[Django] #17903: `ModelBackend.get_all_permissions` returns permissions for inactive users
Django
------------------------------+--------------------
Reporter: SmileyChris | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: SVN
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------
The documentation states:
> Returns a set of permission strings that the user has, both through
group and user permissions
Since `has_perm` returns `False` for an inactive user, I wouldn't think it
should return an empty string.
The behaviour is not as explicitly stated as the other perm methods
though, they say:
> If the user is inactive, this method will always return False.
r14797 changed `get_all_permissions` to return all strings for superusers,
which seems to imply this should work the same way that has_perm does, not
just provide a list of permission strings directly assigned (or via
groups) to the user.
--
Ticket URL: <https://code.djangoproject.com/ticket/17903>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Type: Bug | Status: new
Component: contrib.auth | Version: SVN
Keywords: | Triage Stage: Design
Has patch: 0 | decision needed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by SmileyChris):
* needs_better_patch: => 0
* stage: Unreviewed => Design decision needed
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/17903#comment:1>
Django
Reporter: SmileyChris | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* stage: Design decision needed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/17903#comment:2>
Django
Reporter: SmileyChris | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by jorgecarleitao):
I checked the code, and get_all_permissions does the following:
{{{
If user is anonymous: returns empty set.
else: joins "user permissions" with "user groups permissions".
}}}
In "user groups permissions", if the user is superuser, this returns all
permissions.
So, in {{{has_perm}}} and {{{has_module_perms}}} we check for
{{{is_active}}}, but in {{{get_all_permissions}}} we check for
{{{is_anonymous}}}.
Shouldn't this be consistent?
--
Ticket URL: <https://code.djangoproject.com/ticket/17903#comment:3>
Django
Reporter: SmileyChris | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jorgecarleitao):
* cc: jorgecarleitao (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/17903#comment:4>
Django
Reporter: SmileyChris | Owner: jorgecarleitao
Type: Bug | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jorgecarleitao):
* status: new => assigned
* owner: nobody => jorgecarleitao
Comment:
Pull request [https://github.com/django/django/pull/2636]
--
Ticket URL: <https://code.djangoproject.com/ticket/17903#comment:5>
Django
Reporter: SmileyChris | Owner: jorgecarleitao
Type: Bug | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jorgecarleitao):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/17903#comment:6>
Django
Reporter: SmileyChris | Owner: jorgecarleitao
Type: Bug | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/17903#comment:7>
Django
Reporter: SmileyChris | Owner: jorgecarleitao
Type: Bug | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jorgecarleitao):
* needs_better_patch: 1 => 0
Comment:
To be consistent, I made both `get_all_permissions`,
`get_user_permissions` and `get_group_permissions` to require the user to
be active.
--
Ticket URL: <https://code.djangoproject.com/ticket/17903#comment:8>
Django
Reporter: SmileyChris | Owner: jorgecarleitao
Type: Bug | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: 0 => 1
Comment:
Left comments for how to improve the tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/17903#comment:9>
Django
Reporter: SmileyChris | Owner: jorgecarleitao
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"c33447a50c1b0a96c6e2261f7c45d2522a3fe28d"]:
{{{
#!CommitTicketReference repository=""
revision="c33447a50c1b0a96c6e2261f7c45d2522a3fe28d"
Fixed #17903 -- Modified ModelBackend to eliminate permissions on inactive
users.
Thanks to @SmileyChris for the report and @timgraham for review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/17903#comment:10>
Django
Reporter: SmileyChris | Owner: jorgecarleitao
Type: Bug | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by Tim Graham <timograham@…>):
In [changeset:"150d88cc2c0866ef65f077387e3e560e9c9c3f80"]:
{{{
#!CommitTicketReference repository=""
revision="150d88cc2c0866ef65f077387e3e560e9c9c3f80"
Restored is_anonymous() check in ModelBackend permission checking removed
in refs #17903.
Thanks Florian Apolloner for raising the issue.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/17903#comment:11>