Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

failure on validate method #20

Closed
GoogleCodeExporter opened this issue Mar 21, 2015 · 4 comments
Closed

failure on validate method #20

GoogleCodeExporter opened this issue Mar 21, 2015 · 4 comments

Comments

@GoogleCodeExporter
Copy link

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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

  • Changed state: Accepted

@GoogleCodeExporter
Copy link
Author

I have slightly modified the regex and commited the fix.

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

  • Changed state: Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant