Code

Opened 19 months ago

Closed 17 months ago

Last modified 17 months ago

#19086 closed Bug (fixed)

Content types and permissions are created for swapped models

Reported by: rizumu Owned by: russellm
Component: contrib.auth Version: master
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

While trying to update django-guardian to support AUTH_USER_MODEL I discovered the contenttypes and permissions are still being created for auth.User

This means the auth.User app is still being loaded and I've traced the problem to this loop:

https://github.com/django/django/blob/master/django/db/backends/__init__.py#L998

If someone could suggest where the correct place to put the check is, I'll be happy to put together a patch and test to reproduce the issue.

I think the following explicit check could work, but it could be better to keep the logic in model._meta for auth.User and check on that instead:

...
for app in models.get_apps():
    for model in models.get_models(app):
        if ('django.contrib.auth' in settings.INSTALLED_APPS and settings.AUTH_USER_MODEL != 'auth.User' and
            model.__name__ == 'User' and model.__module == 'django.contrib.auth.models'):
            continue
        if not model._meta.managed:
            continue
        if not router.allow_syncdb(self.connection.alias, model):
            continue
        tables.add(model._meta.db_table)
        tables.update([f.m2m_db_table() for f in model._meta.local_many_to_many])
...

Attachments (0)

Change History (11)

comment:1 Changed 19 months ago by rizumu

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to rizumu
  • Patch needs improvement unset

comment:2 Changed 19 months ago by rizumu

  • Type changed from Uncategorized to Bug

comment:3 Changed 19 months ago by russellm

  • Summary changed from auth.User should not be loaded when using a custom user model to Content types and permissions are created for swapped models
  • Triage Stage changed from Unreviewed to Accepted

The fact that the swapped model is loaded is intentional - otherwise code wouldn't have a way to use the app cache to find the User object and discover that it wasn't in use.

However, the creation of contenttypes and permissions is *not* intentional; the fix there is to correct the syncdb handlers. I've updated the ticket title to reflect the actual problem being reported.

As an aside - we've deliberately avoided referencing auth.User as a literal anywhere in the code -- so you check for the generalized properties of _meta.swappable and _meta.swapped, rather than checking for a literal of auth.User.

comment:4 Changed 18 months ago by rizumu

I've reached a solution here that prevents the Permission and ContentType from being created, please let me know if this looks good:

https://github.com/rizumu/django/compare/19086

When setting up the test case for CustomUser, I hit the createsuperuser issue:

https://code.djangoproject.com/ticket/19067#comment:13

Despite that, I'm going to write two tests, one to prove that the ContentType is not created and another for the Permission, just wanted to note the tests are dependent on the createsuperuser working first.

Last edited 18 months ago by rizumu (previous) (diff)

comment:5 Changed 18 months ago by ptone

  • Has patch set
  • Needs tests set
  • Patch needs improvement set

Thanks for your work on this.

A couple points:

The test you've added: CustomUserModelBackendTest, doesn't really relate to this ticket at all.

The reason its failing doesn't have anything to do with #19067 (in fact, create_superuser is already tested for CustomUser elsewhere) but because CustomUser does not respond as expected with regard to the permission related methods e.g. self.assertEqual(user.get_all_permissions() == set(['auth.test']), True).

It may be worth considering the skipIfCustomUser to those perm tests in the BaseModelBackendTest class

So yes- the tests still needed are the ones you describe that test that perms/contenttypes are not created for the stock User model - and perhaps ARE created for the custom user model

comment:6 Changed 18 months ago by ptone

  • Owner changed from rizumu to ptone

comment:7 Changed 18 months ago by ptone

  • Owner ptone deleted

comment:8 Changed 18 months ago by russellm

  • Owner set to russellm

comment:9 Changed 17 months ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

In c8985a8a7317042a641e870cb75b3005cc5d67b1:

Fixed #19806 -- Ensure that content types and permissions aren't created for swapped models.

Thanks to rizumu for the report.

comment:10 Changed 17 months ago by russellm

In 3fd8458fb3a30ea9fc6b3e7c08ff2d66a63e5067:

[1.5.x] Fixed #19806 -- Ensure that content types and permissions aren't created for swapped models.

Thanks to rizumu for the report.

Backport of c8985a8a7317042a641e870cb75b3005cc5d67b1.

comment:11 Changed 17 months ago by russellm

Fat fingers mistakenly attributed the last two commits to #19806.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.