diff --git a/CHANGELOG b/CHANGELOG index 74278d21..a709d62c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,4 @@ +- Si on essaie d'accéder au compte que qqn d'autre on a une 404 (et plus une 403) - On ne peut plus modifier des comptes COF depuis l'interface K-Fêt - Le champ de paiement BdA se fait au niveau des attributions - Affiche un message d'erreur plutôt que de crasher si échec de l'envoi du mail diff --git a/kfet/tests/test_statistic.py b/kfet/tests/test_statistic.py index f0ed7f74..eda386b7 100644 --- a/kfet/tests/test_statistic.py +++ b/kfet/tests/test_statistic.py @@ -36,8 +36,7 @@ class TestStats(TestCase): client2 = Client() client2.login(username="Barfoo", password="barfoo") - # 1. FOO should be able to get these pages but BAR receives a Forbidden - # response + # 1. FOO should be able to get these pages but BAR receives a 404 user_urls = [ "/k-fet/accounts/FOO/stat/operations/list", "/k-fet/accounts/FOO/stat/operations?{}".format( @@ -57,7 +56,7 @@ class TestStats(TestCase): resp = client.get(url) self.assertEqual(200, resp.status_code) resp2 = client2.get(url) - self.assertEqual(403, resp2.status_code) + self.assertEqual(404, resp2.status_code) # 2. FOO is a member of the team and can get these pages but BAR # receives a Redirect response diff --git a/kfet/tests/test_views.py b/kfet/tests/test_views.py index cb6c9a0c..a5b564cb 100644 --- a/kfet/tests/test_views.py +++ b/kfet/tests/test_views.py @@ -209,6 +209,25 @@ class AccountReadViewTests(ViewTestCaseMixin, TestCase): auth_user = "team" auth_forbidden = [None, "user"] + # Users with forbidden access users should get a 404 here, to avoid leaking trigrams + # See issue #224 + def test_forbidden(self): + for user in self.auth_forbidden: + self.assertRedirectsToLoginOr404(user, self.url_expected) + self.assertRedirectsToLoginOr404(user, "/k-fet/accounts/NEX") + + def assertRedirectsToLoginOr404(self, user, url): + client = Client() + if user is None: + response = client.get(url) + self.assertRedirects( + response, "/login?next={}".format(url), fetch_redirect_response=False + ) + else: + client.login(username=user, password=user) + response = client.get(url) + self.assertEqual(response.status_code, 404) + def get_users_extra(self): return {"user1": create_user("user1", "001")} @@ -296,6 +315,29 @@ class AccountUpdateViewTests(ViewTestCaseMixin, TestCase): "team1": create_team("team1", "101", perms=["kfet.change_account"]), } + # Users with forbidden access users should get a 404 here, to avoid leaking trigrams + # See issue #224 + def test_forbidden(self): + for method in ["get", "post"]: + for user in self.auth_forbidden: + self.assertRedirectsToLoginOr404(user, method, self.url_expected) + self.assertRedirectsToLoginOr404( + user, method, "/k-fet/accounts/NEX/edit" + ) + + def assertRedirectsToLoginOr404(self, user, method, url): + client = Client() + meth = getattr(client, method) + if user is None: + response = meth(url) + self.assertRedirects( + response, "/login?next={}".format(url), fetch_redirect_response=False + ) + else: + client.login(username=user, password=user) + response = meth(url) + self.assertEqual(response.status_code, 404) + def test_get_ok(self): r = self.client.get(self.url) self.assertEqual(r.status_code, 200) @@ -375,7 +417,7 @@ class AccountDeleteViewTests(ViewTestCaseMixin, TestCase): if Account.objects.get(trigramme=trigramme).readable: expected_code = 200 else: - expected_code = 403 + expected_code = 404 r = self.client.post( reverse(self.url_name, kwargs={"trigramme": trigramme}), {} ) @@ -555,6 +597,27 @@ class AccountStatOperationListViewTests(ViewTestCaseMixin, TestCase): def get_users_extra(self): return {"user1": create_user("user1", "001")} + # Users with forbidden access users should get a 404 here, to avoid leaking trigrams + # See issue #224 + def test_forbidden(self): + for user in self.auth_forbidden: + self.assertRedirectsToLoginOr404(user, self.url_expected) + self.assertRedirectsToLoginOr404( + user, "/k-fet/accounts/NEX/stat/operations/list" + ) + + def assertRedirectsToLoginOr404(self, user, url): + client = Client() + if user is None: + response = client.get(url) + self.assertRedirects( + response, "/login?next={}".format(url), fetch_redirect_response=False + ) + else: + client.login(username=user, password=user) + response = client.get(url) + self.assertEqual(response.status_code, 404) + def test_ok(self): r = self.client.get(self.url) self.assertEqual(r.status_code, 200) @@ -616,6 +679,27 @@ class AccountStatOperationViewTests(ViewTestCaseMixin, TestCase): auth_user = "user1" auth_forbidden = [None, "user", "team"] + # Users with forbidden access users should get a 404 here, to avoid leaking trigrams + # See issue #224 + def test_forbidden(self): + for user in self.auth_forbidden: + self.assertRedirectsToLoginOr404(user, self.url_expected) + self.assertRedirectsToLoginOr404( + user, "/k-fet/accounts/NEX/stat/operations" + ) + + def assertRedirectsToLoginOr404(self, user, url): + client = Client() + if user is None: + response = client.get(url) + self.assertRedirects( + response, "/login?next={}".format(url), fetch_redirect_response=False + ) + else: + client.login(username=user, password=user) + response = client.get(url) + self.assertEqual(response.status_code, 404) + def get_users_extra(self): return {"user1": create_user("user1", "001")} @@ -632,6 +716,27 @@ class AccountStatBalanceListViewTests(ViewTestCaseMixin, TestCase): auth_user = "user1" auth_forbidden = [None, "user", "team"] + # Users with forbidden access users should get a 404 here, to avoid leaking trigrams + # See issue #224 + def test_forbidden(self): + for user in self.auth_forbidden: + self.assertRedirectsToLoginOr404(user, self.url_expected) + self.assertRedirectsToLoginOr404( + user, "/k-fet/accounts/NEX/stat/balance/list" + ) + + def assertRedirectsToLoginOr404(self, user, url): + client = Client() + if user is None: + response = client.get(url) + self.assertRedirects( + response, "/login?next={}".format(url), fetch_redirect_response=False + ) + else: + client.login(username=user, password=user) + response = client.get(url) + self.assertEqual(response.status_code, 404) + def get_users_extra(self): return {"user1": create_user("user1", "001")} @@ -677,6 +782,25 @@ class AccountStatBalanceViewTests(ViewTestCaseMixin, TestCase): auth_user = "user1" auth_forbidden = [None, "user", "team"] + # Users with forbidden access users should get a 404 here, to avoid leaking trigrams + # See issue #224 + def test_forbidden(self): + for user in self.auth_forbidden: + self.assertRedirectsToLoginOr404(user, self.url_expected) + self.assertRedirectsToLoginOr404(user, "/k-fet/accounts/NEX/stat/balance") + + def assertRedirectsToLoginOr404(self, user, url): + client = Client() + if user is None: + response = client.get(url) + self.assertRedirects( + response, "/login?next={}".format(url), fetch_redirect_response=False + ) + else: + client.login(username=user, password=user) + response = client.get(url) + self.assertEqual(response.status_code, 404) + def get_users_extra(self): return {"user1": create_user("user1", "001")} diff --git a/kfet/views.py b/kfet/views.py index 4c8c6f47..dde4f24e 100644 --- a/kfet/views.py +++ b/kfet/views.py @@ -10,7 +10,6 @@ from django.contrib.auth.decorators import login_required, permission_required from django.contrib.auth.mixins import PermissionRequiredMixin from django.contrib.auth.models import Permission, User from django.contrib.messages.views import SuccessMessageMixin -from django.core.exceptions import PermissionDenied from django.db import transaction from django.db.models import Count, F, Prefetch, Sum from django.forms import formset_factory @@ -303,7 +302,7 @@ def account_read(request, trigramme): if not account.readable or ( not request.user.has_perm("kfet.is_team") and request.user != account.user ): - raise PermissionDenied + raise Http404 addcosts = ( OperationGroup.objects.filter(opes__addcost_for=account, opes__canceled_at=None) @@ -327,7 +326,7 @@ def account_update(request, trigramme): # Checking permissions if not request.user.has_perm("kfet.is_team") and request.user != account.user: - raise PermissionDenied + raise Http404 user_info_form = UserInfoForm(instance=account.user) @@ -2226,7 +2225,7 @@ class AccountStatBalanceList(PkUrlMixin, SingleResumeStat): def get_object(self, *args, **kwargs): obj = super().get_object(*args, **kwargs) if self.request.user != obj.user: - raise PermissionDenied + raise Http404 return obj @method_decorator(login_required) @@ -2345,7 +2344,7 @@ class AccountStatBalance(PkUrlMixin, JSONDetailView): def get_object(self, *args, **kwargs): obj = super().get_object(*args, **kwargs) if self.request.user != obj.user: - raise PermissionDenied + raise Http404 return obj @method_decorator(login_required) @@ -2376,7 +2375,7 @@ class AccountStatOperationList(PkUrlMixin, SingleResumeStat): def get_object(self, *args, **kwargs): obj = super().get_object(*args, **kwargs) if self.request.user != obj.user: - raise PermissionDenied + raise Http404 return obj @method_decorator(login_required) @@ -2439,7 +2438,7 @@ class AccountStatOperation(ScaleMixin, PkUrlMixin, JSONDetailView): def get_object(self, *args, **kwargs): obj = super().get_object(*args, **kwargs) if self.request.user != obj.user: - raise PermissionDenied + raise Http404 return obj @method_decorator(login_required)