auth: Add deprecation warnings for anything using auth_return_future

tornado/auth.py

@@ -73,6 +73,7 @@ import hashlib
 import hmac
 import time
 import uuid
+import warnings
 
 from tornado.concurrent import (Future, return_future, chain_future,
                                 future_set_exc_info,
@@ -113,6 +114,9 @@ def _auth_return_future(f):
 
     Note that when using this decorator the ``callback`` parameter
     inside the function will actually be a future.
+
+    .. deprecated:: 5.1
+       Will be removed in 6.0.
     """
     replacer = ArgReplacer(f, 'callback')
 
@@ -121,6 +125,8 @@ def _auth_return_future(f):
         future = Future()
         callback, args, kwargs = replacer.replace(future, args, kwargs)
         if callback is not None:
+            warnings.warn("callback arguments are deprecated, use the returned Future instead",
+                          DeprecationWarning)
             future.add_done_callback(
                 functools.partial(_auth_future_to_callback, callback))
 
@@ -179,6 +185,11 @@ class OpenIdMixin(object):
         is present and `authenticate_redirect` if it is not).
 
         The result of this method will generally be used to set a cookie.
+
+        .. deprecated:: 5.1
+
+           The ``callback`` argument is deprecated and will be removed in 6.0.
+           Use the returned awaitable object instead.
         """
         # Verify the OpenID response via direct request to the OP
         args = dict((k, v[-1]) for k, v in self.request.arguments.items())
@@ -381,6 +392,11 @@ class OAuthMixin(object):
         requests to this service on behalf of the user.  The dictionary will
         also contain other fields such as ``name``, depending on the service
         used.
+
+        .. deprecated:: 5.1
+
+           The ``callback`` argument is deprecated and will be removed in 6.0.
+           Use the returned awaitable object instead.
         """
         future = callback
         request_key = escape.utf8(self.get_argument("oauth_token"))
@@ -648,6 +664,11 @@ class OAuth2Mixin(object):
            :hide:
 
         .. versionadded:: 4.3
+
+        .. deprecated:: 5.1
+
+           The ``callback`` argument is deprecated and will be removed in 6.0.
+           Use the returned awaitable object instead.
         """
         all_args = {}
         if access_token:
@@ -781,6 +802,10 @@ class TwitterMixin(OAuthMixin):
         .. testoutput::
            :hide:
 
+        .. deprecated:: 5.1
+
+           The ``callback`` argument is deprecated and will be removed in 6.0.
+           Use the returned awaitable object instead.
         """
         if path.startswith('http:') or path.startswith('https:'):
             # Raw urls are useful for e.g. search which doesn't follow the
@@ -896,6 +921,10 @@ class GoogleOAuth2Mixin(OAuth2Mixin):
         .. testoutput::
            :hide:
 
+        .. deprecated:: 5.1
+
+           The ``callback`` argument is deprecated and will be removed in 6.0.
+           Use the returned awaitable object instead.
         """  # noqa: E501
         http = self.get_auth_http_client()
         body = urllib_parse.urlencode({
@@ -973,6 +1002,11 @@ class FacebookGraphMixin(OAuth2Mixin):
         .. versionchanged:: 4.5
            The ``session_expires`` field was updated to support changes made to the
            Facebook API in March 2017.
+
+        .. deprecated:: 5.1
+
+           The ``callback`` argument is deprecated and will be removed in 6.0.
+           Use the returned awaitable object instead.
         """
         http = self.get_auth_http_client()
         args = {
@@ -991,6 +1025,7 @@ class FacebookGraphMixin(OAuth2Mixin):
                    functools.partial(self._on_access_token, redirect_uri, client_id,
                                      client_secret, callback, fields))
 
+    @gen.coroutine
     def _on_access_token(self, redirect_uri, client_id, client_secret,
                          future, fields, response):
         if response.error:
@@ -1003,10 +1038,8 @@ class FacebookGraphMixin(OAuth2Mixin):
             "expires_in": args.get("expires_in")
         }
 
-        self.facebook_request(
+        user = yield self.facebook_request(
             path="/me",
-            callback=functools.partial(
-                self._on_get_user_info, future, session, fields),
             access_token=session["access_token"],
             appsecret_proof=hmac.new(key=client_secret.encode('utf8'),
                                      msg=session["access_token"].encode('utf8'),
@@ -1014,7 +1047,6 @@ class FacebookGraphMixin(OAuth2Mixin):
             fields=",".join(fields)
         )
 
-    def _on_get_user_info(self, future, session, fields, user):
         if user is None:
             future_set_result_unless_cancelled(future, None)
             return
@@ -1080,6 +1112,11 @@ class FacebookGraphMixin(OAuth2Mixin):
 
         .. versionchanged:: 3.1
            Added the ability to override ``self._FACEBOOK_BASE_URL``.
+
+        .. deprecated:: 5.1
+
+           The ``callback`` argument is deprecated and will be removed in 6.0.
+           Use the returned awaitable object instead.
         """
         url = self._FACEBOOK_BASE_URL + path
         # Thanks to the _auth_return_future decorator, our "callback"

tornado/test/auth_test.py

@@ -6,6 +6,8 @@
 
 from __future__ import absolute_import, division, print_function
 
+import warnings
+
 from tornado.auth import (
     AuthError, OpenIdMixin, OAuthMixin, OAuth2Mixin,
     GoogleOAuth2Mixin, FacebookGraphMixin, TwitterMixin,
@@ -19,16 +21,18 @@ from tornado.testing import AsyncHTTPTestCase, ExpectLog
 from tornado.web import RequestHandler, Application, asynchronous, HTTPError
 
 
-class OpenIdClientLoginHandler(RequestHandler, OpenIdMixin):
+class OpenIdClientLoginHandlerLegacy(RequestHandler, OpenIdMixin):
     def initialize(self, test):
         self._OPENID_ENDPOINT = test.get_url('/openid/server/authenticate')
 
     @asynchronous
     def get(self):
         if self.get_argument('openid.mode', None):
-            self.get_authenticated_user(
-                self.on_user, http_client=self.settings['http_client'])
-            return
+            with warnings.catch_warnings():
+                warnings.simplefilter('ignore', DeprecationWarning)
+                self.get_authenticated_user(
+                    self.on_user, http_client=self.settings['http_client'])
+                return
         res = self.authenticate_redirect()
         assert isinstance(res, Future)
         assert res.done()
@@ -39,6 +43,23 @@ class OpenIdClientLoginHandler(RequestHandler, OpenIdMixin):
         self.finish(user)
 
 
+class OpenIdClientLoginHandler(RequestHandler, OpenIdMixin):
+    def initialize(self, test):
+        self._OPENID_ENDPOINT = test.get_url('/openid/server/authenticate')
+
+    @gen.coroutine
+    def get(self):
+        if self.get_argument('openid.mode', None):
+            user = yield self.get_authenticated_user(http_client=self.settings['http_client'])
+            if user is None:
+                raise Exception("user is None")
+            self.finish(user)
+            return
+        res = self.authenticate_redirect()
+        assert isinstance(res, Future)
+        assert res.done()
+
+
 class OpenIdServerAuthenticateHandler(RequestHandler):
     def post(self):
         if self.get_argument('openid.mode') != 'check_authentication':
@@ -46,7 +67,7 @@ class OpenIdServerAuthenticateHandler(RequestHandler):
         self.write('is_valid:true')
 
 
-class OAuth1ClientLoginHandler(RequestHandler, OAuthMixin):
+class OAuth1ClientLoginHandlerLegacy(RequestHandler, OAuthMixin):
     def initialize(self, test, version):
         self._OAUTH_VERSION = version
         self._OAUTH_REQUEST_TOKEN_URL = test.get_url('/oauth1/server/request_token')
@@ -59,8 +80,10 @@ class OAuth1ClientLoginHandler(RequestHandler, OAuthMixin):
     @asynchronous
     def get(self):
         if self.get_argument('oauth_token', None):
-            self.get_authenticated_user(
-                self.on_user, http_client=self.settings['http_client'])
+            with warnings.catch_warnings():
+                warnings.simplefilter('ignore', DeprecationWarning)
+                self.get_authenticated_user(
+                    self.on_user, http_client=self.settings['http_client'])
             return
         res = self.authorize_redirect(http_client=self.settings['http_client'])
         assert isinstance(res, Future)
@@ -78,6 +101,34 @@ class OAuth1ClientLoginHandler(RequestHandler, OAuthMixin):
         callback(dict(email='foo@example.com'))
 
 
+class OAuth1ClientLoginHandler(RequestHandler, OAuthMixin):
+    def initialize(self, test, version):
+        self._OAUTH_VERSION = version
+        self._OAUTH_REQUEST_TOKEN_URL = test.get_url('/oauth1/server/request_token')
+        self._OAUTH_AUTHORIZE_URL = test.get_url('/oauth1/server/authorize')
+        self._OAUTH_ACCESS_TOKEN_URL = test.get_url('/oauth1/server/access_token')
+
+    def _oauth_consumer_token(self):
+        return dict(key='asdf', secret='qwer')
+
+    @gen.coroutine
+    def get(self):
+        if self.get_argument('oauth_token', None):
+            user = yield self.get_authenticated_user(http_client=self.settings['http_client'])
+            if user is None:
+                raise Exception("user is None")
+            self.finish(user)
+            return
+        yield self.authorize_redirect(http_client=self.settings['http_client'])
+
+    def _oauth_get_user(self, access_token, callback):
+        if self.get_argument('fail_in_get_user', None):
+            raise Exception("failing in get_user")
+        if access_token != dict(key='uiop', secret='5678'):
+            raise Exception("incorrect access token %r" % access_token)
+        callback(dict(email='foo@example.com'))
+
+
 class OAuth1ClientLoginCoroutineHandler(OAuth1ClientLoginHandler):
     """Replaces OAuth1ClientLoginCoroutineHandler's get() with a coroutine."""
     @gen.coroutine
@@ -172,7 +223,7 @@ class TwitterClientHandler(RequestHandler, TwitterMixin):
         return self.settings['http_client']
 
 
-class TwitterClientLoginHandler(TwitterClientHandler):
+class TwitterClientLoginHandlerLegacy(TwitterClientHandler):
     @asynchronous
     def get(self):
         if self.get_argument("oauth_token", None):
@@ -186,6 +237,18 @@ class TwitterClientLoginHandler(TwitterClientHandler):
         self.finish(user)
 
 
+class TwitterClientLoginHandler(TwitterClientHandler):
+    @gen.coroutine
+    def get(self):
+        if self.get_argument("oauth_token", None):
+            user = yield self.get_authenticated_user()
+            if user is None:
+                raise Exception("user is None")
+            self.finish(user)
+            return
+        yield self.authorize_redirect()
+
+
 class TwitterClientLoginGenEngineHandler(TwitterClientHandler):
     @asynchronous
     @gen.engine
@@ -211,15 +274,17 @@ class TwitterClientLoginGenCoroutineHandler(TwitterClientHandler):
             yield self.authorize_redirect()
 
 
-class TwitterClientShowUserHandler(TwitterClientHandler):
+class TwitterClientShowUserHandlerLegacy(TwitterClientHandler):
     @asynchronous
     @gen.engine
     def get(self):
         # TODO: would be nice to go through the login flow instead of
         # cheating with a hard-coded access token.
-        response = yield gen.Task(self.twitter_request,
-                                  '/users/show/%s' % self.get_argument('name'),
-                                  access_token=dict(key='hjkl', secret='vbnm'))
+        with warnings.catch_warnings():
+            warnings.simplefilter('ignore', DeprecationWarning)
+            response = yield gen.Task(self.twitter_request,
+                                      '/users/show/%s' % self.get_argument('name'),
+                                      access_token=dict(key='hjkl', secret='vbnm'))
         if response is None:
             self.set_status(500)
             self.finish('error from twitter request')
@@ -227,6 +292,22 @@ class TwitterClientShowUserHandler(TwitterClientHandler):
             self.finish(response)
 
 
+class TwitterClientShowUserHandler(TwitterClientHandler):
+    @gen.coroutine
+    def get(self):
+        # TODO: would be nice to go through the login flow instead of
+        # cheating with a hard-coded access token.
+        try:
+            response = yield self.twitter_request(
+                '/users/show/%s' % self.get_argument('name'),
+                access_token=dict(key='hjkl', secret='vbnm'))
+        except AuthError:
+            self.set_status(500)
+            self.finish('error from twitter request')
+        else:
+            self.finish(response)
+
+
 class TwitterClientShowUserFutureHandler(TwitterClientHandler):
     @asynchronous
     @gen.engine
@@ -279,12 +360,17 @@ class AuthTest(AsyncHTTPTestCase):
         return Application(
             [
                 # test endpoints
+                ('/legacy/openid/client/login', OpenIdClientLoginHandlerLegacy, dict(test=self)),
                 ('/openid/client/login', OpenIdClientLoginHandler, dict(test=self)),
+                ('/legacy/oauth10/client/login', OAuth1ClientLoginHandlerLegacy,
+                 dict(test=self, version='1.0')),
                 ('/oauth10/client/login', OAuth1ClientLoginHandler,
                  dict(test=self, version='1.0')),
                 ('/oauth10/client/request_params',
                  OAuth1ClientRequestParametersHandler,
                  dict(version='1.0')),
+                ('/legacy/oauth10a/client/login', OAuth1ClientLoginHandlerLegacy,
+                 dict(test=self, version='1.0a')),
                 ('/oauth10a/client/login', OAuth1ClientLoginHandler,
                  dict(test=self, version='1.0a')),
                 ('/oauth10a/client/login_coroutine',
@@ -297,11 +383,14 @@ class AuthTest(AsyncHTTPTestCase):
 
                 ('/facebook/client/login', FacebookClientLoginHandler, dict(test=self)),
 
+                ('/legacy/twitter/client/login', TwitterClientLoginHandlerLegacy, 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)),
+                ('/legacy/twitter/client/show_user',
+                 TwitterClientShowUserHandlerLegacy, dict(test=self)),
                 ('/twitter/client/show_user',
                  TwitterClientShowUserHandler, dict(test=self)),
                 ('/twitter/client/show_user_future',
@@ -325,6 +414,21 @@ class AuthTest(AsyncHTTPTestCase):
             facebook_api_key='test_facebook_api_key',
             facebook_secret='test_facebook_secret')
 
+    def test_openid_redirect_legacy(self):
+        response = self.fetch('/legacy/openid/client/login', follow_redirects=False)
+        self.assertEqual(response.code, 302)
+        self.assertTrue(
+            '/openid/server/authenticate?' in response.headers['Location'])
+
+    def test_openid_get_user_legacy(self):
+        response = self.fetch('/legacy/openid/client/login?openid.mode=blah'
+                              '&openid.ns.ax=http://openid.net/srv/ax/1.0'
+                              '&openid.ax.type.email=http://axschema.org/contact/email'
+                              '&openid.ax.value.email=foo@example.com')
+        response.rethrow()
+        parsed = json_decode(response.body)
+        self.assertEqual(parsed["email"], "foo@example.com")
+
     def test_openid_redirect(self):
         response = self.fetch('/openid/client/login', follow_redirects=False)
         self.assertEqual(response.code, 302)
@@ -340,6 +444,16 @@ class AuthTest(AsyncHTTPTestCase):
         parsed = json_decode(response.body)
         self.assertEqual(parsed["email"], "foo@example.com")
 
+    def test_oauth10_redirect_legacy(self):
+        response = self.fetch('/legacy/oauth10/client/login', follow_redirects=False)
+        self.assertEqual(response.code, 302)
+        self.assertTrue(response.headers['Location'].endswith(
+            '/oauth1/server/authorize?oauth_token=zxcv'))
+        # the cookie is base64('zxcv')|base64('1234')
+        self.assertTrue(
+            '_oauth_request_token="enhjdg==|MTIzNA=="' in response.headers['Set-Cookie'],
+            response.headers['Set-Cookie'])
+
     def test_oauth10_redirect(self):
         response = self.fetch('/oauth10/client/login', follow_redirects=False)
         self.assertEqual(response.code, 302)
@@ -350,6 +464,15 @@ class AuthTest(AsyncHTTPTestCase):
             '_oauth_request_token="enhjdg==|MTIzNA=="' in response.headers['Set-Cookie'],
             response.headers['Set-Cookie'])
 
+    def test_oauth10_get_user_legacy(self):
+        response = self.fetch(
+            '/legacy/oauth10/client/login?oauth_token=zxcv',
+            headers={'Cookie': '_oauth_request_token=enhjdg==|MTIzNA=='})
+        response.rethrow()
+        parsed = json_decode(response.body)
+        self.assertEqual(parsed['email'], 'foo@example.com')
+        self.assertEqual(parsed['access_token'], dict(key='uiop', secret='5678'))
+
     def test_oauth10_get_user(self):
         response = self.fetch(
             '/oauth10/client/login?oauth_token=zxcv',
@@ -368,6 +491,25 @@ class AuthTest(AsyncHTTPTestCase):
         self.assertTrue('oauth_nonce' in parsed)
         self.assertTrue('oauth_signature' in parsed)
 
+    def test_oauth10a_redirect_legacy(self):
+        response = self.fetch('/legacy/oauth10a/client/login', follow_redirects=False)
+        self.assertEqual(response.code, 302)
+        self.assertTrue(response.headers['Location'].endswith(
+            '/oauth1/server/authorize?oauth_token=zxcv'))
+        # the cookie is base64('zxcv')|base64('1234')
+        self.assertTrue(
+            '_oauth_request_token="enhjdg==|MTIzNA=="' in response.headers['Set-Cookie'],
+            response.headers['Set-Cookie'])
+
+    def test_oauth10a_get_user_legacy(self):
+        response = self.fetch(
+            '/legacy/oauth10a/client/login?oauth_token=zxcv',
+            headers={'Cookie': '_oauth_request_token=enhjdg==|MTIzNA=='})
+        response.rethrow()
+        parsed = json_decode(response.body)
+        self.assertEqual(parsed['email'], 'foo@example.com')
+        self.assertEqual(parsed['access_token'], dict(key='uiop', secret='5678'))
+
     def test_oauth10a_redirect(self):
         response = self.fetch('/oauth10a/client/login', follow_redirects=False)
         self.assertEqual(response.code, 302)
@@ -428,6 +570,9 @@ class AuthTest(AsyncHTTPTestCase):
             '_oauth_request_token="enhjdg==|MTIzNA=="' in response.headers['Set-Cookie'],
             response.headers['Set-Cookie'])
 
+    def test_twitter_redirect_legacy(self):
+        self.base_twitter_redirect('/legacy/twitter/client/login')
+
     def test_twitter_redirect(self):
         self.base_twitter_redirect('/twitter/client/login')
 
@@ -451,6 +596,18 @@ class AuthTest(AsyncHTTPTestCase):
                           u'screen_name': u'foo',
                           u'username': u'foo'})
 
+    def test_twitter_show_user_legacy(self):
+        response = self.fetch('/legacy/twitter/client/show_user?name=somebody')
+        response.rethrow()
+        self.assertEqual(json_decode(response.body),
+                         {'name': 'Somebody', 'screen_name': 'somebody'})
+
+    def test_twitter_show_user_error_legacy(self):
+        with ExpectLog(gen_log, 'Error response HTTP 500'):
+            response = self.fetch('/legacy/twitter/client/show_user?name=error')
+        self.assertEqual(response.code, 500)
+        self.assertEqual(response.body, b'error from twitter request')
+
     def test_twitter_show_user(self):
         response = self.fetch('/twitter/client/show_user?name=somebody')
         response.rethrow()
@@ -458,8 +615,7 @@ class AuthTest(AsyncHTTPTestCase):
                          {'name': 'Somebody', 'screen_name': 'somebody'})
 
     def test_twitter_show_user_error(self):
-        with ExpectLog(gen_log, 'Error response HTTP 500'):
-            response = self.fetch('/twitter/client/show_user?name=error')
+        response = self.fetch('/twitter/client/show_user?name=error')
         self.assertEqual(response.code, 500)
         self.assertEqual(response.body, b'error from twitter request')