From 2e67697d36ec1e6a57619a1810714f48c08729f9 Mon Sep 17 00:00:00 2001
From: shamoon <4887959+shamoon@users.noreply.github.com>
Date: Mon, 31 Jul 2023 20:33:18 -0700
Subject: [PATCH] Note creation / deletion should respect doc permissions
- Disable add note button on frontend
- Explicitly disable add / delete via api
---
.../document-detail.component.html | 2 +-
.../document-notes.component.html | 2 +-
.../document-notes.component.ts | 7 ++-
src/documents/tests/test_api.py | 56 +++++++++++++++++++
src/documents/views.py | 23 ++++++--
5 files changed, 82 insertions(+), 8 deletions(-)
diff --git a/src-ui/src/app/components/document-detail/document-detail.component.html b/src-ui/src/app/components/document-detail/document-detail.component.html
index 6b42fade8..41e7a78d1 100644
--- a/src-ui/src/app/components/document-detail/document-detail.component.html
+++ b/src-ui/src/app/components/document-detail/document-detail.component.html
@@ -174,7 +174,7 @@
Notes {{document.notes.length}}
-
+
diff --git a/src-ui/src/app/components/document-notes/document-notes.component.html b/src-ui/src/app/components/document-notes/document-notes.component.html
index f6c46cd47..c76660e30 100644
--- a/src-ui/src/app/components/document-notes/document-notes.component.html
+++ b/src-ui/src/app/components/document-notes/document-notes.component.html
@@ -8,7 +8,7 @@
diff --git a/src-ui/src/app/components/document-notes/document-notes.component.ts b/src-ui/src/app/components/document-notes/document-notes.component.ts
index b8c7d6fd9..01a104fb2 100644
--- a/src-ui/src/app/components/document-notes/document-notes.component.ts
+++ b/src-ui/src/app/components/document-notes/document-notes.component.ts
@@ -26,6 +26,9 @@ export class DocumentNotesComponent extends ComponentWithPermissions {
@Input()
notes: PaperlessDocumentNote[] = []
+ @Input()
+ addDisabled: boolean = false
+
@Output()
updated: EventEmitter = new EventEmitter()
users: PaperlessUser[]
@@ -61,7 +64,9 @@ export class DocumentNotesComponent extends ComponentWithPermissions {
error: (e) => {
this.networkActive = false
this.toastService.showError(
- $localize`Error saving note: ${e.toString()}`
+ $localize`Error saving note`,
+ 10000,
+ JSON.stringify(e)
)
},
})
diff --git a/src/documents/tests/test_api.py b/src/documents/tests/test_api.py
index 40a1ca4a3..d788cf6a4 100644
--- a/src/documents/tests/test_api.py
+++ b/src/documents/tests/test_api.py
@@ -2369,6 +2369,62 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase):
self.assertEqual(resp_data["note"], "this is a posted note")
+ def test_notes_permissions_aware(self):
+ """
+ GIVEN:
+ - Existing document owned by user2 but with granted view perms for user1
+ WHEN:
+ - API request is made by user1 to add a note or delete
+ THEN:
+ - Notes are neither created nor deleted
+ """
+ user1 = User.objects.create_user(username="test1")
+ user1.user_permissions.add(*Permission.objects.all())
+ user1.save()
+
+ user2 = User.objects.create_user(username="test2")
+ user2.save()
+
+ doc = Document.objects.create(
+ title="test",
+ mime_type="application/pdf",
+ content="this is a document which will have notes added",
+ )
+ doc.owner = user2
+ doc.save()
+
+ self.client.force_authenticate(user1)
+
+ resp = self.client.get(
+ f"/api/documents/{doc.pk}/notes/",
+ format="json",
+ )
+ self.assertEqual(resp.content, b"Insufficient permissions to view")
+ self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
+
+ assign_perm("view_document", user1, doc)
+
+ resp = self.client.post(
+ f"/api/documents/{doc.pk}/notes/",
+ data={"note": "this is a posted note"},
+ )
+ self.assertEqual(resp.content, b"Insufficient permissions to create")
+ self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
+
+ note = Note.objects.create(
+ note="This is a note.",
+ document=doc,
+ user=user2,
+ )
+
+ response = self.client.delete(
+ f"/api/documents/{doc.pk}/notes/?id={note.pk}",
+ format="json",
+ )
+
+ self.assertEqual(response.content, b"Insufficient permissions to delete")
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
def test_delete_note(self):
"""
GIVEN:
diff --git a/src/documents/views.py b/src/documents/views.py
index cd69095fe..d57ad4eea 100644
--- a/src/documents/views.py
+++ b/src/documents/views.py
@@ -502,19 +502,18 @@ class DocumentViewSet(
@action(methods=["get", "post", "delete"], detail=True)
def notes(self, request, pk=None):
+ currentUser = request.user
try:
doc = Document.objects.get(pk=pk)
- if request.user is not None and not has_perms_owner_aware(
- request.user,
+ if currentUser is not None and not has_perms_owner_aware(
+ currentUser,
"view_document",
doc,
):
- return HttpResponseForbidden("Insufficient permissions")
+ return HttpResponseForbidden("Insufficient permissions to view")
except Document.DoesNotExist:
raise Http404
- currentUser = request.user
-
if request.method == "GET":
try:
return Response(self.getNotes(doc))
@@ -525,6 +524,13 @@ class DocumentViewSet(
)
elif request.method == "POST":
try:
+ if currentUser is not None and not has_perms_owner_aware(
+ currentUser,
+ "change_document",
+ doc,
+ ):
+ return HttpResponseForbidden("Insufficient permissions to create")
+
c = Note.objects.create(
document=doc,
note=request.data["note"],
@@ -545,6 +551,13 @@ class DocumentViewSet(
},
)
elif request.method == "DELETE":
+ if currentUser is not None and not has_perms_owner_aware(
+ currentUser,
+ "change_document",
+ doc,
+ ):
+ return HttpResponseForbidden("Insufficient permissions to delete")
+
note = Note.objects.get(id=int(request.GET.get("id")))
note.delete()