Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#14249 closed (fixed)

Inactive users have less permissions then anonymous users with custom backend

Reported by: hvdklauw 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 hvdklauw 4 years ago.
Add support for in active user permissions

Download all attachments as: .zip

Change History (16)

comment:1 Changed 4 years ago by hvdklauw

  • Cc hvdklauw@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 4 years ago by lukeplant

  • Triage Stage changed from Unreviewed to Design 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 4 years ago by hvdklauw

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 4 years ago by Alex

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

Changed 4 years ago by hvdklauw

Add support for in active user permissions

comment:5 Changed 4 years ago by hvdklauw

  • 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 4 years ago by anonymous

  • Version changed from 1.2 to 1.3-alpha

comment:7 Changed 4 years ago by jezdez

  • Triage Stage changed from Design decision needed to Accepted

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

comment:8 Changed 4 years ago by jgelens

  • Cc jgelens@… added

comment:9 Changed 4 years ago by jezdez

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

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

comment:10 Changed 4 years ago by SmileyChris

(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 4 years ago by jezdez

  • Resolution fixed deleted
  • Status changed from closed to reopened

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 4 years ago by SmileyChris

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

comment:13 Changed 4 years ago by hvdklauw

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 4 years ago by jezdez

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

Closing after giving this a bit more thought.

comment:15 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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