Fix credentials usage in BatchHTTPRequest (#376)
diff --git a/googleapiclient/_auth.py b/googleapiclient/_auth.py
index 87d3709..05a1d38 100644
--- a/googleapiclient/_auth.py
+++ b/googleapiclient/_auth.py
@@ -14,6 +14,8 @@
"""Helpers for authentication using oauth2client or google-auth."""
+import httplib2
+
try:
import google.auth
import google.auth.credentials
@@ -29,8 +31,6 @@
except ImportError: # pragma: NO COVER
HAS_OAUTH2CLIENT = False
-from googleapiclient.http import build_http
-
def default_credentials():
"""Returns Application Default Credentials."""
@@ -84,9 +84,49 @@
Union[httplib2.Http, google_auth_httplib2.AuthorizedHttp]: An
authorized http client.
"""
+ from googleapiclient.http import build_http
+
if HAS_GOOGLE_AUTH and isinstance(
credentials, google.auth.credentials.Credentials):
return google_auth_httplib2.AuthorizedHttp(credentials,
http=build_http())
else:
return credentials.authorize(build_http())
+
+
+def refresh_credentials(credentials):
+ # Refresh must use a new http instance, as the one associated with the
+ # credentials could be a AuthorizedHttp or an oauth2client-decorated
+ # Http instance which would cause a weird recursive loop of refreshing
+ # and likely tear a hole in spacetime.
+ refresh_http = httplib2.Http()
+ if HAS_GOOGLE_AUTH and isinstance(
+ credentials, google.auth.credentials.Credentials):
+ request = google_auth_httplib2.Request(refresh_http)
+ return credentials.refresh(request)
+ else:
+ return credentials.refresh(refresh_http)
+
+
+def apply_credentials(credentials, headers):
+ # oauth2client and google-auth have the same interface for this.
+ return credentials.apply(headers)
+
+
+def is_valid(credentials):
+ if HAS_GOOGLE_AUTH and isinstance(
+ credentials, google.auth.credentials.Credentials):
+ return credentials.valid
+ else:
+ return not credentials.access_token_expired
+
+
+def get_credentials_from_http(http):
+ if http is None:
+ return None
+ elif hasattr(http.request, 'credentials'):
+ return http.request.credentials
+ elif hasattr(http, 'credentials'):
+ return http.credentials
+ else:
+ return None
diff --git a/googleapiclient/http.py b/googleapiclient/http.py
index 302e0e7..f5d08a1 100644
--- a/googleapiclient/http.py
+++ b/googleapiclient/http.py
@@ -62,6 +62,7 @@
except ImportError:
from oauth2client import _helpers as util
+from googleapiclient import _auth
from googleapiclient import mimeparse
from googleapiclient.errors import BatchError
from googleapiclient.errors import HttpError
@@ -1126,21 +1127,25 @@
# If there is no http per the request then refresh the http passed in
# via execute()
creds = None
- if request.http is not None and hasattr(request.http.request,
- 'credentials'):
- creds = request.http.request.credentials
- elif http is not None and hasattr(http.request, 'credentials'):
- creds = http.request.credentials
+ request_credentials = False
+
+ if request.http is not None:
+ creds = _auth.get_credentials_from_http(request.http)
+ request_credentials = True
+
+ if creds is None and http is not None:
+ creds = _auth.get_credentials_from_http(http)
+
if creds is not None:
if id(creds) not in self._refreshed_credentials:
- creds.refresh(http)
+ _auth.refresh_credentials(creds)
self._refreshed_credentials[id(creds)] = 1
# Only apply the credentials if we are using the http object passed in,
# otherwise apply() will get called during _serialize_request().
- if request.http is None or not hasattr(request.http.request,
- 'credentials'):
- creds.apply(request.headers)
+ if request.http is None or not request_credentials:
+ _auth.apply_credentials(creds, request.headers)
+
def _id_to_header(self, id_):
"""Convert an id to a Content-ID header value.
@@ -1200,9 +1205,10 @@
msg = MIMENonMultipart(major, minor)
headers = request.headers.copy()
- if request.http is not None and hasattr(request.http.request,
- 'credentials'):
- request.http.request.credentials.apply(headers)
+ if request.http is not None:
+ credentials = _auth.get_credentials_from_http(request.http)
+ if credentials is not None:
+ _auth.apply_credentials(credentials, headers)
# MIMENonMultipart adds its own Content-Type header.
if 'content-type' in headers:
@@ -1409,11 +1415,11 @@
# Special case for OAuth2Credentials-style objects which have not yet been
# refreshed with an initial access_token.
- if getattr(http.request, 'credentials', None) is not None:
- creds = http.request.credentials
- if not getattr(creds, 'access_token', None):
+ creds = _auth.get_credentials_from_http(http)
+ if creds is not None:
+ if not _auth.is_valid(creds):
LOGGER.info('Attempting refresh to obtain initial access_token')
- creds.refresh(http)
+ _auth.refresh_credentials(creds)
self._execute(http, self._order, self._requests)
diff --git a/tests/test_http.py b/tests/test_http.py
index fe74672..0df00ab 100644
--- a/tests/test_http.py
+++ b/tests/test_http.py
@@ -62,12 +62,17 @@
class MockCredentials(Credentials):
"""Mock class for all Credentials objects."""
- def __init__(self, bearer_token):
+ def __init__(self, bearer_token, expired=False):
super(MockCredentials, self).__init__()
self._authorized = 0
self._refreshed = 0
self._applied = 0
self._bearer_token = bearer_token
+ self._access_token_expired = expired
+
+ @property
+ def access_token_expired(self):
+ return self._access_token_expired
def authorize(self, http):
self._authorized += 1
@@ -1128,10 +1133,7 @@
def test_execute_initial_refresh_oauth2(self):
batch = BatchHttpRequest()
callbacks = Callbacks()
- cred = MockCredentials('Foo')
-
- # Pretend this is a OAuth2Credentials object
- cred.access_token = None
+ cred = MockCredentials('Foo', expired=True)
http = HttpMockSequence([
({'status': '200',
diff --git a/tox.ini b/tox.ini
index 2664d2e..7a3bad2 100644
--- a/tox.ini
+++ b/tox.ini
@@ -19,4 +19,4 @@
coverage>=3.6,<3.99
unittest2
mock
-commands = nosetests --with-coverage --cover-package=googleapiclient --nocapture --cover-erase --cover-tests --cover-branches --cover-min-percentage=85
+commands = nosetests --with-coverage --cover-package=googleapiclient --nocapture --cover-erase --cover-tests --cover-branches --cover-min-percentage=85 []