Repository: tornadoweb/tornado

Issue:
FacebookGraphMixin failing
I had to modify `_on_access_token` so that it did not attempt to make a `facebook_request` to `/me`. With the code as it stands, I get this error:

`TypeError: 'Future' object is not callable`

However, removing that extra request fixes this, and I can use the access_token in the next part of my auth pipeline.

Discussion:

Patch:
diff --git a/tornado/auth.py b/tornado/auth.py
index 32d0e226..ff7172aa 100644
--- a/tornado/auth.py
+++ b/tornado/auth.py
@@ -75,7 +75,7 @@ import hmac
 import time
 import uuid
 
-from tornado.concurrent import TracebackFuture, return_future
+from tornado.concurrent import TracebackFuture, return_future, chain_future
 from tornado import gen
 from tornado import httpclient
 from tornado import escape
@@ -985,7 +985,7 @@ class FacebookGraphMixin(OAuth2Mixin):
             future.set_exception(AuthError('Facebook auth error: %s' % str(response)))
             return
 
-        args = escape.parse_qs_bytes(escape.native_str(response.body))
+        args = urlparse.parse_qs(escape.native_str(response.body))
         session = {
             "access_token": args["access_token"][-1],
             "expires": args.get("expires")
@@ -1062,8 +1062,13 @@ class FacebookGraphMixin(OAuth2Mixin):
            Added the ability to override ``self._FACEBOOK_BASE_URL``.
         """
         url = self._FACEBOOK_BASE_URL + path
-        return self.oauth2_request(url, callback, access_token,
-                                   post_args, **args)
+        # Thanks to the _auth_return_future decorator, our "callback"
+        # argument is a Future, which we cannot pass as a callback to
+        # oauth2_request. Instead, have oauth2_request return a
+        # future and chain them together.
+        oauth_future = self.oauth2_request(url, access_token=access_token,
+                                           post_args=post_args, **args)
+        chain_future(oauth_future, callback)
 
 
 def _oauth_signature(consumer_token, method, url, parameters={}, token=None):


Test patch:
diff --git a/tornado/test/auth_test.py b/tornado/test/auth_test.py
index 56de93a5..3ed40e45 100644
--- a/tornado/test/auth_test.py
+++ b/tornado/test/auth_test.py
@@ -5,7 +5,7 @@
 
 
 from __future__ import absolute_import, division, print_function, with_statement
-from tornado.auth import OpenIdMixin, OAuthMixin, OAuth2Mixin, TwitterMixin, AuthError, GoogleOAuth2Mixin
+from tornado.auth import OpenIdMixin, OAuthMixin, OAuth2Mixin, TwitterMixin, AuthError, GoogleOAuth2Mixin, FacebookGraphMixin
 from tornado.concurrent import Future
 from tornado.escape import json_decode
 from tornado import gen
@@ -126,6 +126,38 @@ class OAuth2ClientLoginHandler(RequestHandler, OAuth2Mixin):
         assert res.done()
 
 
+class FacebookClientLoginHandler(RequestHandler, FacebookGraphMixin):
+    def initialize(self, test):
+        self._OAUTH_AUTHORIZE_URL = test.get_url('/facebook/server/authorize')
+        self._OAUTH_ACCESS_TOKEN_URL = test.get_url('/facebook/server/access_token')
+        self._FACEBOOK_BASE_URL = test.get_url('/facebook/server')
+
+    @gen.coroutine
+    def get(self):
+        if self.get_argument("code", None):
+            user = yield self.get_authenticated_user(
+                redirect_uri=self.request.full_url(),
+                client_id=self.settings["facebook_api_key"],
+                client_secret=self.settings["facebook_secret"],
+                code=self.get_argument("code"))
+            self.write(user)
+        else:
+                yield self.authorize_redirect(
+                    redirect_uri=self.request.full_url(),
+                    client_id=self.settings["facebook_api_key"],
+                    extra_params={"scope": "read_stream,offline_access"})
+
+
+class FacebookServerAccessTokenHandler(RequestHandler):
+    def get(self):
+        self.write('access_token=asdf')
+
+
+class FacebookServerMeHandler(RequestHandler):
+    def get(self):
+        self.write('{}')
+
+
 class TwitterClientHandler(RequestHandler, TwitterMixin):
     def initialize(self, test):
         self._OAUTH_REQUEST_TOKEN_URL = test.get_url('/oauth1/server/request_token')
@@ -260,6 +292,8 @@ class AuthTest(AsyncHTTPTestCase):
                  dict(version='1.0a')),
                 ('/oauth2/client/login', OAuth2ClientLoginHandler, dict(test=self)),
 
+                ('/facebook/client/login', FacebookClientLoginHandler, dict(test=self)),
+
                 ('/twitter/client/login', TwitterClientLoginHandler, dict(test=self)),
                 ('/twitter/client/login_gen_engine', TwitterClientLoginGenEngineHandler, dict(test=self)),
                 ('/twitter/client/login_gen_coroutine', TwitterClientLoginGenCoroutineHandler, dict(test=self)),
@@ -271,13 +305,17 @@ class AuthTest(AsyncHTTPTestCase):
                 ('/oauth1/server/request_token', OAuth1ServerRequestTokenHandler),
                 ('/oauth1/server/access_token', OAuth1ServerAccessTokenHandler),
 
+                ('/facebook/server/access_token', FacebookServerAccessTokenHandler),
+                ('/facebook/server/me', FacebookServerMeHandler),
                 ('/twitter/server/access_token', TwitterServerAccessTokenHandler),
                 (r'/twitter/api/users/show/(.*)\.json', TwitterServerShowUserHandler),
                 (r'/twitter/api/account/verify_credentials\.json', TwitterServerVerifyCredentialsHandler),
             ],
             http_client=self.http_client,
             twitter_consumer_key='test_twitter_consumer_key',
-            twitter_consumer_secret='test_twitter_consumer_secret')
+            twitter_consumer_secret='test_twitter_consumer_secret',
+            facebook_api_key='test_facebook_api_key',
+            facebook_secret='test_facebook_secret')
 
     def test_openid_redirect(self):
         response = self.fetch('/openid/client/login', follow_redirects=False)
@@ -358,6 +396,13 @@ class AuthTest(AsyncHTTPTestCase):
         self.assertEqual(response.code, 302)
         self.assertTrue('/oauth2/server/authorize?' in response.headers['Location'])
 
+    def test_facebook_login(self):
+        response = self.fetch('/facebook/client/login', follow_redirects=False)
+        self.assertEqual(response.code, 302)
+        self.assertTrue('/facebook/server/authorize?' in response.headers['Location'])
+        response = self.fetch('/facebook/client/login?code=1234', follow_redirects=False)
+        self.assertEqual(response.code, 200)
+
     def base_twitter_redirect(self, url):
         # Same as test_oauth10a_redirect
         response = self.fetch(url, follow_redirects=False)