Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#14249 closed (fixed)

Inactive users have less permissions then anonymous users with custom backend

Reported by: Harro Owned by: nobody
Component: contrib.auth Version: 1.3-alpha
Severity: Keywords:
Cc: hvdklauw@…, jgelens@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

With the closing of Ticket #12557 a custom backend could specify anonymous user permissions.

However now I have a system where an anonymous user has some permissions and a logged in inactive user (User.is_active == False) has no permissions at all.

I suggest the checks for is_active and is_superuser get removed as a check from the User class itself and instead get moved to the default authentication backend.

That way the default way keeps working the way it currently does, but it will allow developers to use those two properties as they see fit when they implement a custom backend.

Attachments (1)

14249.diff (12.6 KB) - added by Harro 6 years ago.
Add support for in active user permissions

Download all attachments as: .zip

Change History (16)

comment:1 Changed 6 years ago by Harro

Cc: hvdklauw@… added

comment:2 Changed 6 years ago by Luke Plant

Triage Stage: UnreviewedDesign decision needed

I'm inclined to agree, but there is a security related backwards incompatibility: if someone has already implemented an auth backend, then this change will open up a hole where inactive users may get permissions, whereas before they had none. That code will have to be updated to close the hole. So I'll mark design decision needed - please bring it up on django-devs.

Thanks!

comment:3 Changed 6 years ago by Harro

You're right. I overlooked that one.
Also a superuser without permissions might be weird ;-)

I guess we'll have to add another property to the backend as #12557 did to make sure if people update it get's checked.

I'll see if I have time this week to write a patch.

comment:4 Changed 6 years ago by Alex Gaynor

Seems to me we'd want to add a flag to Auth backends, like we did for anonymous and per-object permissions.

Changed 6 years ago by Harro

Attachment: 14249.diff added

Add support for in active user permissions

comment:5 Changed 6 years ago by Harro

Has patch: set

Think that's it.
The in_active user permission system is a bit weird in that the get_all_permissions function does return the permissions for an inactive user.

comment:6 Changed 6 years ago by anonymous

Version: 1.21.3-alpha

comment:7 Changed 6 years ago by Jannis Leidel

Triage Stage: Design decision neededAccepted

Accepting this based on the fact that if omitted this would make the auth backend inconsistent with regard to anonymous users.

comment:8 Changed 6 years ago by Jeffrey Gelens

Cc: jgelens@… added

comment:9 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

(In [15010]) Fixed #14249 -- Added support for inactive users to the auth backend system. Thanks, Harro van der Klauw.

comment:10 Changed 6 years ago by Chris Beaven

(In [15204]) Change the lack of supports_inactive_user on an auth backend to a
PendingDeprecationWarning (refs #14249), fixing some bad links in the
1.3 release docs and a typo.

comment:11 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: closedreopened

Changing this to a pendingdeprecation warning doesn't make sense here, since we are going to require to support inactive users in 1.5.

comment:12 Changed 6 years ago by Chris Beaven

Er, this is what the changed documentation said, and is also the normal deprecation path isn't it?

comment:13 Changed 6 years ago by Harro

What is being deprecated here?

  • The supports_inactive_user flag
  • Or the fact that we you can use backends without the flag in django?

As I read the docs it is indeed a PendingDeprecation for 1.3
a deprecation for 1.4
and a removal of all the checks and everything for 1.5 (We now assume the backend supports inactive users)

comment:14 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: reopenedclosed

Closing after giving this a bit more thought.

comment:15 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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