Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#19086 closed Bug (fixed)

Content types and permissions are created for swapped models

Reported by: Thomas Schreiber Owned by: Russell Keith-Magee
Component: contrib.auth Version: dev
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])
...

Change History (11)

comment:1 by Thomas Schreiber, 11 years ago

Owner: changed from nobody to Thomas Schreiber

comment:2 by Thomas Schreiber, 11 years ago

Type: UncategorizedBug

comment:3 by Russell Keith-Magee, 11 years ago

Summary: auth.User should not be loaded when using a custom user modelContent types and permissions are created for swapped models
Triage Stage: UnreviewedAccepted

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 by Thomas Schreiber, 11 years ago

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 11 years ago by Thomas Schreiber (previous) (diff)

comment:5 by Preston Holmes, 11 years ago

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 by Preston Holmes, 11 years ago

Owner: changed from Thomas Schreiber to Preston Holmes

comment:7 by Preston Holmes, 11 years ago

Owner: Preston Holmes removed

comment:8 by Russell Keith-Magee, 11 years ago

Owner: set to Russell Keith-Magee

comment:9 by Russell Keith-Magee, 11 years ago

Resolution: fixed
Status: newclosed

In c8985a8a7317042a641e870cb75b3005cc5d67b1:

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

Thanks to rizumu for the report.

comment:10 by Russell Keith-Magee, 11 years ago

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 by Russell Keith-Magee, 11 years ago

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

Note: See TracTickets for help on using tickets.
Back to Top