Code

Opened 17 months ago

Closed 16 months ago

Last modified 16 months ago

#19412 closed Bug (fixed)

Document that custom user models require a `user_permissions` M2M

Reported by: cdestigter 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 ptone 17 months ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 17 months ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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.

Changed 17 months ago by ptone

comment:2 Changed 17 months ago by ptone

  • 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 Changed 16 months ago by russellm

@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 Changed 16 months ago by Russell Keith-Magee <russell@…>

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

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 Changed 16 months ago by Russell Keith-Magee <russell@…>

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.

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.