amorfo77/paperless-ngx
1
0
Fork 0
mirror of https://github.com/paperless-ngx/paperless-ngx.git synced 2026-07-26 22:04:56 +00:00
Code Issues Projects Releases Wiki Activity

Security (beta): two small permission check fixes (#13186)

Browse Source
This commit is contained in:
shamoon
2026-07-21 16:50:28 +00:00
committed by GitHub
parent 8404198ec8
commit 8aab34ea73
3 changed files with 125 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/documents/tests/test_api_bulk_edit.py
+92
View File
@@ -1419,6 +1419,30 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase):
self.assertEqual(response.status_code, status.HTTP_200_OK)
m.assert_called_once()
@mock.patch("documents.views.bulk_edit.merge")
def test_merge_and_delete_requires_change_permission(self, m) -> None:
self.setup_mock(m, "merge")
user = User.objects.create_user(username="no-change")
user.user_permissions.add(
Permission.objects.get(codename="add_document"),
Permission.objects.get(codename="delete_document"),
)
self.client.force_authenticate(user=user)
response = self.client.post(
"/api/documents/merge/",
json.dumps(
{
"documents": [self.doc2.id, self.doc3.id],
"delete_originals": True,
},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
m.assert_not_called()
@mock.patch("documents.views.bulk_edit.merge")
def test_merge_invalid_parameters(self, m) -> None:
self.setup_mock(m, "merge")
@@ -1668,6 +1692,74 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase):
self.assertEqual(response.status_code, status.HTTP_200_OK)
m.assert_called_once()
@mock.patch("documents.views.bulk_edit.edit_pdf")
def test_edit_pdf_update_requires_change_permission(self, m) -> None:
self.setup_mock(m, "edit_pdf")
user = User.objects.create_user(username="no-change")
self.client.force_authenticate(user=user)
response = self.client.post(
"/api/documents/edit_pdf/",
json.dumps(
{
"documents": [self.doc2.id],
"operations": [{"page": 1}],
"update_document": True,
},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
m.assert_not_called()
@mock.patch("documents.views.bulk_edit.remove_password")
@mock.patch("documents.views.bulk_edit.edit_pdf")
def test_delete_original_requires_delete_permission(
self,
edit_pdf_mock,
remove_password_mock,
) -> None:
self.setup_mock(edit_pdf_mock, "edit_pdf")
self.setup_mock(remove_password_mock, "remove_password")
user = User.objects.create_user(username="no-delete")
user.user_permissions.add(
Permission.objects.get(codename="add_document"),
Permission.objects.get(codename="change_document"),
)
self.client.force_authenticate(user=user)
cases = [
(
"/api/documents/edit_pdf/",
{
"documents": [self.doc2.id],
"operations": [{"page": 1}],
"delete_original": True,
},
edit_pdf_mock,
),
(
"/api/documents/remove_password/",
{
"documents": [self.doc2.id],
"password": "secret",
"delete_original": True,
},
remove_password_mock,
),
]
for endpoint, payload, operation_mock in cases:
with self.subTest(endpoint=endpoint):
response = self.client.post(
endpoint,
json.dumps(payload),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
operation_mock.assert_not_called()
@mock.patch("documents.views.bulk_edit.remove_password")
def test_remove_password(self, m) -> None:
self.setup_mock(m, "remove_password")
src/documents/tests/test_api_document_versions.py
+20
View File
@@ -669,6 +669,26 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
def test_update_version_requires_global_change_permission(self) -> None:
user = User.objects.create_user(username="add-only")
user.user_permissions.add(Permission.objects.get(codename="add_document"))
root = Document.objects.create(
title="root",
checksum="root",
mime_type="application/pdf",
)
self.client.force_authenticate(user=user)
with mock.patch("documents.views.consume_file") as consume_mock:
resp = self.client.post(
f"/api/documents/{root.id}/update_version/",
{"document": self._make_pdf_upload()},
format="multipart",
)
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
consume_mock.apply_async.assert_not_called()
def test_update_version_returns_404_for_missing_document(self) -> None:
resp = self.client.post(
"/api/documents/9999/update_version/",
src/documents/views.py
+13 -5
View File
@@ -1947,10 +1947,13 @@ class DocumentViewSet(
"root_document",
).get(pk=pk)
root_doc = get_root_document(request_doc)
if request.user is not None and not has_perms_owner_aware(
request.user,
"change_document",
root_doc,
if request.user is not None and (
not request.user.has_perm("documents.change_document")
or not has_perms_owner_aware(
request.user,
"change_document",
root_doc,
)
):
return HttpResponseForbidden("Insufficient permissions")
except Document.DoesNotExist:
@@ -2756,7 +2759,7 @@ class DocumentOperationPermissionMixin(PassUserMixin, DocumentSelectionMixin):
)
or (method == bulk_edit.edit_pdf and parameters.get("update_document"))
):
has_perms = user_is_owner_of_all_documents
has_perms = has_perms and user_is_owner_of_all_documents
# check global add permissions for methods that create documents
if (
@@ -2781,6 +2784,11 @@ class DocumentOperationPermissionMixin(PassUserMixin, DocumentSelectionMixin):
method in [bulk_edit.merge, bulk_edit.split]
and parameters.get("delete_originals")
)
or (
method in [bulk_edit.edit_pdf, bulk_edit.remove_password]
and parameters.get("delete_original")
and not parameters.get("update_document")
)
)
and not user.has_perm("documents.delete_document")
):