profcturner / WAM

Workload Allocation Modeller and Assessment Approval System
GNU Affero General Public License v3.0
4 stars 0 forks source link

Views with PermissionRequiredMixin incorrectly tries to direct to login page #22

Closed profcturner closed 5 years ago

profcturner commented 5 years ago

Some newer views user the PermissionRequiredMixin. Logged in users without the permission are automatically redirected to a missing login page.

They should be cleanly informed that they don't have the permission.

profcturner commented 5 years ago

This is quite puzzling because the Django source code shows:

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())

so that this redirection should only happen is the user is not authenticated, but this is happening with authenticated users.

qofsq commented 5 years ago

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())

I’m probably wrong, as I haven’t looked at what the bold function returns, but that to me says ‘if the user is authenticated then raise the denied exception and return to login page’. Should that maybe be checking for ‘not’ self.request.user.is_authenticated?

profcturner commented 5 years ago

The code above is from: https://github.com/django/django/blob/master/django/contrib/auth/mixins.py BTW.

Hmm, maybe the issue is that I assume the Exception will actually create its warning message and then short circuit this response. Maybe need to run some experiments...

profcturner commented 5 years ago

It looks like this behaviour is intended as a feature, in order to allow users to re-login with higher level permissions, but it's a bit of a pain and I could do with finding a way to avoid it.

profcturner commented 5 years ago

But... it seems to be behaving better since I added 403.html and 404.html templates, so I'm closing this for now.