[patch] Always redirect to referer after login

Just flag availability of a patch here.

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:

Index: acct_mgr/web_ui.py
===================================================================
--- acct_mgr/web_ui.py  (revision 9255)
+++ acct_mgr/web_ui.py  (working copy)
@@ -469,7 +469,7 @@
     # overrides
     def _do_login(self, req):
         if not req.remote_user:
-            req.redirect(self.env.abs_href())
+            self._redirect_back(req)
         res = auth.LoginModule._do_login(self, req)
         if req.args.get('rememberme', '0') == '1':
             # Set the session to expire in 30 days (and not when to browser is
@@ -503,17 +503,6 @@
             return user
         return None
 
-    def _redirect_back(self, req):
-        """Redirect the user back to the URL she came from."""
-        referer = self._referer(req)
-        if referer and not referer.startswith(req.base_url):
-            # don't redirect to external sites
-            referer = None
-        req.redirect(referer or self.env.abs_href())
-
-    def _referer(self, req):
-        return req.args.get('referer') or req.get_header('Referer')
-
     def enabled(self):
         # Users should disable the built-in authentication to use this one
         return not self.env.is_component_enabled(auth.LoginModule)

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.

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.

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.

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?

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.

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

Thanks!

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

Related T#9757 has been resolved.