Mewp / lightopenid

Automatically exported from code.google.com/p/lightopenid
MIT License
4 stars 12 forks source link

failure on validate method #20

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
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);

Original issue reported on code.google.com by mrubio...@gmail.com on 22 Nov 2010 at 8:12

GoogleCodeExporter commented 9 years ago
Probably we want that modifier ungreedy, like so:

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

but I don't think the parameters would get mixed anyway.

Original comment by mrubio...@gmail.com on 22 Nov 2010 at 8:14

GoogleCodeExporter commented 9 years ago
That regex needs some work now that I look at it, but I think I made my point.

Original comment by mrubio...@gmail.com on 22 Nov 2010 at 8:17

GoogleCodeExporter commented 9 years ago
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.

Original comment by mewp...@gmail.com on 22 Nov 2010 at 11:33

GoogleCodeExporter commented 9 years ago
I have slightly modified the regex and commited the fix.

Original comment by mewp...@gmail.com on 22 Nov 2010 at 3:30