Skip to content
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

Add Long Lived User Tokens #392

Closed
wants to merge 16 commits into from
25 changes: 25 additions & 0 deletions docs/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"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".


**Parameters**

* ``app_id`` - ``integer`` Facebook application id that is requesting a code.
* ``app_secret`` - ``string`` Facebook application secret that is attempting
to get a code from the access token.
* ``redirect_uri`` - ``string`` Return URL after successful authentication,
usually parses returned Facebook response for authorisation request.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

**Example**

.. code-block:: python

app_id = 1231241241
app_secret = '123abc456def'
redirect_uri = 'https://domain.com/that-handles-auth-response/'
code = graph.get_code_from_token(app_id, app_secret, redirect_uri)
print(code)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

get_permissions
^^^^^^^^^^^^^^^

Expand Down
20 changes: 20 additions & 0 deletions facebook/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,26 @@ 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_code_from_token(self, app_id, app_secret, redirect_uri):
"""
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.
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This docstring suffers from the same issues as the description in the documentation.

args = {'access_token': self.access_token,
'client_id': app_id,
'client_secret': app_secret,
'redirect_uri': redirect_uri}

try:
code = self.request("{0}/oauth/client_code".format(self.version),
args=args)["code"]
return code
except GraphAPIError:
self.access_token = self.extend_access_token(app_id, app_secret)
code = self.request("{0}/oauth/client_code".format(self.version),
args=args)["code"]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a GraphAPIError has been raised, why are we calling extend_access_token here? Why isn't the code object generated here returned?

def get_access_token_from_code(
self, code, redirect_uri, app_id, app_secret):
"""Get an access token from the "code" returned from an OAuth dialog.
Expand Down
19 changes: 19 additions & 0 deletions test/test_facebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,25 @@ def test_extend_access_token(self):
self.assertEqual(
e.message, "fb_exchange_token parameter not specified")

def test_get_code_from_token(self):
"""
Test that get_code_from_token returns a valid code which can be
exchanged for an access token.
"""
redirect_uri = 'https://localhost/facebook/callback/'
app_token = facebook.GraphAPI().get_app_access_token(self.app_id,
self.secret)
self.create_test_users(self.app_id,
facebook.GraphAPI(app_token), 1)
test_token = self.test_users[0]['access_token']
graph = facebook.GraphAPI(test_token)
self.assertIsNotNone(graph.get_code_from_token(
self.app_id, self.secret, redirect_uri),
'Code not returned by get_code_from_token method')
self.assertIsNotNone(graph.get_access_token_from_code(
self.app_id, self.secret), 'Invalid code returned by'
'get_code_from_token method')

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

def test_bogus_access_token(self):
graph = facebook.GraphAPI(access_token='wrong_token')
self.assertRaises(facebook.GraphAPIError, graph.get_object, 'me')
Expand Down