From f4a31484f5925cbc02b59ebd37554538ab826ca1 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 8 Feb 2024 09:58:40 +0100 Subject: [PATCH] Merge pull request from GHSA-hvp4-vrv2-8wrq * Reproduce issue in tests * Fix context attribute when record exists * Add comments * Move assignment and comment below --- src/kinto_attachment/utils.py | 13 ++++++++++- tests/test_views_attachment.py | 40 ++++++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/kinto_attachment/utils.py b/src/kinto_attachment/utils.py index 7adbc8d..599c855 100644 --- a/src/kinto_attachment/utils.py +++ b/src/kinto_attachment/utils.py @@ -26,7 +26,11 @@ class AttachmentRouteFactory(RouteFactory): def __init__(self, request): - """Attachment is not a Kinto resource. + """ + This class is the `context` object being passed to the + :class:`kinto.core.authorization.AuthorizationPolicy`. + + Attachment is not a Kinto resource. The required permission is: * ``write`` if the related record exists; @@ -43,12 +47,19 @@ def __init__(self, request): existing = resource.get() except httpexceptions.HTTPNotFound: existing = None + if existing: + # Request write permission on the existing record. self.permission_object_id = record_uri(request) self.required_permission = "write" else: + # Request create record permission on the parent collection. self.permission_object_id = collection_uri(request) self.required_permission = "create" + # Set the current object in context, since it is used in the + # authorization policy to distinguish operations on plural endpoints + # from individual objects. See Kinto/kinto#918 + self.current_object = existing def sha256(content): diff --git a/tests/test_views_attachment.py b/tests/test_views_attachment.py index 5b9a241..09ba644 100644 --- a/tests/test_views_attachment.py +++ b/tests/test_views_attachment.py @@ -283,12 +283,18 @@ def test_upload_refused_if_not_authenticated(self): self.headers.pop("Authorization") self.upload(status=401) + def test_upload_replace_refused_if_not_authenticated(self): + self.upload(status=201) + + self.headers.pop("Authorization") + self.upload(status=401) + def test_upload_refused_if_not_allowed(self): self.headers.update(get_user_headers("jean-louis")) self.upload(status=403) def test_upload_replace_refused_if_only_create_allowed(self): - # Allow any authenticated to write in this bucket. + # Allow any authenticated to write in this collection. perm = {"permissions": {"record:create": ["system.Authenticated"]}} self.app.patch_json("/buckets/fennec/collections/fonts", perm, headers=self.headers) self.upload(status=201) @@ -296,8 +302,38 @@ def test_upload_replace_refused_if_only_create_allowed(self): self.headers.update(get_user_headers("jean-louis")) self.upload(status=403) + def test_upload_replace_refused_if_only_bucket_read_is_allowed(self): + # Create a record with attachment. + self.upload(status=201) + + # Now allow anyone to read this bucket. + perm = {"permissions": {"read": ["system.Everyone"]}} + self.app.patch_json("/buckets/fennec", perm, headers=self.headers) + + # And try to replace anonymously. + self.headers.pop("Authorization") + self.upload(status=401) + + def test_upload_replace_refused_if_only_read_is_allowed(self): + # Create a record with attachment. + self.upload(status=201) + + # Now allow anyone to read this collection. + perm_change = [ + {"op": "add", "path": "/permissions", "value": {"read": ["system.Everyone"]}} + ] + self.app.patch_json( + "/buckets/fennec/collections/fonts", + perm_change, + headers={**self.headers, "Content-Type": "application/json-patch+json"}, + ) + + # And try to replace anonymously. + self.headers.pop("Authorization") + self.upload(status=401) + def test_upload_create_accepted_if_create_allowed(self): - # Allow any authenticated to write in this bucket. + # Allow any authenticated to write in this collection. perm = {"permissions": {"record:create": ["system.Authenticated"]}} self.app.patch_json("/buckets/fennec/collections/fonts", perm, headers=self.headers)