Merge pull request #357 from singingwolfboy/oauthlib3

Switch to OAuthlib 3.0.0

Author: singingwolfboy

HISTORY.rst

@@ -4,6 +4,17 @@ History
 UNRELEASED
 ++++++++++
 
+- This project now depends on OAuthlib 3.0.0 and above. It does **not** support
+  versions of OAuthlib before 3.0.0.
+- Updated oauth2 tests to use 'sess' for an OAuth2Session instance instead of `auth`
+  because OAuth2Session objects and methods acceept an `auth` paramether which is
+  typically an instance of `requests.auth.HTTPBasicAuth`
+- `OAuth2Session.fetch_token` previously tried to guess how and where to provide
+  "client" and "user" credentials incorrectly. This was incompatible with some
+  OAuth servers and incompatible with breaking changes in oauthlib that seek to
+  correctly provide the `client_id`. The older implementation also did not raise
+  the correct exceptions when username and password are not present on Legacy
+  clients.
 - Avoid automatic netrc authentication for OAuth2Session.
 
 v1.1.0 (9 January 2019)

requests_oauthlib/oauth2_session.py

@@ -4,6 +4,7 @@
 
 from oauthlib.common import generate_token, urldecode
 from oauthlib.oauth2 import WebApplicationClient, InsecureTransportError
+from oauthlib.oauth2 import LegacyApplicationClient
 from oauthlib.oauth2 import TokenExpiredError, is_secure_transport
 import requests
 
@@ -158,11 +159,14 @@ def authorization_url(self, url, state=None, **kwargs):
 
     def fetch_token(self, token_url, code=None, authorization_response=None,
             body='', auth=None, username=None, password=None, method='POST',
-            timeout=None, headers=None, verify=True, proxies=None, **kwargs):
+            timeout=None, headers=None, verify=True, proxies=None,
+            include_client_id=None, client_secret=None, **kwargs):
         """Generic method for fetching an access token from the token endpoint.
 
         If you are using the MobileApplicationClient you will want to use
-        token_from_fragment instead of fetch_token.
+        `token_from_fragment` instead of `fetch_token`.
+
+        The current implementation enforces the RFC guidelines.
 
         :param token_url: Token endpoint URL, must use HTTPS.
         :param code: Authorization code (used by WebApplicationClients).
@@ -171,15 +175,28 @@ def fetch_token(self, token_url, code=None, authorization_response=None,
                                        WebApplicationClients instead of code.
         :param body: Optional application/x-www-form-urlencoded body to add the
                      include in the token request. Prefer kwargs over body.
-        :param auth: An auth tuple or method as accepted by requests.
-        :param username: Username used by LegacyApplicationClients.
-        :param password: Password used by LegacyApplicationClients.
+        :param auth: An auth tuple or method as accepted by `requests`.
+        :param username: Username required by LegacyApplicationClients to appear
+                         in the request body.
+        :param password: Password required by LegacyApplicationClients to appear
+                         in the request body.
         :param method: The HTTP method used to make the request. Defaults
                        to POST, but may also be GET. Other methods should
                        be added as needed.
-        :param headers: Dict to default request headers with.
         :param timeout: Timeout of the request in seconds.
+        :param headers: Dict to default request headers with.
         :param verify: Verify SSL certificate.
+        :param proxies: The `proxies` argument is passed onto `requests`.
+        :param include_client_id: Should the request body include the
+                                  `client_id` parameter. Default is `None`,
+                                  which will attempt to autodetect. This can be
+                                  forced to always include (True) or never
+                                  include (False).
+        :param client_secret: The `client_secret` paired to the `client_id`.
+                              This is generally required unless provided in the
+                              `auth` tuple. If the value is `None`, it will be
+                              omitted from the request, however if the value is
+                              an empty string, an empty string will be sent.
         :param kwargs: Extra parameters to include in the token request.
         :return: A token dict
         """
@@ -196,23 +213,65 @@ def fetch_token(self, token_url, code=None, authorization_response=None,
                 raise ValueError('Please supply either code or '
                                  'authorization_response parameters.')
 
+        # Earlier versions of this library build an HTTPBasicAuth header out of
+        # `username` and `password`. The RFC states, however these attributes
+        # must be in the request body and not the header.
+        # If an upstream server is not spec compliant and requires them to
+        # appear as an Authorization header, supply an explicit `auth` header
+        # to this function.
+        # This check will allow for empty strings, but not `None`.
+        #
+        # Refernences
+        # 4.3.2 - Resource Owner Password Credentials Grant
+        #         https://tools.ietf.org/html/rfc6749#section-4.3.2
+
+        if isinstance(self._client, LegacyApplicationClient):
+            if username is None:
+                raise ValueError('`LegacyApplicationClient` requires both the '
+                                 '`username` and `password` parameters.')
+            if password is None:
+                raise ValueError('The required paramter `username` was supplied, '
+                                 'but `password` was not.')
+
+        # merge username and password into kwargs for `prepare_request_body`
+        if username is not None:
+            kwargs['username'] = username
+        if password is not None:
+            kwargs['password'] = password
+
+        # is an auth explicitly supplied?
+        if auth is not None:
+            # if we're dealing with the default of `include_client_id` (None):
+            # we will assume the `auth` argument is for an RFC compliant server
+            # and we should not send the `client_id` in the body.
+            # This approach allows us to still force the client_id by submitting
+            # `include_client_id=True` along with an `auth` object.
+            if include_client_id is None:
+                include_client_id = False
+
+        # otherwise we may need to create an auth header
+        else:
+            # since we don't have an auth header, we MAY need to create one
+            # it is possible that we want to send the `client_id` in the body
+            # if so, `include_client_id` should be set to True
+            # otherwise, we will generate an auth header
+            if include_client_id is not True:
+                client_id = self.client_id
+                if client_id:
+                    log.debug('Encoding `client_id` "%s" with `client_secret` '
+                              'as Basic auth credentials.', client_id)
+                    client_secret = client_secret if client_secret is not None else ''
+                    auth = requests.auth.HTTPBasicAuth(client_id, client_secret)
+
+        if include_client_id:
+            # this was pulled out of the params
+            # it needs to be passed into prepare_request_body
+            if client_secret is not None:
+                kwargs['client_secret'] = client_secret
 
         body = self._client.prepare_request_body(code=code, body=body,
-                redirect_uri=self.redirect_uri, username=username,
-                password=password, **kwargs)
-
-        client_id = kwargs.get('client_id', '')
-        if auth is None:
-            if client_id:
-                log.debug('Encoding client_id "%s" with client_secret as Basic auth credentials.', client_id)
-                client_secret = kwargs.get('client_secret', '')
-                client_secret = client_secret if client_secret is not None else ''
-                auth = requests.auth.HTTPBasicAuth(client_id, client_secret)
-            elif username:
-                if password is None:
-                    raise ValueError('Username was supplied, but not password.')
-                log.debug('Encoding username, password as Basic auth credentials.')
-                auth = requests.auth.HTTPBasicAuth(username, password)
+                redirect_uri=self.redirect_uri,
+                include_client_id=include_client_id, **kwargs)
 
         headers = headers or {
             'Accept': 'application/json',
@@ -269,9 +328,11 @@ def refresh_token(self, token_url, refresh_token=None, body='', auth=None,
         :param refresh_token: The refresh_token to use.
         :param body: Optional application/x-www-form-urlencoded body to add the
                      include in the token request. Prefer kwargs over body.
-        :param auth: An auth tuple or method as accepted by requests.
+        :param auth: An auth tuple or method as accepted by `requests`.
         :param timeout: Timeout of the request in seconds.
+        :param headers: A dict of headers to be used by `requests`.
         :param verify: Verify SSL certificate.
+        :param proxies: The `proxies` argument will be passed to `requests`.
         :param kwargs: Extra parameters to include in the token request.
         :return: A token dict
         """

requirements.txt

@@ -1,2 +1,2 @@
 requests>=2.0.0
-oauthlib[signedtoken]>=2.1.0,<3.0.0
+oauthlib[signedtoken]>=3.0.0

setup.py

@@ -44,8 +44,8 @@ def readall(path):
     url='https://github.com/requests/requests-oauthlib',
     packages=['requests_oauthlib', 'requests_oauthlib.compliance_fixes'],
     python_requires=">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*",
-    install_requires=['oauthlib>=2.1.0,<3.0.0', 'requests>=2.0.0'],
-    extras_require={'rsa': ['oauthlib[signedtoken]>=2.1.0,<3.0.0']},
+    install_requires=['oauthlib>=3.0.0', 'requests>=2.0.0'],
+    extras_require={'rsa': ['oauthlib[signedtoken]>=3.0.0']},
     license='ISC',
     classifiers=[
         'Development Status :: 5 - Production/Stable',

tests/test_compliance_fixes.py

@@ -32,13 +32,13 @@ def setUp(self):
         mocker.start()
         self.addCleanup(mocker.stop)
 
-        facebook = OAuth2Session('foo', redirect_uri='https://i.b')
+        facebook = OAuth2Session('someclientid', redirect_uri='https://i.b')
         self.session = facebook_compliance_fix(facebook)
 
     def test_fetch_access_token(self):
         token = self.session.fetch_token(
             'https://graph.facebook.com/oauth/access_token',
-             client_secret='bar',
+             client_secret='someclientsecret',
              authorization_response='https://i.b/?code=hello',
         )
         self.assertEqual(token, {'access_token': 'urlencoded', 'token_type': 'Bearer'})
@@ -55,15 +55,15 @@ def setUp(self):
         self.mocker.start()
         self.addCleanup(self.mocker.stop)
 
-        fitbit = OAuth2Session('foo', redirect_uri='https://i.b')
+        fitbit = OAuth2Session('someclientid', redirect_uri='https://i.b')
         self.session = fitbit_compliance_fix(fitbit)
 
     def test_fetch_access_token(self):
         self.assertRaises(
             InvalidGrantError,
             self.session.fetch_token,
             'https://api.fitbit.com/oauth2/token',
-            client_secret='bar',
+            client_secret='someclientsecret',
             authorization_response='https://i.b/?code=hello',
         )
 
@@ -84,7 +84,7 @@ def test_refresh_token(self):
             InvalidGrantError,
             self.session.refresh_token,
             'https://api.fitbit.com/oauth2/token',
-            auth=requests.auth.HTTPBasicAuth('foo', 'bar')
+            auth=requests.auth.HTTPBasicAuth('someclientid', 'someclientsecret')
         )
 
         self.mocker.post(
@@ -94,7 +94,7 @@ def test_refresh_token(self):
 
         token = self.session.refresh_token(
             'https://api.fitbit.com/oauth2/token',
-            auth=requests.auth.HTTPBasicAuth('foo', 'bar')
+            auth=requests.auth.HTTPBasicAuth('someclientid', 'someclientsecret')
         )
 
         self.assertEqual(token['access_token'], 'access')
@@ -120,13 +120,13 @@ def setUp(self):
         mocker.start()
         self.addCleanup(mocker.stop)
 
-        linkedin = OAuth2Session('foo', redirect_uri='https://i.b')
+        linkedin = OAuth2Session('someclientid', redirect_uri='https://i.b')
         self.session = linkedin_compliance_fix(linkedin)
 
     def test_fetch_access_token(self):
         token = self.session.fetch_token(
             'https://www.linkedin.com/uas/oauth2/accessToken',
-            client_secret='bar',
+            client_secret='someclientsecret',
             authorization_response='https://i.b/?code=hello',
         )
         self.assertEqual(token, {'access_token': 'linkedin', 'token_type': 'Bearer'})
@@ -152,13 +152,13 @@ def setUp(self):
         mocker.start()
         self.addCleanup(mocker.stop)
 
-        mailchimp = OAuth2Session('foo', redirect_uri='https://i.b')
+        mailchimp = OAuth2Session('someclientid', redirect_uri='https://i.b')
         self.session = mailchimp_compliance_fix(mailchimp)
 
     def test_fetch_access_token(self):
         token = self.session.fetch_token(
             "https://login.mailchimp.com/oauth2/token",
-            client_secret='bar',
+            client_secret='someclientsecret',
             authorization_response='https://i.b/?code=hello',
         )
         # Times should be close
@@ -184,13 +184,13 @@ def setUp(self):
         mocker.start()
         self.addCleanup(mocker.stop)
 
-        weibo = OAuth2Session('foo', redirect_uri='https://i.b')
+        weibo = OAuth2Session('someclientid', redirect_uri='https://i.b')
         self.session = weibo_compliance_fix(weibo)
 
     def test_fetch_access_token(self):
         token = self.session.fetch_token(
             'https://api.weibo.com/oauth2/access_token',
-            client_secret='bar',
+            client_secret='someclientsecret',
             authorization_response='https://i.b/?code=hello',
         )
         self.assertEqual(token, {'access_token': 'weibo', 'token_type': 'Bearer'})
@@ -223,7 +223,7 @@ def setUp(self):
         mocker.start()
         self.addCleanup(mocker.stop)
 
-        slack = OAuth2Session('foo', redirect_uri='https://i.b')
+        slack = OAuth2Session('someclientid', redirect_uri='https://i.b')
         self.session = slack_compliance_fix(slack)
 
     def test_protected_request(self):
@@ -293,7 +293,7 @@ def setUp(self):
         mocker.start()
         self.addCleanup(mocker.stop)
 
-        plentymarkets = OAuth2Session('foo', redirect_uri='https://i.b')
+        plentymarkets = OAuth2Session('someclientid', redirect_uri='https://i.b')
         self.session = plentymarkets_compliance_fix(plentymarkets)
 
     def test_fetch_access_token(self):