req.args is _RequestArgs object; use getfirst() to avoid AttributeError: 'list' object has no attribute 'startswith'

[dkg@…]

the trac web frontend's auth module defines _referer this way:

def _referer(self, req):
        return req.args.get('referer') or req.get_header('Referer')

However, it's possible that there are two HTTP request variables named "referer", in which case this function will return a list.

This causes a problem in _redirect_back where the value returned by _referer gets used to invoke .startswith. This results in an error like:

Trac detected an internal error:

AttributeError: 'list' object has no attribute 'startswith'

The simple fix is to replace req.args.get() with req.args.getfirst() in _referer(). I'll attach a patch shortly against 0.12.2.

This bug was also present in 0.11.7.

It was first reported to debian as ​http://bugs.debian.org/634800

[dkg@…]

patch to fix web.auth._referer in trac 0.12.2

[Christian Boos]

Thanks for the patch!

[Stefan]

updated version to first known bugged one (according to the description)

[Remy Blank]

Why do you have two referer request arguments? Shouldn't you send only one? Taking the first one arbitrarily is bound to bite you sooner or later.

[Christian Boos]

Yep. Came to the same conclusion and postpone applying the patch until we now more about this use case.

[anonymous]

See ​the original debian bug report — the author of that bug report needs to send a form variable named "referer" (due to the chain of pages displayed in an openid login performed by mod_auth_openid). It looks to me like the presence of a form variable named "referer" ends up doubling up with the http header provided by the browser, which causes the confusion.

I agree that arbitrarily choosing the first isn't great, but it's a lot better than the error message the user currently sees, which often makes users think that they haven't logged in properly.

Logging the duplicate referers would be fine if you want to do that, but i'd prefer to go ahead and let the user log in directly, even if they get returned to a surprising spot because we've picked the wrong variable.

[Christian Boos]

Replying to anonymous:

See ​the original debian bug report — the author of that bug report needs to send a form variable named "referer" (due to the chain of pages displayed in an openid login performed by mod_auth_openid).

Yes, I've read it, the OP says:

I've written a custom login page for libapache2-mod-auth-openid that includes a post value for referer that is set to the original web page that user was on when they clicked the login link.

The problem may well be an error in this custom login page…

It looks to me like the presence of a form variable named "referer" ends up doubling up with the http header provided by the browser, which causes the confusion.

No, that can't be, these are not the same things. Maybe you should ask him to show us this custom login form, there's probably something wrong with it.

I was also wondering if #7876 had anything to do with this problem: one 'referer' coming from POST and the other from a query string in the <form>'s action (we're supposed to handle this… but who knows).

[jm@…]

Hi,

I posted the upstream debian report.

The custom login page is here:

​https://members.mayfirst.org/openid/login/

You get the full OpenID login workflow by going to our trac installation here: ​https://support.mayfirst.org/ and clicking the login link.

The libapache_mod_auth_openid documentation is also ​available as well as the particular ​page on making a custom login form.

I'm happy to experiment with alternative ways to fix this.

jamie

[dkg@…]

I'm using firebug to try a login on ​https://support.mayfirst.org/. firebug allows me to see the set of variables provided by the browser to the server via the "params" tab associated with the request under the net console. There is exactly one "referer" variable listed in the "params" tab during login. (there is another "Referer" listed in the "headers" tab, though obviously that's an http header and not a CGI variable.

[Christian Boos]

Using Chrome, I see the following query/response information when using Google account on the above /login page:

Request Headers

...
Referer:https://members.mayfirst.org/openid/login/?modauthopenid.error=no_idp_found&modauthopenid.referrer=http%3A%2F%2Fsupport.mayfirst.org%2Flogin%3Freferer%3Dhttps%253A%252F%252Fsupport.mayfirst.org%252F&referer=https%3A%2F%2Fsupport.mayfirst.org%2F
...

Query string parameters

referer:https://members.mayfirst.org/openid/login/?modauthopenid.referrer=http%3A%2F%2Fsupport.mayfirst.org%2Flogin%3F
openid_identifier:https://www.google.com/accounts/o8/id?next=/

Response Headers

Connection:Keep-Alive
Content-Encoding:gzip
Content-Length:583
Content-Type:text/html; charset=iso-8859-1
Date:Thu, 08 Dec 2011 11:23:29 GMT
Keep-Alive:timeout=15, max=98
Location:https://www.google.com/accounts/o8/ud?openid.assoc_handle=XXXXXX&openid.claimed_id=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0%2Fidentifier_select&openid.identity=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0%2Fidentifier_select&openid.mode=checkid_setup&openid.ns=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0&openid.realm=http%3A%2F%2Fsupport.mayfirst.org%2F&openid.return_to=http%3A%2F%2Fsupport.mayfirst.org%2Flogin%3Freferer%3Dhttps%253A%252F%252Fmembers.mayfirst.org%252Fopenid%252Flogin%252F%253Fmodauthopenid.referrer%253Dhttp%25253A%25252F%25252Fsupport.mayfirst.org%25252Flogin%25253F%26openid_identifier%3Dhttps%253A%252F%252Fwww.google.com%252Faccounts%252Fo8%252Fid%253Fnext%253D%252F%26modauthopenid.nonce%3Dzwc6gb6uR4%26referer%3Dhttps%253A%252F%252Fmembers.mayfirst.org%252Fopenid%252Flogin%252F%253Fmodauthopenid.referrer%253Dhttp%25253A%25252F%25252Fsupport.mayfirst.org%25252Flogin%25253F&openid.trust_root=http%3A%2F%2Fsupport.mayfirst.org%2F

IOW, two "referer" request parameters added to the Location URL by mod_auth_openid:

referer%3Dhttps%253A%252F%252Fmembers.mayfirst.org%252Fopenid%252Flogin%252F

referer%3Dhttps%253A%252F%252Fmembers.mayfirst.org%252Fopenid%252Flogin%252F

Quick inspection of the mod_auth_openid code suggests that ​https://github.com/bmuller/mod_auth_openid/blob/master/mod_auth_openid.cpp#L306 might be the problem (if return_to is the Referer header which I didn't verify). Well, regardless of the actual details of the bug, it really seems to be a mod_auth_openid problem.

If this really can't be fixed there, we may apply the patch as a workaround in a future release.

[dkg@…]

I've just opened ​an issue with mod_auth_openid to see if this is something they can resolve.

I'm still curious about what you think trac should do in the situation where it receives multiple referer query parameters, though (whether or not mod_auth_openid is involved). Is showing the user a (rather cryptic) error message at helpful in any significant way?

would a patch that logs the error while selecting the first query parameter be more acceptable?

[Christian Boos]

Well, most of the web ui related code will fail with cryptic errors when given arbitrary request parameters: why would you expect wiki:WikiStart?version=1&version=2 to work or even to give a nice error?

In the case of /login, we happen to support a single "referer" parameter, and don't expect it to be given multiple times.

It's still not exactly clear for me why mod_auth_openid repeats the referer parameter, it could be either because it also sees it in the parameters of the URL in the Referer header and merges this with the explicit parameters, or it could be because the explicit non-openid parameters happen to be duplicated along the way…

I just checked once again, this time I just went to the /login page, put directly the correct URL… and pressed Login:

• Referer header is https://members.mayfirst.org/openid/login/?modauthopenid.referrer=http%3A%2F%2Fsupport.mayfirst.org%2Flogin%3F
• Query string parameter contains: referer: https://support.mayfirst.org/
• Location is https://www.google.com/accounts/o8/ud?openid.assoc_handle=xxxxxxx&openid.claimed_id=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0%2Fidentifier_select&openid.identity=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0%2Fidentifier_select&openid.mode=checkid_setup&openid.ns=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0&openid.realm=http%3A%2F%2Fsupport.mayfirst.org%2F&openid.return_to=http%3A%2F%2Fsupport.mayfirst.org%2Flogin%3F referer%3Dhttps%253A%252F%252Fsupport.mayfirst.org%252F%26openid_identifier%3Dhttps%253A%252F%252Fwww.google.com%252Faccounts%252Fo8%252Fid%253Fnext%253D%252F%26modauthopenid.nonce%3DyXesHM8xhe%26 referer%3Dhttps%253A%252F%252Fsupport.mayfirst.org%252F&openid.trust_root=http%3A%2F%2Fsupport.mayfirst.org%2F

So it's really the second hypothesis, the Referer header doesn't matter and we see that the non-openid parameters get duplicated (here its only referer, but you could easily check by adding <input type="hidden" name="dummy" value="xyz" /> to your form).

[Christian Boos]

Replying to cboos:

Well, most of the web ui related code will fail with cryptic errors when given arbitrary request parameters: why would you expect wiki:WikiStart?version=1&version=2 to work or even to give a nice error?

<off-topic> Note that we could indeed imagine to have a much improved web request handling framework than what we have now, with sophisticated ways to check and document parameters, but by doing this you would risk to move Trac out of the minimalistic web application framework territory ;-) </off-topic>

[dkg@…]

I used firebug to modify the login form and add a new dummy input field, and i now agree that mod_auth_openid is duplicating the parameters it receives when it returns to the page from which login was pressed. Thanks for the helpful diagnosis.

In fact, if i add an extra parameter to the login form, i can coax even more copies of each parameter to show in the final request.

[debacle@…]

Can we conclude, that this is not a Trac defect and the ticket can be closed?

[Daniel Kahn Gillmor <dkg@…>]

Yeah, i think you're right, that this needs to be fixed in mod_auth_openid.