Update src/base_facebook.php #48

wants to merge 1 commit into


None yet
8 participants

eosgood commented Dec 7, 2012

fix logic to not reset persistent data

@eosgood eosgood Update src/base_facebook.php
fix logic to not reset persistent data

konrness commented Dec 7, 2012

There are several other places where clearAllPersistentData() is called after a unsuccessful authentication. I think all instances are too eager since there is a lot of non-auth data stored in the signed request including page like/admin info, user age group, etc.

This also fix a bug ( tested! ) I'm facing with a valid access_token being overridden by an empty/invalid token coming from the fbsr_* cookie (set by the JS-SDK).
Just to make myself clear:

  • I'm using both PHP-SDK and JS-SDK;
  • Authentications are handled by the PHP-SDK;
  • Every page has a $fb->api('/me'); call;
  • In the successfully authenticated page, I'm including the JS-SDK with the "cookie:true" option enabled. This way, sometimes the fbsr_* cookie is filled, and when PHP tries to get a signed request off of this cookie, it can't find a valid access_token (its empty) and still destroy the VALID and authenticated session.

A code comment says that " signed request states there's no access token, anything should be cleared " - Why? Maybe I'm missing something, but, if I have a valid access_token, why would an invalid signed request coming from a cookie overwrites that? If the access_token turns out to be invalid when some request is done, the exception should be handled.

this commit actually works, while ca9472b doesn't.
please merge this pull request in place of the other

@eosgood Thanks for this fix. your commit works fine rather that ca947b

cc @facebook


oyvindkinsey commented Dec 11, 2012

ca947b is a less invasive change than the one in this pull request, and is designed to only handle the case of reusing authorization codes.

For all other issues, please provide additional error descriptions and/or pull requests.

@oyvindkinsey The problem is that the following lines remove any valid access_tokens, because of a invalid $code in the JS generated Signed Request which can't be used by PHP:

458      $this->clearAllPersistentData();
459      return false; // respect the signed request's data, even
460                    // if there's an authorization code or something else

Still have an issue where I get logged out, I am authenticating with the JS SDK and using the PHP SDK for actions. The issue results in an error, "OAuthException: An active access token must be used to query information about the current user." because the JS cookie/token is deemed invalid in PHP after about 10 mins.

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