Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

FacebookRedirectLoginHelper isValidRedirect() returns null when redirect_uri params are empty #470

Closed
mdenison opened this issue Jul 16, 2015 · 3 comments

Comments

@mdenison
Copy link

We recently found an issue in one of our applications.

When we append query parameters which are not set (e.g. http://www.example.com/?param1=set&param2=&param3=set) to the 'redirect_uri', the empty parameter seems to break the validation of isValidRedirect().

The 'state' is being compared on this line of code:
https://github.com/facebook/facebook-php-sdk-v4/blob/4.0.16/src/Facebook/FacebookRedirectLoginHelper.php#L205

Although the state seems to be different when we have a param which is not set, if we set that param everything works fine and authentication works as expected in the complete flow.

Since we haven't found anything that relates to a bug with query parameters in the 'redirect_uri' we are strongly guessing this is a bug within the SDK somewhere.

@SammyK
Copy link
Contributor

SammyK commented Jul 16, 2015

I think there's a very illusive bug happening here. It seems there are lots & lots & lots of people having this issue. I've been trying to track it down with no success so far and even gave up on trying to fix it in one of my projects. It'd be nice to finally kill this bug once & for all. :)

Ping @yguedidi, @gfosco, @devmsh.

@champsupertramp
Copy link

I had the similar issue. We updated the session.save_path() in the .ini file and it works.

@SammyK
Copy link
Contributor

SammyK commented Apr 20, 2016

I'm going to go ahead and close this since this issue seems to have many different solutions which leads me to believe it's environment-specific. I think in v6 of the PHP SDK we should extract all the session handling stuff and have the user track the CSRF token using their own method. :)

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

No branches or pull requests

3 participants