From d55a867d90e059874db1db40e4ec10d066fbfb1e Mon Sep 17 00:00:00 2001 From: Christophe Henry Date: Mon, 28 Oct 2024 16:48:57 +0100 Subject: [PATCH] Harden security on referent's pages revent user that are not referent for this organisation but who are referent for another organisation from managing this organisation. --- aidants_connect_web/decorators.py | 11 ++++++++++- aidants_connect_web/tests/test_decorators.py | 18 ++++++++++++++---- .../test_espace_responsable.py | 19 +++++++++++++++---- .../views/espace_responsable.py | 16 ++++++++-------- 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/aidants_connect_web/decorators.py b/aidants_connect_web/decorators.py index 85aeb2120..f42bfbb6e 100644 --- a/aidants_connect_web/decorators.py +++ b/aidants_connect_web/decorators.py @@ -56,7 +56,16 @@ def user_is_responsable_structure(view=None, redirect_field_name="next"): """ def test(user): - return user.is_responsable_structure() + from aidants_connect_web.models import Organisation + + organisation = getattr(user, "organisation", None) + if not isinstance(organisation, Organisation): + return False + + return ( + Organisation.objects.filter(pk=organisation.pk, responsables=user).count() + > 0 + ) decorator = user_passes_test( test, diff --git a/aidants_connect_web/tests/test_decorators.py b/aidants_connect_web/tests/test_decorators.py index e2f784498..a869a877f 100644 --- a/aidants_connect_web/tests/test_decorators.py +++ b/aidants_connect_web/tests/test_decorators.py @@ -1,4 +1,5 @@ from django.conf import settings +from django.db import transaction from django.test import TestCase, tag from django.utils import timezone @@ -63,8 +64,8 @@ def setUpTestData(cls): cls.responsable_georges = AidantFactory( organisation=cls.aidant_thierry.organisation, can_create_mandats=False, + post__is_organisation_manager=True, ) - cls.responsable_georges.responsable_de.add(cls.aidant_thierry.organisation) cls.responsable_georges.responsable_de.add(OrganisationFactory()) def test_responsable_can_access_decorated_page(self): @@ -73,6 +74,15 @@ def test_responsable_can_access_decorated_page(self): self.assertEqual(response.status_code, 200) def test_non_responsable_user_cannot_access_decorated_page(self): - self.client.force_login(self.aidant_thierry) - response = self.client.get("/espace-responsable/organisation/") - self.assertEqual(response.status_code, 302) + with self.subTest("Aidant is not referent"): + self.client.force_login(self.aidant_thierry) + response = self.client.get("/espace-responsable/organisation/") + self.assertEqual(response.status_code, 302) + + with self.subTest("Aidant is referent of a different organisation"): + with transaction.atomic(): + self.aidant_thierry.responsable_de.add(OrganisationFactory()) + + self.client.force_login(self.aidant_thierry) + response = self.client.get("/espace-responsable/organisation/") + self.assertEqual(response.status_code, 302) diff --git a/aidants_connect_web/tests/test_views/test_espace_responsable/test_espace_responsable.py b/aidants_connect_web/tests/test_views/test_espace_responsable/test_espace_responsable.py index f9ca6f3fb..5236d8790 100644 --- a/aidants_connect_web/tests/test_views/test_espace_responsable/test_espace_responsable.py +++ b/aidants_connect_web/tests/test_views/test_espace_responsable/test_espace_responsable.py @@ -16,9 +16,7 @@ class EspaceResponsableOrganisationPage(TestCase): @classmethod def setUpTestData(cls): - cls.client = Client() - cls.responsable_tom = AidantFactory() - cls.responsable_tom.responsable_de.add(cls.responsable_tom.organisation) + cls.responsable_tom = AidantFactory(post__is_organisation_manager=True) cls.id_organisation = cls.responsable_tom.organisation.id cls.autre_organisation = OrganisationFactory() @@ -103,13 +101,26 @@ def test_I_must_select_at_least_one_demarche(self): ) self.assertEqual(response.status_code, 200) self.assertTemplateUsed( - response, "aidants_connect_web/espace_responsable/organisation.html" + response, espace_responsable.OrganisationView.template_name ) self.assertEqual( response.context_data["form"].errors["demarches"][0], "Vous devez sélectionner au moins une démarche.", ) + def test_I_cant_manage_organisation_without_rights(self): + self.responsable_rachida = AidantFactory(post__is_organisation_manager=True) + self.responsable_rachida.organisations.add(self.responsable_tom.organisation) + + response = self.client.post( + reverse("espace_responsable_organisation"), + data={"demarches": []}, + ) + self.assertNotEquals(response.status_code, 200) + self.assertTemplateNotUsed( + response, espace_responsable.OrganisationView.template_name + ) + @tag("responsable-structure") class EspaceResponsableAidantPage(TestCase): diff --git a/aidants_connect_web/views/espace_responsable.py b/aidants_connect_web/views/espace_responsable.py index e13c52eee..576f82905 100644 --- a/aidants_connect_web/views/espace_responsable.py +++ b/aidants_connect_web/views/espace_responsable.py @@ -82,18 +82,18 @@ class OrganisationView(DetailView, FormView): def dispatch(self, request, *args, **kwargs): self.referent: Aidant = request.user # Needed when following the FormView path - self.object = self.get_object() + self.organisation = self.get_object() return super().dispatch(request, *args, **kwargs) def get_object(self, queryset=None): - self.organisation: Organisation = self.referent.organisation - - if not self.organisation: - django_messages.error( - self.request, "Vous n'êtes pas rattaché à une organisation." + if not hasattr(self, "object"): + self.object = get_object_or_404( + Organisation, + pk=self.referent.organisation.pk, + responsables=self.referent, ) - return redirect("espace_aidant_home") - return self.organisation + + return self.object def get_context_data(self, **kwargs): return {