diff --git a/AUTHORS b/AUTHORS index a42eabf8db..539f1c5cc3 100644 --- a/AUTHORS +++ b/AUTHORS @@ -86,6 +86,7 @@ answer newbie questions, and generally made Django that much better: Artem Gnilov Arthur Arthur Koziel + Arthur Rio Arvis Bickovskis Aryeh Leib Taurog A S Alam diff --git a/django/contrib/auth/management/__init__.py b/django/contrib/auth/management/__init__.py index 14c25a7399..deda238c78 100644 --- a/django/contrib/auth/management/__init__.py +++ b/django/contrib/auth/management/__init__.py @@ -60,7 +60,7 @@ def create_permissions(app_config, verbosity=2, interactive=True, using=DEFAULT_ for klass in app_config.get_models(): # Force looking up the content types in the current database # before creating foreign keys to them. - ctype = ContentType.objects.db_manager(using).get_for_model(klass) + ctype = ContentType.objects.db_manager(using).get_for_model(klass, for_concrete_model=False) ctypes.add(ctype) for perm in _get_all_permissions(klass._meta): diff --git a/django/contrib/auth/migrations/0011_update_proxy_permissions.py b/django/contrib/auth/migrations/0011_update_proxy_permissions.py new file mode 100644 index 0000000000..0e66649695 --- /dev/null +++ b/django/contrib/auth/migrations/0011_update_proxy_permissions.py @@ -0,0 +1,48 @@ +from django.db import migrations +from django.db.models import Q + + +def update_proxy_model_permissions(apps, schema_editor, reverse=False): + """ + Update the content_type of proxy model permissions to use the ContentType + of the proxy model. + """ + Permission = apps.get_model('auth', 'Permission') + ContentType = apps.get_model('contenttypes', 'ContentType') + for Model in apps.get_models(): + opts = Model._meta + if not opts.proxy: + continue + proxy_default_permissions_codenames = [ + '%s_%s' % (action, opts.model_name) + for action in opts.default_permissions + ] + permissions_query = Q(codename__in=proxy_default_permissions_codenames) + for codename, name in opts.permissions: + permissions_query = permissions_query | Q(codename=codename, name=name) + concrete_content_type = ContentType.objects.get_for_model(Model, for_concrete_model=True) + proxy_content_type = ContentType.objects.get_for_model(Model, for_concrete_model=False) + old_content_type = proxy_content_type if reverse else concrete_content_type + new_content_type = concrete_content_type if reverse else proxy_content_type + Permission.objects.filter( + permissions_query, + content_type=old_content_type, + ).update(content_type=new_content_type) + + +def revert_proxy_model_permissions(apps, schema_editor): + """ + Update the content_type of proxy model permissions to use the ContentType + of the concrete model. + """ + update_proxy_model_permissions(apps, schema_editor, reverse=True) + + +class Migration(migrations.Migration): + dependencies = [ + ('auth', '0010_alter_group_name_max_length'), + ('contenttypes', '0002_remove_content_type_name'), + ] + operations = [ + migrations.RunPython(update_proxy_model_permissions, revert_proxy_model_permissions), + ] diff --git a/docs/releases/2.2.txt b/docs/releases/2.2.txt index d3c8e9abcc..069e098664 100644 --- a/docs/releases/2.2.txt +++ b/docs/releases/2.2.txt @@ -439,6 +439,28 @@ Use this instead:: alias = property(operator.attrgetter('base')) +Permissions for proxy models +---------------------------- + +:ref:`Permissions for proxy models ` are now +created using the content type of the proxy model rather than the content type +of the concrete model. A migration will update existing permissions when you +run :djadmin:`migrate`. + +In the admin, the change is transparent for proxy models having the same +``app_label`` as their concrete model. However, in older versions, users with +permissions for a proxy model with a *different* ``app_label`` than its +concrete model couldn't access the model in the admin. That's now fixed, but +you might want to audit the permissions assignments for such proxy models +(``[add|view|change|delete]_myproxy``) prior to upgrading to ensure the new +access is appropriate. + +Finally, proxy model permission strings must be updated to use their own +``app_label``. For example, for ``app.MyProxyModel`` inheriting from +``other_app.ConcreteModel``, update +``user.has_perm('other_app.add_myproxymodel')`` to +``user.has_perm('app.add_myproxymodel')``. + Miscellaneous ------------- diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt index 328b9c69bf..640130beb0 100644 --- a/docs/topics/auth/default.txt +++ b/docs/topics/auth/default.txt @@ -262,6 +262,20 @@ The permission can then be assigned to a attribute or to a :class:`~django.contrib.auth.models.Group` via its ``permissions`` attribute. +.. admonition:: Proxy models need their own content type + + If you want to create :ref:`permissions for a proxy model + `, pass ``for_concrete_model=False`` to + :meth:`.ContentTypeManager.get_for_model` to get the appropriate + ``ContentType``:: + + content_type = ContentType.objects.get_for_model(BlogPostProxy, for_concrete_model=False) + + .. versionchanged:: 2.2 + + In older versions, proxy models use the content type of the concrete + model. + Permission caching ------------------ @@ -303,6 +317,44 @@ the user from the database. For example:: ... +.. _proxy-models-permissions-topic: + +Proxy models +------------ + +Proxy models work exactly the same way as concrete models. Permissions are +created using the own content type of the proxy model. Proxy models don't +inherit the permissions of the concrete model they subclass:: + + class Person(models.Model): + class Meta: + permissions = (('can_eat_pizzas', 'Can eat pizzas'),) + + class Student(Person): + class Meta: + proxy = True + permissions = (('can_deliver_pizzas', 'Can deliver pizzas'),) + + >>> # Fetch the content type for the proxy model. + >>> content_type = ContentType.objects.get_for_model(Student, for_concrete_model=False) + >>> student_permissions = Permission.objects.filter(content_type=content_type) + >>> [p.codename for p in student_permissions] + ['add_student', 'change_student', 'delete_student', 'view_student', + 'can_deliver_pizzas'] + >>> for permission in student_permissions: + ... user.user_permissions.add(permission) + >>> user.has_perm('app.add_person') + False + >>> user.has_perm('app.can_eat_pizzas') + False + >>> user.has_perms(('app.add_student', 'app.can_deliver_pizzas')) + True + +.. versionchanged:: 2.2 + + In older versions, permissions for proxy models use the content type of + the concrete model rather than content type of the proxy model. + .. _auth-web-requests: Authentication in Web requests diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index d09e7fa084..0b0ad41e2e 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -43,7 +43,8 @@ from .models import ( Restaurant, RowLevelChangePermissionModel, Section, ShortMessage, Simple, Sketch, State, Story, StumpJoke, Subscriber, SuperVillain, Telegram, Thing, Topping, UnchangeableObject, UndeletableObject, UnorderedObject, - UserMessenger, Villain, Vodcast, Whatsit, Widget, Worker, WorkHour, + UserMessenger, UserProxy, Villain, Vodcast, Whatsit, Widget, Worker, + WorkHour, ) @@ -1075,6 +1076,7 @@ site.register(Ingredient) site.register(NotReferenced) site.register(ExplicitlyProvidedPK, GetFormsetsArgumentCheckingAdmin) site.register(ImplicitlyGeneratedPK, GetFormsetsArgumentCheckingAdmin) +site.register(UserProxy) # Register core models we need in our tests site.register(User, UserAdmin) diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index eef8106df4..d134b34923 100644 --- a/tests/admin_views/models.py +++ b/tests/admin_views/models.py @@ -976,3 +976,9 @@ class Author(models.Model): class Authorship(models.Model): book = models.ForeignKey(Book, models.CASCADE) author = models.ForeignKey(Author, models.CASCADE) + + +class UserProxy(User): + """Proxy a model with a different app_label.""" + class Meta: + proxy = True diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 93a93454d9..0cc16509ff 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -52,7 +52,8 @@ from .models import ( Report, Restaurant, RowLevelChangePermissionModel, SecretHideout, Section, ShortMessage, Simple, State, Story, SuperSecretHideout, SuperVillain, Telegram, TitleTranslation, Topping, UnchangeableObject, UndeletableObject, - UnorderedObject, Villain, Vodcast, Whatsit, Widget, Worker, WorkHour, + UnorderedObject, UserProxy, Villain, Vodcast, Whatsit, Widget, Worker, + WorkHour, ) ERROR_MESSAGE = "Please enter the correct username and password \ @@ -1362,10 +1363,10 @@ class CustomModelAdminTest(AdminViewBasicTestCase): self.assertEqual(response.status_code, 200) -def get_perm(Model, perm): +def get_perm(Model, codename): """Return the permission object, for the Model""" - ct = ContentType.objects.get_for_model(Model) - return Permission.objects.get(content_type=ct, codename=perm) + ct = ContentType.objects.get_for_model(Model, for_concrete_model=False) + return Permission.objects.get(content_type=ct, codename=codename) @override_settings( @@ -2384,6 +2385,81 @@ class AdminViewPermissionsTest(TestCase): ) +@override_settings( + ROOT_URLCONF='admin_views.urls', + TEMPLATES=[{ + 'BACKEND': 'django.template.backends.django.DjangoTemplates', + 'APP_DIRS': True, + 'OPTIONS': { + 'context_processors': [ + 'django.contrib.auth.context_processors.auth', + 'django.contrib.messages.context_processors.messages', + ], + }, + }], +) +class AdminViewProxyModelPermissionsTests(TestCase): + """Tests for proxy models permissions in the admin.""" + + @classmethod + def setUpTestData(cls): + cls.viewuser = User.objects.create_user(username='viewuser', password='secret', is_staff=True) + cls.adduser = User.objects.create_user(username='adduser', password='secret', is_staff=True) + cls.changeuser = User.objects.create_user(username='changeuser', password='secret', is_staff=True) + cls.deleteuser = User.objects.create_user(username='deleteuser', password='secret', is_staff=True) + # Setup permissions. + opts = UserProxy._meta + cls.viewuser.user_permissions.add(get_perm(UserProxy, get_permission_codename('view', opts))) + cls.adduser.user_permissions.add(get_perm(UserProxy, get_permission_codename('add', opts))) + cls.changeuser.user_permissions.add(get_perm(UserProxy, get_permission_codename('change', opts))) + cls.deleteuser.user_permissions.add(get_perm(UserProxy, get_permission_codename('delete', opts))) + # UserProxy instances. + cls.user_proxy = UserProxy.objects.create(username='user_proxy', password='secret') + + def test_add(self): + self.client.force_login(self.adduser) + url = reverse('admin:admin_views_userproxy_add') + data = { + 'username': 'can_add', + 'password': 'secret', + 'date_joined_0': '2019-01-15', + 'date_joined_1': '16:59:10', + } + response = self.client.post(url, data, follow=True) + self.assertEqual(response.status_code, 200) + self.assertTrue(UserProxy.objects.filter(username='can_add').exists()) + + def test_view(self): + self.client.force_login(self.viewuser) + response = self.client.get(reverse('admin:admin_views_userproxy_changelist')) + self.assertContains(response, '

Select user proxy to view

') + self.assertEqual(response.status_code, 200) + response = self.client.get(reverse('admin:admin_views_userproxy_change', args=(self.user_proxy.pk,))) + self.assertContains(response, '

View user proxy

') + self.assertContains(response, '
user_proxy
') + + def test_change(self): + self.client.force_login(self.changeuser) + data = { + 'password': self.user_proxy.password, + 'username': self.user_proxy.username, + 'date_joined_0': self.user_proxy.date_joined.strftime('%Y-%m-%d'), + 'date_joined_1': self.user_proxy.date_joined.strftime('%H:%M:%S'), + 'first_name': 'first_name', + } + url = reverse('admin:admin_views_userproxy_change', args=(self.user_proxy.pk,)) + response = self.client.post(url, data) + self.assertRedirects(response, reverse('admin:admin_views_userproxy_changelist')) + self.assertEqual(UserProxy.objects.get(pk=self.user_proxy.pk).first_name, 'first_name') + + def test_delete(self): + self.client.force_login(self.deleteuser) + url = reverse('admin:admin_views_userproxy_delete', args=(self.user_proxy.pk,)) + response = self.client.post(url, {'post': 'yes'}, follow=True) + self.assertEqual(response.status_code, 200) + self.assertFalse(UserProxy.objects.filter(pk=self.user_proxy.pk).exists()) + + @override_settings(ROOT_URLCONF='admin_views.urls') class AdminViewsNoUrlTest(TestCase): """Regression test for #17333""" diff --git a/tests/auth_tests/models/__init__.py b/tests/auth_tests/models/__init__.py index 3422255d33..785e6953ff 100644 --- a/tests/auth_tests/models/__init__.py +++ b/tests/auth_tests/models/__init__.py @@ -6,14 +6,16 @@ from .invalid_models import CustomUserNonUniqueUsername from .is_active import IsActiveTestUser1 from .minimal import MinimalUser from .no_password import NoPasswordUser +from .proxy import Proxy, UserProxy from .uuid_pk import UUIDUser from .with_foreign_key import CustomUserWithFK, Email from .with_integer_username import IntegerUsernameUser from .with_last_login_attr import UserWithDisabledLastLoginField __all__ = ( - 'CustomUser', 'CustomUserWithoutIsActiveField', 'CustomPermissionsUser', - 'CustomUserWithFK', 'Email', 'ExtensionUser', 'IsActiveTestUser1', - 'MinimalUser', 'NoPasswordUser', 'UUIDUser', 'CustomUserNonUniqueUsername', - 'IntegerUsernameUser', 'UserWithDisabledLastLoginField', + 'CustomPermissionsUser', 'CustomUser', 'CustomUserNonUniqueUsername', + 'CustomUserWithFK', 'CustomUserWithoutIsActiveField', 'Email', + 'ExtensionUser', 'IntegerUsernameUser', 'IsActiveTestUser1', 'MinimalUser', + 'NoPasswordUser', 'Proxy', 'UUIDUser', 'UserProxy', + 'UserWithDisabledLastLoginField', ) diff --git a/tests/auth_tests/models/proxy.py b/tests/auth_tests/models/proxy.py new file mode 100644 index 0000000000..9e82a398c2 --- /dev/null +++ b/tests/auth_tests/models/proxy.py @@ -0,0 +1,22 @@ +from django.contrib.auth.models import User +from django.db import models + + +class Concrete(models.Model): + pass + + +class Proxy(Concrete): + class Meta: + proxy = True + permissions = ( + ('display_proxys', 'May display proxys information'), + ) + + +class UserProxy(User): + class Meta: + proxy = True + permissions = ( + ('use_different_app_label', 'May use a different app label'), + ) diff --git a/tests/auth_tests/test_management.py b/tests/auth_tests/test_management.py index 87f2ae1790..d0a91f3261 100644 --- a/tests/auth_tests/test_management.py +++ b/tests/auth_tests/test_management.py @@ -6,7 +6,7 @@ from io import StringIO from unittest import mock from django.apps import apps -from django.contrib.auth import management +from django.contrib.auth import get_permission_codename, management from django.contrib.auth.management import ( create_permissions, get_default_username, ) @@ -23,6 +23,7 @@ from django.utils.translation import gettext_lazy as _ from .models import ( CustomUser, CustomUserNonUniqueUsername, CustomUserWithFK, Email, + UserProxy, ) MOCK_INPUT_KEY_TO_PROMPTS = { @@ -985,3 +986,18 @@ class CreatePermissionsTests(TestCase): ContentType.objects.filter(app_label='auth', model='group').delete() # This fails with a foreign key constraint without the fix. create_permissions(apps.get_app_config('auth'), interactive=False, verbosity=0) + + def test_permission_with_proxy_content_type_created(self): + """ + A proxy model's permissions use its own content type rather than the + content type of the concrete model. + """ + opts = UserProxy._meta + codename = get_permission_codename('add', opts) + self.assertTrue( + Permission.objects.filter( + content_type__model=opts.model_name, + content_type__app_label=opts.app_label, + codename=codename, + ).exists() + ) diff --git a/tests/auth_tests/test_migrations.py b/tests/auth_tests/test_migrations.py new file mode 100644 index 0000000000..5ff2f6b4b3 --- /dev/null +++ b/tests/auth_tests/test_migrations.py @@ -0,0 +1,154 @@ +from importlib import import_module + +from django.apps import apps +from django.contrib.auth.models import Permission, User +from django.contrib.contenttypes.models import ContentType +from django.test import TestCase + +from .models import Proxy, UserProxy + +update_proxy_permissions = import_module('django.contrib.auth.migrations.0011_update_proxy_permissions') + + +class ProxyModelWithDifferentAppLabelTests(TestCase): + available_apps = [ + 'auth_tests', + 'django.contrib.auth', + 'django.contrib.contenttypes', + ] + + def setUp(self): + """ + Create proxy permissions with content_type to the concrete model + rather than the proxy model (as they were before Django 2.2 and + migration 11). + """ + Permission.objects.all().delete() + self.concrete_content_type = ContentType.objects.get_for_model(UserProxy) + self.default_permission = Permission.objects.create( + content_type=self.concrete_content_type, + codename='add_userproxy', + name='Can add userproxy', + ) + self.custom_permission = Permission.objects.create( + content_type=self.concrete_content_type, + codename='use_different_app_label', + name='May use a different app label', + ) + + def test_proxy_model_permissions_contenttype(self): + proxy_model_content_type = ContentType.objects.get_for_model(UserProxy, for_concrete_model=False) + self.assertEqual(self.default_permission.content_type, self.concrete_content_type) + self.assertEqual(self.custom_permission.content_type, self.concrete_content_type) + update_proxy_permissions.update_proxy_model_permissions(apps, None) + self.default_permission.refresh_from_db() + self.assertEqual(self.default_permission.content_type, proxy_model_content_type) + self.custom_permission.refresh_from_db() + self.assertEqual(self.custom_permission.content_type, proxy_model_content_type) + + def test_user_has_now_proxy_model_permissions(self): + user = User.objects.create() + user.user_permissions.add(self.default_permission) + user.user_permissions.add(self.custom_permission) + for permission in [self.default_permission, self.custom_permission]: + self.assertTrue(user.has_perm('auth.' + permission.codename)) + self.assertFalse(user.has_perm('auth_tests.' + permission.codename)) + update_proxy_permissions.update_proxy_model_permissions(apps, None) + # Reload user to purge the _perm_cache. + user = User._default_manager.get(pk=user.pk) + for permission in [self.default_permission, self.custom_permission]: + self.assertFalse(user.has_perm('auth.' + permission.codename)) + self.assertTrue(user.has_perm('auth_tests.' + permission.codename)) + + def test_migrate_backwards(self): + update_proxy_permissions.update_proxy_model_permissions(apps, None) + update_proxy_permissions.revert_proxy_model_permissions(apps, None) + self.default_permission.refresh_from_db() + self.assertEqual(self.default_permission.content_type, self.concrete_content_type) + self.custom_permission.refresh_from_db() + self.assertEqual(self.custom_permission.content_type, self.concrete_content_type) + + def test_user_keeps_same_permissions_after_migrating_backward(self): + user = User.objects.create() + user.user_permissions.add(self.default_permission) + user.user_permissions.add(self.custom_permission) + for permission in [self.default_permission, self.custom_permission]: + self.assertTrue(user.has_perm('auth.' + permission.codename)) + self.assertFalse(user.has_perm('auth_tests.' + permission.codename)) + update_proxy_permissions.update_proxy_model_permissions(apps, None) + update_proxy_permissions.revert_proxy_model_permissions(apps, None) + # Reload user to purge the _perm_cache. + user = User._default_manager.get(pk=user.pk) + for permission in [self.default_permission, self.custom_permission]: + self.assertTrue(user.has_perm('auth.' + permission.codename)) + self.assertFalse(user.has_perm('auth_tests.' + permission.codename)) + + +class ProxyModelWithSameAppLabelTests(TestCase): + available_apps = [ + 'auth_tests', + 'django.contrib.auth', + 'django.contrib.contenttypes', + ] + + def setUp(self): + """ + Create proxy permissions with content_type to the concrete model + rather than the proxy model (as they were before Django 2.2 and + migration 11). + """ + Permission.objects.all().delete() + self.concrete_content_type = ContentType.objects.get_for_model(Proxy) + self.default_permission = Permission.objects.create( + content_type=self.concrete_content_type, + codename='add_proxy', + name='Can add proxy', + ) + self.custom_permission = Permission.objects.create( + content_type=self.concrete_content_type, + codename='display_proxys', + name='May display proxys information', + ) + + def test_proxy_model_permissions_contenttype(self): + proxy_model_content_type = ContentType.objects.get_for_model(Proxy, for_concrete_model=False) + self.assertEqual(self.default_permission.content_type, self.concrete_content_type) + self.assertEqual(self.custom_permission.content_type, self.concrete_content_type) + update_proxy_permissions.update_proxy_model_permissions(apps, None) + self.default_permission.refresh_from_db() + self.custom_permission.refresh_from_db() + self.assertEqual(self.default_permission.content_type, proxy_model_content_type) + self.assertEqual(self.custom_permission.content_type, proxy_model_content_type) + + def test_user_still_has_proxy_model_permissions(self): + user = User.objects.create() + user.user_permissions.add(self.default_permission) + user.user_permissions.add(self.custom_permission) + for permission in [self.default_permission, self.custom_permission]: + self.assertTrue(user.has_perm('auth_tests.' + permission.codename)) + update_proxy_permissions.update_proxy_model_permissions(apps, None) + # Reload user to purge the _perm_cache. + user = User._default_manager.get(pk=user.pk) + for permission in [self.default_permission, self.custom_permission]: + self.assertTrue(user.has_perm('auth_tests.' + permission.codename)) + + def test_migrate_backwards(self): + update_proxy_permissions.update_proxy_model_permissions(apps, None) + update_proxy_permissions.revert_proxy_model_permissions(apps, None) + self.default_permission.refresh_from_db() + self.assertEqual(self.default_permission.content_type, self.concrete_content_type) + self.custom_permission.refresh_from_db() + self.assertEqual(self.custom_permission.content_type, self.concrete_content_type) + + def test_user_keeps_same_permissions_after_migrating_backward(self): + user = User.objects.create() + user.user_permissions.add(self.default_permission) + user.user_permissions.add(self.custom_permission) + for permission in [self.default_permission, self.custom_permission]: + self.assertTrue(user.has_perm('auth_tests.' + permission.codename)) + update_proxy_permissions.update_proxy_model_permissions(apps, None) + update_proxy_permissions.revert_proxy_model_permissions(apps, None) + # Reload user to purge the _perm_cache. + user = User._default_manager.get(pk=user.pk) + for permission in [self.default_permission, self.custom_permission]: + self.assertTrue(user.has_perm('auth_tests.' + permission.codename))