From 64c792b11ffb67387f0d402728d3b086bb1cfe3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Sat, 21 Dec 2019 16:26:59 +0100 Subject: [PATCH] Disambiguation in kfet's permission handling In some places we used to refer to permissions based on their codename only (the part after the dot "." in the following examples) which can be ambiguous. Typically, we might define permissions like "bds.is_team" or "cof.is_team" in the near future ;) --- kfet/management/commands/loadkfetdevdata.py | 8 +++----- kfet/open/tests.py | 14 ++++++++++---- kfet/tests/test_statistic.py | 2 +- kfet/tests/test_views.py | 17 ++++++++++------- kfet/views.py | 21 +++++++++++++-------- 5 files changed, 37 insertions(+), 25 deletions(-) diff --git a/kfet/management/commands/loadkfetdevdata.py b/kfet/management/commands/loadkfetdevdata.py index 0543be80..43154d6e 100644 --- a/kfet/management/commands/loadkfetdevdata.py +++ b/kfet/management/commands/loadkfetdevdata.py @@ -6,7 +6,7 @@ import os import random from datetime import timedelta -from django.contrib.auth.models import ContentType, Group, Permission, User +from django.contrib.auth.models import Group, Permission, User from django.core.management import call_command from django.utils import timezone @@ -41,11 +41,9 @@ class Command(MyBaseCommand): group_chef.save() group_boy.save() - permissions_chef = Permission.objects.filter( - content_type__in=ContentType.objects.filter(app_label="kfet") - ) + permissions_chef = Permission.objects.filter(content_type__app_label="kfet",) permissions_boy = Permission.objects.filter( - codename__in=["is_team", "perform_deposit"] + content_type__app_label="kfet", codename__in=["is_team", "perform_deposit"] ) group_chef.permissions.add(*permissions_chef) diff --git a/kfet/open/tests.py b/kfet/open/tests.py index 4e652cb6..ae9bfa4b 100644 --- a/kfet/open/tests.py +++ b/kfet/open/tests.py @@ -84,7 +84,8 @@ class OpenKfetTest(ChannelTestCase): def test_export_team(self): """Export all values for a team member.""" user = User.objects.create_user("team", "", "team") - user.user_permissions.add(Permission.objects.get(codename="is_team")) + is_team = Permission.objects.get_by_natural_key("is_team", "kfet", "account") + user.user_permissions.add(is_team) export = self.kfet_open.export(user) self.assertSetEqual(set(["status", "admin_status", "force_close"]), set(export)) @@ -114,8 +115,12 @@ class OpenKfetViewsTest(ChannelTestCase): # get some permissions perms = { - "kfet.is_team": Permission.objects.get(codename="is_team"), - "kfet.can_force_close": Permission.objects.get(codename="can_force_close"), + "kfet.is_team": Permission.objects.get_by_natural_key( + "is_team", "kfet", "account" + ), + "kfet.can_force_close": Permission.objects.get_by_natural_key( + "can_force_close", "kfet", "account" + ), } # authenticated user and its client @@ -199,7 +204,8 @@ class OpenKfetConsumerTest(ChannelTestCase): """Team user is added to kfet.open.team group.""" # setup team user and its client t = User.objects.create_user("team", "", "team") - t.user_permissions.add(Permission.objects.get(codename="is_team")) + is_team = Permission.objects.get_by_natural_key("is_team", "kfet", "account") + t.user_permissions.add(is_team) c = WSClient() c.force_login(t) diff --git a/kfet/tests/test_statistic.py b/kfet/tests/test_statistic.py index eda386b7..9fafada4 100644 --- a/kfet/tests/test_statistic.py +++ b/kfet/tests/test_statistic.py @@ -18,7 +18,7 @@ class TestStats(TestCase): user.set_password("foobar") user.save() Account.objects.create(trigramme="FOO", cofprofile=user.profile) - perm = Permission.objects.get(codename="is_team") + perm = Permission.objects.get_by_natural_key("is_team", "kfet", "account") user.user_permissions.add(perm) user2 = User.objects.create(username="Barfoo") diff --git a/kfet/tests/test_views.py b/kfet/tests/test_views.py index decc8915..0a5c4e49 100644 --- a/kfet/tests/test_views.py +++ b/kfet/tests/test_views.py @@ -1855,7 +1855,7 @@ class KPsulPerformOperationsViewTests(ViewTestCaseMixin, TestCase): json_data = json.loads(resp.content.decode("utf-8")) self.assertEqual( json_data["errors"]["missing_perms"], - ["Enregistrer des commandes avec commentaires"], + ["[kfet] Enregistrer des commandes avec commentaires"], ) def test_group_on_acc_frozen(self): @@ -1898,7 +1898,7 @@ class KPsulPerformOperationsViewTests(ViewTestCaseMixin, TestCase): self.assertEqual(resp.status_code, 403) json_data = json.loads(resp.content.decode("utf-8")) self.assertEqual( - json_data["errors"]["missing_perms"], ["Forcer le gel d'un compte"] + json_data["errors"]["missing_perms"], ["[kfet] Forcer le gel d'un compte"] ) def test_invalid_group_checkout(self): @@ -2373,7 +2373,9 @@ class KPsulPerformOperationsViewTests(ViewTestCaseMixin, TestCase): self.assertEqual(resp.status_code, 403) json_data = json.loads(resp.content.decode("utf-8")) - self.assertEqual(json_data["errors"]["missing_perms"], ["Effectuer une charge"]) + self.assertEqual( + json_data["errors"]["missing_perms"], ["[kfet] Effectuer une charge"] + ) def test_withdraw(self): data = dict( @@ -2648,7 +2650,8 @@ class KPsulPerformOperationsViewTests(ViewTestCaseMixin, TestCase): self.assertEqual(resp.status_code, 403) json_data = json.loads(resp.content.decode("utf-8")) self.assertEqual( - json_data["errors"]["missing_perms"], ["Modifier la balance d'un compte"] + json_data["errors"]["missing_perms"], + ["[kfet] Modifier la balance d'un compte"], ) def test_invalid_edit_expects_comment(self): @@ -2956,7 +2959,7 @@ class KPsulPerformOperationsViewTests(ViewTestCaseMixin, TestCase): json_data = json.loads(resp.content.decode("utf-8")) self.assertEqual( json_data["errors"], - {"missing_perms": ["Enregistrer des commandes en négatif"]}, + {"missing_perms": ["[kfet] Enregistrer des commandes en négatif"]}, ) def test_invalid_negative_exceeds_allowed_duration_from_config(self): @@ -3780,7 +3783,7 @@ class KPsulCancelOperationsViewTests(ViewTestCaseMixin, TestCase): json_data = json.loads(resp.content.decode("utf-8")) self.assertEqual( json_data["errors"], - {"missing_perms": ["Annuler des commandes non récentes"]}, + {"missing_perms": ["[kfet] Annuler des commandes non récentes"]}, ) def test_already_canceled(self): @@ -3926,7 +3929,7 @@ class KPsulCancelOperationsViewTests(ViewTestCaseMixin, TestCase): json_data = json.loads(resp.content.decode("utf-8")) self.assertEqual( json_data["errors"], - {"missing_perms": ["Enregistrer des commandes en négatif"]}, + {"missing_perms": ["[kfet] Enregistrer des commandes en négatif"]}, ) def test_partial_0(self): diff --git a/kfet/views.py b/kfet/views.py index a5babe52..655e856d 100644 --- a/kfet/views.py +++ b/kfet/views.py @@ -3,6 +3,7 @@ import heapq import statistics from collections import defaultdict from decimal import Decimal +from typing import List from urllib.parse import urlencode from django.contrib import messages @@ -993,15 +994,19 @@ def kpsul_update_addcost(request): return JsonResponse(data) -def get_missing_perms(required_perms, user): - missing_perms_codenames = [ - (perm.split("."))[1] for perm in required_perms if not user.has_perm(perm) - ] - missing_perms = list( - Permission.objects.filter(codename__in=missing_perms_codenames).values_list( - "name", flat=True +def get_missing_perms(required_perms: List[str], user: User) -> List[str]: + def get_perm_description(app_label: str, codename: str) -> str: + name = Permission.objects.values_list("name", flat=True).get( + codename=codename, content_type__app_label=app_label ) - ) + return "[{}] {}".format(app_label, name) + + missing_perms = [ + get_perm_description(*perm.split(".")) + for perm in required_perms + if not user.has_perm(perm) + ] + return missing_perms