Add Long Lived User Tokens #392
Conversation
Added support for long lived user access tokens. This includes the api call to exchange a short lived token for a long lived token and the api call to get a code to be redeemed for a long lived token.
facebook/__init__.py
Outdated
| @@ -311,6 +311,34 @@ def get_app_access_token(self, app_id, app_secret, offline=False): | |||
| return self.request("{0}/oauth/access_token".format(self.version), | |||
| args=args)["access_token"] | |||
|
|
|||
| def get_long_lived_token(self, app_id, app_secret): | |||
There was a problem hiding this comment.
Isn't this exactly the same as the extend_access_token method?
|
Ahh, I had assumed the extend_access_token method was a way to refresh the access token, not create a long lived access token. |
| @@ -330,6 +330,31 @@ Generates Facebook login URL to request access token and permissions. | |||
| fb_login_url = graph.auth_url(app_id, canvas_url, perms) | |||
| print(fb_login_url) | |||
|
|
|||
| get_code_from_token | |||
There was a problem hiding this comment.
Since not every method is documented, this file has gotten a bit confusing. In general, though, methods should appear in the same order that they appear in the code (so this should be between delete_object and auth_url).
| https://developers.facebook.com/docs/facebook-login/access-tokens/expiration-and-extension/ | ||
|
|
||
| Exchanges an existing long livd user access token on the server side for a | ||
| code that can be redeemed for a long lived user token on the client side. |
There was a problem hiding this comment.
"Lived" is spelled wrong.
I don't think this sentence is very clear. This isn't a client-side SDK, so it can be assumed that everything is happening "on the server side".
| to get a code from the access token. | ||
| * ``redirect_uri`` - ``string`` Return URL after successful authentication, | ||
| usually parses returned Facebook response for authorisation request. | ||
|
|
There was a problem hiding this comment.
The "Parameters" section uses the same grammar as the auth_url method's documentation, but that section is wrong. Check any other method for better examples.
| redirect_uri = 'https://domain.com/that-handles-auth-response/' | ||
| code = graph.get_code_from_token(app_id, app_secret, redirect_uri) | ||
| print(code) | ||
|
|
There was a problem hiding this comment.
This example isn't useful. Because the code needs to be used by client-side code, I think it will be difficult to create a generally useful example and that this method does not necessarily need one.
| Gets a code to be exchanged for a long lived token. | ||
| Uses a pre-existing server side long lived token to return a code | ||
| that can be redeemed for a client side long lived token. | ||
| """ |
There was a problem hiding this comment.
This docstring suffers from the same issues as the description in the documentation.
| self.access_token = self.extend_access_token(app_id, app_secret) | ||
| code = self.request("{0}/oauth/client_code".format(self.version), | ||
| args=args)["code"] | ||
|
|
There was a problem hiding this comment.
If a GraphAPIError has been raised, why are we calling extend_access_token here? Why isn't the code object generated here returned?
| self.assertIsNotNone(graph.get_access_token_from_code( | ||
| self.app_id, self.secret), 'Invalid code returned by' | ||
| 'get_code_from_token method') | ||
|
|
There was a problem hiding this comment.
Why does this test call both get_code_from_token and get_access_token_from_code? Should it be separated into two different tests?
I don't think it's safe to use assertIsNotNone here - this assumes that either method has succeeded as long as it doesn't return None. I think it would be better to test failure as well (e.g. using an expired token or a bad redirect URI).
Fix #300.