feat(Proof): improve the permission strategy

open_prices/api/prices/tests.py

@@ -330,14 +330,17 @@ def setUpTestData(cls):
         cls.user_proof = ProofFactory(
             type=proof_constants.TYPE_RECEIPT, owner=cls.user_session.user.user_id
         )
-        cls.proof_2 = ProofFactory()
+        cls.proof_2 = ProofFactory(type=proof_constants.TYPE_PRICE_TAG)
         cls.data = {
             **PRICE_8001505005707,
             "location_osm_id": 652825274,
             "location_osm_type": "NODE",
             "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

@@ -72,7 +72,8 @@ def test_proof_list(self):
         # thanks to select_related, we only have 2 queries:
         # - 1 to count the number of proofs of the user
         # - 1 to get the proofs and their associated locations (select_related)
-        with self.assertNumQueries(2):
+        # - 1 to get the proof predictions (prefetch_related)
+        with self.assertNumQueries(3):
             response = self.client.get(self.url)
             self.assertEqual(response.status_code, 200)
             data = response.data
@@ -110,10 +111,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(type=proof_constants.TYPE_PRICE_TAG, 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,
@@ -122,8 +125,10 @@ def setUpTestData(cls):
 
     def test_proof_list_filter_by_type(self):
         url = self.url + "?type=RECEIPT"
-        response = self.client.get(url)
-        self.assertEqual(response.data["total"], 1)
+        response = self.client.get(
+            url, headers={"Authorization": f"Bearer {self.user_session.token}"}
+        )
+        self.assertEqual(response.data["total"], 2)
         self.assertEqual(response.data["items"][0]["price_count"], 15)
 
     def test_proof_list_filter_by_owner(self):
@@ -156,6 +161,12 @@ def test_proof_detail(self):
         response = self.client.get(self.url)
         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}"}
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data["id"], self.proof.id)
         self.assertIn("predictions", response.data)  # returned in "detail"
         self.assertEqual(len(response.data["predictions"]), 1)
         prediction = response.data["predictions"][0]
@@ -186,14 +197,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,
@@ -325,6 +333,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
         )
@@ -337,6 +347,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):
@@ -368,3 +383,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 IsAuthenticatedOrReadOnly
+from rest_framework.permissions import BasePermission
 from rest_framework.request import Request
 from rest_framework.response import Response
 
@@ -24,6 +24,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,
@@ -32,7 +48,7 @@ class ProofViewSet(
     viewsets.GenericViewSet,
 ):
     authentication_classes = [CustomAuthentication]
-    permission_classes = [IsAuthenticatedOrReadOnly]
+    permission_classes = [ProofPermission]
     http_method_names = ["get", "post", "patch", "delete"]  # disable "put"
     queryset = Proof.objects.all()
     serializer_class = ProofFullSerializer
@@ -42,16 +58,22 @@ class ProofViewSet(
     ordering = ["created"]
 
     def get_queryset(self):
-        queryset = self.queryset
         if self.request.method in ["GET"]:
-            queryset = queryset.select_related("location")
-            if self.action == "retrieve":
-                queryset = queryset.prefetch_related("predictions")
+            # 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"]:
-            # only return proofs owned by the current user
-            if self.request.user.is_authenticated:
-                queryset = queryset.filter(owner=self.request.user.user_id)
-        return queryset
+            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):
         if self.request.method == "PATCH":
@@ -99,8 +121,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,11 +463,16 @@ def clean(self, *args, **kwargs):
                 )
 
             if proof:
-                if proof.owner != self.owner:
+                if (
+                    proof.owner != self.owner
+                    and proof.type != proof_constants.TYPE_PRICE_TAG
+                ):
                     validation_errors = utils.add_validation_error(
                         validation_errors,
                         "proof",
-                        "Proof does not belong to the current user",
+                        "Proof does not belong to the current user. "
+                        "Adding price to proof a user does not own is "
+                        "only allowed for PRICE_TAG proofs",
                     )
                 if not self.id:  # skip these checks on update
                     if proof.type in proof_constants.TYPE_SINGLE_SHOP_LIST:

open_prices/prices/tests.py

@@ -358,7 +358,14 @@ def test_price_proof_validation(self):
             currency="EUR",
             owner=user_session.user.user_id,
         )
-        proof_2 = ProofFactory()
+        proof_gdpr = ProofFactory(
+            type=proof_constants.TYPE_GDPR_REQUEST,
+            location_osm_id=169450062,
+            location_osm_type=location_constants.OSM_TYPE_NODE,
+            date="2024-10-01",
+            currency="EUR",
+        )
+        proof_price_tag = ProofFactory(type=proof_constants.TYPE_PRICE_TAG)
         # proof not set
         PriceFactory(proof_id=None, owner=user_proof_receipt.owner)
         # proof unknown
@@ -374,16 +381,27 @@ def test_price_proof_validation(self):
             currency=user_proof_receipt.currency,
             owner=user_proof_receipt.owner,
         )
-        # different price & proof owner
+        # difference price and proof owner should raise a ValidationError
+        # if the proof type is different than PRICE_TAG
         self.assertRaises(
             ValidationError,
             PriceFactory,
-            proof_id=proof_2.id,  # different
+            proof_id=proof_gdpr.id,  # gdpr proof
+            location_osm_id=proof_gdpr.location_osm_id,
+            location_osm_type=proof_gdpr.location_osm_type,
+            date=proof_gdpr.date,
+            currency=proof_gdpr.currency,
+            owner=user_proof_receipt.owner,  # different owner
+        )
+        # different price & proof owner should not raise a ValidationError
+        # for PRICE_TAG proofs
+        PriceFactory(
+            proof_id=proof_price_tag.id,
             location_osm_id=user_proof_receipt.location_osm_id,
             location_osm_type=user_proof_receipt.location_osm_type,
             date=user_proof_receipt.date,
             currency=user_proof_receipt.currency,
-            owner=user_proof_receipt.owner,
+            owner=user_proof_receipt.owner,  # different owner
         )
         # proof location_osm_id & location_osm_type
         self.assertRaises(

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/proofs/tests.py

@@ -155,7 +155,11 @@ class ProofQuerySetTest(TestCase):
     def setUpTestData(cls):
         cls.proof_without_price = ProofFactory(type=proof_constants.TYPE_PRICE_TAG)
         cls.proof_with_price = ProofFactory(type=proof_constants.TYPE_GDPR_REQUEST)
-        PriceFactory(proof_id=cls.proof_with_price.id, price=1.0)
+        PriceFactory(
+            proof_id=cls.proof_with_price.id,
+            price=1.0,
+            owner=cls.proof_with_price.owner,
+        )
 
     def test_has_type_single_shop(self):
         self.assertEqual(Proof.objects.count(), 2)
@@ -204,6 +208,7 @@ def setUpTestData(cls):
             price=2.0,
             currency="EUR",
             date="2024-06-30",
+            owner=cls.proof_receipt.owner,
         )
 
     def test_is_type_single_shop(self):

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