From cf3ff0a570c19768dc66de1a19f80e24976abce2 Mon Sep 17 00:00:00 2001
From: Ben Timby <btimby@smartfile.com>
Date: Fri, 3 Mar 2017 10:08:17 -0500
Subject: [PATCH 1/7] Implement changes to allow control over token refresh
 authentication.

---
 requests_oauthlib/oauth2_session.py | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/requests_oauthlib/oauth2_session.py b/requests_oauthlib/oauth2_session.py
index 5c46c2d1..fc64fc3a 100644
--- a/requests_oauthlib/oauth2_session.py
+++ b/requests_oauthlib/oauth2_session.py
@@ -334,13 +334,21 @@ def request(self, method, url, data=None, headers=None, withhold_token=False,
                     log.debug('Auto refresh is set, attempting to refresh at %s.',
                               self.auto_refresh_url)
 
-                    # We mustn't pass auth twice.
-                    auth = kwargs.pop('auth', None)
-                    if client_id and client_secret and (auth is None):
-                        log.debug('Encoding client_id "%s" with client_secret as Basic auth credentials.', client_id)
-                        auth = requests.auth.HTTPBasicAuth(client_id, client_secret)
+                    refresh_kwargs = {}
+                    # If caller provided auto_refresh_kwargs, don't send kwargs
+                    if not self.auto_refresh_kwargs:
+                        # Send kargs (preserve behavior) if caller did not
+                        # specify auto_refresh_kwargs.
+                        refresh_kwargs.update(kwargs)
+                        # If caller did not specify auth in kwargs, provide one.
+                        if client_id and client_secret and 'auth' not in refresh_kwargs:
+                            log.debug('Encoding client_id "%s" with client_secret as Basic auth credentials.', client_id)
+                            refresh_kwargs['auth'] = \
+                                requests.auth.HTTPBasicAuth(client_id, client_secret)
+                    # Call refresh_token(), it merges self.auto_refresh_kwargs
+                    # if provided by caller.
                     token = self.refresh_token(
-                        self.auto_refresh_url, auth=auth, **kwargs
+                        self.auto_refresh_url, **refresh_kwargs
                     )
                     if self.token_updater:
                         log.debug('Updating token to %s using %s.',

From f22426b9318504317f768e272c57da2382bcf1a0 Mon Sep 17 00:00:00 2001
From: Ben Timby <btimby@smartfile.com>
Date: Fri, 3 Mar 2017 10:34:17 -0500
Subject: [PATCH 2/7] Place refresh_token() arguments under caller's control.

---
 requests_oauthlib/oauth2_session.py | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/requests_oauthlib/oauth2_session.py b/requests_oauthlib/oauth2_session.py
index fc64fc3a..eaac60b1 100644
--- a/requests_oauthlib/oauth2_session.py
+++ b/requests_oauthlib/oauth2_session.py
@@ -279,9 +279,6 @@ def refresh_token(self, token_url, refresh_token=None, body='', auth=None,
 
         refresh_token = refresh_token or self.token.get('refresh_token')
 
-        log.debug('Adding auto refresh key word arguments %s.',
-                  self.auto_refresh_kwargs)
-        kwargs.update(self.auto_refresh_kwargs)
         body = self._client.prepare_refresh_body(body=body,
                 refresh_token=refresh_token, scope=self.scope, **kwargs)
         log.debug('Prepared refresh token request body %s', body)
@@ -334,21 +331,9 @@ def request(self, method, url, data=None, headers=None, withhold_token=False,
                     log.debug('Auto refresh is set, attempting to refresh at %s.',
                               self.auto_refresh_url)
 
-                    refresh_kwargs = {}
-                    # If caller provided auto_refresh_kwargs, don't send kwargs
-                    if not self.auto_refresh_kwargs:
-                        # Send kargs (preserve behavior) if caller did not
-                        # specify auto_refresh_kwargs.
-                        refresh_kwargs.update(kwargs)
-                        # If caller did not specify auth in kwargs, provide one.
-                        if client_id and client_secret and 'auth' not in refresh_kwargs:
-                            log.debug('Encoding client_id "%s" with client_secret as Basic auth credentials.', client_id)
-                            refresh_kwargs['auth'] = \
-                                requests.auth.HTTPBasicAuth(client_id, client_secret)
-                    # Call refresh_token(), it merges self.auto_refresh_kwargs
-                    # if provided by caller.
+                    # Call refresh_token(), pass any kwargs registered to it.
                     token = self.refresh_token(
-                        self.auto_refresh_url, **refresh_kwargs
+                        self.auto_refresh_url, **self.auth_refresh_kwargs
                     )
                     if self.token_updater:
                         log.debug('Updating token to %s using %s.',

From e5e888a1e722331a007df7ec1ab0ba1a5e0e5f0c Mon Sep 17 00:00:00 2001
From: Ben Timby <btimby@smartfile.com>
Date: Fri, 3 Mar 2017 11:39:29 -0500
Subject: [PATCH 3/7] Revert previous prototypes. Preserve behavior, with
 warnings.

---
 requests_oauthlib/oauth2_session.py | 26 ++++++++++++++++++++++++--
 tests/test_oauth2_session.py        | 10 +++++++++-
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/requests_oauthlib/oauth2_session.py b/requests_oauthlib/oauth2_session.py
index eaac60b1..b207d43e 100644
--- a/requests_oauthlib/oauth2_session.py
+++ b/requests_oauthlib/oauth2_session.py
@@ -283,6 +283,15 @@ def refresh_token(self, token_url, refresh_token=None, body='', auth=None,
                 refresh_token=refresh_token, scope=self.scope, **kwargs)
         log.debug('Prepared refresh token request body %s', body)
 
+        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 ''
+                log.warning('client_id provided, but client_secret missing')
+                auth = requests.auth.HTTPBasicAuth(client_id, client_secret)
+
         if headers is None:
             headers = {
                 'Accept': 'application/json',
@@ -314,6 +323,11 @@ def request(self, method, url, data=None, headers=None, withhold_token=False,
         """Intercept all requests and add the OAuth 2 token if present."""
         if not is_secure_transport(url):
             raise InsecureTransportError()
+
+        if client_id or client_secret:
+            log.warning(
+                'Specify token refresh authentication in auto_refresh_kwargs.')
+
         if self.token and not withhold_token:
             log.debug('Invoking %d protected resource request hooks.',
                       len(self.compliance_hook['protected_request']))
@@ -331,9 +345,17 @@ def request(self, method, url, data=None, headers=None, withhold_token=False,
                     log.debug('Auto refresh is set, attempting to refresh at %s.',
                               self.auto_refresh_url)
 
-                    # Call refresh_token(), pass any kwargs registered to it.
+                    refresh_kwargs = self.auto_refresh_kwargs.copy()
+
+                    auth = kwargs.pop('auth', None)
+                    if auth:
+                        log.warning('Specify token refresh authentication in '
+                                    'auto_refresh_kwargs.')
+                        refresh_kwargs['auth'] = auth
+
                     token = self.refresh_token(
-                        self.auto_refresh_url, **self.auth_refresh_kwargs
+                        self.auto_refresh_url, client_id=client_id,
+                        client_secret=client_secret, **refresh_kwargs
                     )
                     if self.token_updater:
                         log.debug('Updating token to %s using %s.',
diff --git a/tests/test_oauth2_session.py b/tests/test_oauth2_session.py
index a222df00..ce56149d 100644
--- a/tests/test_oauth2_session.py
+++ b/tests/test_oauth2_session.py
@@ -15,7 +15,7 @@
 from oauthlib.oauth2 import WebApplicationClient, MobileApplicationClient
 from oauthlib.oauth2 import LegacyApplicationClient, BackendApplicationClient
 from requests_oauthlib import OAuth2Session, TokenUpdated
-
+from requests.auth import HTTPBasicAuth
 
 fake_time = time.time()
 
@@ -138,6 +138,14 @@ def fake_refresh_with_auth(r, **kwargs):
             auth.send = fake_refresh_with_auth
             auth.get('https://i.b', client_id='foo', client_secret='bar')
 
+        # Make sure auth param is passed through
+        for client in self.clients:
+            auth = OAuth2Session(client=client, token=self.expired_token,
+                    auto_refresh_url='https://i.b/refresh',
+                    token_updater=token_updater)
+            auth.send = fake_refresh_with_auth
+            auth.get('https://i.b', auth=HTTPBasicAuth('foo', 'bar'))
+
     @mock.patch("time.time", new=lambda: fake_time)
     def test_token_from_fragment(self):
         mobile = MobileApplicationClient(self.client_id)

From 5e910162d2345bd8ab9bfba096d2bd0bae790c04 Mon Sep 17 00:00:00 2001
From: Ben Timby <btimby@smartfile.com>
Date: Fri, 3 Mar 2017 12:53:44 -0500
Subject: [PATCH 4/7] Print and log warnings.

---
 requests_oauthlib/oauth2_session.py | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/requests_oauthlib/oauth2_session.py b/requests_oauthlib/oauth2_session.py
index b207d43e..ed6780c6 100644
--- a/requests_oauthlib/oauth2_session.py
+++ b/requests_oauthlib/oauth2_session.py
@@ -1,15 +1,33 @@
 from __future__ import unicode_literals
 
 import logging
+import warnings
 
 from oauthlib.common import generate_token, urldecode
 from oauthlib.oauth2 import WebApplicationClient, InsecureTransportError
 from oauthlib.oauth2 import TokenExpiredError, is_secure_transport
+
 import requests
 
 log = logging.getLogger(__name__)
 
 
+# Preserve so we can call it.
+old_showwarning = warnings.showwarning
+
+def logwarning(message, category, filename, lineno, file=None):
+    """
+    Log warnings as well as print them.
+    """
+    log.warning(
+        '%s:%s: %s:%s' % 
+        (filename, lineno, category.__name__, message))
+    old_showwarning(message, category, filename, lineno, file=file)
+
+# Install our new handler.
+warnings.showwarning = logwarning
+
+
 class TokenUpdated(Warning):
     def __init__(self, token):
         super(TokenUpdated, self).__init__()
@@ -289,7 +307,7 @@ def refresh_token(self, token_url, refresh_token=None, body='', auth=None,
                 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 ''
-                log.warning('client_id provided, but client_secret missing')
+                warnings.warn('client_id provided, but client_secret missing')
                 auth = requests.auth.HTTPBasicAuth(client_id, client_secret)
 
         if headers is None:
@@ -325,7 +343,7 @@ def request(self, method, url, data=None, headers=None, withhold_token=False,
             raise InsecureTransportError()
 
         if client_id or client_secret:
-            log.warning(
+            warnings.warn(
                 'Specify token refresh authentication in auto_refresh_kwargs.')
 
         if self.token and not withhold_token:
@@ -349,7 +367,7 @@ def request(self, method, url, data=None, headers=None, withhold_token=False,
 
                     auth = kwargs.pop('auth', None)
                     if auth:
-                        log.warning('Specify token refresh authentication in '
+                        warnings.warn('Specify token refresh authentication in '
                                     'auto_refresh_kwargs.')
                         refresh_kwargs['auth'] = auth
 

From fd7eaa56f2e3aa6db882990189c9b56d3d2644f9 Mon Sep 17 00:00:00 2001
From: Ben Timby <btimby@smartfile.com>
Date: Fri, 3 Mar 2017 13:42:02 -0500
Subject: [PATCH 5/7] Final edits.

---
 requests_oauthlib/oauth2_session.py | 49 +++++++++++++++++++----------
 tests/test_oauth2_session.py        | 24 +++++++++++++-
 2 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/requests_oauthlib/oauth2_session.py b/requests_oauthlib/oauth2_session.py
index ed6780c6..884a6718 100644
--- a/requests_oauthlib/oauth2_session.py
+++ b/requests_oauthlib/oauth2_session.py
@@ -297,19 +297,11 @@ def refresh_token(self, token_url, refresh_token=None, body='', auth=None,
 
         refresh_token = refresh_token or self.token.get('refresh_token')
 
+        log.debug('Kwargs: %s', kwargs)
         body = self._client.prepare_refresh_body(body=body,
                 refresh_token=refresh_token, scope=self.scope, **kwargs)
         log.debug('Prepared refresh token request body %s', body)
 
-        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 ''
-                warnings.warn('client_id provided, but client_secret missing')
-                auth = requests.auth.HTTPBasicAuth(client_id, client_secret)
-
         if headers is None:
             headers = {
                 'Accept': 'application/json',
@@ -363,17 +355,42 @@ def request(self, method, url, data=None, headers=None, withhold_token=False,
                     log.debug('Auto refresh is set, attempting to refresh at %s.',
                               self.auto_refresh_url)
 
+                    # Someday this code can be eliminated, we should be able to
+                    # simply pass **self.auto_refresh_kwargs to
+                    # `refresh_token()` what follows is to maintain
+                    # compatibility
+
+                    # Start with kwargs explicitly requested for refresh.
                     refresh_kwargs = self.auto_refresh_kwargs.copy()
 
-                    auth = kwargs.pop('auth', None)
-                    if auth:
-                        warnings.warn('Specify token refresh authentication in '
-                                    'auto_refresh_kwargs.')
-                        refresh_kwargs['auth'] = auth
+                    if 'auth' in kwargs:
+                        # If user supplied `auth` to `request()` warn them to
+                        # instead use `auto_refresh_kwargs`. But honor their
+                        # intent for now.
+                        warnings.warn('auth argument supplied. Please specify '
+                                      'token refresh authentication in '
+                                      'auto_refresh_kwargs.')
+                        refresh_kwargs['auth'] = kwargs.pop('auth')
+                    elif client_id:
+                        # If no auth was provided, but `client_id` and
+                        # `client_secret` were, warn the user and create an
+                        # HTTP Basic auth header. It is preferred if the user
+                        # instead place an auth param into `auto_refresh_kwargs`
+                        warnings.warn('client_id argument supplied, Please '
+                                      'specify token refresh authentication in'
+                                      'auto_refresh_kwargs')
+                        log.debug('Encoding client_id "%s" with client_secret as Basic auth credentials.', client_id)
+                        if not client_secret:
+                            warnings.warn('client_id provided, but '
+                                          'client_secret missing')
+                            client_secret = ''
+                        refresh_kwargs['auth'] = \
+                            requests.auth.HTTPBasicAuth(client_id, client_secret)
+
+                    # End of compat. code.
 
                     token = self.refresh_token(
-                        self.auto_refresh_url, client_id=client_id,
-                        client_secret=client_secret, **refresh_kwargs
+                        self.auto_refresh_url, **refresh_kwargs
                     )
                     if self.token_updater:
                         log.debug('Updating token to %s using %s.',
diff --git a/tests/test_oauth2_session.py b/tests/test_oauth2_session.py
index ce56149d..ec850e14 100644
--- a/tests/test_oauth2_session.py
+++ b/tests/test_oauth2_session.py
@@ -131,6 +131,9 @@ def fake_refresh_with_auth(r, **kwargs):
             resp.text = json.dumps(self.token)
             return resp
 
+        # Make sure client_id and client_secret are converted to Basic Auth.
+        # This is a deprecated implicit auth method and should be removed in
+        # favor of auto_refresh_kwargs.
         for client in self.clients:
             auth = OAuth2Session(client=client, token=self.expired_token,
                     auto_refresh_url='https://i.b/refresh',
@@ -138,7 +141,8 @@ def fake_refresh_with_auth(r, **kwargs):
             auth.send = fake_refresh_with_auth
             auth.get('https://i.b', client_id='foo', client_secret='bar')
 
-        # Make sure auth param is passed through
+        # Make sure auth param is passed through, this is a deprecated implicit
+        # auth method and should be removed in favor of auto_refresh_kwargs.
         for client in self.clients:
             auth = OAuth2Session(client=client, token=self.expired_token,
                     auto_refresh_url='https://i.b/refresh',
@@ -146,6 +150,24 @@ def fake_refresh_with_auth(r, **kwargs):
             auth.send = fake_refresh_with_auth
             auth.get('https://i.b', auth=HTTPBasicAuth('foo', 'bar'))
 
+        def explicit_refresh_post(r, **kwargs):
+            if "/refresh" in r.url:
+                self.assertIn('foo-client-id', r.body)
+                self.assertIn('foo-client-secret', r.body)
+                self.assertNotIn("Authorization", r.headers)
+            resp = mock.MagicMock()
+            resp.text = json.dumps(self.token)
+            return resp
+
+        for client in self.clients:
+            auth = OAuth2Session(client=client, token=self.expired_token,
+                    auto_refresh_url='https://i.b/refresh',
+                    auto_refresh_kwargs={'client_id': 'foo-client-id',
+                    'client_secret': 'foo-client-secret'},
+                    token_updater=token_updater)
+            auth.send = explicit_refresh_post
+            auth.post('https://i.b')
+
     @mock.patch("time.time", new=lambda: fake_time)
     def test_token_from_fragment(self):
         mobile = MobileApplicationClient(self.client_id)

From eee296aae06f19e7c8363008366ce1537b7d233f Mon Sep 17 00:00:00 2001
From: Ben Timby <btimby@smartfile.com>
Date: Fri, 3 Mar 2017 13:46:21 -0500
Subject: [PATCH 6/7] Remove old duplicate warning.

---
 requests_oauthlib/oauth2_session.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/requests_oauthlib/oauth2_session.py b/requests_oauthlib/oauth2_session.py
index 884a6718..c5b8035b 100644
--- a/requests_oauthlib/oauth2_session.py
+++ b/requests_oauthlib/oauth2_session.py
@@ -334,10 +334,6 @@ def request(self, method, url, data=None, headers=None, withhold_token=False,
         if not is_secure_transport(url):
             raise InsecureTransportError()
 
-        if client_id or client_secret:
-            warnings.warn(
-                'Specify token refresh authentication in auto_refresh_kwargs.')
-
         if self.token and not withhold_token:
             log.debug('Invoking %d protected resource request hooks.',
                       len(self.compliance_hook['protected_request']))

From 6960d8da97fd1edafe9bf2e2f81e4ee9cbc2e3ea Mon Sep 17 00:00:00 2001
From: Ben Timby <btimby@smartfile.com>
Date: Fri, 3 Mar 2017 15:18:27 -0500
Subject: [PATCH 7/7] Address review.

---
 requests_oauthlib/oauth2_session.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/requests_oauthlib/oauth2_session.py b/requests_oauthlib/oauth2_session.py
index c5b8035b..eba305c5 100644
--- a/requests_oauthlib/oauth2_session.py
+++ b/requests_oauthlib/oauth2_session.py
@@ -297,7 +297,6 @@ def refresh_token(self, token_url, refresh_token=None, body='', auth=None,
 
         refresh_token = refresh_token or self.token.get('refresh_token')
 
-        log.debug('Kwargs: %s', kwargs)
         body = self._client.prepare_refresh_body(body=body,
                 refresh_token=refresh_token, scope=self.scope, **kwargs)
         log.debug('Prepared refresh token request body %s', body)
@@ -333,7 +332,6 @@ def request(self, method, url, data=None, headers=None, withhold_token=False,
         """Intercept all requests and add the OAuth 2 token if present."""
         if not is_secure_transport(url):
             raise InsecureTransportError()
-
         if self.token and not withhold_token:
             log.debug('Invoking %d protected resource request hooks.',
                       len(self.compliance_hook['protected_request']))
@@ -359,7 +357,7 @@ def request(self, method, url, data=None, headers=None, withhold_token=False,
                     # Start with kwargs explicitly requested for refresh.
                     refresh_kwargs = self.auto_refresh_kwargs.copy()
 
-                    if 'auth' in kwargs:
+                    if kwargs.get('auth', None):
                         # If user supplied `auth` to `request()` warn them to
                         # instead use `auto_refresh_kwargs`. But honor their
                         # intent for now.