Add tests for admin auth

devproject/urls.py

@@ -43,7 +43,7 @@ urlpatterns = [
         name="django-i18n",
     ),
     # Uncomment next line if you plan to use Django admin for 3rd party apps
-    # url(r'^django-admin/', admin.site.urls),
+    url(r'^django-admin/', admin.site.urls),
 ]
 
 

misago/admin/auth.py

@@ -7,49 +7,48 @@ from django.utils.translation import gettext as _
 
 from ..conf import settings
 
-KEY_TOKEN = "misago_admin_session_token"
-KEY_UPDATED = "misago_admin_session_updated"
-
-
-def make_user_admin_token(user):
-    formula = (str(user.pk), user.email, user.password, settings.SECRET_KEY)
-    return md5(":".join(formula).encode()).hexdigest()
-
+TOKEN_KEY = "misago_admin_session_token"
+UPDATED_KEY = "misago_admin_session_updated"
 
 # Admin session state controls
-def is_admin_session(request):
+def is_admin_authorized(request):
     if request.user.is_anonymous:
         return False
 
     if not request.user.is_staff:
         return False
 
-    admin_token = request.session.get(KEY_TOKEN)
+    admin_token = request.session.get(TOKEN_KEY)
     if not admin_token == make_user_admin_token(request.user):
         return False
 
-    updated = request.session.get(KEY_UPDATED, 0)
+    updated = request.session.get(UPDATED_KEY, 0)
     if updated < time() - (settings.MISAGO_ADMIN_SESSION_EXPIRATION * 60):
         if updated:
-            request.session.pop(KEY_UPDATED, None)
+            request.session.pop(UPDATED_KEY, None)
             messages.info(request, _("Your admin session has expired."))
         return False
 
     return True
 
 
-def start_admin_session(request, user):
-    request.session[KEY_TOKEN] = make_user_admin_token(user)
-    request.session[KEY_UPDATED] = int(time())
+def authorize_admin(request):
+    request.session[TOKEN_KEY] = make_user_admin_token(request.user)
+    request.session[UPDATED_KEY] = int(time())
 
 
-def update_admin_session(request):
-    request.session[KEY_UPDATED] = int(time())
+def update_admin_authorization(request):
+    request.session[UPDATED_KEY] = int(time())
 
 
-def close_admin_session(request):
-    request.session.pop(KEY_TOKEN, None)
-    request.session.pop(KEY_UPDATED, None)
+def remove_admin_authorization(request):
+    request.session.pop(TOKEN_KEY, None)
+    request.session.pop(UPDATED_KEY, None)
+
+
+def make_user_admin_token(user):
+    formula = (str(user.pk), user.email, user.password, settings.SECRET_KEY)
+    return md5(":".join(formula).encode()).hexdigest()
 
 
 # Login/logout exposed
@@ -65,14 +64,14 @@ def django_login_handler(sender, **kwargs):
     except AttributeError:
         admin_namespace = False
     if admin_namespace and user.is_staff:
-        start_admin_session(request, user)
+        authorize_admin(request)
 
 
 dj_auth.signals.user_logged_in.connect(django_login_handler)
 
 
 def django_logout_handler(sender, **kwargs):
-    close_admin_session(kwargs["request"])
+    remove_admin_authorization(kwargs["request"])
 
 
 dj_auth.signals.user_logged_out.connect(django_logout_handler)

misago/admin/middleware.py

@@ -1,7 +1,11 @@
 from django.shortcuts import redirect
 from django.utils.deprecation import MiddlewareMixin
 
-from . import auth
+from .auth import (
+    is_admin_authorized,
+    remove_admin_authorization,
+    update_admin_authorization
+)
 from .views import get_protected_namespace
 from .views.auth import login
 
@@ -10,9 +14,13 @@ class AdminAuthMiddleware(MiddlewareMixin):
     def process_view(self, request, view_func, view_args, view_kwargs):
         request.admin_namespace = get_protected_namespace(request)
         if request.admin_namespace:
-            if not auth.is_admin_session(request):
-                auth.close_admin_session(request)
-                if request.resolver_match.url_name == "index":
-                    return login(request)
-                return redirect("%s:index" % request.admin_namespace)
-            auth.update_admin_session(request)
+            return self.check_admin_authorization(request)
+
+    def check_admin_authorization(self, request):
+        if not is_admin_authorized(request):
+            remove_admin_authorization(request)
+            if request.resolver_match.url_name == "index":
+                return login(request)
+            return redirect("%s:index" % request.admin_namespace)
+
+        update_admin_authorization(request)

misago/admin/tests/test_admin_views.py

@@ -6,31 +6,6 @@ from ..test import AdminTestCase
 from ..views import get_protected_namespace
 
 
-class MockRequest:
-    def __init__(self, path):
-        self.path = path
-
-
-class AdminProtectedNamespaceTests(TestCase):
-    def test_valid_cases(self):
-        """get_protected_namespace returns true for protected links"""
-        TEST_CASES = ("", "somewhere/", "ejksajdlksajldjskajdlksajlkdas")
-
-        links_prefix = reverse("misago:admin:index")
-
-        for case in TEST_CASES:
-            request = MockRequest(links_prefix + case)
-            self.assertEqual(get_protected_namespace(request), "misago:admin")
-
-    def test_invalid_cases(self):
-        """get_protected_namespace returns none for other links"""
-        TEST_CASES = ("/", "/somewhere/", "/ejksajdlksajldjskajdlksajlkdas")
-
-        for case in TEST_CASES:
-            request = MockRequest(case)
-            self.assertEqual(get_protected_namespace(request), None)
-
-
 class AdminViewAccessTests(AdminTestCase):
     def test_admin_denies_non_staff_non_superuser(self):
         """admin middleware rejects user thats non staff and non superuser"""

misago/admin/tests/test_auth.py

@@ -0,0 +1,115 @@
+from time import time
+from unittest.mock import Mock
+
+import pytest
+from django.contrib.messages import get_messages
+from django.test import override_settings
+
+from ..auth import (
+    TOKEN_KEY,
+    UPDATED_KEY,
+    authorize_admin,
+    is_admin_authorized,
+    remove_admin_authorization,
+    update_admin_authorization,
+)
+
+
+@pytest.fixture
+def admin_request(superuser):
+    request = Mock(session={}, user=superuser)
+    authorize_admin(request)
+    return request
+
+
+def test_authorizing_admin_updates_request_session(user):
+    request = Mock(session={}, user=user)
+    authorize_admin(request)
+    assert request.session
+
+
+def test_staff_user_can_be_authorized(staffuser):
+    request = Mock(session={}, user=staffuser)
+    authorize_admin(request)
+    assert is_admin_authorized(request)
+
+
+def test_non_staff_user_admin_authorization_is_never_valid(user):
+    request = Mock(session={}, user=user)
+    authorize_admin(request)
+    assert not is_admin_authorized(request)
+
+
+def test_anonymous_user_admin_authorization_is_never_valid(user, anonymous_user):
+    request = Mock(session={}, user=user)
+    authorize_admin(request)
+    request.user = anonymous_user
+    assert not is_admin_authorized(request)
+
+
+def test_superuser_without_staff_flag_admin_authorization_is_never_valid(
+    staffuser
+):
+    request = Mock(session={}, user=staffuser)
+    authorize_admin(request)
+    request.user.is_staff = False
+    assert not is_admin_authorized(request)
+
+
+def test_admin_authorization_is_invalidated_by_user_pk_change(
+    admin_request, superuser
+):
+    admin_request.user.pk = superuser.pk + 1
+    assert not is_admin_authorized(admin_request)
+
+
+def test_admin_authorization_is_invalidated_by_user_email_change(admin_request):
+    admin_request.user.email = "changed@example.com"
+    assert not is_admin_authorized(admin_request)
+
+
+def test_admin_authorization_is_invalidated_by_user_password_change(admin_request):
+    admin_request.user.set_password("changed-password")
+    assert not is_admin_authorized(admin_request)
+
+
+def test_admin_authorization_is_invalidated_by_secret_key_change(admin_request):
+    with override_settings(SECRET_KEY="changed-secret-key"):
+        assert not is_admin_authorized(admin_request)
+
+
+def test_admin_authorization_is_invalidated_by_token_change(admin_request):
+    admin_request.session[TOKEN_KEY] = "authorization-token-changed"
+    assert not is_admin_authorized(admin_request)
+
+
+@override_settings(MISAGO_ADMIN_SESSION_EXPIRATION=5)
+def test_admin_authorization_is_invalidated_by_token_expiration(admin_request):
+    admin_request.session[UPDATED_KEY] = time() - 5 * 60 - 1
+    assert not is_admin_authorized(admin_request)
+
+
+def test_updating_authorization_extends_authorization_expiration_time(admin_request):
+    admin_request.session[UPDATED_KEY] = 0
+    update_admin_authorization(admin_request)
+    assert admin_request.session[UPDATED_KEY]
+
+
+def test_updating_authorization_validates_authorization(admin_request):
+    admin_request.session[UPDATED_KEY] = 0
+    update_admin_authorization(admin_request)
+    assert is_admin_authorized(admin_request)
+
+
+def test_removing_authorization_removes_autorization_from_request_session(
+    admin_request
+):
+    admin_request.session[UPDATED_KEY] = 0
+    remove_admin_authorization(admin_request)
+    assert not admin_request.session
+
+
+def test_removing_authorization_invalidates_autorization(admin_request):
+    admin_request.session[UPDATED_KEY] = 0
+    remove_admin_authorization(admin_request)
+    assert not is_admin_authorized(admin_request)

misago/admin/tests/test_login_to_admin.py

@@ -1,7 +1,7 @@
 from django.urls import reverse
 
 from ...test import assert_contains
-from ..auth import is_admin_session
+from ..auth import is_admin_authorized
 
 admin_link = reverse("misago:admin:index")
 
@@ -50,7 +50,7 @@ def test_user_with_staff_status_is_logged_to_admin(client, staffuser, user_passw
     response = client.post(
         admin_link, {"username": staffuser.username, "password": user_password}
     )
-    assert is_admin_session(response.wsgi_request)
+    assert is_admin_authorized(response.wsgi_request)
     assert response.wsgi_request.user == staffuser
 
 

misago/admin/tests/test_logout_from_admin.py

@@ -2,7 +2,7 @@ from django.contrib.messages import get_messages
 from django.urls import reverse
 
 from ...test import assert_contains
-from ..auth import is_admin_session
+from ..auth import is_admin_authorized
 
 admin_logout_link = reverse("misago:admin:logout")
 site_logout_link = reverse("misago:logout")
@@ -14,7 +14,7 @@ def test_admin_can_logout_from_admin_site_but_stay_logged(
 ):
     response = admin_client.post(admin_logout_link)
     assert response.wsgi_request.user == superuser
-    assert not is_admin_session(response.wsgi_request)
+    assert not is_admin_authorized(response.wsgi_request)
 
 
 def test_admin_is_redirected_to_site_on_logout(admin_client, superuser):
@@ -32,10 +32,10 @@ def test_admin_is_displayed_a_message_after_logout(admin_client, superuser):
 def test_admin_can_logout_from_entire_site(admin_client):
     response = admin_client.post(site_logout_link)
     assert response.wsgi_request.user.is_anonymous
-    assert not is_admin_session(response.wsgi_request)
+    assert not is_admin_authorized(response.wsgi_request)
 
 
 def test_admin_is_redirected_to_site_on_logout(admin_client, superuser):
     response = admin_client.post(admin_logout_link)
     assert response.status_code == 302
-    assert response["location"] == site_link
+    assert response["location"] == site_link

misago/admin/tests/test_protected_urls_detection.py

@@ -0,0 +1,49 @@
+from unittest.mock import Mock
+
+from django.urls import reverse
+
+from ..views import get_protected_namespace
+
+django_admin_url = reverse("admin:index")
+misago_admin_url = reverse("misago:admin:index")
+site_url = reverse("misago:index")
+
+
+def test_request_to_misago_admin_root_url_is_protected():
+    request = Mock(path=misago_admin_url)
+    assert get_protected_namespace(request) == "misago:admin"
+
+
+def test_request_to_misago_admin_subpath_url_is_protected():
+    request = Mock(path=misago_admin_url + "users/")
+    assert get_protected_namespace(request) == "misago:admin"
+
+
+def test_request_to_django_admin_root_url_is_protected():
+    request = Mock(path=django_admin_url)
+    assert get_protected_namespace(request) == "admin"
+
+
+def test_request_to_django_admin_subpath_url_is_protected():
+    request = Mock(path=django_admin_url + "users/")
+    assert get_protected_namespace(request) == "admin"
+
+
+def test_request_to_django_admin_subpath_url_is_protected():
+    request = Mock(path=django_admin_url + "users/")
+    assert get_protected_namespace(request) == "admin"
+
+
+def test_request_to_site_root_url_is_not_protected():
+    request = Mock(path=site_url)
+    assert get_protected_namespace(request) is None
+
+
+def test_request_to_site_subpath_url_is_not_protected():
+    request = Mock(path=site_url + "t/some-thread-123/")
+    assert get_protected_namespace(request) is None
+
+
+def test_request_to_site_non_reversable_url_is_not_protected():
+    request = Mock(path=site_url + "somewhere-custom/")
+    assert get_protected_namespace(request) is None

misago/admin/views/__init__.py

@@ -3,7 +3,7 @@ from django.shortcuts import render as dj_render
 
 from ...conf import settings
 from .. import site
-from ..auth import is_admin_session, update_admin_session
+from ..auth import is_admin_authorized, update_admin_authorization
 from .auth import login
 
 
@@ -56,8 +56,8 @@ def protected_admin_view(f):
     def decorator(request, *args, **kwargs):
         protected_view = get_protected_namespace(request)
         if protected_view:
-            if is_admin_session(request):
-                update_admin_session(request)
+            if is_admin_authorized(request):
+                update_admin_authorization(request)
                 return f(request, *args, **kwargs)
             request.admin_namespace = protected_view
             return login(request)

misago/admin/views/auth.py

@@ -35,7 +35,7 @@ def login(request):
 @never_cache
 def logout(request):
     if request.method == "POST":
-        auth.close_admin_session(request)
+        auth.remove_admin_authorization(request)
         messages.info(request, _("Your admin session has been closed."))
         return redirect("misago:index")
     return redirect("misago:admin:index")

misago/admin/views/errorpages.py

@@ -2,12 +2,12 @@ from django.shortcuts import redirect
 
 from . import get_protected_namespace, protected_admin_view, render
 from ...core.utils import get_exception_message
-from ..auth import is_admin_session, update_admin_session
+from ..auth import is_admin_authorized, update_admin_authorization
 
 # Magic error page used by admin
 @protected_admin_view
 def _error_page(request, code, exception=None, default_message=None):
-    if not is_admin_session(request):
+    if not is_admin_authorized(request):
         return redirect("misago:admin:index")
 
     template_pattern = "misago/admin/errorpages/%s.html" % code
@@ -25,7 +25,7 @@ def _error_page(request, code, exception=None, default_message=None):
 def admin_error_page(f):
     def decorator(request, *args, **kwargs):
         if get_protected_namespace(request):
-            update_admin_session(request)
+            update_admin_authorization(request)
             return _error_page(request, *args, **kwargs)
         return f(request, *args, **kwargs)
 
@@ -34,8 +34,8 @@ def admin_error_page(f):
 
 # Magic CSRF fail page for Admin
 def _csrf_failure(request, reason=""):
-    if is_admin_session(request):
-        update_admin_session(request)
+    if is_admin_authorized(request):
+        update_admin_authorization(request)
         response = render(
             request,
             "misago/admin/errorpages/csrf_failure_authenticated.html",

misago/conftest.py

@@ -1,7 +1,7 @@
 import pytest
 
 from .acl import ACL_CACHE, useracl
-from .admin.auth import start_admin_session
+from .admin.auth import authorize_admin
 from .conf import SETTINGS_CACHE
 from .conf.dynamicsettings import DynamicSettings
 from .conf.staticsettings import StaticSettings
@@ -86,6 +86,6 @@ def superuser_acl(superuser, cache_versions):
 def admin_client(mocker, client, superuser):
     client.force_login(superuser)
     session = client.session
-    start_admin_session(mocker.Mock(session=session), superuser)
+    authorize_admin(mocker.Mock(session=session, user=superuser))
     session.save()
     return client

misago/users/views/admin/users.py

@@ -6,7 +6,7 @@ from django.shortcuts import redirect
 from django.utils.translation import gettext_lazy as _
 
 from ....acl.useracl import get_user_acl
-from ....admin.auth import start_admin_session
+from ....admin.auth import authorize_admin
 from ....admin.views import generic
 from ....categories.models import Category
 from ....core.mail import mail_users
@@ -305,13 +305,13 @@ class EditUser(UserAdmin, generic.ModelFormView):
             target.set_password(form.cleaned_data["new_password"])
 
             if target.pk == request.user.pk:
-                start_admin_session(request, target)
+                authorize_admin(request, target)
                 update_session_auth_hash(request, target)
 
         if form.cleaned_data.get("email"):
             target.set_email(form.cleaned_data["email"])
             if target.pk == request.user.pk:
-                start_admin_session(request, target)
+                authorize_admin(request, target)
 
         if form.cleaned_data.get("is_avatar_locked"):
             if not target.old_is_avatar_locked: