Allow a Backend to Globally Fail Authentication

[umbrae@…]

Allows a backend to fail all authentication - I.E. if a user exists in a backend but the password is wrong, it's very possible that the backend might just want to kick the user out instead of letting it go through the rest of the authentication backends and possibly succeeding.

Kind of like PAM's 'required' option.

[umbrae@…]

Patch to implement 'required' functionality

[umbrae@…]

A (hopefully) viewable version of the previous diff

[umbrae@…]

Well, I've obviously shown that I'm pretty incompetent when it comes to using tickets. (Sorry guys, I'm a newbie.)

Here's the diff:

Index: trunk/django/contrib/auth/__init__.py
===================================================================
--- trunk/django/contrib/auth/__init__.py       (revision 3596)
+++ trunk/django/contrib/auth/__init__.py       (working copy)
@@ -1,4 +1,4 @@
-from django.core.exceptions import ImproperlyConfigured
+from django.core.exceptions import ImproperlyConfigured, PermissionDenied

 SESSION_KEY = '_auth_user_id'
 BACKEND_SESSION_KEY = '_auth_user_backend'
@@ -35,6 +35,9 @@
         except TypeError:
             # This backend doesn't accept these credentials as arguments. Try the next one.
             continue
+        except PermissionDenied:
+            # This backend says to stop in our tracks - this user should not be allowed in at all.
+            return None
         if user is None:
             continue
         # Annotate the user object with the path of the backend.

[Gary Wilson]

Jacob, did you mean to close this or mark it accepted?

I think this we be more flexible if it were part of the AUTHENTICATION_BACKENDS settings instead of hardcoding it into a backend. Authentication backend re-use would go out the window if people started raising PermissionDenied in their backends.

[Chris Beaven]

I agree with gwilson, it's probably better to have a new setting to control it (it seems more of a project-level choice).

I'll push back to design decision anyway

[Malcolm Tredinnick]

Any auth backend should be able to raise "stop here" (return "no", not "don't care"). What special value to return is up to the implementor. It's purely part of the auth backend, not a setting.

[Ashutosh Dwivedi]

the implementation along with test cases submitted by aashu_dwivedi

[Ashutosh Dwivedi]

I am not sure but do i need to update the documentation also ?

[Łukasz Rekucki]

Yes, otherwise people won't even know the feature is available.

Some comments to the patch:

• Your tests fail (did you runned them?):

  Traceback (most recent call last):
    File "/home/lrekucki/django/django-gh/django/contrib/auth/tests/auth_backends.py", line 388, in test_authenticates
      self.assertEqual(authenticate(username='test',password='test'),self.user1)
  NameError: global name 'authenticate' is not defined
  

• Some of your code (like "except PermissionDenied" in django/contrib/__init__.py) is indented with tabs. Mixin spaces and tabs is bad. Django prefers 4 spaces as indentation.
• You should also format your code according to PEP8 (i.e. whitespace around operators, whitespace after comma in argument list).

[Ashutosh Dwivedi]

corrected patch

[Ashutosh Dwivedi]

Added corrected patch with the required addition to the documentation .

[Łukasz Rekucki]

Fixed indentation in the previous patch. Tweaked the docs a bit.

[Łukasz Rekucki]

On a bureaucratic note: you shouldn't mark your own patches as "Ready for checkin". Instead update the appropriate flags and someone else will review the patch (also see the ​contributing howto).

Thanks for your work on this, aashu_dwivedi.

[Aymeric Augustin]

#15716 was a duplicate.

[anonymous]

PermissionDeniedBackend should set supports_anonymous_user and supports_object_permissions to True since that will be the default soon (and the only option). And if we are already on it: Shouldn't the permission part of the backends support the same mechanism?

[dan.julius@…]

Fixed value of supports_anonymous_user and supports_object_permissions

[dan.julius@…]

Modified PermissionDeniedBackend class:

• Set supports_anonymous_user and supports_object_permissions to True
• Added supports_inactive_user = True (which will be required in v1.5)

Implementing similar functionality for permissions will require design change regarding get_all_permissions.
Since each backend provides a list of permissions it allows, the list would be inconsistent if a backend raised the PermissionDenied exception on a permission already in the list.

[Aymeric Augustin]

Revert apparent spam. If this is really fixed, please provide a short explanation, for instance a commit reference or superseding ticket.

[Jacob]

Milestone 1.4 deleted

[Carl Meyer]

Please don't mark tickets fixed without explanation.

[Daniel Roseman]

Update patch to apply against 1.4

[Jannis Leidel]

Please update the docs release notes about the changes and use override_settings in the tests for easier settings switching per test.

[namn]

ticket2550d.diff

[namn]

Patch revised.

[Jannis Leidel <jannis@…>]

In 1520748dac95a7f114e4bb2feeee04d46c720494:

Fixed #2550 -- Allow the auth backends to raise the PermissionDenied exception to completely stop the authentication chain. Many thanks to namn, danielr, Dan Julius, Łukasz Rekucki, Aashu Dwivedi and umbrae for working this over the years.