Skip to content

Commit

Permalink
Harden security on referent's pages
Browse files Browse the repository at this point in the history
revent user that are not referent for this organisation
but who are referent for another organisation from managing this organisation.
  • Loading branch information
christophehenry committed Oct 29, 2024
1 parent 9aa37e9 commit d55a867
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 17 deletions.
11 changes: 10 additions & 1 deletion aidants_connect_web/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 14 additions & 4 deletions aidants_connect_web/tests/test_decorators.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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):
Expand All @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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):
Expand Down
16 changes: 8 additions & 8 deletions aidants_connect_web/views/espace_responsable.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit d55a867

Please sign in to comment.