From ab086899a00fe56303be39be5dec7ccfe0cd14b9 Mon Sep 17 00:00:00 2001 From: Jon Wayne Parrott Date: Thu, 23 Feb 2017 09:20:14 -0800 Subject: [PATCH] Remove one-time token behavior of JWT Credentials (#117) --- google/auth/jwt.py | 95 ++++++---------------------- google/oauth2/service_account.py | 9 ++- system_tests/test_grpc.py | 5 +- tests/oauth2/test_service_account.py | 6 +- tests/test_jwt.py | 51 +++++++-------- 5 files changed, 58 insertions(+), 108 deletions(-) diff --git a/google/auth/jwt.py b/google/auth/jwt.py index 418be6ba8..087dbd9e9 100644 --- a/google/auth/jwt.py +++ b/google/auth/jwt.py @@ -45,8 +45,6 @@ import datetime import json -from six.moves import urllib - from google.auth import _helpers from google.auth import _service_account_info from google.auth import credentials @@ -246,11 +244,7 @@ class Credentials(credentials.Signing, """Credentials that use a JWT as the bearer token. These credentials require an "audience" claim. This claim identifies the - intended recipient of the bearer token. You can set the audience when - you construct these credentials, however, these credentials can also set - the audience claim automatically if not specified. In this case, whenever - a request is made the credentials will automatically generate a one-time - JWT with the request URI as the audience. + intended recipient of the bearer token. The constructor arguments determine the claims for the JWT that is sent with requests. Usually, you'll construct these credentials with @@ -260,13 +254,15 @@ class Credentials(credentials.Signing, JSON file:: credentials = jwt.Credentials.from_service_account_file( - 'service-account.json') + 'service-account.json', + audience='https://speech.googleapis.com') If you already have the service account file loaded and parsed:: service_account_info = json.load(open('service_account.json')) credentials = jwt.Credentials.from_service_account_info( - service_account_info) + service_account_info, + audience='https://speech.googleapis.com') Both helper methods pass on arguments to the constructor, so you can specify the JWT claims:: @@ -280,7 +276,10 @@ class Credentials(credentials.Signing, :class:`~google.auth.crypt.Signer` instance:: credentials = jwt.Credentials( - signer, issuer='your-issuer', subject='your-subject') + signer, + issuer='your-issuer', + subject='your-subject', + audience=''https://speech.googleapis.com'') The claims are considered immutable. If you want to modify the claims, you can easily create another instance using :meth:`with_claims`:: @@ -289,7 +288,7 @@ class Credentials(credentials.Signing, audience='https://vision.googleapis.com') """ - def __init__(self, signer, issuer=None, subject=None, audience=None, + def __init__(self, signer, issuer, subject, audience, additional_claims=None, token_lifetime=_DEFAULT_TOKEN_LIFETIME_SECS): """ @@ -298,8 +297,7 @@ def __init__(self, signer, issuer=None, subject=None, audience=None, issuer (str): The `iss` claim. subject (str): The `sub` claim. audience (str): the `aud` claim. The intended audience for the - credentials. If not specified, a new JWT will be generated for - every request and will use the request URI as the audience. + credentials. additional_claims (Mapping[str, str]): Any additional claims for the JWT payload. token_lifetime (int): The amount of time in seconds for @@ -334,7 +332,8 @@ def _from_signer_and_info(cls, signer, info, **kwargs): ValueError: If the info is not in the expected format. """ kwargs.setdefault('subject', info['client_email']) - return cls(signer, issuer=info['client_email'], **kwargs) + kwargs.setdefault('issuer', info['client_email']) + return cls(signer, **kwargs) @classmethod def from_service_account_info(cls, info, **kwargs): @@ -381,9 +380,8 @@ def with_claims(self, issuer=None, subject=None, audience=None, claim will be used. subject (str): The `sub` claim. If unspecified the current subject claim will be used. - audience (str): the `aud` claim. If not specified, a new - JWT will be generated for every request and will use - the request URI as the audience. + audience (str): the `aud` claim. If unspecified the current + audience claim will be used. additional_claims (Mapping[str, str]): Any additional claims for the JWT payload. This will be merged with the current additional claims. @@ -399,12 +397,9 @@ def with_claims(self, issuer=None, subject=None, audience=None, additional_claims=self._additional_claims.copy().update( additional_claims or {})) - def _make_jwt(self, audience=None): + def _make_jwt(self): """Make a signed JWT. - Args: - audience (str): Overrides the instance's current audience claim. - Returns: Tuple[bytes, datetime]: The encoded JWT and the expiration. """ @@ -414,10 +409,10 @@ def _make_jwt(self, audience=None): payload = { 'iss': self._issuer, - 'sub': self._subject or self._issuer, + 'sub': self._subject, 'iat': _helpers.datetime_to_secs(now), 'exp': _helpers.datetime_to_secs(expiry), - 'aud': audience or self._audience, + 'aud': self._audience, } payload.update(self._additional_claims) @@ -426,22 +421,6 @@ def _make_jwt(self, audience=None): return jwt, expiry - def _make_one_time_jwt(self, uri): - """Makes a one-off JWT with the URI as the audience. - - Args: - uri (str): The request URI. - - Returns: - bytes: The encoded JWT. - """ - parts = urllib.parse.urlsplit(uri) - # Strip query string and fragment - audience = urllib.parse.urlunsplit( - (parts.scheme, parts.netloc, parts.path, None, None)) - token, _ = self._make_jwt(audience=audience) - return token - def refresh(self, request): """Refreshes the access token. @@ -452,15 +431,8 @@ def refresh(self, request): # (pylint doesn't correctly recognize overridden methods.) self.token, self.expiry = self._make_jwt() + @_helpers.copy_docstring(credentials.Signing) def sign_bytes(self, message): - """Signs the given message. - - Args: - message (bytes): The message to sign. - - Returns: - bytes: The message signature. - """ return self._signer.sign(message) @property @@ -472,32 +444,3 @@ def signer_email(self): @_helpers.copy_docstring(credentials.Signing) def signer(self): return self._signer - - def before_request(self, request, method, url, headers): - """Performs credential-specific before request logic. - - If an audience is specified it will refresh the credentials if - necessary. If no audience is specified it will generate a one-time - token for the request URI. In either case, it will set the - authorization header in headers to the token. - - Args: - request (Any): Unused. - method (str): The request's HTTP method. - url (str): The request's URI. - headers (Mapping): The request's headers. - """ - # pylint: disable=unused-argument - # (pylint doesn't correctly recognize overridden methods.) - - # If this set of credentials has a pre-set audience, just ensure that - # there is a valid token and apply the auth headers. - if self._audience: - if not self.valid: - self.refresh(request) - self.apply(headers) - # Otherwise, generate a one-time token using the URL - # (without the query string and fragment) as the audience. - else: - token = self._make_one_time_jwt(url) - self.apply(headers, token=token) diff --git a/google/oauth2/service_account.py b/google/oauth2/service_account.py index b71b8acc8..a4fc1b6c1 100644 --- a/google/oauth2/service_account.py +++ b/google/oauth2/service_account.py @@ -204,7 +204,7 @@ def from_service_account_file(cls, filename, **kwargs): filename, require=['client_email', 'token_uri']) return cls._from_signer_and_info(signer, info, **kwargs) - def to_jwt_credentials(self): + def to_jwt_credentials(self, audience): """Creates a :class:`google.auth.jwt.Credentials` instance from this instance. @@ -223,13 +223,18 @@ def to_jwt_credentials(self): jwt_creds = jwt.Credentials.from_service_account_file( 'service_account.json') + Args: + audience (str): the `aud` claim. The intended audience for the + credentials. + Returns: google.auth.jwt.Credentials: A new Credentials instance. """ return jwt.Credentials( self._signer, issuer=self._service_account_email, - subject=self._service_account_email) + subject=self._service_account_email, + audience=audience) @property def service_account_email(self): diff --git a/system_tests/test_grpc.py b/system_tests/test_grpc.py index 73467fe7a..8a4419a90 100644 --- a/system_tests/test_grpc.py +++ b/system_tests/test_grpc.py @@ -40,7 +40,10 @@ def test_grpc_request_with_regular_credentials(http_request): def test_grpc_request_with_jwt_credentials(http_request): credentials, project_id = google.auth.default() - credentials = credentials.to_jwt_credentials() + audience = 'https://{}/google.pubsub.v1.Publisher'.format( + publisher_client.PublisherClient.SERVICE_ADDRESS) + credentials = credentials.to_jwt_credentials( + audience=audience) channel = google.auth.transport.grpc.secure_authorized_channel( credentials, diff --git a/tests/oauth2/test_service_account.py b/tests/oauth2/test_service_account.py index 8ae545a1f..1bce67029 100644 --- a/tests/oauth2/test_service_account.py +++ b/tests/oauth2/test_service_account.py @@ -112,9 +112,11 @@ def test_from_service_account_file_args(self): assert credentials._additional_claims == additional_claims def test_to_jwt_credentials(self): - jwt_from_svc = self.credentials.to_jwt_credentials() + jwt_from_svc = self.credentials.to_jwt_credentials( + audience=mock.sentinel.audience) jwt_from_info = jwt.Credentials.from_service_account_info( - SERVICE_ACCOUNT_INFO) + SERVICE_ACCOUNT_INFO, + audience=mock.sentinel.audience) assert isinstance(jwt_from_svc, jwt.Credentials) assert jwt_from_svc._signer.key_id == jwt_from_info._signer.key_id diff --git a/tests/test_jwt.py b/tests/test_jwt.py index df09ece61..0716fc369 100644 --- a/tests/test_jwt.py +++ b/tests/test_jwt.py @@ -206,17 +206,20 @@ class TestCredentials: @pytest.fixture(autouse=True) def credentials_fixture(self, signer): self.credentials = jwt.Credentials( - signer, self.SERVICE_ACCOUNT_EMAIL) + signer, self.SERVICE_ACCOUNT_EMAIL, self.SERVICE_ACCOUNT_EMAIL, + self.AUDIENCE) def test_from_service_account_info(self): with open(SERVICE_ACCOUNT_JSON_FILE, 'r') as fh: info = json.load(fh) - credentials = jwt.Credentials.from_service_account_info(info) + credentials = jwt.Credentials.from_service_account_info( + info, audience=self.AUDIENCE) assert credentials._signer.key_id == info['private_key_id'] assert credentials._issuer == info['client_email'] assert credentials._subject == info['client_email'] + assert credentials._audience == self.AUDIENCE def test_from_service_account_info_args(self): info = SERVICE_ACCOUNT_INFO.copy() @@ -235,11 +238,12 @@ def test_from_service_account_file(self): info = SERVICE_ACCOUNT_INFO.copy() credentials = jwt.Credentials.from_service_account_file( - SERVICE_ACCOUNT_JSON_FILE) + SERVICE_ACCOUNT_JSON_FILE, audience=self.AUDIENCE) assert credentials._signer.key_id == info['private_key_id'] assert credentials._issuer == info['client_email'] assert credentials._subject == info['client_email'] + assert credentials._audience == self.AUDIENCE def test_from_service_account_file_args(self): info = SERVICE_ACCOUNT_INFO.copy() @@ -259,6 +263,18 @@ def test_default_state(self): # Expiration hasn't been set yet assert not self.credentials.expired + def test_with_claims(self): + new_audience = 'new_audience' + new_credentials = self.credentials.with_claims( + audience=new_audience) + + assert new_credentials._signer == self.credentials._signer + assert new_credentials._issuer == self.credentials._issuer + assert new_credentials._subject == self.credentials._subject + assert new_credentials._audience == new_audience + assert (new_credentials._additional_claims == + self.credentials._additional_claims) + def test_sign_bytes(self): to_sign = b'123' signature = self.credentials.sign_bytes(to_sign) @@ -292,43 +308,24 @@ def test_expired(self): now.return_value = self.credentials.expiry + one_day assert self.credentials.expired - def test_before_request_one_time_token(self): + def test_before_request(self): headers = {} self.credentials.refresh(None) self.credentials.before_request( - mock.Mock(), 'GET', 'http://example.com?a=1#3', headers) - - header_value = headers['authorization'] - _, token = header_value.split(' ') - - # This should be a one-off token, so it shouldn't be the same as the - # credentials' stored token. - assert token != self.credentials.token - - payload = self._verify_token(token) - assert payload['aud'] == 'http://example.com' - - def test_before_request_with_preset_audience(self): - headers = {} - - credentials = self.credentials.with_claims(audience=self.AUDIENCE) - credentials.refresh(None) - credentials.before_request( None, 'GET', 'http://example.com?a=1#3', headers) header_value = headers['authorization'] _, token = header_value.split(' ') # Since the audience is set, it should use the existing token. - assert token.encode('utf-8') == credentials.token + assert token.encode('utf-8') == self.credentials.token payload = self._verify_token(token) assert payload['aud'] == self.AUDIENCE def test_before_request_refreshes(self): - credentials = self.credentials.with_claims(audience=self.AUDIENCE) - assert not credentials.valid - credentials.before_request( + assert not self.credentials.valid + self.credentials.before_request( None, 'GET', 'http://example.com?a=1#3', {}) - assert credentials.valid + assert self.credentials.valid