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

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

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

Comments

Projects
None yet
3 participants
@mdenison

mdenison commented Jul 16, 2015

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

This comment has been minimized.

Show comment
Hide comment
@SammyK

SammyK Jul 16, 2015

Collaborator

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.

Collaborator

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

This comment has been minimized.

Show comment
Hide comment
@champsupertramp

champsupertramp Nov 6, 2015

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

champsupertramp commented Nov 6, 2015

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

@SammyK SammyK added the duplicate label Apr 20, 2016

@SammyK

This comment has been minimized.

Show comment
Hide comment
@SammyK

SammyK Apr 20, 2016

Collaborator

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. :)

Collaborator

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 join this conversation on GitHub. Already have an account? Sign in to comment