Merge pull request #3903 from paperless-ngx/fix/issue-3902

Fix: note creation / deletion should respect doc permissions
This commit is contained in:
shamoon 2023-08-02 08:33:44 -07:00 committed by GitHub
commit 557e1790dd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 82 additions and 8 deletions

View File

@ -174,7 +174,7 @@
<li [ngbNavItem]="DocumentDetailNavIDs.Notes" *ngIf="notesEnabled">
<a ngbNavLink i18n>Notes <span *ngIf="document?.notes.length" class="badge text-bg-secondary ms-1">{{document.notes.length}}</span></a>
<ng-template ngbNavContent>
<app-document-notes [documentId]="documentId" [notes]="document?.notes" (updated)="notesUpdated($event)"></app-document-notes>
<app-document-notes [documentId]="documentId" [notes]="document?.notes" [addDisabled]="!userCanEdit" (updated)="notesUpdated($event)"></app-document-notes>
</ng-template>
</li>

View File

@ -8,7 +8,7 @@
</div>
<div class="form-group mt-2 d-flex justify-content-end align-items-center">
<div *ngIf="networkActive" class="spinner-border spinner-border-sm fw-normal me-auto" role="status"></div>
<button type="button" class="btn btn-primary btn-sm" [disabled]="networkActive" (click)="addNote()" i18n>Add note</button>
<button type="button" class="btn btn-primary btn-sm" [disabled]="networkActive || addDisabled" (click)="addNote()" i18n>Add note</button>
</div>
</form>
<hr>

View File

@ -26,6 +26,9 @@ export class DocumentNotesComponent extends ComponentWithPermissions {
@Input()
notes: PaperlessDocumentNote[] = []
@Input()
addDisabled: boolean = false
@Output()
updated: EventEmitter<PaperlessDocumentNote[]> = 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)
)
},
})

View File

@ -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:

View File

@ -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()