Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#19412 closed Bug (fixed)

Document that custom user models require a `user_permissions` M2M

Reported by: Craig de Stigter Owned by: nobody
Component: Documentation Version: 1.5-beta-1
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

It appears that django.contrib.auth.backends.ModelBackend requires that user models have a user_permissions ManyToManyField.

cf https://groups.google.com/d/msg/django-developers/K2cA2C3VeQw/6YoF1J9Sjx0J

This requirement isn't documented AFAICT. It should probably be mentioned in the Custom User Models docs where the other User requirements are listed.

Marking RB since this is an issue with a new feature

Attachments (1)

modelbackend-perms.diff (2.2 KB ) - added by Preston Holmes 11 years ago.

Download all attachments as: .zip

Change History (6)

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

Triage Stage: UnreviewedAccepted

user_permissions is only required if you try to check permissions on the user object, and that should only happen if you port over the permission check methods from AbstractUser.

However, the relationship between the ModelBackend and permissions definitely deserves to be documented.

There may also be some crossover here with a request from django-dev to abstract out the AbstractUser permissions bits into a mixin model that can be re-used.

by Preston Holmes, 11 years ago

Attachment: modelbackend-perms.diff added

comment:2 by Preston Holmes, 11 years ago

Has patch: set
Needs documentation: set
Needs tests: set
Patch needs improvement: set

I think we could make the backend more resilient by changing the behavior to return no permissions for user models that don't define the permission related fields.

Essentially - a user that doesn't define permissions, has no permissions - but doesn't raise any error.

I've attached a patch demonstrating this - and can test and document if this seems a valid approach.

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

@ptone: I can see what you're driving at, but I'm not sure that's the right approach. Yes, the ModelBackend defines has_perm et al, but they're accessed via the permission methods on the User. If you've got a custom user, and you've added the methods that explicitly call the authentication backends looking for permissions, I think it's reasonable to assume that you've also installed all the plumbing to support those permissions calls (or that you've defined a custom auth backend that uses a different approach).

I think the PermissionsMixin (a mixin that encapsulates all the user_permission bits into a single class) is a better approach -- this has been requested on django-dev anyway, and it ensures that if someone is going to be calling into ModelBackend, they've got all the pieces they need.

comment:4 by Russell Keith-Magee <russell@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 47e1df896b17aaaa97b73ef64010a7df4ea3d8d6:

Fixed #19412 -- Added PermissionsMixin to the auth.User heirarchy.

This makes it easier to make a ModelBackend-compliant (with regards to
permissions) User model.

Thanks to cdestigter for the report about the relationship between
ModelBackend and permissions, and to the many users on django-dev that
contributed to the discussion about mixins.

comment:5 by Russell Keith-Magee <russell@…>, 11 years ago

In 311bd0055d6302ce42d3831f95d8b255986ddc40:

[1.5.X} Fixed #19412 -- Added PermissionsMixin to the auth.User heirarchy.

This makes it easier to make a ModelBackend-compliant (with regards to
permissions) User model.

Thanks to cdestigter for the report about the relationship between
ModelBackend and permissions, and to the many users on django-dev that
contributed to the discussion about mixins.

Backport of 47e1df896b17aaaa97b73ef64010a7df4ea3d8d6 from master.

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