Skip to content

Commit f08d36f

Browse files
replaceafillsevein
andcommitted
Avoid unauthenticated access to admin area
* Fix urlpatterns of administration package * Fix LOGIN_EXEMPT_URLS regexps Co-authored-by: Jesús García Crespo <[email protected]>
1 parent 220fbca commit f08d36f

File tree

6 files changed

+139
-30
lines changed

6 files changed

+139
-30
lines changed

src/dashboard/src/components/administration/urls.py

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,72 +29,72 @@
2929
urlpatterns = [
3030
url(r"^$", views.administration, name="administration_index"),
3131
url(
32-
r"reports/failures/delete/(?P<report_id>\w+)/$",
32+
r"^reports/failures/delete/(?P<report_id>\w+)/$",
3333
views.failure_report_delete,
3434
name="failure_report_delete",
3535
),
3636
url(
37-
r"reports/failures/(?P<report_id>\w+)/$",
37+
r"^reports/failures/(?P<report_id>\w+)/$",
3838
views.failure_report,
3939
name="failure_report",
4040
),
41-
url(r"reports/failures/$", views.failure_report, name="reports_failures_index"),
42-
url(r"dips/as/$", views_dip_upload.admin_as, name="dips_as"),
43-
url(r"dips/atom/$", views_dip_upload.admin_atom, name="dips_atom_index"),
41+
url(r"^reports/failures/$", views.failure_report, name="reports_failures_index"),
42+
url(r"^dips/as/$", views_dip_upload.admin_as, name="dips_as"),
43+
url(r"^dips/atom/$", views_dip_upload.admin_atom, name="dips_atom_index"),
4444
url(
45-
r"dips/atom/edit_levels/$",
45+
r"^dips/atom/edit_levels/$",
4646
views.atom_levels_of_description,
4747
name="atom_levels_of_description",
4848
),
4949
url(r"^i18n/", include("django.conf.urls.i18n", namespace="django_i18n")),
5050
url(
51-
r"language/$",
51+
r"^language/$",
5252
TemplateView.as_view(template_name="administration/language.html"),
5353
name="admin_set_language",
5454
),
55-
url(r"storage/$", views.storage, name="storage"),
56-
url(r"usage/$", views.usage, name="usage"),
57-
url(r"usage/clear/(?P<dir_id>\w+)/$", views.usage_clear, name="usage_clear"),
58-
url(r"processing/$", views_processing.list, name="processing"),
59-
url(r"processing/add/$", views_processing.edit, name="processing_add"),
55+
url(r"^storage/$", views.storage, name="storage"),
56+
url(r"^usage/$", views.usage, name="usage"),
57+
url(r"^usage/clear/(?P<dir_id>\w+)/$", views.usage_clear, name="usage_clear"),
58+
url(r"^processing/$", views_processing.list, name="processing"),
59+
url(r"^processing/add/$", views_processing.edit, name="processing_add"),
6060
url(
61-
r"processing/edit/{}/$".format(ProcessingConfigurationForm.NAME_URL_REGEX),
61+
r"^processing/edit/{}/$".format(ProcessingConfigurationForm.NAME_URL_REGEX),
6262
views_processing.edit,
6363
name="processing_edit",
6464
),
6565
url(
66-
r"processing/reset/{}/$".format(ProcessingConfigurationForm.NAME_URL_REGEX),
66+
r"^processing/reset/{}/$".format(ProcessingConfigurationForm.NAME_URL_REGEX),
6767
views_processing.reset,
6868
name="processing_reset",
6969
),
7070
url(
71-
r"processing/delete/{}/$".format(ProcessingConfigurationForm.NAME_URL_REGEX),
71+
r"^processing/delete/{}/$".format(ProcessingConfigurationForm.NAME_URL_REGEX),
7272
views_processing.delete,
7373
name="processing_delete",
7474
),
7575
url(
76-
r"processing/download/{}/$".format(ProcessingConfigurationForm.NAME_URL_REGEX),
76+
r"^processing/download/{}/$".format(ProcessingConfigurationForm.NAME_URL_REGEX),
7777
views_processing.download,
7878
name="processing_download",
7979
),
80-
url(r"premis/agent/$", views.premis_agent, name="premis_agent"),
81-
url(r"handle/$", views.handle_config, name="handle"),
82-
url(r"api/$", views.api, name="api"),
83-
url(r"general/$", views.general, name="general"),
84-
url(r"version/$", views.version, name="version"),
80+
url(r"^premis/agent/$", views.premis_agent, name="premis_agent"),
81+
url(r"^handle/$", views.handle_config, name="handle"),
82+
url(r"^api/$", views.api, name="api"),
83+
url(r"^general/$", views.general, name="general"),
84+
url(r"^version/$", views.version, name="version"),
8585
url(
86-
r"taxonomy/term/(?P<term_uuid>" + settings.UUID_REGEX + ")/$",
86+
r"^taxonomy/term/(?P<term_uuid>" + settings.UUID_REGEX + ")/$",
8787
views.term_detail,
8888
name="term",
8989
),
9090
url(
91-
r"taxonomy/term/(?P<term_uuid>" + settings.UUID_REGEX + ")/delete/$",
91+
r"^taxonomy/term/(?P<term_uuid>" + settings.UUID_REGEX + ")/delete/$",
9292
views.term_delete,
9393
),
9494
url(
95-
r"taxonomy/(?P<taxonomy_uuid>" + settings.UUID_REGEX + ")/$",
95+
r"^taxonomy/(?P<taxonomy_uuid>" + settings.UUID_REGEX + ")/$",
9696
views.terms,
9797
name="terms",
9898
),
99-
url(r"taxonomy/$", views.taxonomy, name="taxonomy"),
99+
url(r"^taxonomy/$", views.taxonomy, name="taxonomy"),
100100
]

src/dashboard/src/components/api/urls.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
urlpatterns = [
2727
url(r"transfer/approve", views.approve_transfer),
2828
url(r"transfer/unapproved", views.unapproved_transfers),
29-
url(r"transfer/completed", views.completed_transfers),
29+
url(r"transfer/completed", views.completed_transfers, name="completed_transfers"),
3030
url(
3131
r"transfer/status/(?P<unit_uuid>" + settings.UUID_REGEX + ")",
3232
views.status,
@@ -49,9 +49,13 @@
4949
url(r"^(?P<unit_type>transfer|ingest)/delete/", views.mark_completed_hidden),
5050
url(r"^ingest/reingest/approve", views.reingest_approve),
5151
url(r"^ingest/reingest", views.reingest, {"target": "ingest"}),
52-
url(r"^ingest/completed", views.completed_ingests),
52+
url(r"^ingest/completed", views.completed_ingests, name="completed_ingests"),
5353
url(r"^ingest/copy_metadata_files/$", views.copy_metadata_files_api),
54-
url(r"administration/dips/atom/levels/$", views.get_levels_of_description),
54+
url(
55+
r"administration/dips/atom/levels/$",
56+
views.get_levels_of_description,
57+
name="get_levels_of_description",
58+
),
5559
url(
5660
r"administration/dips/atom/fetch_levels/$",
5761
views.fetch_levels_of_description_from_atom,

src/dashboard/src/installer/middleware.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232

3333
def _load_exempt_urls():
3434
global EXEMPT_URLS
35-
EXEMPT_URLS = [re_compile(settings.LOGIN_URL.lstrip("/"))]
35+
EXEMPT_URLS = [re_compile("{}$".format(settings.LOGIN_URL.lstrip("/")))]
3636
if hasattr(settings, "LOGIN_EXEMPT_URLS"):
3737
EXEMPT_URLS += [re_compile(expr) for expr in settings.LOGIN_EXEMPT_URLS]
3838

src/dashboard/src/settings/base.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,15 @@ def _get_settings_from_file(path):
484484
# login-related settings
485485
LOGIN_URL = "/administration/accounts/login/"
486486
LOGIN_REDIRECT_URL = "/"
487-
LOGIN_EXEMPT_URLS = [r"^administration/accounts/login", r"^api", r"^jsi18n"]
487+
LOGIN_EXEMPT_URLS = [
488+
# Allow users to authenticate via the log-in page.
489+
r"^administration/accounts/login$",
490+
# Authentication in the API namespace is handled by components.api.views
491+
# using Tastypie's MultiAuthentication. We don't match the end of the
492+
# string here purposely.
493+
r"^api",
494+
r"^jsi18n",
495+
]
488496
# Django debug toolbar
489497
try:
490498
import debug_toolbar # noqa: F401

src/dashboard/tests/test_auth.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
# -*- coding: utf-8 -*-
2+
3+
from __future__ import unicode_literals
4+
5+
import json
6+
7+
from django.conf import settings
8+
from django.contrib.auth import get_user_model
9+
from django.core.urlresolvers import reverse
10+
from django.test import TestCase
11+
from django.test.client import Client
12+
from tastypie.models import ApiKey
13+
14+
from components import helpers
15+
from components.helpers import generate_api_key
16+
17+
18+
class TestAuth(TestCase):
19+
"""Authentication sanity checks."""
20+
21+
fixtures = ["test_user"]
22+
23+
API_URLS = (
24+
reverse("api:completed_transfers"),
25+
reverse("api:completed_ingests"),
26+
reverse("api:get_levels_of_description"),
27+
)
28+
29+
def setUp(self):
30+
helpers.set_setting("dashboard_uuid", "test-uuid")
31+
32+
def authenticate(self):
33+
self.client = Client()
34+
self.client.login(username="test", password="test")
35+
36+
def test_site_requires_auth(self):
37+
for url in [
38+
reverse("transfer:transfer_index"),
39+
reverse("ingest:ingest_index"),
40+
reverse("administration:api"),
41+
reverse("administration:general"),
42+
reverse("administration:premis_agent"),
43+
# Verify that exempted URLs cannot be used to access other areas
44+
# that are restricted.
45+
"{}/{}".format(settings.LOGIN_URL.rstrip("/"), "transfer/"),
46+
"{}/{}".format(settings.LOGIN_URL.rstrip("/"), "abcdefgh/api/"),
47+
"{}/{}".format(settings.LOGIN_URL.rstrip("/"), "version"),
48+
]:
49+
response = self.client.get(url)
50+
51+
self.assertRedirects(response, settings.LOGIN_URL)
52+
53+
def test_site_performs_session_auth(self):
54+
self.authenticate()
55+
56+
for url in [
57+
reverse("transfer:transfer_index"),
58+
reverse("ingest:ingest_index"),
59+
reverse("administration:api"),
60+
]:
61+
response = self.client.get(url, follow=False)
62+
63+
self.assertEqual(response.status_code, 200)
64+
65+
def test_api_requires_auth(self):
66+
for url in self.API_URLS:
67+
response = self.client.get(url)
68+
69+
self.assertEqual(response.status_code, 403)
70+
self.assertEqual(
71+
response.content.decode("utf8"),
72+
json.dumps({"message": "API key not valid.", "error": True}),
73+
)
74+
75+
def test_api_authenticates_via_key(self):
76+
user = get_user_model().objects.get(pk=1)
77+
generate_api_key(user)
78+
key = ApiKey.objects.get(user=user).key
79+
80+
for url in self.API_URLS:
81+
response = self.client.get(
82+
url, HTTP_AUTHORIZATION="ApiKey test:{}".format(key), follow=False
83+
)
84+
85+
self.assertEqual(response.status_code, 200)
86+
87+
def test_api_authenticates_via_session(self):
88+
self.authenticate()
89+
90+
for url in self.API_URLS:
91+
response = self.client.get(url, follow=False)
92+
93+
self.assertEqual(response.status_code, 200)

src/dashboard/tests/test_middleware.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ def test_unauthenticated_user_can_access_exempt_url(self):
3636

3737
self.assertEqual(response.status_code, 404)
3838

39+
# installer.middleware.EXEMPT_URLS is a global *list*
40+
# that has been mutated in this test, this restores it back
41+
_load_exempt_urls()
42+
3943
def test_unauthenticated_user_is_sent_to_login_page(self):
4044
helpers.set_setting("dashboard_uuid", "test-uuid")
4145

0 commit comments

Comments
 (0)