Edgewall Software
Modify

Opened 18 years ago

Closed 16 years ago

#3048 closed defect (fixed)

Issue with redirect after login

Reported by: simon-code@… Owned by: osimons
Priority: normal Milestone: 0.11
Component: general Version: devel
Severity: normal Keywords: path login referer
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

Regarding trunk/trac/web/auth.py@3174#L190 - auth.py / LoginModule / def _redirect_back(self.req)

The test for "same site" is not quite sufficient, as it will get the result wrong when directly linking to /login when one projectname is a substring of another. A simple example is "sandbox" and "sandbox2". It actually gives errors both ways:

  1. Link from sandbox to sandbox2/login will get you to the project, but not actually logged in properly. Why this happens, I have not been able to figure out. It just sees me as 'anonymous', even after doing the "login" link manually afterwards. Any ideas? Something else comparing projectnames giving false positives? I have a "home" module that I've made for front page replacement (default component), but it does not receive the authentication information. It is build to "Trac standards", and doesn't do anything complicated that should suggest a problem here.
  2. Linking from /sandbox2 to /sandbox/login will actually not take you anywhere, as it does not get caught by the if-test (substring match). The redirect back does not make sense. I made a small change to the if-test, and this issue seems to work correctly:
            if referer and not (referer == req.base_url \
                        or referer.startswith(req.base_url + '/')):
    

This fix for issue 2 will catch both where the referrer is just /sandbox2 (no specific module/page) and where it is like /sandbox2/wiki/WikiStart (by closing the projectname with a '/' testing for /sandbox/).

I have made a cross-project personalised navigation, and have no issues of any kind with projects that are not substrings of each other.

Attachments (0)

Change History (5)

comment:1 by docwhat, 18 years ago

Keywords: path login referer added
Severity: normalmajor
Version: 0.9.4devel

I think to be correct, you would have to look like this:

if referer and \
   not ( referer == req.base_url or\
         referer.startswith(req.base_url.rstrip('/')+'/') ):

Though, this doesn't fix the problem that if login or logout are in https://, while the main site is in http:// and you don't write stupid RewriteRules to bounce the user back (why do that, anyway?)…

if referer:
   referer_without_protocol = referer.split('://',1)[-1]
   base_without_protocol = req.base_url.split('://',1)[-1]
   if referer_without_protocol != base_without_protocol and \
      not referer_without_protocol.startswith(base_without_protocol.rstrip('/')+'/'):
       referer = None

I'll happily do a patch for this, if someone wants (I actually already have one).

You can reach me at http://docwhat.gerf.org/ The email is the same as the hostname but the first dot is an at-sign.

Ciao!

comment:2 by Christian Boos, 17 years ago

Keywords: verify added
Milestone: 0.10.5

Verify how the fix for #2553 would affect this one.

in reply to:  2 comment:3 by osimons, 16 years ago

Keywords: verify removed
Milestone: 0.10.50.11.1
Owner: changed from Jonas Borgström to osimons

Replying to cboos:

Verify how the fix for #2553 would affect this one.

Problem 1) is now 'worksforme' with current trunk. I cannot make this problem appear anymore, but have no idea when it disappeared or why.

Problem 2) is still very much alive. I'll add it to my todo list.

comment:4 by osimons, 16 years ago

Milestone: 0.11.10.11
Status: newassigned

OK, the original patch still solves this, but using the rstrip() from first comment to make the matching correct. I'd go with the harmless approach here though, and not add more than a correct matching of projectname, leaving the redirect working as before:

  • trac/web/auth.py

     
    198198    def _redirect_back(self, req):
    199199        """Redirect the user back to the URL she came from."""
    200200        referer = req.get_header('Referer')
    201         if referer and not referer.startswith(req.base_url):
     201        if referer and not (referer == req.base_url or \
     202                referer.startswith(req.base_url.rstrip('/')+'/')):
    202203            # only redirect to referer if it is from the same site
    203204            referer = None
    204205        req.redirect(referer or req.abs_href())

If no one objects or has a better approach, I'll commit it.

comment:5 by osimons, 16 years ago

Resolution: fixed
Severity: majornormal
Status: assignedclosed

Patch for issue 2) applied in [6279]. Closing.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain osimons.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from osimons to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.