My favorites | Sign in
Project Home Downloads Wiki Issues Source
READ-ONLY: This project has been archived. For more information see this post.
Search
for
  Advanced search   Search tips   Subscriptions
Issue 20: failure on validate method
1 person starred this issue and may be notified of changes. Back to list
Status:  Fixed
Owner:  ----
Closed:  Nov 2010


 
Reported by mrubio...@gmail.com, Nov 22, 2010
Because the returnUrl is set to the current url(request uri) but with the query string stripped, it fails to return true when the url has one.

At this point, unless I'm missing something about the involved protocols, I don't see why should it be stripped, or why do we need the expression "($this->data['openid_return_to'] != $this->returnUrl)" at all.

In case it's actually needed, we can still get the returnUrl WITH the query string we provided, by replacing, in the __construct method:

- $uri = strpos($uri, '?') ? substr($uri, 0, strpos($uri, '?')) : $uri;

for this

+ $uri = preg_replace('#((?<=\?)|&)openid\..*(?>=&|$)#','',$uri);
Nov 22, 2010
#1 mrubio...@gmail.com
Probably we want that modifier ungreedy, like so:

$uri = preg_replace('#((?<=\?)|&)openid\..*?(?>=&|$)#','',$uri);

but I don't think the parameters would get mixed anyway.
Nov 22, 2010
#2 mrubio...@gmail.com
That regex needs some work now that I look at it, but I think I made my point.
Nov 22, 2010
Project Member #3 mewp...@gmail.com
We need the "($this->data['openid_return_to'] != $this->returnUrl)", because without it, someone could send an openid response meant for another relying party, and it would still validate. That would enable a rogue RP to authenticate into LightOpenID RP-s, effectively hijacking the user's account on those sites.

Besides, the protocol requires that openid.return_to has to be checked, and it's enough reason to do that. See section 11 and 11.1 of the spec for the details.

As for the regex, I'll test it later and commit the fix if it works.
Status: Accepted
Nov 22, 2010
Project Member #4 mewp...@gmail.com
I have slightly modified the regex and commited the fix.
Status: Fixed

Powered by Google Project Hosting