[Django] #36843: RenamePermission might operate on unrelated models

[Django]

#36843: RenamePermission might operate on unrelated models
-----------------------------+-------------------------------------------
Reporter: Jacob Walls | Type: Bug
Status: new | Component: contrib.auth
Version: 6.0 | Severity: Release blocker
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+-------------------------------------------
As of Django 6.0 (#27489), when a migration contains a `RenameModel`
operation, ephemeral `RenamePermission` operations are injected to sync
the name & codename of the model's permission records.

As mentioned in
https://github.com/django/django/pull/20339#discussion_r2653481704, we
discovered that the ephemeral operation might operate on unrelated models
owing to its use of `model__icontains`:

{{{#!py
ctypes = ContentType.objects.using(db).filter(
app_label=self.app_label, model__icontains=old_model.lower()
)
for permission in Permission.objects.using(db).filter(
content_type_id__in=ctypes.values("id")
):
}}}

To reproduce:

- Have `Song` and `SongProfile` models, assign a view permission on
`SongProfile` to a user
- Rename `Song` to `Son`, migrate
- `RenamePermission` adjusts the four permissions on each model (8 perms
total) to all have "son" in the name & codename
- Permissions are now missing for `SongProfile` , so they get regenerated
by the post_migrate handler
- existing user/group assignments (e.g. the one created above) are now
pointing to the wrong permission ("add_son" with a content type pointing
to `SongProfile`)


Unlike #36793, which hinged on the existence of stale records causing
conflicts (possibly from reapplying or reversing migrations in
environments that had stale permissions before Django 6), this scenario
doesn't involve any duplicates, and the models in step 2 don't need to be
the same as step 1, just have common substrings. I reused a model for
convenience, but the point is that an unrelated model's permissions are
being affected.

Suggesting to revert of f02b49d2f3bf84f5225de920ca510149f1f9f1da and that
we address the issues identified in
https://github.com/django/django/pull/20481 (which was a first pass at
identifying & fixing) with more breathing room in a future version.
--
Ticket URL: <https://code.djangoproject.com/ticket/36843>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36843: RenamePermission might operate on unrelated models

---------------------------------+---------------------------------------
Reporter: Jacob Walls | Owner: Jacob Walls
Type: Bug | Status: assigned
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

---------------------------------+---------------------------------------
Changes (by Jacob Walls):

* owner: (none) => Jacob Walls
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36843#comment:1>

[Django]

#36843: RenamePermission might operate on unrelated models
---------------------------------+---------------------------------------
Reporter: Jacob Walls | Owner: Jacob Walls
Type: Bug | Status: assigned
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+---------------------------------------

Description changed by Jacob Walls:

Old description:

New description:

As of Django 6.0 (#27489), when a migration contains a `RenameModel`
operation, ephemeral `RenamePermission` operations are injected to sync
the name & codename of the model's permission records.

As mentioned in
https://github.com/django/django/pull/20339#discussion_r2653481704, we
discovered that the ephemeral operation might operate on unrelated models
owing to its use of `model__icontains`:

{{{#!py
ctypes = ContentType.objects.using(db).filter(
app_label=self.app_label, model__icontains=old_model.lower()
)
for permission in Permission.objects.using(db).filter(
content_type_id__in=ctypes.values("id")
):
}}}

To reproduce:

- Have `Song` and `SongProfile` models, migrate (this produces permission
records during `post_migrate`)
- assign a view permission on `SongProfile` to a user

- Rename `Song` to `Son`, migrate
- `RenamePermission` adjusts the four permissions on each model (8 perms
total) to all have "son" in the name & codename
- Permissions are now missing for `SongProfile` , so they get regenerated
by the post_migrate handler
- existing user/group assignments (e.g. the one created above) are now
pointing to the wrong permission ("add_son" with a content type pointing
to `SongProfile`)


Unlike #36793, which hinged on the existence of stale records causing
conflicts (possibly from reapplying or reversing migrations in
environments that had stale permissions before Django 6), this scenario
doesn't involve any duplicates, and the models in step 2 don't need to be
the same as step 1, just have common substrings. I reused a model for
convenience, but the point is that an unrelated model's permissions are
being affected.

Suggesting to revert of f02b49d2f3bf84f5225de920ca510149f1f9f1da and that
we address the issues identified in
https://github.com/django/django/pull/20481 (which was a first pass at
identifying & fixing) with more breathing room in a future version.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/36843#comment:2>

[Django]

#36843: RenamePermission might operate on unrelated models
---------------------------------+---------------------------------------
Reporter: Jacob Walls | Owner: Jacob Walls
Type: Bug | Status: assigned
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Unreviewed

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+---------------------------------------

Changes (by Jacob Walls):

* has_patch: 0 => 1

Comment:

[https://github.com/django/django/pull/20497 PR] to revert #27489
--
Ticket URL: <https://code.djangoproject.com/ticket/36843#comment:3>

[Django]

#36843: RenamePermission might operate on unrelated models
---------------------------------+---------------------------------------
Reporter: Jacob Walls | Owner: Jacob Walls
Type: Bug | Status: assigned
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+---------------------------------------

Changes (by Natalia Bidart):

* stage: Unreviewed => Accepted

Comment:

I can confirm the described problem. After following the steps, I get:
{{{
django-postgis=# select * from django_content_type where model ilike
'%son%';
id | app_label | model
----+-----------+-------------
60 | testapp | songprofile
59 | testapp | son
(2 rows)

django-postgis=# select * from auth_permission where content_type_id in
(60, 59);
id | name | content_type_id | codename
-----+-------------------------+-----------------+--------------------
233 | Can add son | 59 | add_son
234 | Can change son | 59 | change_son
235 | Can delete son | 59 | delete_son
236 | Can view son | 59 | view_son
237 | Can add son | 60 | add_son
238 | Can change son | 60 | change_son
239 | Can delete son | 60 | delete_son
240 | Can view son | 60 | view_son
241 | Can add song profile | 60 | add_songprofile
242 | Can change song profile | 60 | change_songprofile
243 | Can delete song profile | 60 | delete_songprofile
244 | Can view song profile | 60 | view_songprofile
(12 rows)
}}}

And the user that originally got a view perm for songprofile, can now view
son instances.
--
Ticket URL: <https://code.djangoproject.com/ticket/36843#comment:4>

[Django]

#36843: RenamePermission might operate on unrelated models

-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36843#comment:5>

[Django]

#36843: RenamePermission might operate on unrelated models
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
| Walls

Type: Bug | Status: closed
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls <jacobtylerwalls@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"030c63d329c4814da221528e823a4aaaaa40e4f1" 030c63d3]:
{{{#!CommitTicketReference repository=""
revision="030c63d329c4814da221528e823a4aaaaa40e4f1"
Fixed #36843, #36793 -- Reverted "Fixed #27489 -- Renamed permissions upon
model renaming in migrations."

This reverts commits f02b49d2f3bf84f5225de920ca510149f1f9f1da and
6e89271a8507fe272d11814975500a1b40303a04.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36843#comment:6>

[Django]

#36843: RenamePermission might operate on unrelated models
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
| Walls
Type: Bug | Status: closed
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls <jacobtylerwalls@…>):

In [changeset:"42bab76258162e1df77d080aa69cac5d803ae90a" 42bab762]:
{{{#!CommitTicketReference repository=""
revision="42bab76258162e1df77d080aa69cac5d803ae90a"
[6.0.x] Fixed #36843, #36793 -- Reverted "Fixed #27489 -- Renamed

permissions upon model renaming in migrations."

This reverts commits f02b49d2f3bf84f5225de920ca510149f1f9f1da and
6e89271a8507fe272d11814975500a1b40303a04.

Backport of 030c63d329c4814da221528e823a4aaaaa40e4f1 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36843#comment:7>