mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-08-10 04:43:19 +00:00
Fix: deny deactivated users in permission filtering and auto-login (#13623)
* Fix: Hardening sweep, ensure a user is active, not just authenticated * Missed this test
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
Reference in New Issue
Block a user