Rework perms again, this time to configure the perms for each app ind…

…ividually

smartmin/apps.py

@@ -0,0 +1,11 @@
+from django.apps import AppConfig
+from django.db.models.signals import post_migrate
+
+
+class SmartminConfig(AppConfig):
+    name = "smartmin"
+
+    def ready(self):
+        from .perms import sync_permissions
+
+        post_migrate.connect(sync_permissions)

smartmin/models.py

@@ -8,13 +8,8 @@
 
 from django.conf import settings
 from django.db import models
-from django.db.models.signals import post_migrate
 from django.utils import timezone
 
-from .perms import sync_permissions
-
-post_migrate.connect(sync_permissions)
-
 
 class SmartImportRowError(Exception):
     def __init__(self, message):

smartmin/perms.py

@@ -1,40 +1,14 @@
 import re
 
-from django.apps import apps
 from django.conf import settings
 from django.contrib.auth.models import Group, Permission
+from django.contrib.contenttypes.management import create_contenttypes
 from django.contrib.contenttypes.models import ContentType
 
-permissions_app_name = None
-perm_desc_regex = re.compile(r"(?P<app>\w+)\.(?P<codename>\w+)(?P<wild>\.\*)?")
+perm_desc_regex = re.compile(r"(?P<app>\w+)\.(?P<contenttype>[a-z0-9]+)((_(?P<perm>\w+))|(\.(?P<wild>\*)))")
 
 
-def get_permissions_app_name():
-    """
-    Gets the app after which smartmin permissions should be installed. This can be specified by PERMISSIONS_APP in the
-    Django settings or defaults to the last app with models
-    """
-    global permissions_app_name
-
-    if not permissions_app_name:
-        permissions_app_name = getattr(settings, "PERMISSIONS_APP", None)
-
-        if not permissions_app_name:
-            app_names_with_models = [a.name for a in apps.get_app_configs() if a.models_module is not None]
-            if app_names_with_models:
-                permissions_app_name = app_names_with_models[-1]
-
-    return permissions_app_name
-
-
-def is_permissions_app(app_config):
-    """
-    Returns whether this is the app after which permissions should be installed.
-    """
-    return app_config.name == get_permissions_app_name()
-
-
-def update_group_permissions(group, permissions: list):
+def update_group_permissions(app, group, permissions: list):
     """
     Checks the the passed in role (can be user, group or AnonymousUser)  has all the passed
     in permissions, granting them if necessary.
@@ -43,14 +17,18 @@ def update_group_permissions(group, permissions: list):
     new_permissions = []
 
     for perm_desc in permissions:
-        app_label, codename, wild = _parse_perm_desc(perm_desc)
+        app_label, content_type, perm = _parse_perm_desc(perm_desc)
+
+        # ignore if this permission is for a type not in this app
+        if app_label != app.label:
+            continue
 
-        if wild:
+        if perm == "*":
             codenames = Permission.objects.filter(
-                content_type__app_label=app_label, codename__startswith=f"{codename}_"
+                content_type__app_label=app_label, codename__startswith=f"{content_type}_"
             ).values_list("codename", flat=True)
         else:
-            codenames = [codename]
+            codenames = [f"{content_type}_{perm}"]
 
         perms = []
         for codename in codenames:
@@ -64,66 +42,65 @@ def update_group_permissions(group, permissions: list):
         group.permissions.add(*perms)
 
     # remove any that are extra
-    for perm in group.permissions.select_related("content_type").all():
+    for perm in group.permissions.filter(content_type__app_label=app.label):
         if (perm.content_type.app_label, perm.codename) not in new_permissions:
             group.permissions.remove(perm)
 
 
 def sync_permissions(sender, **kwargs):
     """
-    1. Ensures all permissions decribed by the PERMISSIONS setting exist in the database.
-    2. Ensures all permissions granted by the GROUP_PERMISSIONS setting are granted to the appropriate groups.
+    1. Ensures all permissions for this app described by the PERMISSIONS setting exist in the database.
+    2. Ensures all permissions for this app granted by the GROUP_PERMISSIONS setting are granted.
     """
 
-    if not is_permissions_app(sender):
-        return
+    # the content types app also listens for post_migrate signals but since order isn't guaranteed, we need to
+    # manually invoke what it does for this app to be sure that the content types are created
+    create_contenttypes(sender)
 
     # for each of our items
     for natural_key, permissions in getattr(settings, "PERMISSIONS", {}).items():
-        # if the natural key '*' then that means add to all objects
+        # if wild, we add these permissions to all content types defined by this app
         if natural_key == "*":
-            # for each of our content types
-            for content_type in ContentType.objects.all():
+            for content_type in ContentType.objects.filter(app_label=sender.label):
                 for permission in permissions:
                     _ensure_permission_exists(content_type, permission)
 
-        # otherwise, this is on a specific content type, add for each of those
+        # otherwise check if this type belongs to this app and if so add the permissions to that type only
         else:
-            app, model = natural_key.split(".")
-            try:
-                content_type = ContentType.objects.get_by_natural_key(app, model)
-            except ContentType.DoesNotExist:
-                continue
-
-            # add each permission
-            for permission in permissions:
-                _ensure_permission_exists(content_type, permission)
+            app_label, model = natural_key.split(".")
+            if app_label == sender.label:
+                try:
+                    content_type = ContentType.objects.get_by_natural_key(app_label, model)
+                except ContentType.DoesNotExist:
+                    raise ValueError(f"No such content type: {app_label}.{model}")
+
+                # add each permission
+                for permission in permissions:
+                    _ensure_permission_exists(content_type, permission)
 
     # for each of our items
     for name, permissions in getattr(settings, "GROUP_PERMISSIONS", {}).items():
         # get or create the group
-        (group, created) = Group.objects.get_or_create(name=name)
-        if created:
-            pass
+        group, created = Group.objects.get_or_create(name=name)
 
-        update_group_permissions(group, permissions)
+        update_group_permissions(sender, group, permissions)
 
 
 def _parse_perm_desc(desc: str) -> tuple:
     """
     Parses a permission descriptor into its app_label, model and permission parts, e.g.
-        app.model.*      => app, model, True
-        app.model_perm   => app, model_perm, False
+        app.model.*      => app, model, *
+        app.model_perm   => app, model, perm
     """
 
     match = perm_desc_regex.match(desc)
     if not match:
         raise ValueError(f"Invalid permission descriptor: {desc}")
 
-    return match.group("app"), match.group("codename"), bool(match.group("wild"))
+    return match.group("app"), match.group("contenttype"), match.group("perm") or match.group("wild")
 
 
-def _ensure_permission_exists(content_type: str, permission: str):
+def _ensure_permission_exists(content_type, permission: str):
     """
     Adds the passed in permission to that content type.  Note that the permission passed
     in should be a single word, or verb.  The proper 'codename' will be generated from that.

test_runner/authors/models.py

@@ -0,0 +1,7 @@
+from django.db import models
+
+from smartmin.models import SmartModel
+
+
+class Author(SmartModel):
+    name = models.CharField(max_length=128)

test_runner/blog/tests.py

@@ -3,6 +3,7 @@
 from unittest.mock import patch
 from zoneinfo import ZoneInfo
 
+from django.apps import apps
 from django.conf import settings
 from django.contrib.auth.models import Group, User
 from django.core import mail
@@ -478,25 +479,27 @@ def test_integrity_error(self):
     def test_version(self):
         self.assertTrue(smartmin.__version__ is not None)
 
-    def test_management(self):
+    def test_permissions(self):
         authors = Group.objects.get(name="Authors")
+        blog_app = apps.get_app_config("blog")
 
         def perms(g):
             return set(
                 g.permissions.values_list(Concat(F("content_type__app_label"), Value("."), F("codename")), flat=True)
             )
 
         with self.assertRaises(ValueError):
-            update_group_permissions(authors, ("blog.",))
+            update_group_permissions(blog_app, authors, ("blog.",))
         with self.assertRaises(ValueError):
-            update_group_permissions(authors, ("blog.post.too.many.dots",))
+            update_group_permissions(blog_app, authors, ("blog.post.too.many.dots",))
         with self.assertRaises(ValueError):
-            update_group_permissions(authors, ("blog.category.not_valid_either",))
+            update_group_permissions(blog_app, authors, ("blog.category.not_valid_either",))
         with self.assertRaises(ValueError):
-            update_group_permissions(authors, ("blog.category_not_valid_either",))
+            update_group_permissions(blog_app, authors, ("blog.category_not_valid_either",))
 
         self.assertEqual(
             {
+                "authors.author_read",
                 "blog.category_create",
                 "blog.category_delete",
                 "blog.category_list",
@@ -520,11 +523,12 @@ def perms(g):
         )  # no change
 
         # reduce our permission set to not include categories
-        update_group_permissions(authors, permissions=("blog.post.*", "blog.foo.*"))
+        update_group_permissions(blog_app, authors, permissions=("blog.post.*", "blog.foo.*"))
 
         # category permissions should have been removed
         self.assertEqual(
             {
+                "authors.author_read",
                 "blog.post_author",
                 "blog.post_create",
                 "blog.post_csv_import",
@@ -543,9 +547,9 @@ def perms(g):
         )
 
         # reduce our permission to specific post permissions
-        update_group_permissions(authors, permissions=("blog.post_create", "blog.post_list"))
+        update_group_permissions(blog_app, authors, permissions=("blog.post_create", "blog.post_list"))
 
-        self.assertEqual({"blog.post_create", "blog.post_list"}, perms(authors))
+        self.assertEqual({"authors.author_read", "blog.post_create", "blog.post_list"}, perms(authors))
 
     def test_smart_model(self):
         d1 = datetime(2016, 12, 31, 9, 20, 30, 123456, tzinfo=ZoneInfo("Africa/Kigali"))

test_runner/settings.py

@@ -142,11 +142,9 @@
     "django.contrib.staticfiles",
     "smartmin",
     "smartmin.users",
+    "test_runner.authors",
     "test_runner.blog",
-    # Uncomment the next line to enable the admin:
     "django.contrib.admin",
-    # Uncomment the next line to enable admin documentation:
-    # 'django.contrib.admindocs',
     "smartmin.csv_imports",
 )
 
@@ -177,8 +175,8 @@
         "read",  # can read an object, viewing it's details
         "update",  # can update an object
         "delete",  # can delete an object,
-        "list",
-    ),  # can view a list of the objects
+        "list",  # can view a list of the objects
+    ),
     "blog.post": (
         "author",
         "exclude",
@@ -190,16 +188,14 @@
         "list_no_pagination",
     ),
     "auth.user": ("profile",),
-    # invalid content type for test
-    "blog.foo": ("nothing",),
 }
 
 # assigns the permissions that each group should have, here creating an Administrator group with
 # authority to create and change users
 GROUP_PERMISSIONS = {
-    "Administrator": ("auth.user.*",),
-    "Editors": ("blog.post_update", "blog.post_list", "auth.user_profile"),
-    "Authors": ("blog.post.*", "blog.category.*"),
+    "Administrator": ("authors.author.*",),
+    "Editors": ("blog.post_update", "blog.post_list"),
+    "Authors": ("blog.post.*", "blog.category.*", "authors.author_read"),
 }
 
 LOGIN_URL = "/users/login/"