From 09db2385451c9bb46de991da5a37acb11686b290 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Fri, 12 Jun 2020 01:53:38 +0000 Subject: [PATCH 1/7] feat: make load_credentials_from_file public and alllow scopes --- google/auth/__init__.py | 4 ++-- google/auth/_default.py | 16 ++++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/google/auth/__init__.py b/google/auth/__init__.py index 6b4b78b29..5ca20a362 100644 --- a/google/auth/__init__.py +++ b/google/auth/__init__.py @@ -16,10 +16,10 @@ import logging -from google.auth._default import default +from google.auth._default import default, load_credentials_from_file -__all__ = ["default"] +__all__ = ["default", "load_credentials_from_file"] # Set default logging handler to avoid "No handler found" warnings. diff --git a/google/auth/_default.py b/google/auth/_default.py index d7110a10d..f09abf78b 100644 --- a/google/auth/_default.py +++ b/google/auth/_default.py @@ -69,8 +69,8 @@ def _warn_about_problematic_credentials(credentials): warnings.warn(_CLOUD_SDK_CREDENTIALS_WARNING) -def _load_credentials_from_file(filename): - """Loads credentials from a file. +def load_credentials_from_file(filename, scopes=None): + """Loads Google credentials from a file. The credentials file must be a service account key or stored authorized user credentials. @@ -78,6 +78,10 @@ def _load_credentials_from_file(filename): Args: filename (str): The full path to the credentials file. + scopes (Sequence[str]): The list of scopes for the credentials. If + specified, the credentials will automatically be scoped if + necessary. + Returns: Tuple[google.auth.credentials.Credentials, Optional[str]]: Loaded credentials and the project ID. Authorized user credentials do not @@ -109,7 +113,7 @@ def _load_credentials_from_file(filename): from google.oauth2 import credentials try: - credentials = credentials.Credentials.from_authorized_user_info(info) + credentials = credentials.Credentials.from_authorized_user_info(info, scopes=scopes) except ValueError as caught_exc: msg = "Failed to load authorized user credentials from {}".format(filename) new_exc = exceptions.DefaultCredentialsError(msg, caught_exc) @@ -122,7 +126,7 @@ def _load_credentials_from_file(filename): from google.oauth2 import service_account try: - credentials = service_account.Credentials.from_service_account_info(info) + credentials = service_account.Credentials.from_service_account_info(info, scopes=scopes) except ValueError as caught_exc: msg = "Failed to load service account credentials from {}".format(filename) new_exc = exceptions.DefaultCredentialsError(msg, caught_exc) @@ -148,7 +152,7 @@ def _get_gcloud_sdk_credentials(): if not os.path.isfile(credentials_filename): return None, None - credentials, project_id = _load_credentials_from_file(credentials_filename) + credentials, project_id = load_credentials_from_file(credentials_filename) if not project_id: project_id = _cloud_sdk.get_project_id() @@ -162,7 +166,7 @@ def _get_explicit_environ_credentials(): explicit_file = os.environ.get(environment_vars.CREDENTIALS) if explicit_file is not None: - credentials, project_id = _load_credentials_from_file( + credentials, project_id = load_credentials_from_file( os.environ[environment_vars.CREDENTIALS] ) From 4b6b34ccd1c12df79317d64b9c39129da7c21027 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Fri, 12 Jun 2020 02:07:48 +0000 Subject: [PATCH 2/7] test: update tests --- google/auth/_default.py | 8 ++++-- tests/test__default.py | 54 ++++++++++++++++++++++++++++------------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/google/auth/_default.py b/google/auth/_default.py index f09abf78b..a5e9965cf 100644 --- a/google/auth/_default.py +++ b/google/auth/_default.py @@ -113,7 +113,9 @@ def load_credentials_from_file(filename, scopes=None): from google.oauth2 import credentials try: - credentials = credentials.Credentials.from_authorized_user_info(info, scopes=scopes) + credentials = credentials.Credentials.from_authorized_user_info( + info, scopes=scopes + ) except ValueError as caught_exc: msg = "Failed to load authorized user credentials from {}".format(filename) new_exc = exceptions.DefaultCredentialsError(msg, caught_exc) @@ -126,7 +128,9 @@ def load_credentials_from_file(filename, scopes=None): from google.oauth2 import service_account try: - credentials = service_account.Credentials.from_service_account_info(info, scopes=scopes) + credentials = service_account.Credentials.from_service_account_info( + info, scopes=scopes + ) except ValueError as caught_exc: msg = "Failed to load service account credentials from {}".format(filename) new_exc = exceptions.DefaultCredentialsError(msg, caught_exc) diff --git a/tests/test__default.py b/tests/test__default.py index 35000b04d..df7bdb3c2 100644 --- a/tests/test__default.py +++ b/tests/test__default.py @@ -43,77 +43,97 @@ SERVICE_ACCOUNT_FILE_DATA = json.load(fh) LOAD_FILE_PATCH = mock.patch( - "google.auth._default._load_credentials_from_file", + "google.auth._default.load_credentials_from_file", return_value=(mock.sentinel.credentials, mock.sentinel.project_id), autospec=True, ) -def test__load_credentials_from_missing_file(): +def test_load_credentials_from_missing_file(): with pytest.raises(exceptions.DefaultCredentialsError) as excinfo: - _default._load_credentials_from_file("") + _default.load_credentials_from_file("") assert excinfo.match(r"not found") -def test__load_credentials_from_file_invalid_json(tmpdir): +def test_load_credentials_from_file_invalid_json(tmpdir): jsonfile = tmpdir.join("invalid.json") jsonfile.write("{") with pytest.raises(exceptions.DefaultCredentialsError) as excinfo: - _default._load_credentials_from_file(str(jsonfile)) + _default.load_credentials_from_file(str(jsonfile)) assert excinfo.match(r"not a valid json file") -def test__load_credentials_from_file_invalid_type(tmpdir): +def test_load_credentials_from_file_invalid_type(tmpdir): jsonfile = tmpdir.join("invalid.json") jsonfile.write(json.dumps({"type": "not-a-real-type"})) with pytest.raises(exceptions.DefaultCredentialsError) as excinfo: - _default._load_credentials_from_file(str(jsonfile)) + _default.load_credentials_from_file(str(jsonfile)) assert excinfo.match(r"does not have a valid type") -def test__load_credentials_from_file_authorized_user(): - credentials, project_id = _default._load_credentials_from_file(AUTHORIZED_USER_FILE) +def test_load_credentials_from_file_authorized_user(): + credentials, project_id = _default.load_credentials_from_file(AUTHORIZED_USER_FILE) assert isinstance(credentials, google.oauth2.credentials.Credentials) assert project_id is None -def test__load_credentials_from_file_authorized_user_bad_format(tmpdir): +def test_load_credentials_from_file_authorized_user_bad_format(tmpdir): filename = tmpdir.join("authorized_user_bad.json") filename.write(json.dumps({"type": "authorized_user"})) with pytest.raises(exceptions.DefaultCredentialsError) as excinfo: - _default._load_credentials_from_file(str(filename)) + _default.load_credentials_from_file(str(filename)) assert excinfo.match(r"Failed to load authorized user") assert excinfo.match(r"missing fields") -def test__load_credentials_from_file_authorized_user_cloud_sdk(): +def test_load_credentials_from_file_authorized_user_cloud_sdk(): with pytest.warns(UserWarning, match="Cloud SDK"): - credentials, project_id = _default._load_credentials_from_file( + credentials, project_id = _default.load_credentials_from_file( AUTHORIZED_USER_CLOUD_SDK_FILE ) assert isinstance(credentials, google.oauth2.credentials.Credentials) assert project_id is None -def test__load_credentials_from_file_service_account(): - credentials, project_id = _default._load_credentials_from_file(SERVICE_ACCOUNT_FILE) +def test_load_credentials_from_file_authorized_user_cloud_sdk_with_scopes(): + with pytest.warns(UserWarning, match="Cloud SDK"): + credentials, project_id = _default.load_credentials_from_file( + AUTHORIZED_USER_CLOUD_SDK_FILE, + scopes=["https://www.google.com/calendar/feeds"], + ) + assert isinstance(credentials, google.oauth2.credentials.Credentials) + assert project_id is None + assert credentials.scopes == ["https://www.google.com/calendar/feeds"] + + +def test_load_credentials_from_file_service_account(): + credentials, project_id = _default.load_credentials_from_file(SERVICE_ACCOUNT_FILE) + assert isinstance(credentials, service_account.Credentials) + assert project_id == SERVICE_ACCOUNT_FILE_DATA["project_id"] + + +def test_load_credentials_from_file_service_account(): + credentials, project_id = _default.load_credentials_from_file( + SERVICE_ACCOUNT_FILE, scopes=["https://www.google.com/calendar/feeds"] + ) assert isinstance(credentials, service_account.Credentials) assert project_id == SERVICE_ACCOUNT_FILE_DATA["project_id"] + assert credentials.scopes == ["https://www.google.com/calendar/feeds"] -def test__load_credentials_from_file_service_account_bad_format(tmpdir): +def test_load_credentials_from_file_service_account_bad_format(tmpdir): filename = tmpdir.join("serivce_account_bad.json") filename.write(json.dumps({"type": "service_account"})) with pytest.raises(exceptions.DefaultCredentialsError) as excinfo: - _default._load_credentials_from_file(str(filename)) + _default.load_credentials_from_file(str(filename)) assert excinfo.match(r"Failed to load service account") assert excinfo.match(r"missing fields") From 04abd9e84ac9041c648ce332cbfcfe0c03098cf2 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Mon, 15 Jun 2020 20:19:45 +0000 Subject: [PATCH 3/7] fix: raise error for json with no type --- google/auth/_default.py | 67 ++++++++++++++++++++--------------------- tests/test__default.py | 12 ++++++++ 2 files changed, 45 insertions(+), 34 deletions(-) diff --git a/google/auth/_default.py b/google/auth/_default.py index a5e9965cf..bb33db552 100644 --- a/google/auth/_default.py +++ b/google/auth/_default.py @@ -77,7 +77,6 @@ def load_credentials_from_file(filename, scopes=None): Args: filename (str): The full path to the credentials file. - scopes (Sequence[str]): The list of scopes for the credentials. If specified, the credentials will automatically be scoped if necessary. @@ -109,41 +108,41 @@ def load_credentials_from_file(filename, scopes=None): # credentials file or an authorized user credentials file. credential_type = info.get("type") - if credential_type == _AUTHORIZED_USER_TYPE: - from google.oauth2 import credentials - - try: - credentials = credentials.Credentials.from_authorized_user_info( - info, scopes=scopes - ) - except ValueError as caught_exc: - msg = "Failed to load authorized user credentials from {}".format(filename) - new_exc = exceptions.DefaultCredentialsError(msg, caught_exc) - six.raise_from(new_exc, caught_exc) - # Authorized user credentials do not contain the project ID. - _warn_about_problematic_credentials(credentials) - return credentials, None - - elif credential_type == _SERVICE_ACCOUNT_TYPE: - from google.oauth2 import service_account - - try: - credentials = service_account.Credentials.from_service_account_info( - info, scopes=scopes - ) - except ValueError as caught_exc: - msg = "Failed to load service account credentials from {}".format(filename) - new_exc = exceptions.DefaultCredentialsError(msg, caught_exc) - six.raise_from(new_exc, caught_exc) - return credentials, info.get("project_id") + if credential_type is not None: + if credential_type == _AUTHORIZED_USER_TYPE: + from google.oauth2 import credentials - else: - raise exceptions.DefaultCredentialsError( - "The file {file} does not have a valid type. " - "Type is {type}, expected one of {valid_types}.".format( - file=filename, type=credential_type, valid_types=_VALID_TYPES - ) + try: + credentials = credentials.Credentials.from_authorized_user_info( + info, scopes=scopes + ) + except ValueError as caught_exc: + msg = "Failed to load authorized user credentials from {}".format(filename) + new_exc = exceptions.DefaultCredentialsError(msg, caught_exc) + six.raise_from(new_exc, caught_exc) + # Authorized user credentials do not contain the project ID. + _warn_about_problematic_credentials(credentials) + return credentials, None + + elif credential_type == _SERVICE_ACCOUNT_TYPE: + from google.oauth2 import service_account + + try: + credentials = service_account.Credentials.from_service_account_info( + info, scopes=scopes + ) + except ValueError as caught_exc: + msg = "Failed to load service account credentials from {}".format(filename) + new_exc = exceptions.DefaultCredentialsError(msg, caught_exc) + six.raise_from(new_exc, caught_exc) + return credentials, info.get("project_id") + + raise exceptions.DefaultCredentialsError( + "The file {file} does not have a valid type. " + "Type is {type}, expected one of {valid_types}.".format( + file=filename, type=credential_type, valid_types=_VALID_TYPES ) + ) def _get_gcloud_sdk_credentials(): diff --git a/tests/test__default.py b/tests/test__default.py index df7bdb3c2..179b66734 100644 --- a/tests/test__default.py +++ b/tests/test__default.py @@ -39,6 +39,8 @@ SERVICE_ACCOUNT_FILE = os.path.join(DATA_DIR, "service_account.json") +CLIENT_SECRETS_FILE = os.path.join(DATA_DIR, "client_secrets.json") + with open(SERVICE_ACCOUNT_FILE) as fh: SERVICE_ACCOUNT_FILE_DATA = json.load(fh) @@ -82,6 +84,16 @@ def test_load_credentials_from_file_authorized_user(): assert project_id is None +def test_load_credentials_from_file_no_type(tmpdir): + # use the client_secrets.json, which is valid json but not a + # loadable credentials type + with pytest.raises(exceptions.DefaultCredentialsError) as excinfo: + _default.load_credentials_from_file(CLIENT_SECRETS_FILE) + + assert excinfo.match(r"does not have a valid type") + assert excinfo.match(r"Type is None") + + def test_load_credentials_from_file_authorized_user_bad_format(tmpdir): filename = tmpdir.join("authorized_user_bad.json") filename.write(json.dumps({"type": "authorized_user"})) From 21083c2686b9256bd17af67c9e58c05ad958afe6 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Mon, 15 Jun 2020 20:21:12 +0000 Subject: [PATCH 4/7] test: fix test names --- tests/test__default.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test__default.py b/tests/test__default.py index 179b66734..d7ffa9dea 100644 --- a/tests/test__default.py +++ b/tests/test__default.py @@ -131,7 +131,7 @@ def test_load_credentials_from_file_service_account(): assert project_id == SERVICE_ACCOUNT_FILE_DATA["project_id"] -def test_load_credentials_from_file_service_account(): +def test_load_credentials_from_file_service_account_with_scopes(): credentials, project_id = _default.load_credentials_from_file( SERVICE_ACCOUNT_FILE, scopes=["https://www.google.com/calendar/feeds"] ) From 33d3457199a520b4ec5246e0509b0148f685f340 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Tue, 16 Jun 2020 19:52:18 +0000 Subject: [PATCH 5/7] refactor: simplify control flow --- google/auth/_default.py | 62 ++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/google/auth/_default.py b/google/auth/_default.py index bb33db552..dc65c5786 100644 --- a/google/auth/_default.py +++ b/google/auth/_default.py @@ -77,7 +77,7 @@ def load_credentials_from_file(filename, scopes=None): Args: filename (str): The full path to the credentials file. - scopes (Sequence[str]): The list of scopes for the credentials. If + scopes (Optional[Sequence[str]]): The list of scopes for the credentials. If specified, the credentials will automatically be scoped if necessary. @@ -108,42 +108,42 @@ def load_credentials_from_file(filename, scopes=None): # credentials file or an authorized user credentials file. credential_type = info.get("type") - if credential_type is not None: - if credential_type == _AUTHORIZED_USER_TYPE: - from google.oauth2 import credentials - - try: - credentials = credentials.Credentials.from_authorized_user_info( - info, scopes=scopes - ) - except ValueError as caught_exc: - msg = "Failed to load authorized user credentials from {}".format(filename) - new_exc = exceptions.DefaultCredentialsError(msg, caught_exc) - six.raise_from(new_exc, caught_exc) - # Authorized user credentials do not contain the project ID. - _warn_about_problematic_credentials(credentials) - return credentials, None - - elif credential_type == _SERVICE_ACCOUNT_TYPE: - from google.oauth2 import service_account - - try: - credentials = service_account.Credentials.from_service_account_info( - info, scopes=scopes - ) - except ValueError as caught_exc: - msg = "Failed to load service account credentials from {}".format(filename) - new_exc = exceptions.DefaultCredentialsError(msg, caught_exc) - six.raise_from(new_exc, caught_exc) - return credentials, info.get("project_id") - - raise exceptions.DefaultCredentialsError( + if credential_type is None or credential_type not in _VALID_TYPES: + raise exceptions.DefaultCredentialsError( "The file {file} does not have a valid type. " "Type is {type}, expected one of {valid_types}.".format( file=filename, type=credential_type, valid_types=_VALID_TYPES ) ) + if credential_type == _AUTHORIZED_USER_TYPE: + from google.oauth2 import credentials + + try: + credentials = credentials.Credentials.from_authorized_user_info( + info, scopes=scopes + ) + except ValueError as caught_exc: + msg = "Failed to load authorized user credentials from {}".format(filename) + new_exc = exceptions.DefaultCredentialsError(msg, caught_exc) + six.raise_from(new_exc, caught_exc) + # Authorized user credentials do not contain the project ID. + _warn_about_problematic_credentials(credentials) + return credentials, None + + elif credential_type == _SERVICE_ACCOUNT_TYPE: + from google.oauth2 import service_account + + try: + credentials = service_account.Credentials.from_service_account_info( + info, scopes=scopes + ) + except ValueError as caught_exc: + msg = "Failed to load service account credentials from {}".format(filename) + new_exc = exceptions.DefaultCredentialsError(msg, caught_exc) + six.raise_from(new_exc, caught_exc) + return credentials, info.get("project_id") + def _get_gcloud_sdk_credentials(): """Gets the credentials and project ID from the Cloud SDK.""" From 9c043c6a1c7bd2a0a330b025f50534e7a062c978 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim Date: Tue, 16 Jun 2020 22:58:05 +0000 Subject: [PATCH 6/7] fix: raise coverage --- google/auth/_default.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/google/auth/_default.py b/google/auth/_default.py index dc65c5786..6bb025d4f 100644 --- a/google/auth/_default.py +++ b/google/auth/_default.py @@ -108,14 +108,6 @@ def load_credentials_from_file(filename, scopes=None): # credentials file or an authorized user credentials file. credential_type = info.get("type") - if credential_type is None or credential_type not in _VALID_TYPES: - raise exceptions.DefaultCredentialsError( - "The file {file} does not have a valid type. " - "Type is {type}, expected one of {valid_types}.".format( - file=filename, type=credential_type, valid_types=_VALID_TYPES - ) - ) - if credential_type == _AUTHORIZED_USER_TYPE: from google.oauth2 import credentials @@ -144,6 +136,14 @@ def load_credentials_from_file(filename, scopes=None): six.raise_from(new_exc, caught_exc) return credentials, info.get("project_id") + else: + raise exceptions.DefaultCredentialsError( + "The file {file} does not have a valid type. " + "Type is {type}, expected one of {valid_types}.".format( + file=filename, type=credential_type, valid_types=_VALID_TYPES + ) + ) + def _get_gcloud_sdk_credentials(): """Gets the credentials and project ID from the Cloud SDK.""" From 26db61ccd673e043759dac88f069ab7a4b5cc105 Mon Sep 17 00:00:00 2001 From: Sijun Liu Date: Thu, 18 Jun 2020 00:01:27 -0700 Subject: [PATCH 7/7] test: update test --- tests/test__default.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test__default.py b/tests/test__default.py index b78f52ac7..3c87b35eb 100644 --- a/tests/test__default.py +++ b/tests/test__default.py @@ -118,7 +118,7 @@ def test_load_credentials_from_file_authorized_user_cloud_sdk(): assert project_id is None # No warning if the json file has quota project id. - credentials, project_id = _default._load_credentials_from_file( + credentials, project_id = _default.load_credentials_from_file( AUTHORIZED_USER_CLOUD_SDK_WITH_QUOTA_PROJECT_ID_FILE ) assert isinstance(credentials, google.oauth2.credentials.Credentials)