Re: [Django] #12233: Redirect logged in user
Django
--------------------------------------+------------------------------------
Reporter: dmathieu | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version:
Severity: Normal | Resolution:
Keywords: authentication,login | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by lgp171188@…):
Bump. If it is still not in, would love to see it in asap.
--
Ticket URL: <https://code.djangoproject.com/ticket/12233#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: dmathieu | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version:
Severity: Normal | Resolution:
Keywords: authentication,login | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by kmtracey):
Before this can go in, someone interested in the feature needs to correct
the current patch to fix the problems !SmileyChris noted above. The
current patch adds an arg in the middle of existing args, this is
backwards-incompatible. We also can't make changes to the arguments to a
documented function
(https://docs.djangoproject.com/en/1.4/topics/auth/#django.contrib.auth.views.login)
without updating the docs. So, if you'd like to "bump" this ticket along,
the way to do that would be to provide an updated patch (or pull request)
that fixes the problems with the existing one.
--
Ticket URL: <https://code.djangoproject.com/ticket/12233#comment:11>
Django
Reporter: dmathieu | Owner:
Type: New feature | aaronbassett
Component: contrib.auth | Status: assigned
Severity: Normal | Version:
Keywords: | Resolution:
authentication,login | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by aaronbassett):
* status: new => assigned
* owner: nobody => aaronbassett
--
Ticket URL: <https://code.djangoproject.com/ticket/12233#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: dmathieu | Owner:
Type: New feature | aaronbassett
Component: contrib.auth | Status: assigned
Severity: Normal | Version:
Keywords: | Resolution:
authentication,login | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by buchanae):
I've done some work on this here:
https://github.com/abuchanan/django/tree/ticket_12233
Still needs work.
I changed the argument name to "redirect_authenticated_user" and the value
type to Boolean (whether authenticated users should be redirected) which
defaults to False because I figured that would be more backwards-
compatible.
The redirect URL would use the same mechanism as normal user login.
I'm not sure yet how to handle a case where the redirect url is the login
page (creating an infinite redirect loop). I guess I could chalk that up
to user error and just let it happen.
I've also considered that this may be cleaner as a view decorator.
Thoughts on that?
Something to the effect of:
{{{
@redirect_authenticated_user(redirect_field_name=REDIRECT_FIELD_NAME,
redirect_url=settings.LOGIN_REDIRECT_URL)
def your_view(request): pass
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/12233#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: dmathieu | Owner:
Type: New feature | aaronbassett
Component: contrib.auth | Status: assigned
Severity: Normal | Version:
Keywords: | Resolution:
authentication,login | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: buchanae (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/12233#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: dmathieu | Owner:
Type: New feature | aaronbassett
Component: contrib.auth | Status: assigned
Severity: Normal | Version:
Keywords: | Resolution:
authentication,login | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: torsten@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/12233#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: dmathieu | Owner:
Type: New feature | aaronbassett
Component: contrib.auth | Status: assigned
Severity: Normal | Version:
Keywords: | Resolution:
authentication,login | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by olethanh):
I have improved the branch a little bit and rebased it on top of master in
https://github.com/olethanh/django/tree/ticket_12233
I added a simple loop detection which will raise an Exception but it won't
catch all cases and improved the docs
Not sure what's the best way to handle this redirection problem either:
* We could just do nothing and let the user browser automatically detect
it but then the admin won't know that something is wrong.
* We cannot detect this at start time since there is no settings for the
LOGIN_URL and a project can potentially have multiple login url.
* We can raise an Exception if we detect it (what is currently done in my
branch)
* We can return a 500 if we detect it (what's was previously done in my
branch)
* But more importantly we don't really have a reliable way to detect it,
my current patch won't catch redirection if they have GET parameters
inside or if we redirect to another login url or if there is another
redirection afterwards.
Not sure how important this problem is since it is a corner case that will
only happen if the settings are badly configured.
--
Ticket URL: <https://code.djangoproject.com/ticket/12233#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: dmathieu | Owner:
Type: New feature | aaronbassett
Component: contrib.auth | Status: assigned
Severity: Normal | Version:
Keywords: | Resolution:
authentication,login | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: olethanh (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/12233#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: dmathieu | Owner:
Type: New feature | aaronbassett
Component: contrib.auth | Status: assigned
Severity: Normal | Version:
Keywords: | Resolution:
authentication,login | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by erikr):
Raising an exception makes sense to me if we can't detect it earlier. The
exception will probably produce a 500 to the user, and a useful error
message to the end user. There should be a warning about this issue in the
docs though.
Could you make a proper pull request? That makes it much easier to review
and keep track of comments. Add a reference to this ticket in the PR, and
post the link to the PR here.
--
Ticket URL: <https://code.djangoproject.com/ticket/12233#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: dmathieu | Owner:
Type: New feature | Status: assigned
Component: contrib.auth | Version:
Severity: Normal | Resolution:
authentication,login |
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by olethanh):
I've updated my branch, rebased it on master, improved the documentation
and the tests.
Opened a PR here : https://github.com/django/django/pull/5604
--
Ticket URL: <https://code.djangoproject.com/ticket/12233#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: dmathieu | Owner:
| aaronbassett
Type: New feature | Status: assigned
Component: contrib.auth | Version:
Severity: Normal | Resolution:
authentication,login | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/12233#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: dmathieu | Owner:
| aaronbassett
Type: New feature | Status: assigned
Component: contrib.auth | Version:
Severity: Normal | Resolution:
authentication,login |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
Left a few more comments for improvement. Please bump back to RFC when
updated.
--
Ticket URL: <https://code.djangoproject.com/ticket/12233#comment:21>