Last Comment Bug 603500 - Replace Django's @permission_required
: Replace Django's @permission_required
Status: VERIFIED FIXED
:
Product: support.mozilla.org
Classification: Other
Component: Code Quality (show other bugs)
: unspecified
: All All
: P3 normal (vote)
: 2.3
Assigned To: James Socol [:jsocol, :james]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-11 16:52 PDT by James Socol [:jsocol, :james]
Modified: 2010-10-29 16:04 PDT (History)
1 user (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description James Socol [:jsocol, :james] 2010-10-11 16:52:01 PDT
The built-in @permission_required decorator in Django has an annoying behavior of always sending users to the login page, even if they're already logged in. This is confusing and leads to false-positive bug reports about session issues.

A while ago I wrote a replacement decorator for another project, it's designed to act as a drop-in replacement: http://gist.github.com/592040, except that it only redirects to the login page if users are not authenticated, and returns a normal HttpResponseForbidden if they are.

Pros: more accurate user experience.
Cons: it's copied and forked from Django, so we have to maintain it ourselves. Also I would need to write a test or two specifically for it.

Thoughts? Is it worth it?
Comment 1 James Socol [:jsocol, :james] 2010-10-11 17:20:04 PDT
The more I think about this the more necessary it will be, assuming the log in view in 2.4 does what I imagine it will do: redirect you automatically if you're already logged in. In that case, imagine user Bob visits /foo, and doesn't have permissions to view it. He's redirected to /login, which, because he's already authenticated, redirects him back to /foo, which redirects him back to /login, ad infinitum.
Comment 2 James Socol [:jsocol, :james] 2010-10-12 10:31:03 PDT
Per discussion, we'll make the switch. I'll start with the linked decorator and tweak it to work with our setup, and include tests.
Comment 3 James Socol [:jsocol, :james] 2010-10-14 09:26:36 PDT
http://github.com/jsocol/kitsune/commit/5a947024e5

For clarity and posterity:

While navigating to a URL requiring permission, let's say /flagged

If you:                            You will see:
are logged out                     a redirect to the login page.
are logged in without permission   an error.
are logged in with permission      the page.
Comment 4 Rebecca Billings [:rbillings] 2010-10-29 16:04:23 PDT
Verified all three states

Note You need to log in before you can comment on or make changes to this bug.