From 4f8ebdd09835ad5c010c122ec669c1b224801302 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Fri, 26 Apr 2019 08:51:58 +0200 Subject: [PATCH] [2.2.x] Fixed #30351 -- Handled pre-existing permissions in proxy model permissions data migration. Regression in 181fb60159e54d442d3610f4afba6f066a6dac05. Backport of 98296f86b340c8c9c968375d59f1d3a3479e60c2 from master --- .../0011_update_proxy_permissions.py | 30 +++++++++++++++---- docs/releases/2.2.1.txt | 5 ++++ tests/auth_tests/test_migrations.py | 25 ++++++++++++++++ 3 files changed, 55 insertions(+), 5 deletions(-) diff --git a/django/contrib/auth/migrations/0011_update_proxy_permissions.py b/django/contrib/auth/migrations/0011_update_proxy_permissions.py index 0e66649695..c3f617f438 100644 --- a/django/contrib/auth/migrations/0011_update_proxy_permissions.py +++ b/django/contrib/auth/migrations/0011_update_proxy_permissions.py @@ -1,5 +1,18 @@ -from django.db import migrations +import sys + +from django.core.management.color import color_style +from django.db import migrations, transaction from django.db.models import Q +from django.db.utils import IntegrityError + +WARNING = """ + A problem arose migrating proxy model permissions for {old} to {new}. + + Permission(s) for {new} already existed. + Codenames Q: {query} + + Ensure to audit ALL permissions for {old} and {new}. +""" def update_proxy_model_permissions(apps, schema_editor, reverse=False): @@ -7,6 +20,7 @@ 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. """ + style = color_style() Permission = apps.get_model('auth', 'Permission') ContentType = apps.get_model('contenttypes', 'ContentType') for Model in apps.get_models(): @@ -24,10 +38,16 @@ def update_proxy_model_permissions(apps, schema_editor, reverse=False): 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) + try: + with transaction.atomic(): + Permission.objects.filter( + permissions_query, + content_type=old_content_type, + ).update(content_type=new_content_type) + except IntegrityError: + old = '{}_{}'.format(old_content_type.app_label, old_content_type.model) + new = '{}_{}'.format(new_content_type.app_label, new_content_type.model) + sys.stdout.write(style.WARNING(WARNING.format(old=old, new=new, query=permissions_query))) def revert_proxy_model_permissions(apps, schema_editor): diff --git a/docs/releases/2.2.1.txt b/docs/releases/2.2.1.txt index 98f800d455..9696488e14 100644 --- a/docs/releases/2.2.1.txt +++ b/docs/releases/2.2.1.txt @@ -59,3 +59,8 @@ Bugfixes * Increased the default timeout when using ``Watchman`` to 5 seconds to prevent falling back to ``StatReloader`` on larger projects and made it customizable via the ``DJANGO_WATCHMAN_TIMEOUT`` environment variable (:ticket:`30361`). + +* Fixed a regression in Django 2.2 that caused a crash when migrating + permissions for proxy models if the target permissions already existed. For + example, when a permission had been created manually or a model had been + migrated from concrete to proxy (:ticket:`30351`). diff --git a/tests/auth_tests/test_migrations.py b/tests/auth_tests/test_migrations.py index 5ff2f6b4b3..98c5b5964a 100644 --- a/tests/auth_tests/test_migrations.py +++ b/tests/auth_tests/test_migrations.py @@ -4,6 +4,7 @@ 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 django.test.utils import captured_stdout from .models import Proxy, UserProxy @@ -152,3 +153,27 @@ class ProxyModelWithSameAppLabelTests(TestCase): 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_with_existing_target_permission(self): + """ + Permissions may already exist: + + - Old workaround was to manually create permissions for proxy models. + - Model may have been concrete and then converted to proxy. + + Output a reminder to audit relevant permissions. + """ + proxy_model_content_type = ContentType.objects.get_for_model(Proxy, for_concrete_model=False) + Permission.objects.create( + content_type=proxy_model_content_type, + codename='add_proxy', + name='Can add proxy', + ) + Permission.objects.create( + content_type=proxy_model_content_type, + codename='display_proxys', + name='May display proxys information', + ) + with captured_stdout() as stdout: + update_proxy_permissions.update_proxy_model_permissions(apps, None) + self.assertIn('A problem arose migrating proxy model permissions', stdout.getvalue())