diff --git a/oauth2client/client.py b/oauth2client/client.py index 5bdf911..0b5ef55 100644 --- a/oauth2client/client.py +++ b/oauth2client/client.py @@ -556,7 +556,7 @@ class OAuth2Credentials(Credentials): http: httplib2.Http, an http object to be used to make the refresh request. """ - self._refresh(http.request) + self._refresh(http) def revoke(self, http): """Revokes a refresh_token and makes the credentials void. @@ -565,7 +565,7 @@ class OAuth2Credentials(Credentials): http: httplib2.Http, an http object to be used to make the revoke request. """ - self._revoke(http.request) + self._revoke(http) def apply(self, headers): """Add the authorization to the headers. @@ -606,7 +606,7 @@ class OAuth2Credentials(Credentials): Returns: A set of strings containing the canonical list of scopes. """ - self._retrieve_scopes(http.request) + self._retrieve_scopes(http) return self.scopes @classmethod @@ -745,7 +745,7 @@ class OAuth2Credentials(Credentials): return headers - def _refresh(self, http_request): + def _refresh(self, http): """Refreshes the access_token. This method first checks by reading the Storage object if available. @@ -753,15 +753,13 @@ class OAuth2Credentials(Credentials): refresh is completed. Args: - http_request: callable, a callable that matches the method - signature of httplib2.Http.request, used to make the - refresh request. + http: an object to be used to make HTTP requests. Raises: HttpAccessTokenRefreshError: When the refresh fails. """ if not self.store: - self._do_refresh_request(http_request) + self._do_refresh_request(http) else: self.store.acquire_lock() try: @@ -773,17 +771,15 @@ class OAuth2Credentials(Credentials): logger.info('Updated access_token read from Storage') self._updateFromCredential(new_cred) else: - self._do_refresh_request(http_request) + self._do_refresh_request(http) finally: self.store.release_lock() - def _do_refresh_request(self, http_request): + def _do_refresh_request(self, http): """Refresh the access_token using the refresh_token. Args: - http_request: callable, a callable that matches the method - signature of httplib2.Http.request, used to make the - refresh request. + http: an object to be used to make HTTP requests. Raises: HttpAccessTokenRefreshError: When the refresh fails. @@ -793,7 +789,7 @@ class OAuth2Credentials(Credentials): logger.info('Refreshing access_token') resp, content = transport.request( - http_request, self.token_uri, method='POST', + http, self.token_uri, method='POST', body=body, headers=headers) content = _helpers._from_bytes(content) if resp.status == http_client.OK: @@ -833,23 +829,19 @@ class OAuth2Credentials(Credentials): pass raise HttpAccessTokenRefreshError(error_msg, status=resp.status) - def _revoke(self, http_request): + def _revoke(self, http): """Revokes this credential and deletes the stored copy (if it exists). Args: - http_request: callable, a callable that matches the method - signature of httplib2.Http.request, used to make the - revoke request. + http: an object to be used to make HTTP requests. """ - self._do_revoke(http_request, self.refresh_token or self.access_token) + self._do_revoke(http, self.refresh_token or self.access_token) - def _do_revoke(self, http_request, token): + def _do_revoke(self, http, token): """Revokes this credential and deletes the stored copy (if it exists). Args: - http_request: callable, a callable that matches the method - signature of httplib2.Http.request, used to make the - refresh request. + http: an object to be used to make HTTP requests. token: A string used as the token to be revoked. Can be either an access_token or refresh_token. @@ -860,7 +852,7 @@ class OAuth2Credentials(Credentials): logger.info('Revoking token') query_params = {'token': token} token_revoke_uri = _update_query_params(self.revoke_uri, query_params) - resp, content = transport.request(http_request, token_revoke_uri) + resp, content = transport.request(http, token_revoke_uri) if resp.status == http_client.OK: self.invalid = True else: @@ -876,23 +868,19 @@ class OAuth2Credentials(Credentials): if self.store: self.store.delete() - def _retrieve_scopes(self, http_request): + def _retrieve_scopes(self, http): """Retrieves the list of authorized scopes from the OAuth2 provider. Args: - http_request: callable, a callable that matches the method - signature of httplib2.Http.request, used to make the - revoke request. + http: an object to be used to make HTTP requests. """ - self._do_retrieve_scopes(http_request, self.access_token) + self._do_retrieve_scopes(http, self.access_token) - def _do_retrieve_scopes(self, http_request, token): + def _do_retrieve_scopes(self, http, token): """Retrieves the list of authorized scopes from the OAuth2 provider. Args: - http_request: callable, a callable that matches the method - signature of httplib2.Http.request, used to make the - refresh request. + http: an object to be used to make HTTP requests. token: A string used as the token to identify the credentials to the provider. @@ -904,7 +892,7 @@ class OAuth2Credentials(Credentials): query_params = {'access_token': token, 'fields': 'scope'} token_info_uri = _update_query_params(self.token_info_uri, query_params) - resp, content = transport.request(http_request, token_info_uri) + resp, content = transport.request(http, token_info_uri) content = _helpers._from_bytes(content) if resp.status == http_client.OK: d = json.loads(content) @@ -977,19 +965,25 @@ class AccessTokenCredentials(OAuth2Credentials): data['user_agent']) return retval - def _refresh(self, http_request): + def _refresh(self, http): + """Refreshes the access token. + + Args: + http: unused HTTP object. + + Raises: + AccessTokenCredentialsError: always + """ raise AccessTokenCredentialsError( 'The access_token is expired or invalid and can\'t be refreshed.') - def _revoke(self, http_request): + def _revoke(self, http): """Revokes the access_token and deletes the store if available. Args: - http_request: callable, a callable that matches the method - signature of httplib2.Http.request, used to make the - revoke request. + http: an object to be used to make HTTP requests. """ - self._do_revoke(http_request, self.access_token) + self._do_revoke(http, self.access_token) def _detect_gce_environment(): @@ -1511,15 +1505,13 @@ class AssertionCredentials(GoogleCredentials): """Generate assertion string to be used in the access token request.""" raise NotImplementedError - def _revoke(self, http_request): + def _revoke(self, http): """Revokes the access_token and deletes the store if available. Args: - http_request: callable, a callable that matches the method - signature of httplib2.Http.request, used to make the - revoke request. + http: an object to be used to make HTTP requests. """ - self._do_revoke(http_request, self.access_token) + self._do_revoke(http, self.access_token) def sign_blob(self, blob): """Cryptographically sign a blob (of bytes). diff --git a/oauth2client/contrib/_metadata.py b/oauth2client/contrib/_metadata.py index 4c4588a..1a18bd4 100644 --- a/oauth2client/contrib/_metadata.py +++ b/oauth2client/contrib/_metadata.py @@ -32,15 +32,13 @@ METADATA_ROOT = 'http://metadata.google.internal/computeMetadata/v1/' METADATA_HEADERS = {'Metadata-Flavor': 'Google'} -def get(http_request, path, root=METADATA_ROOT, recursive=None): +def get(http, path, root=METADATA_ROOT, recursive=None): """Fetch a resource from the metadata server. Args: + http: an object to be used to make HTTP requests. path: A string indicating the resource to retrieve. For example, 'instance/service-accounts/defualt' - http_request: A callable that matches the method - signature of httplib2.Http.request. Used to make the request to the - metadataserver. root: A string indicating the full path to the metadata server root. recursive: A boolean indicating whether to do a recursive query of metadata. See @@ -57,7 +55,7 @@ def get(http_request, path, root=METADATA_ROOT, recursive=None): url = _helpers._add_query_parameter(url, 'recursive', recursive) response, content = transport.request( - http_request, url, headers=METADATA_HEADERS) + http, url, headers=METADATA_HEADERS) if response.status == http_client.OK: decoded = _helpers._from_bytes(content) @@ -71,16 +69,15 @@ def get(http_request, path, root=METADATA_ROOT, recursive=None): 'metadata service. Response:\n{1}'.format(url, response)) -def get_service_account_info(http_request, service_account='default'): +def get_service_account_info(http, service_account='default'): """Get information about a service account from the metadata server. Args: + http: an object to be used to make HTTP requests. service_account: An email specifying the service account for which to look up information. Default will be information for the "default" service account of the current compute engine instance. - http_request: A callable that matches the method - signature of httplib2.Http.request. Used to make the request to the - metadata server. + Returns: A dictionary with information about the specified service account, for example: @@ -92,21 +89,19 @@ def get_service_account_info(http_request, service_account='default'): } """ return get( - http_request, + http, 'instance/service-accounts/{0}/'.format(service_account), recursive=True) -def get_token(http_request, service_account='default'): +def get_token(http, service_account='default'): """Fetch an oauth token for the Args: + http: an object to be used to make HTTP requests. service_account: An email specifying the service account this token should represent. Default will be a token for the "default" service account of the current compute engine instance. - http_request: A callable that matches the method - signature of httplib2.Http.request. Used to make the request to the - metadataserver. Returns: A tuple of (access token, token expiration), where access token is the @@ -114,7 +109,7 @@ def get_token(http_request, service_account='default'): that indicates when the access token will expire. """ token_json = get( - http_request, + http, 'instance/service-accounts/{0}/token'.format(service_account)) token_expiry = client._UTCNOW() + datetime.timedelta( seconds=token_json['expires_in']) diff --git a/oauth2client/contrib/appengine.py b/oauth2client/contrib/appengine.py index 514fd58..a3df769 100644 --- a/oauth2client/contrib/appengine.py +++ b/oauth2client/contrib/appengine.py @@ -157,17 +157,15 @@ class AppAssertionCredentials(client.AssertionCredentials): data = json.loads(json_data) return AppAssertionCredentials(data['scope']) - def _refresh(self, http_request): - """Refreshes the access_token. + def _refresh(self, http): + """Refreshes the access token. Since the underlying App Engine app_identity implementation does its own caching we can skip all the storage hoops and just to a refresh using the API. Args: - http_request: callable, a callable that matches the method - signature of httplib2.Http.request, used to make the - refresh request. + http: unused HTTP object Raises: AccessTokenRefreshError: When the refresh fails. diff --git a/oauth2client/contrib/devshell.py b/oauth2client/contrib/devshell.py index b8bb978..c1906eb 100644 --- a/oauth2client/contrib/devshell.py +++ b/oauth2client/contrib/devshell.py @@ -117,7 +117,12 @@ class DevshellCredentials(client.GoogleCredentials): user_agent) self._refresh(None) - def _refresh(self, http_request): + def _refresh(self, http): + """Refreshes the access token. + + Args: + http: unused HTTP object + """ self.devshell_response = _SendRecv() self.access_token = self.devshell_response.access_token expires_in = self.devshell_response.expires_in diff --git a/oauth2client/contrib/gce.py b/oauth2client/contrib/gce.py index 9e1d9d0..d1510db 100644 --- a/oauth2client/contrib/gce.py +++ b/oauth2client/contrib/gce.py @@ -98,42 +98,38 @@ class AppAssertionCredentials(client.AssertionCredentials): Returns: A set of strings containing the canonical list of scopes. """ - self._retrieve_info(http.request) + self._retrieve_info(http) return self.scopes - def _retrieve_info(self, http_request): - """Validates invalid service accounts by retrieving service account info. + def _retrieve_info(self, http): + """Retrieves service account info for invalid credentials. Args: - http_request: callable, a callable that matches the method - signature of httplib2.Http.request, used to make the - request to the metadata server + http: an object to be used to make HTTP requests. """ if self.invalid: info = _metadata.get_service_account_info( - http_request, + http, service_account=self.service_account_email or 'default') self.invalid = False self.service_account_email = info['email'] self.scopes = info['scopes'] - def _refresh(self, http_request): - """Refreshes the access_token. + def _refresh(self, http): + """Refreshes the access token. Skip all the storage hoops and just refresh using the API. Args: - http_request: callable, a callable that matches the method - signature of httplib2.Http.request, used to make - the refresh request. + http: an object to be used to make HTTP requests. Raises: HttpAccessTokenRefreshError: When the refresh fails. """ try: - self._retrieve_info(http_request) + self._retrieve_info(http) self.access_token, self.token_expiry = _metadata.get_token( - http_request, service_account=self.service_account_email) + http, service_account=self.service_account_email) except http_client.HTTPException as err: raise client.HttpAccessTokenRefreshError(str(err)) diff --git a/oauth2client/service_account.py b/oauth2client/service_account.py index 2414751..540bfaa 100644 --- a/oauth2client/service_account.py +++ b/oauth2client/service_account.py @@ -649,9 +649,22 @@ class _JWTAccessCredentials(ServiceAccountCredentials): return result def refresh(self, http): + """Refreshes the access_token. + + The HTTP object is unused since no request needs to be made to + get a new token, it can just be generated locally. + + Args: + http: unused HTTP object + """ self._refresh(None) - def _refresh(self, http_request): + def _refresh(self, http): + """Refreshes the access_token. + + Args: + http: unused HTTP object + """ self.access_token, self.token_expiry = self._create_token() def _create_token(self, additional_claims=None): diff --git a/oauth2client/transport.py b/oauth2client/transport.py index ed256da..79a61f1 100644 --- a/oauth2client/transport.py +++ b/oauth2client/transport.py @@ -275,6 +275,7 @@ def request(http, uri, method='GET', body=None, headers=None, tuple, a pair of a httplib2.Response with the status code and other headers and the bytes of the content returned. """ + # NOTE: Allowing http or http.request is temporary (See Issue 601). http_callable = getattr(http, 'request', http) return http_callable(uri, method=method, body=body, headers=headers, redirections=redirections, diff --git a/tests/contrib/test_gce.py b/tests/contrib/test_gce.py index d71a391..dbd7ff3 100644 --- a/tests/contrib/test_gce.py +++ b/tests/contrib/test_gce.py @@ -67,8 +67,7 @@ class AppAssertionCredentialsTests(unittest2.TestCase): @mock.patch('oauth2client.contrib._metadata.get_service_account_info', return_value=SERVICE_ACCOUNT_INFO) def test_refresh_token(self, get_info, get_token): - http_request = mock.MagicMock() - http_mock = mock.MagicMock(request=http_request) + http_mock = object() credentials = gce.AppAssertionCredentials() credentials.invalid = False credentials.service_account_email = 'a@example.com' @@ -76,12 +75,12 @@ class AppAssertionCredentialsTests(unittest2.TestCase): credentials.get_access_token(http=http_mock) self.assertEqual(credentials.access_token, 'A') self.assertTrue(credentials.access_token_expired) - get_token.assert_called_with(http_request, + get_token.assert_called_with(http_mock, service_account='a@example.com') credentials.get_access_token(http=http_mock) self.assertEqual(credentials.access_token, 'B') self.assertFalse(credentials.access_token_expired) - get_token.assert_called_with(http_request, + get_token.assert_called_with(http_mock, service_account='a@example.com') get_info.assert_not_called() @@ -114,8 +113,7 @@ class AppAssertionCredentialsTests(unittest2.TestCase): @mock.patch('oauth2client.contrib._metadata.get_service_account_info', return_value=SERVICE_ACCOUNT_INFO) def test_retrieve_scopes(self, metadata): - http_request = mock.MagicMock() - http_mock = mock.MagicMock(request=http_request) + http_mock = object() credentials = gce.AppAssertionCredentials() self.assertTrue(credentials.invalid) self.assertIsNone(credentials.scopes) @@ -124,19 +122,18 @@ class AppAssertionCredentialsTests(unittest2.TestCase): self.assertFalse(credentials.invalid) credentials.retrieve_scopes(http_mock) # Assert scopes weren't refetched - metadata.assert_called_once_with(http_request, + metadata.assert_called_once_with(http_mock, service_account='default') @mock.patch('oauth2client.contrib._metadata.get_service_account_info', side_effect=http_client.HTTPException('No Such Email')) def test_retrieve_scopes_bad_email(self, metadata): - http_request = mock.MagicMock() - http_mock = mock.MagicMock(request=http_request) + http_mock = object() credentials = gce.AppAssertionCredentials(email='b@example.com') with self.assertRaises(http_client.HTTPException): credentials.retrieve_scopes(http_mock) - metadata.assert_called_once_with(http_request, + metadata.assert_called_once_with(http_mock, service_account='b@example.com') def test_save_to_well_known_file(self): diff --git a/tests/contrib/test_metadata.py b/tests/contrib/test_metadata.py index 6907f39..3929fd0 100644 --- a/tests/contrib/test_metadata.py +++ b/tests/contrib/test_metadata.py @@ -79,16 +79,16 @@ class TestMetadata(unittest2.TestCase): 'oauth2client.client._UTCNOW', return_value=datetime.datetime.min) def test_get_token_success(self, now): - http_request = request_mock( + http = request_mock( http_client.OK, 'application/json', json.dumps({'access_token': 'a', 'expires_in': 100}) ) - token, expiry = _metadata.get_token(http_request=http_request) + token, expiry = _metadata.get_token(http=http) self.assertEqual(token, 'a') self.assertEqual( expiry, datetime.datetime.min + datetime.timedelta(seconds=100)) - http_request.assert_called_once_with( + http.assert_called_once_with( EXPECTED_URL + '/token', **EXPECTED_KWARGS )