From 898c604b3b0d01094b715acadae76e35bdd15b65 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Mon, 5 Oct 2020 08:55:15 +1100 Subject: [PATCH] Fix incorrect permission names - Uses the app_model name, *NOT* the name of the database table - Adds extra tests to ensure that permissions get assigned and removed correctly --- InvenTree/users/models.py | 19 ++++++---- InvenTree/users/tests.py | 79 ++++++++++++++++++++++++++++++++++----- 2 files changed, 80 insertions(+), 18 deletions(-) diff --git a/InvenTree/users/models.py b/InvenTree/users/models.py index 8f349e2f18..09f2a046d1 100644 --- a/InvenTree/users/models.py +++ b/InvenTree/users/models.py @@ -72,8 +72,8 @@ class RuleSet(models.Model): ], 'purchase_order': [ 'company_company', - 'part_supplierpart', - 'part_supplierpricebreak', + 'company_supplierpart', + 'company_supplierpricebreak', 'order_purchaseorder', 'order_purchaseorderattachment', 'order_purchaseorderlineitem', @@ -90,9 +90,9 @@ class RuleSet(models.Model): # Database models we ignore permission sets for RULESET_IGNORE = [ # Core django models (not user configurable) - 'django_admin_log', - 'django_content_type', - 'django_session', + 'admin_logentry', + 'contenttypes_contenttype', + 'sessions_session', # Models which currently do not require permissions 'common_colortheme', @@ -275,9 +275,12 @@ def update_group_roles(group, debug=False): (permission_name, model) = perm.split('_') - content_type = ContentType.objects.get(app_label=app, model=model) - - permission = Permission.objects.get(content_type=content_type, codename=perm) + try: + content_type = ContentType.objects.get(app_label=app, model=model) + permission = Permission.objects.get(content_type=content_type, codename=perm) + except ContentType.DoesNotExist: + print(f"Error: Could not find permission matching '{permission_string}'") + permission = None return permission diff --git a/InvenTree/users/tests.py b/InvenTree/users/tests.py index d87649e2db..d14ffc4950 100644 --- a/InvenTree/users/tests.py +++ b/InvenTree/users/tests.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals from django.test import TestCase from django.apps import apps +from django.contrib.auth.models import Group from users.models import RuleSet @@ -53,13 +54,15 @@ class RuleSetModelTest(TestCase): available_models = apps.get_models() - available_tables = [] + available_tables = set() + # Extract each available database model and construct a formatted string for model in available_models: - table_name = model.objects.model._meta.db_table - available_tables.append(table_name) + label = model.objects.model._meta.label + label = label.replace('.', '_').lower() + available_tables.add(label) - assigned_models = [] + assigned_models = set() # Now check that each defined model is a valid table name for key in RuleSet.RULESET_MODELS.keys(): @@ -68,26 +71,32 @@ class RuleSetModelTest(TestCase): for m in models: - assigned_models.append(m) + assigned_models.add(m) - missing_models = [] + missing_models = set() for model in available_tables: if model not in assigned_models and model not in RuleSet.RULESET_IGNORE: - missing_models.append(model) + missing_models.add(model) if len(missing_models) > 0: print("The following database models are not covered by the defined RuleSet permissions:") for m in missing_models: print("-", m) - extra_models = [] + extra_models = set() - defined_models = assigned_models + RuleSet.RULESET_IGNORE + defined_models = set() + + for model in assigned_models: + defined_models.add(model) + + for model in RuleSet.RULESET_IGNORE: + defined_models.add(model) for model in defined_models: if model not in available_tables: - extra_models.append(model) + extra_models.add(model) if len(extra_models) > 0: print("The following RuleSet permissions do not match a database model:") @@ -96,3 +105,53 @@ class RuleSetModelTest(TestCase): self.assertEqual(len(missing_models), 0) self.assertEqual(len(extra_models), 0) + + def test_permission_assign(self): + """ + Test that the permission assigning works! + """ + + # Create a new group + group = Group.objects.create(name="Test group") + + rulesets = group.rule_sets.all() + + # Rulesets should have been created automatically for this group + self.assertEqual(rulesets.count(), len(RuleSet.RULESET_CHOICES)) + + # Check that all permissions have been assigned permissions? + permission_set = set() + + for models in RuleSet.RULESET_MODELS.values(): + + for model in models: + permission_set.add(model) + + # Every ruleset by default sets one permission, the "view" permission set + self.assertEqual(group.permissions.count(), len(permission_set)) + + # Add some more rules + for rule in rulesets: + rule.can_add = True + rule.can_change = True + + rule.save() + + group.save() + + # There should now be three permissions for each rule set + self.assertEqual(group.permissions.count(), 3 * len(permission_set)) + + # Now remove *all* permissions + for rule in rulesets: + rule.can_view = False + rule.can_add = False + rule.can_change = False + rule.can_delete = False + + rule.save() + + group.save() + + # There should now not be any permissions assigned to this group + self.assertEqual(group.permissions.count(), 0)