#7687 closed defect (fixed)
[patch] Always redirect to referer after login
Reported by: | Owned by: | Steffen Hoffmann | |
---|---|---|---|
Priority: | normal | Component: | AccountManagerPlugin |
Severity: | minor | Keywords: | login redirect |
Cc: | Trac Release: | 0.11 |
Description
Currently, trac honours referer only when user was presented with login form and enters her username/password; if the user was sent to login page while already being logged in, she is not redirected back, but rather redirected to front page.
This makes it difficult to create user-friendly links to private trac pages (e.g. new ticket or ticket view), that would ask user login beforehands.
This is fixed easily:
-
0.11/acct_mgr/web_ui.py
diff -u -r accountmanagerplugin-orig/0.11/acct_mgr/web_ui.py accountmanagerplugin/0.11/acct_mgr/web_ui.py
old new 473 473 # overrides 474 474 def _do_login(self, req): 475 475 if not req.remote_user: 476 req.redirect(self.env.abs_href())476 self._redirect_back(req) 477 477 res = auth.LoginModule._do_login(self, req) 478 478 if req.args.get('rememberme', '0') == '1': 479 479 # Set the session to expire in 30 days (and not when to browser is
Attachments (0)
Change History (11)
comment:1 Changed 14 years ago by
Keywords: | login redirect added |
---|---|
Summary: | Always redirect to referer after login → [patch] Always redirect to referer after login |
comment:2 follow-up: 6 Changed 14 years ago by
The patch is correct and fixes the issue. This will become very useful once #T540 is closed, as it relies on the fact that an already logged-in user going to /login
is redirected to the referrer.
Also, the LoginModule._redirect_back()
and LoginModule._referer()
methods don't need to be overridden, as they are present and correct in Trac core (even with a bug fixed). They may even be extended in #T540, so the best would be to remove them from AccountManagerPlugin altogether (on trunk).
Here's the full patch:
-
acct_mgr/web_ui.py
469 469 # overrides 470 470 def _do_login(self, req): 471 471 if not req.remote_user: 472 req.redirect(self.env.abs_href())472 self._redirect_back(req) 473 473 res = auth.LoginModule._do_login(self, req) 474 474 if req.args.get('rememberme', '0') == '1': 475 475 # Set the session to expire in 30 days (and not when to browser is … … 503 503 return user 504 504 return None 505 505 506 def _redirect_back(self, req):507 """Redirect the user back to the URL she came from."""508 referer = self._referer(req)509 if referer and not referer.startswith(req.base_url):510 # don't redirect to external sites511 referer = None512 req.redirect(referer or self.env.abs_href())513 514 def _referer(self, req):515 return req.args.get('referer') or req.get_header('Referer')516 517 506 def enabled(self): 518 507 # Users should disable the built-in authentication to use this one 519 508 return not self.env.is_component_enabled(auth.LoginModule)
comment:3 follow-up: 4 Changed 14 years ago by
Type: | enhancement → defect |
---|
Thanks! I agree, trac.web.auth.LoginModule has better _redirect_back and the same _referer, no need to override them here
I marked the ticket as defect, as the behaviour it fixes was inconsistent. (The proper redirect occurs for previously not logged in users, but does not occur for the logged in ones, IIRC).
I doubt this ticket has any relation to #T540, as you yourself commented there: RSS readers can not use login forms.
comment:4 Changed 14 years ago by
Replying to Danya <dendik@kodomo.fbb.msu.ru>:
I doubt this ticket has any relation to #T540, as you yourself commented there: RSS readers can not use login forms.
It actually does. With one of the solutions in #T540, we "route" RSS feed links through /login
, which is then supposed to redirect back. Due to this issue, RSS links are broken when using form-based login. That is, if a logged-in user clicks an RSS link in the browser, she is currently redirected to WikStart
, instead of going to the requested RSS feed.
comment:5 Changed 14 years ago by
This makes sense for HTTP-based logins, not for form-based, since RSS clients can not handle form-based logins. You mentioned it yourself in #T540 (comment 93).
Anyway, RSS is not the only target for this ticket.
comment:6 Changed 14 years ago by
Owner: | changed from John Hampton to Steffen Hoffmann |
---|---|
Status: | new → assigned |
Replying to rblank:
The patch is correct and fixes the issue.
I've added this patch to my patch stack now, but have no clue, how to test it myself. The behavior reported here seems to persist. Any hints?
comment:7 Changed 14 years ago by
Log in to a Trac instance configured for form-based login. Then change the URL in your browser to:
http://my.site/login?referer=http://my.site/search
With the unpatched AccountManagerPlugin, you are redirected to WikiStart. With the patch, you are redirected to the search page.
comment:8 Changed 14 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
(In [9263]): AccountManagerPlugin: Remove private redirection after login, closes #7687.
This might have been a fix for a previously bad or inconsistent behaviour. But meanwhile it changed into a hindrance to make redirection to referer after login work properly in current Trac.
comment:10 Changed 14 years ago by
(In [9404]) AccountManagerPlugin: Provide fallback for undefined HTTP referer after successful login attempt, closes #3783, refs #3233 and #7687.
Redirect to corresponding Trac project's base URL, if referrer is undefined.
We prefer req.abs_href()
over req.base_url
, as a req object is available
and should always contain the needed information even with option base_url
unset in trac.ini . See T#5064 for even more details.
Just flag availability of a patch here.