From e25f173c1964e7b98d444be3ef93733d335eeccf Mon Sep 17 00:00:00 2001 From: imperosol Date: Sun, 26 Apr 2026 22:35:13 +0200 Subject: [PATCH] apply review comments --- api/tests/test_third_party_auth.py | 28 +++++++++++++--- api/views.py | 51 +++++++++++++++++++++++------- locale/fr/LC_MESSAGES/django.po | 29 ++++++++++++----- 3 files changed, 84 insertions(+), 24 deletions(-) diff --git a/api/tests/test_third_party_auth.py b/api/tests/test_third_party_auth.py index 39faebce..1fca8ccb 100644 --- a/api/tests/test_third_party_auth.py +++ b/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 @@ class TestThirdPartyAuth(TestCase): 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 @@ class TestThirdPartyAuth(TestCase): 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.", + ) + ] diff --git a/api/views.py b/api/views.py index 6b66db03..9f519eaa 100644 --- a/api/views.py +++ b/api/views.py @@ -3,10 +3,11 @@ from urllib.parse import unquote 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.schemas import UserProfileSchema 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 @@ class ThirdPartyAuthView(LoginRequiredMixin, FormView): 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 @@ class ThirdPartyAuthView(LoginRequiredMixin, FormView): 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) diff --git a/locale/fr/LC_MESSAGES/django.po b/locale/fr/LC_MESSAGES/django.po index c2a94b6f..6af3ad84 100644 --- a/locale/fr/LC_MESSAGES/django.po +++ b/locale/fr/LC_MESSAGES/django.po @@ -6,7 +6,7 @@ msgid "" msgstr "" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2026-05-12 11:12+0200\n" +"POT-Creation-Date: 2026-05-23 15:09+0200\n" "PO-Revision-Date: 2016-07-18\n" "Last-Translator: Maréchal \n" @@ -148,14 +148,25 @@ msgid "" "href=\"%(sith_cgu_link)s\">the Students' Association applies as soon as " "the form is submitted." msgstr "" -"Les politiques de confidentialité de %(app)s et de l'Association des Etudiants s'appliquent dès la soumission " -"du formulaire." +"Les politiques de confidentialité de %(app)s et de l'Association des Etudiants " +"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,7 +178,9 @@ 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 "" @@ -175,9 +188,9 @@ msgid "" "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"