From a46f6d20c646a491e23154b1ef035a46b78e2e05 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Tue, 11 Jun 2024 11:21:18 -0500 Subject: [PATCH 1/2] Reorganize permissions code --- smartmin/__init__.py | 2 - smartmin/management/__init__.py | 168 -------------------------------- smartmin/perms.py | 35 ------- smartmin/users/models.py | 5 + smartmin/users/perms.py | 136 ++++++++++++++++++++++++++ test_runner/blog/tests.py | 79 ++++++++++++--- 6 files changed, 205 insertions(+), 220 deletions(-) delete mode 100644 smartmin/perms.py create mode 100644 smartmin/users/perms.py diff --git a/smartmin/__init__.py b/smartmin/__init__.py index 3e0ce7e..3a223dd 100644 --- a/smartmin/__init__.py +++ b/smartmin/__init__.py @@ -1,3 +1 @@ -from __future__ import unicode_literals - __version__ = "5.0.2" diff --git a/smartmin/management/__init__.py b/smartmin/management/__init__.py index 6f55757..e69de29 100644 --- a/smartmin/management/__init__.py +++ b/smartmin/management/__init__.py @@ -1,168 +0,0 @@ -import sys - -from django.apps import apps -from django.conf import settings -from django.contrib.auth.models import Group, Permission -from django.contrib.contenttypes.models import ContentType -from django.core.exceptions import ObjectDoesNotExist -from django.db.models.signals import post_migrate - -from smartmin.perms import assign_perm, remove_perm - -permissions_app_name = None - - -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 check_role_permissions(role, permissions, current_permissions): - """ - Checks the the passed in role (can be user, group or AnonymousUser) has all the passed - in permissions, granting them if necessary. - """ - role_permissions = [] - - # get all the current permissions, we'll remove these as we verify they should still be granted - for permission in permissions: - splits = permission.split(".") - if len(splits) != 2 and len(splits) != 3: - sys.stderr.write(" invalid permission %s, ignoring\n" % permission) - continue - - app = splits[0] - codenames = [] - - if len(splits) == 2: - codenames.append(splits[1]) - else: - (object, action) = splits[1:] - - # if this is a wildcard, then query our database for all the permissions that exist on this object - if action == "*": - for perm in Permission.objects.filter(codename__startswith="%s_" % object, content_type__app_label=app): - codenames.append(perm.codename) - # otherwise, this is an error, continue - else: - sys.stderr.write(" invalid permission %s, ignoring\n" % permission) - continue - - if len(codenames) == 0: - continue - - for codename in codenames: - # the full codename for this permission - full_codename = "%s.%s" % (app, codename) - - # this marks all the permissions which should remain - role_permissions.append(full_codename) - - try: - assign_perm(full_codename, role) - except ObjectDoesNotExist: - pass - # sys.stderr.write(" unknown permission %s, ignoring\n" % permission) - - # remove any that are extra - for permission in current_permissions: - if isinstance(permission, str): - key = permission - else: - key = "%s.%s" % (permission.content_type.app_label, permission.codename) - - if key not in role_permissions: - remove_perm(key, role) - - -def check_all_group_permissions(sender, **kwargs): - """ - Checks that all the permissions specified in our settings.py are set for our groups. - """ - if not is_permissions_app(sender): - return - - config = getattr(settings, "GROUP_PERMISSIONS", dict()) - - # for each of our items - for name, permissions in config.items(): - # get or create the group - (group, created) = Group.objects.get_or_create(name=name) - if created: - pass - - check_role_permissions(group, permissions, group.permissions.all()) - - -def add_permission(content_type, permission): - """ - 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. - """ - # build our permission slug - codename = "%s_%s" % (content_type.model, permission) - - # sys.stderr.write("Checking %s permission for %s\n" % (permission, content_type.name)) - - # does it already exist - if not Permission.objects.filter(content_type=content_type, codename=codename): - Permission.objects.create( - content_type=content_type, codename=codename, name="Can %s %s" % (permission, content_type.name) - ) - # sys.stderr.write("Added %s permission for %s\n" % (permission, content_type.name)) - - -def check_all_permissions(sender, **kwargs): - """ - This syncdb checks our PERMISSIONS setting in settings.py and makes sure all those permissions - actually exit. - """ - if not is_permissions_app(sender): - return - - config = getattr(settings, "PERMISSIONS", dict()) - - # for each of our items - for natural_key, permissions in config.items(): - # if the natural key '*' then that means add to all objects - if natural_key == "*": - # for each of our content types - for content_type in ContentType.objects.all(): - for permission in permissions: - add_permission(content_type, permission) - - # otherwise, this is on a specific content type, add for each of those - 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: - add_permission(content_type, permission) - - -post_migrate.connect(check_all_permissions) -post_migrate.connect(check_all_group_permissions) diff --git a/smartmin/perms.py b/smartmin/perms.py deleted file mode 100644 index 29a5f57..0000000 --- a/smartmin/perms.py +++ /dev/null @@ -1,35 +0,0 @@ -from django.contrib.auth.models import Permission - - -def assign_perm(perm, group): - """ - Assigns a permission to a group - """ - 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) - - group.permissions.add(perm) - return perm - - -def remove_perm(perm, group): - """ - Removes a permission from a group - """ - 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) - - group.permissions.remove(perm) - return diff --git a/smartmin/users/models.py b/smartmin/users/models.py index d534e7d..2108055 100644 --- a/smartmin/users/models.py +++ b/smartmin/users/models.py @@ -4,8 +4,13 @@ from django.conf import settings from django.contrib.auth.hashers import check_password 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) + def is_password_complex(password): has_caps = re.search("[A-Z]+", password) diff --git a/smartmin/users/perms.py b/smartmin/users/perms.py new file mode 100644 index 0000000..9a7b9bb --- /dev/null +++ b/smartmin/users/perms.py @@ -0,0 +1,136 @@ +import re + +from django.apps import apps +from django.conf import settings +from django.contrib.auth.models import Group, Permission +from django.contrib.contenttypes.models import ContentType + +permissions_app_name = None +perm_desc_regex = re.compile(r"(?P\w+)\.(?P\w+)(?P\.\*)?") + + +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): + """ + Checks the the passed in role (can be user, group or AnonymousUser) has all the passed + in permissions, granting them if necessary. + """ + + new_permissions = [] + + for perm_desc in permissions: + app_label, codename, wild = _parse_perm_desc(perm_desc) + + if wild: + codenames = Permission.objects.filter( + content_type__app_label=app_label, codename__startswith=f"{codename}_" + ).values_list("codename", flat=True) + else: + codenames = [codename] + + perms = [] + for codename in codenames: + try: + perms.append(Permission.objects.get(content_type__app_label=app_label, codename=codename)) + except Permission.DoesNotExist: + raise ValueError(f"Cannot grant permission {app_label}.{codename} as it does not exist.") + + new_permissions.append((app_label, codename)) + + group.permissions.add(*perms) + + # remove any that are extra + for perm in group.permissions.select_related("content_type").all(): + 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. + """ + + if not is_permissions_app(sender): + return + + # 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 natural_key == "*": + # for each of our content types + for content_type in ContentType.objects.all(): + for permission in permissions: + _ensure_permission_exists(content_type, permission) + + # otherwise, this is on a specific content type, add for each of those + 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) + + # 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 + + update_group_permissions(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 + """ + + 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")) + + +def _ensure_permission_exists(content_type: str, 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. + """ + + codename = f"{content_type.model}_{permission}" # build our permission slug + + Permission.objects.get_or_create( + content_type=content_type, codename=codename, defaults={"name": f"Can {permission} {content_type.name}"} + ) diff --git a/test_runner/blog/tests.py b/test_runner/blog/tests.py index d36c8da..5200c5f 100644 --- a/test_runner/blog/tests.py +++ b/test_runner/blog/tests.py @@ -1,26 +1,26 @@ import json from datetime import datetime, timedelta, timezone as tzone from unittest.mock import patch - from zoneinfo import ZoneInfo from django.conf import settings from django.contrib.auth.models import Group, User from django.core import mail from django.core.files.uploadedfile import SimpleUploadedFile +from django.db.models import F, Value +from django.db.models.functions import Concat from django.test import TestCase, override_settings from django.test.client import Client -from django.test.utils import override_settings from django.urls import reverse from django.utils import timezone import smartmin from smartmin.csv_imports.models import ImportTask -from smartmin.management import check_role_permissions from smartmin.models import SmartImportRowError from smartmin.templatetags.smartmin import get, get_value_from_view, user_as_string, view_as_json from smartmin.tests import SmartminTest from smartmin.users.models import FailedLogin, PasswordHistory, RecoveryToken, is_password_complex +from smartmin.users.perms import update_group_permissions from smartmin.views import smart_url from smartmin.widgets import DatePickerWidget, ImageThumbnailWidget from test_runner.blog.models import Category, Post @@ -481,22 +481,71 @@ def test_version(self): def test_management(self): authors = Group.objects.get(name="Authors") + 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.",)) + with self.assertRaises(ValueError): + update_group_permissions(authors, ("blog.post.too.many.dots",)) + with self.assertRaises(ValueError): + update_group_permissions(authors, ("blog.category.not_valid_either",)) + with self.assertRaises(ValueError): + update_group_permissions(authors, ("blog.category_not_valid_either",)) + + self.assertEqual( + { + "blog.category_create", + "blog.category_delete", + "blog.category_list", + "blog.category_read", + "blog.category_update", + "blog.post_author", + "blog.post_create", + "blog.post_csv_import", + "blog.post_delete", + "blog.post_exclude", + "blog.post_exclude2", + "blog.post_list", + "blog.post_list_no_pagination", + "blog.post_messages", + "blog.post_read", + "blog.post_readonly", + "blog.post_readonly2", + "blog.post_update", + }, + perms(authors), + ) # no change + # reduce our permission set to not include categories - permissions = ( - "blog.post.*", - "blog.post.too.many.dots", - "blog.category.not_valid_either", - "blog.", - "blog.foo.*", - ) + update_group_permissions(authors, permissions=("blog.post.*", "blog.foo.*")) - self.assertEqual(18, authors.permissions.all().count()) + # category permissions should have been removed + self.assertEqual( + { + "blog.post_author", + "blog.post_create", + "blog.post_csv_import", + "blog.post_delete", + "blog.post_exclude", + "blog.post_exclude2", + "blog.post_list", + "blog.post_list_no_pagination", + "blog.post_messages", + "blog.post_read", + "blog.post_readonly", + "blog.post_readonly2", + "blog.post_update", + }, + perms(authors), + ) - # check that they are reassigned - check_role_permissions(authors, permissions, authors.permissions.all()) + # reduce our permission to specific post permissions + update_group_permissions(authors, permissions=("blog.post_create", "blog.post_list")) - # removing all category actions should bring us to 10 - self.assertEqual(13, authors.permissions.all().count()) + self.assertEqual({"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")) From 78aa651887305bc0c1b74ed1982f72feeed71da4 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Tue, 11 Jun 2024 14:20:34 -0500 Subject: [PATCH 2/2] Move to root app --- smartmin/models.py | 5 +++++ smartmin/{users => }/perms.py | 0 smartmin/users/models.py | 5 ----- test_runner/blog/tests.py | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) rename smartmin/{users => }/perms.py (100%) diff --git a/smartmin/models.py b/smartmin/models.py index ed6d295..cc82e1c 100644 --- a/smartmin/models.py +++ b/smartmin/models.py @@ -8,8 +8,13 @@ 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): diff --git a/smartmin/users/perms.py b/smartmin/perms.py similarity index 100% rename from smartmin/users/perms.py rename to smartmin/perms.py diff --git a/smartmin/users/models.py b/smartmin/users/models.py index 2108055..d534e7d 100644 --- a/smartmin/users/models.py +++ b/smartmin/users/models.py @@ -4,13 +4,8 @@ from django.conf import settings from django.contrib.auth.hashers import check_password 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) - def is_password_complex(password): has_caps = re.search("[A-Z]+", password) diff --git a/test_runner/blog/tests.py b/test_runner/blog/tests.py index 5200c5f..d0f0041 100644 --- a/test_runner/blog/tests.py +++ b/test_runner/blog/tests.py @@ -17,10 +17,10 @@ import smartmin from smartmin.csv_imports.models import ImportTask from smartmin.models import SmartImportRowError +from smartmin.perms import update_group_permissions from smartmin.templatetags.smartmin import get, get_value_from_view, user_as_string, view_as_json from smartmin.tests import SmartminTest from smartmin.users.models import FailedLogin, PasswordHistory, RecoveryToken, is_password_complex -from smartmin.users.perms import update_group_permissions from smartmin.views import smart_url from smartmin.widgets import DatePickerWidget, ImageThumbnailWidget from test_runner.blog.models import Category, Post