Opened 14 years ago

Closed 11 years ago

#15716 closed New feature (fixed)

The has_perm() method of authorization backends should be able to explicitly deny permission

Reported by: German M. Bravo Owned by: jorgecarleitao
Component: contrib.auth Version:
Severity: Normal Keywords:
Cc: German M. Bravo, albrecht.andi@…, jorgecarleitao Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

For authorization backends, has_perm() should return True if the authorization is granted, False if it's not, and None if it doesn't know about certain permission.

Reading django's source code, I see that if has_perm(), in the backends, returns False, it keeps trying other backends for a successful authorization (i.e. for any other has_perm() in any other backends returning True)

I was thinking, shouldn't a has_perm() returning False be a definitive "False" (as in no permission or permission denied and stop trying?) My take would be perhaps has_perm() returning None in case the backend simply doesn't know about the asked permission, so only then django keeps on trying in other backends, but if has_perm() otherwise returns False, it should always mean an absolute Permission Denied.

This would allow cases where several authorization backends are set up and some backends handle explicit permission denied rules.

Change History (13)

comment:1 by Aymeric Augustin, 14 years ago

Resolution: duplicate
Status: newclosed

Duplicate of #2550

comment:2 by German M. Bravo, 14 years ago

Resolution: duplicate
Status: closedreopened

I reopened the ticket since #15716 is not entirely a duplicate of #2550, since ticket #15716 is about authorization, and #2550 is about authentication... they are both related nonetheless.

I would suggest, in my opinion, for the case of authorization in this ticket, to use an irrevocable False for permission denied, True for permission granted and either None or an Exception (or both) for "Permission Unknown" or "Don't care".

In case of has_perm() returning None or raising a PermissionUnkown Exception, django would keep on looking for the permission in other backends, otherwise return the permission from the backend.

comment:3 by Chris Beaven, 14 years ago

Component: Uncategorizedcontrib.auth
Triage Stage: UnreviewedDesign decision needed
Version: 1.2

We can't just change False to mean "permission denied" since that would be entirely backwards incompatible. I'd say it would probably be better to follow the example of #2550 and allow has_perm to raise PermissionDenied, short-circuiting the remainder of the permissions checks.

comment:4 by German M. Bravo, 14 years ago

I absolutely agree SmileyChris, just to keep backward compatibility, though not being this the same case as in #2550, where authenticate() returns a User object or None (in which the PermissionDenied exception is the obvious solution) and had we had the opportunity to choose, False meaning Permission Denied, I'd say is better.

The edge case in which using False to mean Permission Denied here could hit backward compatibility would be only when multiple authorization backends are used, chaining multiple fallback calls to has_perm(), which I don't know how often or serious that'd be. In all other cases, it seems using False to mean Permission Denied shouldn't pose a problem, and by design we'd have a more solid, elegant, intuitive and logical implementation of has_perm(), not allowing, or denying, a permission when it returns False, as it should. But I suppose the consequences of this decision call for further discussion.

comment:5 by Luke Plant, 14 years ago

Type: New feature

comment:6 by Luke Plant, 14 years ago

Severity: Normal

comment:7 by Chris Beaven, 14 years ago

Easy pickings: unset
Summary: in authoriation backends, has_perm() should return None if it doesn't know about a pemissionThe has_perm() method of authorization backends should be able to explicitly deny permission
Triage Stage: Design decision neededAccepted

I reassert that I want this feature to use an exception to explicitly deny that permission.

I'm leaving it open as a close relative to #2550, since this deals with authorization rather than authentication.

comment:8 by German M. Bravo, 13 years ago

Cc: German M. Bravo added
UI/UX: unset

comment:9 by Andi Albrecht, 12 years ago

Cc: albrecht.andi@… added

comment:10 by Aymeric Augustin, 12 years ago

Status: reopenednew

comment:11 by jorgecarleitao, 11 years ago

Cc: jorgecarleitao added

I believe the option of raising an exception is superior to return False since it allows exceptions that tell us why the permission was denied. A False would only provide information on whether it was forbidden or not, which can be too restrictive for future development of backends.

comment:12 by jorgecarleitao, 11 years ago

Has patch: set
Owner: changed from nobody to jorgecarleitao
Status: newassigned

comment:13 by Tim Graham <timograham@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 2e364a0aacb49a5160896b1ca5a2619baa3f4d9b:

Fixed #15716 - Authentication backends can short-circuit authorization.

Authorization backends can now raise PermissionDenied in "has_perm"
and "has_module_perms" to short-circuit authorization process.

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