diff --git a/django/contrib/auth/apps.py b/django/contrib/auth/apps.py index 109a34dff6..79c44e78f2 100644 --- a/django/contrib/auth/apps.py +++ b/django/contrib/auth/apps.py @@ -1,9 +1,9 @@ from django.apps import AppConfig -from django.contrib.auth.checks import check_user_model from django.core import checks from django.db.models.signals import post_migrate from django.utils.translation import ugettext_lazy as _ +from .checks import check_models_permissions, check_user_model from .management import create_permissions @@ -15,3 +15,4 @@ class AuthConfig(AppConfig): post_migrate.connect(create_permissions, dispatch_uid="django.contrib.auth.management.create_permissions") checks.register(check_user_model, checks.Tags.models) + checks.register(check_models_permissions, checks.Tags.models) diff --git a/django/contrib/auth/checks.py b/django/contrib/auth/checks.py index cbafb40dca..d1b0a44fdd 100644 --- a/django/contrib/auth/checks.py +++ b/django/contrib/auth/checks.py @@ -1,10 +1,14 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals +from itertools import chain + from django.apps import apps from django.conf import settings from django.core import checks +from .management import _get_builtin_permissions + def check_user_model(app_configs=None, **kwargs): if app_configs is None: @@ -70,3 +74,72 @@ def check_user_model(app_configs=None, **kwargs): ) return errors + + +def check_models_permissions(app_configs=None, **kwargs): + if app_configs is None: + models = apps.get_models() + else: + models = chain.from_iterable(app_config.get_models() for app_config in app_configs) + + Permission = apps.get_model('auth', 'Permission') + permission_name_max_length = Permission._meta.get_field('name').max_length + errors = [] + + for model in models: + opts = model._meta + builtin_permissions = dict(_get_builtin_permissions(opts)) + # Check builtin permission name length. + max_builtin_permission_name_length = max(len(name) for name in builtin_permissions.values()) + if max_builtin_permission_name_length > permission_name_max_length: + verbose_name_max_length = ( + permission_name_max_length - (max_builtin_permission_name_length - len(opts.verbose_name_raw)) + ) + errors.append( + checks.Error( + "The verbose_name of model '%s.%s' must be at most %d characters " + "for its builtin permission names to be at most %d characters." % ( + opts.app_label, opts.object_name, verbose_name_max_length, permission_name_max_length + ), + obj=model, + id='auth.E007', + ) + ) + codenames = set() + for codename, name in opts.permissions: + # Check custom permission name length. + if len(name) > permission_name_max_length: + errors.append( + checks.Error( + "The permission named '%s' of model '%s.%s' is longer than %d characters." % ( + name, opts.app_label, opts.object_name, permission_name_max_length + ), + obj=model, + id='auth.E008', + ) + ) + # Check custom permissions codename clashing. + if codename in builtin_permissions: + errors.append( + checks.Error( + "The permission codenamed '%s' clashes with a builtin permission " + "for model '%s.%s'." % ( + codename, opts.app_label, opts.object_name + ), + obj=model, + id='auth.E005', + ) + ) + elif codename in codenames: + errors.append( + checks.Error( + "The permission codenamed '%s' is duplicated for model '%s.%s'." % ( + codename, opts.app_label, opts.object_name + ), + obj=model, + id='auth.E006', + ) + ) + codenames.add(codename) + + return errors diff --git a/django/contrib/auth/management/__init__.py b/django/contrib/auth/management/__init__.py index 6380d2f0c3..3ab9458ad3 100644 --- a/django/contrib/auth/management/__init__.py +++ b/django/contrib/auth/management/__init__.py @@ -9,19 +9,17 @@ import unicodedata from django.apps import apps from django.contrib.auth import get_permission_codename from django.core import exceptions -from django.core.management.base import CommandError from django.db import DEFAULT_DB_ALIAS, router from django.utils import six from django.utils.encoding import DEFAULT_LOCALE_ENCODING -def _get_all_permissions(opts, ctype): +def _get_all_permissions(opts): """ Returns (codename, name) for all permissions in the given opts. """ builtin = _get_builtin_permissions(opts) custom = list(opts.permissions) - _check_permission_clashing(custom, builtin, ctype) return builtin + custom @@ -37,26 +35,6 @@ def _get_builtin_permissions(opts): return perms -def _check_permission_clashing(custom, builtin, ctype): - """ - Check that permissions for a model do not clash. Raises CommandError if - there are duplicate permissions. - """ - pool = set() - builtin_codenames = set(p[0] for p in builtin) - for codename, _name in custom: - if codename in pool: - raise CommandError( - "The permission codename '%s' is duplicated for model '%s.%s'." % - (codename, ctype.app_label, ctype.model_class().__name__)) - elif codename in builtin_codenames: - raise CommandError( - "The permission codename '%s' clashes with a builtin permission " - "for model '%s.%s'." % - (codename, ctype.app_label, ctype.model_class().__name__)) - pool.add(codename) - - def create_permissions(app_config, verbosity=2, interactive=True, using=DEFAULT_DB_ALIAS, **kwargs): if not app_config.models_module: return @@ -71,9 +49,6 @@ def create_permissions(app_config, verbosity=2, interactive=True, using=DEFAULT_ from django.contrib.contenttypes.models import ContentType - permission_name_max_length = Permission._meta.get_field('name').max_length - verbose_name_max_length = permission_name_max_length - 11 # len('Can change ') prefix - # This will hold the permissions we're looking for as # (content_type, (codename, name)) searched_perms = list() @@ -84,17 +59,8 @@ def create_permissions(app_config, verbosity=2, interactive=True, using=DEFAULT_ # before creating foreign keys to them. ctype = ContentType.objects.db_manager(using).get_for_model(klass) - if len(klass._meta.verbose_name) > verbose_name_max_length: - raise exceptions.ValidationError( - "The verbose_name of %s.%s is longer than %s characters" % ( - ctype.app_label, - ctype.model, - verbose_name_max_length, - ) - ) - ctypes.add(ctype) - for perm in _get_all_permissions(klass._meta, ctype): + for perm in _get_all_permissions(klass._meta): searched_perms.append((ctype, perm)) # Find all the Permissions that have a content_type for a model we're @@ -111,18 +77,6 @@ def create_permissions(app_config, verbosity=2, interactive=True, using=DEFAULT_ for ct, (codename, name) in searched_perms if (ct.pk, codename) not in all_perms ] - # Validate the permissions before bulk_creation to avoid cryptic database - # error when the name is longer than 255 characters - for perm in perms: - if len(perm.name) > permission_name_max_length: - raise exceptions.ValidationError( - "The permission name %s of %s.%s is longer than %s characters" % ( - perm.name, - perm.content_type.app_label, - perm.content_type.model, - permission_name_max_length, - ) - ) Permission.objects.using(using).bulk_create(perms) if verbosity >= 2: for perm in perms: diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index b54cd7bef2..c82bd2bc98 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -432,6 +432,16 @@ Auth ``USERNAME_FIELD``. * **auth.W004**: ```` is named as the ``USERNAME_FIELD``, but it is not unique. +* **auth.E005**: The permission codenamed ```` clashes with a builtin + permission for model ````. +* **auth.E006**: The permission codenamed ```` is duplicated for model + ````. +* **auth.E007**: The :attr:`verbose_name + ` of model ```` must be at most + 244 characters for its builtin permission names + to be at most 255 characters. +* **auth.E008**: The permission named ```` of model ```` is longer + than 255 characters. Content Types diff --git a/tests/auth_tests/test_checks.py b/tests/auth_tests/test_checks.py index 392e72e62c..3ff78d9851 100644 --- a/tests/auth_tests/test_checks.py +++ b/tests/auth_tests/test_checks.py @@ -1,6 +1,8 @@ from __future__ import unicode_literals -from django.contrib.auth.checks import check_user_model +from django.contrib.auth.checks import ( + check_models_permissions, check_user_model, +) from django.contrib.auth.models import AbstractBaseUser from django.core import checks from django.db import models @@ -80,3 +82,83 @@ class UserModelChecksTests(SimpleTestCase): id='auth.W004', ), ]) + + +@isolate_apps('auth_tests', attr_name='apps') +@override_system_checks([check_models_permissions]) +class ModelsPermissionsChecksTests(SimpleTestCase): + def test_clashing_default_permissions(self): + class Checked(models.Model): + class Meta: + permissions = [ + ('change_checked', 'Can edit permission (duplicate)') + ] + errors = checks.run_checks(self.apps.get_app_configs()) + self.assertEqual(errors, [ + checks.Error( + "The permission codenamed 'change_checked' clashes with a builtin " + "permission for model 'auth_tests.Checked'.", + obj=Checked, + id='auth.E005', + ), + ]) + + def test_non_clashing_custom_permissions(self): + class Checked(models.Model): + class Meta: + permissions = [ + ('my_custom_permission', 'Some permission'), + ('other_one', 'Some other permission'), + ] + errors = checks.run_checks(self.apps.get_app_configs()) + self.assertEqual(errors, []) + + def test_clashing_custom_permissions(self): + class Checked(models.Model): + class Meta: + permissions = [ + ('my_custom_permission', 'Some permission'), + ('other_one', 'Some other permission'), + ('my_custom_permission', 'Some permission with duplicate permission code'), + ] + errors = checks.run_checks(self.apps.get_app_configs()) + self.assertEqual(errors, [ + checks.Error( + "The permission codenamed 'my_custom_permission' is duplicated for " + "model 'auth_tests.Checked'.", + obj=Checked, + id='auth.E006', + ), + ]) + + def test_verbose_name_max_length(self): + class Checked(models.Model): + class Meta: + verbose_name = 'some ridiculously long verbose name that is out of control' * 5 + errors = checks.run_checks(self.apps.get_app_configs()) + self.assertEqual(errors, [ + checks.Error( + "The verbose_name of model 'auth_tests.Checked' must be at most 244 " + "characters for its builtin permission names to be at most 255 characters.", + obj=Checked, + id='auth.E007', + ), + ]) + + def test_custom_permission_name_max_length(self): + custom_permission_name = 'some ridiculously long verbose name that is out of control' * 5 + + class Checked(models.Model): + class Meta: + permissions = [ + ('my_custom_permission', custom_permission_name), + ] + errors = checks.run_checks(self.apps.get_app_configs()) + self.assertEqual(errors, [ + checks.Error( + "The permission named '%s' of model 'auth_tests.Checked' is longer " + "than 255 characters." % custom_permission_name, + obj=Checked, + id='auth.E008', + ), + ]) diff --git a/tests/auth_tests/test_management.py b/tests/auth_tests/test_management.py index 3f31e7d66a..34b3465b9c 100644 --- a/tests/auth_tests/test_management.py +++ b/tests/auth_tests/test_management.py @@ -11,7 +11,6 @@ from django.contrib.auth.management.commands import ( ) from django.contrib.auth.models import Group, Permission, User from django.contrib.contenttypes.models import ContentType -from django.core import exceptions from django.core.management import call_command from django.core.management.base import CommandError from django.test import TestCase, mock, override_settings @@ -567,48 +566,12 @@ class PermissionTestCase(TestCase): def setUp(self): self._original_permissions = Permission._meta.permissions[:] self._original_default_permissions = Permission._meta.default_permissions - self._original_verbose_name = Permission._meta.verbose_name def tearDown(self): Permission._meta.permissions = self._original_permissions Permission._meta.default_permissions = self._original_default_permissions - Permission._meta.verbose_name = self._original_verbose_name ContentType.objects.clear_cache() - def test_duplicated_permissions(self): - """ - Test that we show proper error message if we are trying to create - duplicate permissions. - """ - auth_app_config = apps.get_app_config('auth') - - # check duplicated default permission - Permission._meta.permissions = [ - ('change_permission', 'Can edit permission (duplicate)')] - msg = ( - "The permission codename 'change_permission' clashes with a " - "builtin permission for model 'auth.Permission'." - ) - with self.assertRaisesMessage(CommandError, msg): - create_permissions(auth_app_config, verbosity=0) - - # check duplicated custom permissions - Permission._meta.permissions = [ - ('my_custom_permission', 'Some permission'), - ('other_one', 'Some other permission'), - ('my_custom_permission', 'Some permission with duplicate permission code'), - ] - msg = "The permission codename 'my_custom_permission' is duplicated for model 'auth.Permission'." - with self.assertRaisesMessage(CommandError, msg): - create_permissions(auth_app_config, verbosity=0) - - # should not raise anything - Permission._meta.permissions = [ - ('my_custom_permission', 'Some permission'), - ('other_one', 'Some other permission'), - ] - create_permissions(auth_app_config, verbosity=0) - def test_default_permissions(self): auth_app_config = apps.get_app_config('auth') @@ -631,32 +594,3 @@ class PermissionTestCase(TestCase): self.assertEqual(Permission.objects.filter( content_type=permission_content_type, ).count(), 1) - - def test_verbose_name_length(self): - auth_app_config = apps.get_app_config('auth') - - permission_content_type = ContentType.objects.get_by_natural_key('auth', 'permission') - Permission.objects.filter(content_type=permission_content_type).delete() - Permission._meta.verbose_name = "some ridiculously long verbose name that is out of control" * 5 - - msg = "The verbose_name of auth.permission is longer than 244 characters" - with self.assertRaisesMessage(exceptions.ValidationError, msg): - create_permissions(auth_app_config, verbosity=0) - - def test_custom_permission_name_length(self): - auth_app_config = apps.get_app_config('auth') - - ContentType.objects.get_by_natural_key('auth', 'permission') - custom_perm_name = 'a' * 256 - Permission._meta.permissions = [ - ('my_custom_permission', custom_perm_name), - ] - try: - msg = ( - "The permission name %s of auth.permission is longer than " - "255 characters" % custom_perm_name - ) - with self.assertRaisesMessage(exceptions.ValidationError, msg): - create_permissions(auth_app_config, verbosity=0) - finally: - Permission._meta.permissions = []