Opened 2 years ago

Closed 2 years ago

Last modified 2 years 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 2 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 2 years 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 2 years ago by ptone

comment:2 Changed 2 years 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 2 years 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 2 years 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 2 years 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.

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