Modify

Opened 4 years ago

Closed 4 years ago

Last modified 22 months ago

#7687 closed defect (fixed)

[patch] Always redirect to referer after login

Reported by: Danya <dendik@…> Owned by: hasienda
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 4 years ago by hasienda

  • Keywords login redirect added
  • Summary changed from Always redirect to referer after login to [patch] Always redirect to referer after login

Just flag availability of a patch here.

comment:2 follow-up: Changed 4 years ago by rblank

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 follow-up: Changed 4 years ago by Danya <dendik@…>

  • Type changed from enhancement to 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 in reply to: ↑ 3 Changed 4 years ago by rblank

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 4 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 4 years ago by hasienda

  • Owner changed from pacopablo to hasienda
  • Status changed from new to 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 4 years ago by rblank

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 4 years ago by hasienda

  • Resolution set to fixed
  • Status changed from assigned to 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:9 Changed 4 years ago by rblank

Thanks!

comment:10 Changed 4 years ago by hasienda

(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 4 years ago by hasienda

Related T#9757 has been resolved.

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.