feat(Proof): improve the permission strategy

open_prices/api/prices/tests.py

@@ -338,6 +338,9 @@ def setUpTestData(cls):
             "proof_id": cls.user_proof.id,
             "source": "test",
         }
+        cls.data_2 = cls.data.copy()
+        cls.data_2["product_code"] = "1402506209800"
+        cls.data_2["location_osm_id"] = 169424088
 
     def test_price_create_without_proof(self):
         data = self.data.copy()
@@ -383,14 +386,15 @@ def test_price_create_with_proof(self):
             content_type="application/json",
         )
         self.assertEqual(response.status_code, 400)
-        # not proof owner
+        # Users are allowed to add price on proofs they don't own
         response = self.client.post(
             self.url,
-            {**self.data, "proof_id": self.proof_2.id},
+            {**self.data_2, "proof_id": self.proof_2.id},
             headers={"Authorization": f"Bearer {self.user_session.token}"},
             content_type="application/json",
         )
-        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.status_code, 201)
+        self.assertEqual(response.data["product_code"], "1402506209800")
         # authenticated
         response = self.client.post(
             self.url,

open_prices/api/proofs/tests.py

@@ -68,14 +68,10 @@ def setUpTestData(cls):
         )
 
     def test_proof_list(self):
-        # anonymous
+        # all proofs are read-accessible to anyone
         response = self.client.get(self.url)
-        self.assertEqual(response.status_code, 403)
-        # wrong token
-        response = self.client.get(
-            self.url, headers={"Authorization": f"Bearer {self.user_session.token}X"}
-        )
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data["total"], 3)  # All proofs are returned
         # authenticated
         # thanks to select_related and prefetch_related, we only have 6
         # queries:
@@ -91,8 +87,8 @@ def test_proof_list(self):
             )
             self.assertEqual(response.status_code, 200)
             data = response.data
-            self.assertEqual(data["total"], 2)  # only user's proofs
-            self.assertEqual(len(data["items"]), 2)
+            self.assertEqual(data["total"], 3)  # only user's proofs
+            self.assertEqual(len(data["items"]), 3)
             item = data["items"][0]
             self.assertEqual(item["id"], self.proof.id)  # default order
             self.assertIn("predictions", item)
@@ -125,7 +121,7 @@ def test_proof_list_order_by(self):
         response = self.client.get(
             url, headers={"Authorization": f"Bearer {self.user_session.token}"}
         )
-        self.assertEqual(response.data["total"], 2)
+        self.assertEqual(response.data["total"], 3)
         self.assertEqual(response.data["items"][0]["price_count"], 50)
 
 
@@ -134,10 +130,12 @@ class ProofListFilterApiTest(TestCase):
     def setUpTestData(cls):
         cls.url = reverse("api:proofs-list")
         cls.user_session = SessionFactory()
+        # type RECEIPT
         cls.proof = ProofFactory(
             **PROOF, price_count=15, owner=cls.user_session.user.user_id
         )
-        ProofFactory(price_count=0)
+        # type RECEIPT, but not owned by the user
+        ProofFactory(type=proof_constants.TYPE_RECEIPT, price_count=15)
         ProofFactory(
             type=proof_constants.TYPE_PRICE_TAG,
             price_count=50,
@@ -149,7 +147,7 @@ def test_proof_list_filter_by_type(self):
         response = self.client.get(
             url, headers={"Authorization": f"Bearer {self.user_session.token}"}
         )
-        self.assertEqual(response.data["total"], 1)
+        self.assertEqual(response.data["total"], 2)
         self.assertEqual(response.data["items"][0]["price_count"], 15)
 
 
@@ -173,12 +171,8 @@ def test_proof_detail(self):
         self.assertEqual(response.data["detail"], "No Proof matches the given query.")
         # anonymous
         response = self.client.get(self.url)
-        self.assertEqual(response.status_code, 403)
-        # wrong token
-        response = self.client.get(
-            self.url, headers={"Authorization": f"Bearer {self.user_session_1.token}X"}
-        )
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data["id"], self.proof.id)
         # authenticated
         response = self.client.get(
             self.url, headers={"Authorization": f"Bearer {self.user_session_1.token}"}
@@ -207,14 +201,11 @@ def tearDown(self):
     def test_proof_create(self):
         # anonymous
         response = self.client.post(self.url, self.data)
-        self.assertEqual(response.status_code, 403)
-        # wrong token
-        response = self.client.post(
-            self.url,
-            self.data,
-            headers={"Authorization": f"Bearer {self.user_session.token}X"},
-        )
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 201)
+        self.assertTrue(response.data["file_path"] is not None)
+        self.assertEqual(response.data["owner"], None)
+        self.assertEqual(response.data["anonymous"], True)
+
         # authenticated
         response = self.client.post(
             self.url,
@@ -346,6 +337,8 @@ class ProofDeleteApiTest(TestCase):
     def setUpTestData(cls):
         cls.user_session_1 = SessionFactory()
         cls.user_session_2 = SessionFactory()
+        cls.user_session_3 = SessionFactory()
+        cls.moderator = SessionFactory(user__is_moderator=True)
         cls.proof = ProofFactory(
             **PROOF, price_count=15, owner=cls.user_session_1.user.user_id
         )
@@ -358,6 +351,11 @@ def setUpTestData(cls):
             date=cls.proof.date,
             owner=cls.proof.owner,
         )
+        proof_2_data = PROOF.copy()
+        proof_2_data["date"] = "2024-06-06"
+        cls.proof_2 = ProofFactory(
+            **proof_2_data, price_count=0, owner=cls.user_session_3.user.user_id
+        )
         cls.url = reverse("api:proofs-detail", args=[cls.proof.id])
 
     def test_proof_delete(self):
@@ -389,3 +387,14 @@ def test_proof_delete(self):
         self.assertEqual(
             Proof.objects.filter(owner=self.user_session_1.user.user_id).count(), 0
         )
+
+        # Delete request from a moderator, should work
+        # Proof 1 was deleted, let's delete proof 2
+        url_proof_2 = reverse("api:proofs-detail", args=[self.proof_2.id])
+        response = self.client.delete(
+            url_proof_2,
+            headers={"Authorization": f"Bearer {self.moderator.token}"},
+        )
+        self.assertEqual(response.status_code, 204)
+        self.assertEqual(response.data, None)
+        self.assertEqual(Proof.objects.filter(id=self.proof_2.id).count(), 0)

open_prices/api/proofs/views.py

@@ -4,7 +4,7 @@
 from rest_framework import filters, mixins, status, viewsets
 from rest_framework.decorators import action
 from rest_framework.parsers import MultiPartParser
-from rest_framework.permissions import IsAuthenticated
+from rest_framework.permissions import BasePermission
 from rest_framework.request import Request
 from rest_framework.response import Response
 
@@ -23,6 +23,22 @@
 from open_prices.proofs.utils import store_file
 
 
+class ProofPermission(BasePermission):
+    def has_permission(self, request, view):
+        if view.action in ("retrieve", "list"):
+            # Allow any user (even anonymous) to view any proof
+            return True
+        elif request.method == "POST" and request.path.startswith(
+            "/api/v1/proofs/upload"
+        ):
+            # Allow anonymous users to upload proofs
+            return True
+
+        # Require authenticated user for the rest ("destroy", "update",
+        # Gemini proof processing)
+        return request.user and request.user.is_authenticated
+
+
 class ProofViewSet(
     mixins.ListModelMixin,
     mixins.RetrieveModelMixin,
@@ -31,28 +47,31 @@ class ProofViewSet(
     viewsets.GenericViewSet,
 ):
     authentication_classes = [CustomAuthentication]
-    permission_classes = [IsAuthenticated]
+    permission_classes = [ProofPermission]
     http_method_names = ["get", "post", "patch", "delete"]  # disable "put"
-    queryset = Proof.objects.none()
+    queryset = Proof.objects.all()
     serializer_class = ProofFullSerializer
     filter_backends = [DjangoFilterBackend, filters.OrderingFilter]
     filterset_class = ProofFilter
     ordering_fields = ["date", "price_count", "created"]
     ordering = ["created"]
 
     def get_queryset(self):
-        # only return proofs owned by the current user
-        if self.request.user.is_authenticated:
-            queryset = Proof.objects.filter(owner=self.request.user.user_id)
-            if self.request.method in ["GET"]:
-                # Select all proofs along with their locations using a select
-                # related query (1 single query)
-                # Then prefetch all the predictions related to the proof using
-                # a prefetch related query (only 1 query for all proofs)
-                return queryset.select_related("location").prefetch_related(
-                    "predictions"
-                )
-            return queryset
+        if self.request.method in ["GET"]:
+            # Select all proofs along with their locations using a select
+            # related query (1 single query)
+            # Then prefetch all the predictions related to the proof using
+            # a prefetch related query (only 1 query for all proofs)
+            return self.queryset.select_related("location").prefetch_related(
+                "predictions"
+            )
+        elif self.request.method in ["PATCH", "DELETE"]:
+            if self.request.user.is_moderator:
+                return self.queryset
+            # for patch and delete actions, only return proofs owned by the
+            # current user if not a moderator
+            return self.queryset.filter(owner=self.request.user.user_id)
+
         return self.queryset
 
     def get_serializer_class(self):
@@ -99,8 +118,13 @@ def upload(self, request: Request) -> Response:
         serializer.is_valid(raise_exception=True)
         # get source
         source = get_source_from_request(self.request)
+        # Here, user is either a `open_prices.users.models.User` (if
+        # authenticated) or an `django.contrib.auth.models.AnonymousUser`
+        # (if not authenticated)
+        user = self.request.user
+        owner = None if user.is_anonymous else user.user_id
         # save
-        proof = serializer.save(owner=self.request.user.user_id, source=source)
+        proof = serializer.save(owner=owner, source=source, anonymous=user.is_anonymous)
         # return full proof
         return Response(ProofFullSerializer(proof).data, status=status.HTTP_201_CREATED)
 

open_prices/prices/models.py

@@ -463,12 +463,6 @@ def clean(self, *args, **kwargs):
                 )
 
             if proof:
-                if proof.owner != self.owner:
-                    validation_errors = utils.add_validation_error(
-                        validation_errors,
-                        "proof",
-                        "Proof does not belong to the current user",
-                    )
                 if not self.id:  # skip these checks on update
                     if proof.type in proof_constants.TYPE_SINGLE_SHOP_LIST:
                         for PROOF_FIELD in Price.DUPLICATE_PROOF_FIELDS:

open_prices/prices/tests.py

@@ -374,10 +374,8 @@ def test_price_proof_validation(self):
             currency=user_proof_receipt.currency,
             owner=user_proof_receipt.owner,
         )
-        # different price & proof owner
-        self.assertRaises(
-            ValidationError,
-            PriceFactory,
+        # different price & proof owner should not raise a ValidationError
+        PriceFactory(
             proof_id=proof_2.id,  # different
             location_osm_id=user_proof_receipt.location_osm_id,
             location_osm_type=user_proof_receipt.location_osm_type,

open_prices/proofs/factories.py

@@ -12,6 +12,7 @@ class ProofFactory(DjangoModelFactory):
     class Meta:
         model = Proof
 
+    owner = factory.Faker("user_name")
     file_path = factory.Faker("file_path")
     mimetype = "image/jpeg"
     type = factory.fuzzy.FuzzyChoice(proof_constants.TYPE_LIST)

open_prices/proofs/migrations/0007_proof_anonymous.py

@@ -0,0 +1,17 @@
+# Generated by Django 5.1 on 2024-12-05 14:21
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("proofs", "0006_add_proof_prediction_table"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="proof",
+            name="anonymous",
+            field=models.BooleanField(default=False),
+        ),
+    ]

open_prices/proofs/models.py

@@ -99,6 +99,7 @@ class Proof(models.Model):
     price_count = models.PositiveIntegerField(default=0, blank=True, null=True)
 
     owner = models.CharField(blank=True, null=True)
+    anonymous = models.BooleanField(default=False)
     source = models.CharField(blank=True, null=True)
 
     created = models.DateTimeField(default=timezone.now)

open_prices/users/models.py

@@ -33,9 +33,14 @@ class Meta:
         verbose_name = "User"
         verbose_name_plural = "Users"
 
+    @property
     def is_authenticated(self):
         return True
 
+    @property
+    def is_anonymous(self):
+        return False
+
     def update_price_count(self):
         from open_prices.prices.models import Price