apply review comments

Author: imperosol

api/tests/test_third_party_auth.py

@@ -1,6 +1,7 @@
 from unittest import mock
 from unittest.mock import Mock
 
+from django.contrib.messages import Message, get_messages
 from django.db.models import Max
 from django.test import TestCase
 from django.urls import reverse
@@ -87,7 +88,15 @@ def test_wrong_signature(self):
         del self.query["signature"]
         self.query["signature"] = hmac_hexdigest(new_key, self.query)
         res = self.client.get(reverse("api-link:third-party-auth", query=self.query))
-        assert res.status_code == 403
+        assert list(get_messages(res.wsgi_request)) == [
+            Message(
+                level=40,
+                message=(
+                    "La signature est incorrecte. "
+                    "Nous ne pouvons pas garantir l'authenticité de la requête."
+                ),
+            )
+        ]
 
     def test_cgu_not_accepted(self):
         self.client.force_login(self.user)
@@ -102,13 +111,24 @@ def test_cgu_not_accepted(self):
         assert res.status_code == 200
 
     def test_invalid_client(self):
+        self.client.force_login(self.user)
         self.query["client_id"] = ApiClient.objects.aggregate(res=Max("id"))["res"] + 1
         res = self.client.get(reverse("api-link:third-party-auth", query=self.query))
-        assert res.status_code == 403
+        assert list(get_messages(res.wsgi_request)) == [
+            Message(
+                level=40,
+                message="Les données fournies pour l'authentification sont incorrectes.",
+            )
+        ]
 
     def test_missing_parameter(self):
-        """Test that a 403 is raised if there is a missing parameter."""
+        self.client.force_login(self.user)
         del self.query["username"]
         self.query["signature"] = hmac_hexdigest(self.api_client.hmac_key, self.query)
         res = self.client.get(reverse("api-link:third-party-auth", query=self.query))
-        assert res.status_code == 403
+        assert list(get_messages(res.wsgi_request)) == [
+            Message(
+                level=40,
+                message="Les données fournies pour l'authentification sont incorrectes.",
+            )
+        ]

api/views.py

@@ -3,10 +3,11 @@
 
 import pydantic
 import requests
+import sentry_sdk
 from django.conf import settings
 from django.contrib import messages
-from django.contrib.auth.mixins import LoginRequiredMixin
-from django.core.exceptions import PermissionDenied
+from django.contrib.auth.mixins import AccessMixin, LoginRequiredMixin
+from django.shortcuts import render
 from django.urls import reverse, reverse_lazy
 from django.utils.translation import gettext as _
 from django.views.generic import FormView, TemplateView
@@ -20,16 +21,19 @@
 from core.utils import hmac_hexdigest
 
 
-class ThirdPartyAuthView(LoginRequiredMixin, FormView):
+class ThirdPartyAuthView(AccessMixin, FormView):
     form_class = ThirdPartyAuthForm
     template_name = "api/third_party/auth.jinja"
     success_url = reverse_lazy("core:index")
 
-    def parse_params(self) -> ThirdPartyAuthParamsSchema:
+    def parse_params(self) -> ThirdPartyAuthParamsSchema | None:
         """Parse and check the authentication parameters.
 
-        Raises:
-            PermissionDenied: if the verification failed.
+        If parsing fails, messages will be created using the django message
+        infrastructure.
+
+        Returns:
+            The parses parameters, or None if the parsing failed.
         """
         # This is here rather than in ThirdPartyAuthForm because
         # the given parameters and their signature are checked during both
@@ -39,20 +43,39 @@ def parse_params(self) -> ThirdPartyAuthParamsSchema:
         params = {key: unquote(val) for key, val in params.items()}
         try:
             params = ThirdPartyAuthParamsSchema(**params)
-        except pydantic.ValidationError as e:
-            raise PermissionDenied("Wrong data format") from e
+        except pydantic.ValidationError:
+            messages.error(
+                self.request, _("The data provided for authentication is incorrect")
+            )
+            return None
         client: ApiClient = get_object_or_none(ApiClient, id=params.client_id)
         if not client:
-            raise PermissionDenied
+            messages.error(
+                self.request, _("The data provided for authentication is incorrect")
+            )
+            return None
         if not hmac.compare_digest(
             hmac_hexdigest(client.hmac_key, params.model_dump(exclude={"signature"})),
             params.signature,
         ):
-            raise PermissionDenied("Bad signature")
+            messages.error(
+                self.request,
+                _(
+                    "The signature is incorrect. "
+                    "We cannot ensure the provenance of the request."
+                ),
+            )
+            return None
         return params
 
     def dispatch(self, request, *args, **kwargs):
+        if not request.user.is_authenticated:
+            return self.handle_no_permission()
         self.params = self.parse_params()
+        if not self.params:
+            # if parameters parsing failed, shortcut the operation and display
+            # an empty page with just the error messages.
+            return render(request, "core/base.jinja")
         return super().dispatch(request, *args, **kwargs)
 
     def get(self, *args, **kwargs):
@@ -73,10 +96,14 @@ def form_valid(self, form):
         client = ApiClient.objects.get(id=form.cleaned_data["client_id"])
         user = UserProfileSchema.from_orm(self.request.user).model_dump()
         data = {"user": user, "signature": hmac_hexdigest(client.hmac_key, user)}
-        response = requests.post(form.cleaned_data["callback_url"], json=data)
+        try:
+            ok = requests.post(form.cleaned_data["callback_url"], json=data).ok
+        except requests.RequestException as e:
+            sentry_sdk.capture_exception(e)
+            ok = False
         self.success_url = reverse(
             "api-link:third-party-auth-result",
-            kwargs={"result": "success" if response.ok else "failure"},
+            kwargs={"result": "success" if ok else "failure"},
         )
         return super().form_valid(form)
 

locale/fr/LC_MESSAGES/django.po

@@ -148,14 +148,25 @@ msgid ""
 "href=\"%(sith_cgu_link)s\">the Students' Association</a> applies as soon as "
 "the form is submitted."
 msgstr ""
-"Les politiques de confidentialité de <a href=\"%(privacy_link)s\">%(app)s</a> et de <a "
-"href=\"%(sith_cgu_link)s\">l'Association des Etudiants</a> s'appliquent dès la soumission "
-"du formulaire."
+"Les politiques de confidentialité de <a href=\"%(privacy_link)s\">%(app)s</"
+"a> et de <a href=\"%(sith_cgu_link)s\">l'Association des Etudiants</a> "
+"s'appliquent dès la soumission du formulaire."
 
 #: api/templates/api/third_party/auth.jinja
 msgid "Confirmation of identity"
 msgstr "Confirmation d'identité"
 
+#: api/views.py
+msgid "The data provided for authentication is incorrect"
+msgstr "Les données fournies pour l'authentification sont incorrectes."
+
+#: api/views.py
+msgid ""
+"The signature is incorrect. We cannot ensure the provenance of the request."
+msgstr ""
+"La signature est incorrecte. Nous ne pouvons pas garantir l'authenticité de "
+"la requête."
+
 #: api/views.py
 #, python-format
 msgid ""
@@ -167,17 +178,19 @@ msgstr ""
 
 #: api/views.py
 msgid "You have been successfully authenticated. You can now close this page."
-msgstr "Vous avez été authentifié avec succès. Vous pouvez maintenant fermer cette page."
+msgstr ""
+"Vous avez été authentifié avec succès. Vous pouvez maintenant fermer cette "
+"page."
 
 #: api/views.py
 msgid ""
 "Your authentication on the AE website was successful, but an error happened "
 "during the interaction with the third-party application. Please contact the "
 "managers of the latter."
 msgstr ""
-"Votre authentification sur le site AE a fonctionné, mais une erreur est arrivée "
-"durant l'interaction avec l'application tierce. Veuillez contacter les responsables "
-"de cette dernière."
+"Votre authentification sur le site AE a fonctionné, mais une erreur est "
+"arrivée durant l'interaction avec l'application tierce. Veuillez contacter "
+"les responsables de cette dernière."
 
 #: club/forms.py
 msgid "Users to add"