Modify

Opened 14 years ago

Closed 14 years ago

Last modified 12 years ago

#7687 closed defect (fixed)

[patch] Always redirect to referer after login

Reported by: Danya <dendik@…> 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  
    473473    # overrides
    474474    def _do_login(self, req):
    475475        if not req.remote_user:
    476             req.redirect(self.env.abs_href())
     476            self._redirect_back(req)
    477477        res = auth.LoginModule._do_login(self, req)
    478478        if req.args.get('rememberme', '0') == '1':
    479479            # 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 Steffen Hoffmann

Keywords: login redirect added
Summary: Always redirect to referer after login[patch] Always redirect to referer after login

Just flag availability of a patch here.

comment:2 Changed 14 years ago by Remy Blank

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

     
    469469    # overrides
    470470    def _do_login(self, req):
    471471        if not req.remote_user:
    472             req.redirect(self.env.abs_href())
     472            self._redirect_back(req)
    473473        res = auth.LoginModule._do_login(self, req)
    474474        if req.args.get('rememberme', '0') == '1':
    475475            # Set the session to expire in 30 days (and not when to browser is
     
    503503            return user
    504504        return None
    505505
    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 sites
    511             referer = None
    512         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 
    517506    def enabled(self):
    518507        # Users should disable the built-in authentication to use this one
    519508        return not self.env.is_component_enabled(auth.LoginModule)

comment:3 Changed 14 years ago by Danya <dendik@…>

Type: enhancementdefect

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 in reply to:  3 Changed 14 years ago by Remy Blank

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 Danya <dendik@…>

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 in reply to:  2 Changed 14 years ago by Steffen Hoffmann

Owner: changed from John Hampton to Steffen Hoffmann
Status: newassigned

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 Remy Blank

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 Steffen Hoffmann

Resolution: fixed
Status: assignedclosed

(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:9 Changed 14 years ago by Remy Blank

Thanks!

comment:10 Changed 14 years ago by Steffen Hoffmann

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

comment:11 Changed 14 years ago by Steffen Hoffmann

Related T#9757 has been resolved.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.