diff --git a/src/documents/filters.py b/src/documents/filters.py index 4dac785ee..40ae8978b 100644 --- a/src/documents/filters.py +++ b/src/documents/filters.py @@ -1047,6 +1047,12 @@ class PermittedObjectsFilter(BaseFilterBackend): perm_codename: str | None = None def filter_queryset(self, request, queryset, view): + # Before the superuser and owner-only paths, neither of which consults + # permitted_object_ids. Scoped to authenticated users so anonymous + # access (AnonymousUser.is_active is False) keeps its existing + # unowned-only behaviour. + if request.user.is_authenticated and not request.user.is_active: + return queryset.none() if request.user.is_superuser: return queryset if not self.include_granted: diff --git a/src/documents/permissions.py b/src/documents/permissions.py index 82b5d9fe8..ef1bc281c 100644 --- a/src/documents/permissions.py +++ b/src/documents/permissions.py @@ -54,11 +54,15 @@ class PaperlessObjectPermissions(DjangoObjectPermissions): class PaperlessAdminPermissions(BasePermission): def has_permission(self, request, view): - return request.user.is_staff + return request.user.is_active and request.user.is_staff def has_global_statistics_permission(user: User | None) -> bool: - if user is None or not getattr(user, "is_authenticated", False): + if ( + user is None + or not getattr(user, "is_active", False) + or not getattr(user, "is_authenticated", False) + ): return False return getattr(user, "is_superuser", False) or user.has_perm( @@ -67,7 +71,11 @@ def has_global_statistics_permission(user: User | None) -> bool: def has_system_status_permission(user: User | None) -> bool: - if user is None or not getattr(user, "is_authenticated", False): + if ( + user is None + or not getattr(user, "is_active", False) + or not getattr(user, "is_authenticated", False) + ): return False return ( @@ -188,6 +196,13 @@ def permitted_object_ids( if user is None or not getattr(user, "is_authenticated", False): return base_qs.filter(owner__isnull=True).values_list("id", flat=True) + # Deactivated users get nothing, deactivated superusers included, so this + # has to come before the superuser shortcut. guardian's + # ObjectPermissionChecker denies inactive users, but get_objects_for_user + # (the pattern this replaces) does not, so it would not be inherited. + if not getattr(user, "is_active", False): + return base_qs.none().values_list("id", flat=True) + if getattr(user, "is_superuser", False): return base_qs.values_list("id", flat=True) diff --git a/src/documents/tests/test_permission_filtering_security.py b/src/documents/tests/test_permission_filtering_security.py index b0bd31b9c..84daaebbf 100644 --- a/src/documents/tests/test_permission_filtering_security.py +++ b/src/documents/tests/test_permission_filtering_security.py @@ -496,6 +496,28 @@ class TestPermittedObjectIdsGenericModels: expected_hidden=[strangers.pk], ) + @pytest.mark.parametrize("is_superuser", [False, True]) + def test_inactive_user_sees_nothing(self, model, factory, perm, is_superuser): + suffix = f"{model.__name__}_{is_superuser}" + user = User.objects.create_user( + username=f"inactive_{suffix}", + is_active=False, + is_superuser=is_superuser, + ) + other = User.objects.create_user(username=f"other_{suffix}") + granted = factory(owner=other) + assign_perm(perm, user, granted) + + assert_visible_document_ids( + permitted_object_ids(user, model, perm), + expected_visible=[], + expected_hidden=[ + factory(owner=None).pk, + factory(owner=user).pk, + granted.pk, + ], + ) + def test_unowned_object_visible_to_everyone(self, model, factory, perm): user = User.objects.create_user(username=f"user_{model.__name__}") unowned = factory(owner=None) diff --git a/src/documents/tests/test_permitted_objects_filter.py b/src/documents/tests/test_permitted_objects_filter.py index d3df22805..b087f091b 100644 --- a/src/documents/tests/test_permitted_objects_filter.py +++ b/src/documents/tests/test_permitted_objects_filter.py @@ -68,3 +68,44 @@ class TestPermittedObjectsFilter: visible_ids = set(result.values_list("id", flat=True)) assert visible_ids == {owned.pk} assert granted.pk not in visible_ids + + @pytest.mark.parametrize( + ("username", "is_superuser"), + [("inactive", False), ("inactive_super", True)], + ) + def test_inactive_user_sees_nothing(self, username: str, *, is_superuser: bool): + user = User.objects.create_user( + username=username, + is_active=False, + is_superuser=is_superuser, + ) + TagFactory(owner=None) + TagFactory(owner=user) + granted = TagFactory(owner=User.objects.create_user(username=f"o_{username}")) + assign_perm("view_tag", user, granted) + request = APIRequestFactory().get("/") + request.user = user + + result = PermittedObjectsFilter().filter_queryset( + request, + Tag.objects.all(), + _DummyView(), + ) + assert result.count() == 0 + + def test_inactive_user_sees_nothing_with_include_granted_false(self): + user = User.objects.create_user(username="inactive_owner", is_active=False) + TagFactory(owner=user) + TagFactory(owner=None) + request = APIRequestFactory().get("/") + request.user = user + + class _OwnerOnlyFilter(PermittedObjectsFilter): + include_granted = False + + result = _OwnerOnlyFilter().filter_queryset( + request, + Tag.objects.all(), + _DummyView(), + ) + assert result.count() == 0 diff --git a/src/paperless/auth.py b/src/paperless/auth.py index 2e7f00bf2..e9a49b7b7 100644 --- a/src/paperless/auth.py +++ b/src/paperless/auth.py @@ -19,7 +19,10 @@ class AutoLoginMiddleware(MiddlewareMixin): if request.path.startswith("/api/token/") and request.method == "POST": return None try: - request.user = User.objects.get(username=settings.AUTO_LOGIN_USERNAME) + request.user = User.objects.get( + username=settings.AUTO_LOGIN_USERNAME, + is_active=True, + ) auth.login( request=request, user=request.user, diff --git a/src/paperless/tests/test_auth_middleware.py b/src/paperless/tests/test_auth_middleware.py new file mode 100644 index 000000000..fec355d7d --- /dev/null +++ b/src/paperless/tests/test_auth_middleware.py @@ -0,0 +1,53 @@ +from django.contrib.auth.models import AnonymousUser +from django.contrib.auth.models import User +from django.test import RequestFactory +from django.test import TestCase +from django.test import override_settings + +from paperless.auth import AutoLoginMiddleware + + +@override_settings(AUTO_LOGIN_USERNAME="autologin") +class TestAutoLoginMiddleware(TestCase): + def setUp(self) -> None: + super().setUp() + self.factory = RequestFactory() + self.middleware = AutoLoginMiddleware(lambda request: None) + + def _process(self, request): + # login() needs a session to write to + request.session = self.client.session + self.middleware.process_request(request) + return request + + def test_active_user_is_logged_in(self) -> None: + """ + GIVEN: + - AUTO_LOGIN_USERNAME names an active user + WHEN: + - A request is processed by the middleware + THEN: + - That user is attached to the request + """ + user = User.objects.create_user(username="autologin") + + request = self._process(self.factory.get("/")) + + self.assertEqual(request.user, user) + + def test_deactivated_user_is_not_logged_in(self) -> None: + """ + GIVEN: + - AUTO_LOGIN_USERNAME names a user who has been deactivated + WHEN: + - A request is processed by the middleware + THEN: + - The request is left anonymous rather than authenticated as them + """ + User.objects.create_user(username="autologin", is_active=False) + + request = self.factory.get("/") + request.user = AnonymousUser() + self._process(request) + + self.assertFalse(request.user.is_authenticated)