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

missing setAccessToken #91

Closed
wants to merge 1 commit into from
Closed

Conversation

kl3ryk
Copy link

@kl3ryk kl3ryk commented Sep 3, 2013

This missing line was causing problem when used with the following scenario:

$facebookAdapter->getAccessToken();
//[...] some actions
$facebookAdapter->setExtendedAccessToken();
$facebookAdapter->getAccessToken();

there was an error

{"error":{"message":"Error validating verification code. Please make sure your redirect_uri is identical to the one you used in the OAuth dialog request","type":"OAuthException","code":100}}

Why?
Because here:
https://github.com/facebook/facebook-php-sdk/blob/master/src/base_facebook.php#L382
session is destroyed.

and then here:
https://github.com/facebook/facebook-php-sdk/blob/master/src/base_facebook.php#L399
cached version of access token is not returned.

And then there is a request to exchange code to access token, but our code is already expired.

This problem exists when we already passed the facebook authentication process and our url is like this:

http://example.com?code=CODE&state=STATE#_=_

@gfosco
Copy link
Contributor

gfosco commented Oct 28, 2013

Sorry it took this long to respond to this. Thanks for your contribution. In order for us to consider merging this, can you sign the Contributor License Agreement? https://developers.facebook.com/opensource/cla

Once signed, comment here and I will re-open this.

@gfosco gfosco closed this Oct 28, 2013
@kl3ryk
Copy link
Author

kl3ryk commented Oct 28, 2013

Done :)

@gfosco gfosco reopened this Oct 28, 2013
@gfosco
Copy link
Contributor

gfosco commented Oct 31, 2013

Is it possible to add a test case for this that hits both scenarios?

@kl3ryk
Copy link
Author

kl3ryk commented Nov 4, 2013

Yes i will provide them in this week :)

@gfosco
Copy link
Contributor

gfosco commented Dec 17, 2013

Hi @kl3ryk will you be adding test cases?

Thanks

@gfosco gfosco closed this Jan 9, 2014
@janisto
Copy link

janisto commented Feb 5, 2014

Why didn't you merge this?

When I use setExtendedAccessToken(), I now have to use getPersistentData() and setAccessToken().

@kl3ryk
Copy link
Author

kl3ryk commented Feb 5, 2014

@janisto they didnt merged it because i didn't provided test cases yet. But until march i don't have much time to prepare test cases. So if you have anything i can just push it.

I will find some time after 11 march :). @gfosco will you reopen issue then?

@roman02
Copy link

roman02 commented Feb 14, 2014

kl3ryk, let's assume that he will reopen the issue, as he has done in the past.

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