Let shortcut 'remove_perm' work with global permission objects

Fixes #606
django-guardian · Sep 18, 2020 · 9a60121 · 9a60121
1 parent 9c3edfb
commit 9a60121
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 9 deletions.
diff --git a/guardian/shortcuts.py b/guardian/shortcuts.py
@@ -162,17 +162,18 @@ def remove_perm(perm, user_or_group=None, obj=None):
     """
     user, group = get_identity(user_or_group)
     if obj is None:
-        try:
-            app_label, codename = perm.split('.', 1)
-        except ValueError:
-            raise ValueError("For global permissions, first argument must be in"
-                             " format: 'app_label.codename' (is %r)" % perm)
-        perm = Permission.objects.get(content_type__app_label=app_label,
-                                      codename=codename)
+        if not isinstance(perm, Permission):
+            try:
+                app_label, codename = perm.split('.', 1)
+            except ValueError:
+                raise ValueError("For global permissions, first argument must be in"
+                                 " format: 'app_label.codename' (is %r)" % perm)
+            perm = Permission.objects.get(content_type__app_label=app_label,
+                                          codename=codename)
         if user:
             user.user_permissions.remove(perm)
             return
-        elif group:
+        if group:
             group.permissions.remove(perm)
             return
 

diff --git a/guardian/testapp/tests/test_shortcuts.py b/guardian/testapp/tests/test_shortcuts.py
@@ -109,7 +109,9 @@ def test_group_assign_perm_queryset(self):
 
     def test_user_assign_perm_global(self):
         perm = assign_perm("contenttypes.change_contenttype", self.user)
+        assign_perm(self.get_permission("delete_contenttype"), self.group)
         self.assertTrue(self.user.has_perm("contenttypes.change_contenttype"))
+        self.assertTrue(self.user.has_perm("contenttypes.delete_contenttype"))
         self.assertTrue(isinstance(perm, Permission))
 
     def test_group_assign_perm_global(self):
@@ -160,7 +162,7 @@ def test_group_assign_perm_list(self):
         for obj in self.ctype_list:
             self.assertTrue(check.has_perm("add_contenttype", obj))
             self.assertTrue(check.has_perm("change_contenttype", obj))
-            self.assertTrue(check.has_perm("delete_contenttype", obj)) 
+            self.assertTrue(check.has_perm("delete_contenttype", obj))
 
 
 class MultipleIdentitiesOperationsTest(ObjectPermissionTestCase):
@@ -272,9 +274,13 @@ def test_group_remove_perm_queryset(self):
     def test_user_remove_perm_global(self):
         # assign perm first
         perm = "contenttypes.change_contenttype"
+        perm_obj = self.get_permission("delete_contenttype")
         assign_perm(perm, self.user)
+        assign_perm(perm_obj, self.user)
         remove_perm(perm, self.user)
+        remove_perm(perm_obj, self.user)
         self.assertFalse(self.user.has_perm(perm))
+        self.assertFalse(self.user.has_perm(perm_obj.codename))
 
     def test_group_remove_perm_global(self):
         # assign perm first