id,summary,reporter,owner,description,type,status,component,version,severity,resolution,keywords,cc,stage,has_patch,needs_docs,needs_tests,needs_better_patch,easy,ui_ux 28379,Fix redirect loop in AccessMixin if a user lack permissions,Dylan Verheul,Dylan Verheul,"`AccessMixin` and derived classes offer a great way to handle permission testing for class based views. However, the current default workflow for handling failed permissions can lead to an endless loop of redirects, and is hard to fix without resorting to writing a custom `AccessMixin` for each project. **Why is the current workflow wrong?** The default way of handling ""no permission"" is to check for `AccessMixin.raise_exception` (default: `False`). If `AccessMixin.raise_exception` is `True`, then the `PermissionDenied` exception is raised. Else (the default behavior), the user is redirected to the `login_url`. The `PermissionDenied` exception can be customized in Django by providing a `403.html`. In this template, it is easy to explain to the user that access was denied, and to check for possible solutions, such as loggin in an authorized user. In my opinion, this should always be the default. Lack of permissions in a Python application calls for an exception to be raised. However, the default behavior is to redirect to the `login_url`. What happens to a **user that has not yet logged in** this scenario? 1. The anonymous user visits a URL, let's call it `show_me_something` 2. Permission is denied 3. The user is redirected to a login page, without any explanation 4. Upon successful login, the user will probably be redirected to the original intent `show_me_something` This isn't pretty, the user doesn't know what is going on. In my apps I would always go for the exception to be raised and to show a 403 page that explains that the uses has insufficient rights to view `show_me_something`, offer the user a link to login and signup, and carry over the url to `show_me_something` as a redirect upon succesful login. What's worse is what happens to a user that is already logged in. 1. The authenticated user visits a URL, let's call it `show_me_something` 2. Permission is denied 3. The user is redirected to a login page, without any explanation, but already logged in. Some login pages may even refuse access to a logged in user and redirect them. More confusion! 4. The only logical thing for the user to do is to provide login credentials again. 5. Upon successful login, the user will probably be redirected to the original intent `show_me_something`, where permission will be denied again, goto 1. **What should be improved** My suggestions are: 1. All in all I'd say always raise the `PermissionDenied` exception and have one way to handle missing permissions (DRY). That would mean at least changing the default value for `AccessMixin.raise_exception` to `True`, and possible even deprecate it. Currently, setting `raise_exception` to False is a recipe for writing views that confuse users and even make them end up in endless loops. 2. If raising the exception is not the behavior in the AccessMixin, never redirect a logged in user to the `login_url`, but raise `PermissionDenied` instead. So change `handle_no_permission` to: {{{ def handle_no_permission(self): if self.raise_exception or self.request.user.is_authenticated: raise PermissionDenied(self.get_permission_denied_message()) return redirect_to_login(self.request.get_full_path(), self.get_login_url(), self.get_redirect_field_name()) }}} ",Bug,closed,contrib.auth,1.11,Normal,fixed,permissions,Carlton Gibson,Ready for checkin,1,0,0,0,0,0