Fixed #11154, #22270 -- Made proxy model permissions use correct content type.

Co-Authored-By: Simon Charette <charette.s@gmail.com>
Co-Authored-By: Antoine Catton <acatton@fusionbox.com>

AUTHORS

@@ -86,6 +86,7 @@ answer newbie questions, and generally made Django that much better:
     Artem Gnilov <boobsd@gmail.com>
     Arthur <avandorp@gmail.com>
     Arthur Koziel <http://arthurkoziel.com>
+    Arthur Rio <arthur.rio44@gmail.com>
     Arvis Bickovskis <viestards.lists@gmail.com>
     Aryeh Leib Taurog <http://www.aryehleib.com/>
     A S Alam <aalam@users.sf.net>

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):

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),
+    ]

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 <proxy-models-permissions-topic>` 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
 -------------
 

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
+    <proxy-models-permissions-topic>`, 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

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)

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

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, '<h1>Select user proxy to view</h1>')
+        self.assertEqual(response.status_code, 200)
+        response = self.client.get(reverse('admin:admin_views_userproxy_change', args=(self.user_proxy.pk,)))
+        self.assertContains(response, '<h1>View user proxy</h1>')
+        self.assertContains(response, '<div class="readonly">user_proxy</div>')
+
+    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"""

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',
 )

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'),
+        )

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()
+        )

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))