From 7f7d92d63ffee91859fc819416af78cef3baf574 Mon Sep 17 00:00:00 2001 From: Zev Goldstein Date: Fri, 23 Jul 2021 15:52:46 -0400 Subject: [PATCH] fix: do not use the GAE APIs on gen2+ runtimes (#807) * fix: do not use the GAE APIs on gen2+ runtimes Currently, this library uses the App Engine API in all environments if it can be imported successfully. This assumption made sense when the API was only available on gen1, but this is no longer the case. See https://github.com/GoogleCloudPlatform/appengine-python-standard In order to comply with AIP-4115, we must treat GAE gen2+ as a "compute engine equivalent environment" even if the GAE APIs are importable. In other words, google.auth.default() must never return an app_engine.Credental on GAE gen2+.Currently, this library uses the App Engine API in all environments if it can be imported successfully. This assumption made sense when the API was only available on gen1, but this is no longer the case. See https://github.com/GoogleCloudPlatform/appengine-python-standard In order to comply with AIP-4115, we must treat GAE gen2+ as a "compute engine equivalent environment" even if the GAE APIs are importable. In other words, google.auth.default() should not return an app_engine.Credental on GAE gen2+. * blacken Co-authored-by: arithmetic1728 <58957152+arithmetic1728@users.noreply.github.com> --- google/auth/_default.py | 5 +++ google/auth/environment_vars.py | 6 ++++ tests/test__default.py | 57 +++++++++++++++++++++++++++--- tests/test_app_engine.py | 2 ++ tests_async/test__default_async.py | 57 +++++++++++++++++++++++++++--- 5 files changed, 119 insertions(+), 8 deletions(-) diff --git a/google/auth/_default.py b/google/auth/_default.py index 4dc0725e7..f7e308f3e 100644 --- a/google/auth/_default.py +++ b/google/auth/_default.py @@ -230,6 +230,11 @@ def _get_explicit_environ_credentials(): def _get_gae_credentials(): """Gets Google App Engine App Identity credentials and project ID.""" + # If not GAE gen1, prefer the metadata service even if the GAE APIs are + # available as per https://google.aip.dev/auth/4115. + if os.environ.get(environment_vars.LEGACY_APPENGINE_RUNTIME) != "python27": + return None, None + # While this library is normally bundled with app_engine, there are # some cases where it's not available, so we tolerate ImportError. try: diff --git a/google/auth/environment_vars.py b/google/auth/environment_vars.py index f02774181..d36d6c4af 100644 --- a/google/auth/environment_vars.py +++ b/google/auth/environment_vars.py @@ -60,6 +60,12 @@ The default value is false. Users have to explicitly set this value to true in order to use client certificate to establish a mutual TLS channel.""" +LEGACY_APPENGINE_RUNTIME = "APPENGINE_RUNTIME" +"""Gen1 environment variable defining the App Engine Runtime. + +Used to distinguish between GAE gen1 and GAE gen2+. +""" + # AWS environment variables used with AWS workload identity pools to retrieve # AWS security credentials and the AWS region needed to create a serialized # signed requests to the AWS STS GetCalledIdentity API that can be exchanged diff --git a/tests/test__default.py b/tests/test__default.py index e13689625..a515f3813 100644 --- a/tests/test__default.py +++ b/tests/test__default.py @@ -447,7 +447,9 @@ def app_identity(monkeypatch): yield app_identity_module -def test__get_gae_credentials(app_identity): +@mock.patch.dict(os.environ) +def test__get_gae_credentials_gen1(app_identity): + os.environ[environment_vars.LEGACY_APPENGINE_RUNTIME] = "python27" app_identity.get_application_id.return_value = mock.sentinel.project credentials, project_id = _default._get_gae_credentials() @@ -456,18 +458,65 @@ def test__get_gae_credentials(app_identity): assert project_id == mock.sentinel.project +@mock.patch.dict(os.environ) +def test__get_gae_credentials_gen2(): + os.environ["GAE_RUNTIME"] = "python37" + credentials, project_id = _default._get_gae_credentials() + assert credentials is None + assert project_id is None + + +@mock.patch.dict(os.environ) +def test__get_gae_credentials_gen2_backwards_compat(): + # compat helpers may copy GAE_RUNTIME to APPENGINE_RUNTIME + # for backwards compatibility with code that relies on it + os.environ[environment_vars.LEGACY_APPENGINE_RUNTIME] = "python37" + os.environ["GAE_RUNTIME"] = "python37" + credentials, project_id = _default._get_gae_credentials() + assert credentials is None + assert project_id is None + + +def test__get_gae_credentials_env_unset(): + assert environment_vars.LEGACY_APPENGINE_RUNTIME not in os.environ + assert "GAE_RUNTIME" not in os.environ + credentials, project_id = _default._get_gae_credentials() + assert credentials is None + assert project_id is None + + +@mock.patch.dict(os.environ) def test__get_gae_credentials_no_app_engine(): + # test both with and without LEGACY_APPENGINE_RUNTIME setting + assert environment_vars.LEGACY_APPENGINE_RUNTIME not in os.environ + import sys - with mock.patch.dict("sys.modules"): - sys.modules["google.auth.app_engine"] = None + with mock.patch.dict(sys.modules, {"google.auth.app_engine": None}): + credentials, project_id = _default._get_gae_credentials() + assert credentials is None + assert project_id is None + + os.environ[environment_vars.LEGACY_APPENGINE_RUNTIME] = "python27" credentials, project_id = _default._get_gae_credentials() assert credentials is None assert project_id is None +@mock.patch.dict(os.environ) +@mock.patch.object(app_engine, "app_identity", new=None) def test__get_gae_credentials_no_apis(): - assert _default._get_gae_credentials() == (None, None) + # test both with and without LEGACY_APPENGINE_RUNTIME setting + assert environment_vars.LEGACY_APPENGINE_RUNTIME not in os.environ + + credentials, project_id = _default._get_gae_credentials() + assert credentials is None + assert project_id is None + + os.environ[environment_vars.LEGACY_APPENGINE_RUNTIME] = "python27" + credentials, project_id = _default._get_gae_credentials() + assert credentials is None + assert project_id is None @mock.patch( diff --git a/tests/test_app_engine.py b/tests/test_app_engine.py index e335ff7ed..6a788b9e9 100644 --- a/tests/test_app_engine.py +++ b/tests/test_app_engine.py @@ -52,6 +52,7 @@ def test_get_project_id(app_identity): assert app_engine.get_project_id() == mock.sentinel.project +@mock.patch.object(app_engine, "app_identity", new=None) def test_get_project_id_missing_apis(): with pytest.raises(EnvironmentError) as excinfo: assert app_engine.get_project_id() @@ -86,6 +87,7 @@ def test_sign(self, app_identity): class TestCredentials(object): + @mock.patch.object(app_engine, "app_identity", new=None) def test_missing_apis(self): with pytest.raises(EnvironmentError) as excinfo: app_engine.Credentials() diff --git a/tests_async/test__default_async.py b/tests_async/test__default_async.py index 527a8da45..b67230342 100644 --- a/tests_async/test__default_async.py +++ b/tests_async/test__default_async.py @@ -284,7 +284,9 @@ def app_identity(monkeypatch): yield app_identity_module -def test__get_gae_credentials(app_identity): +@mock.patch.dict(os.environ) +def test__get_gae_credentials_gen1(app_identity): + os.environ[environment_vars.LEGACY_APPENGINE_RUNTIME] = "python27" app_identity.get_application_id.return_value = mock.sentinel.project credentials, project_id = _default._get_gae_credentials() @@ -293,18 +295,65 @@ def test__get_gae_credentials(app_identity): assert project_id == mock.sentinel.project +@mock.patch.dict(os.environ) +def test__get_gae_credentials_gen2(): + os.environ["GAE_RUNTIME"] = "python37" + credentials, project_id = _default._get_gae_credentials() + assert credentials is None + assert project_id is None + + +@mock.patch.dict(os.environ) +def test__get_gae_credentials_gen2_backwards_compat(): + # compat helpers may copy GAE_RUNTIME to APPENGINE_RUNTIME + # for backwards compatibility with code that relies on it + os.environ[environment_vars.LEGACY_APPENGINE_RUNTIME] = "python37" + os.environ["GAE_RUNTIME"] = "python37" + credentials, project_id = _default._get_gae_credentials() + assert credentials is None + assert project_id is None + + +def test__get_gae_credentials_env_unset(): + assert environment_vars.LEGACY_APPENGINE_RUNTIME not in os.environ + assert "GAE_RUNTIME" not in os.environ + credentials, project_id = _default._get_gae_credentials() + assert credentials is None + assert project_id is None + + +@mock.patch.dict(os.environ) def test__get_gae_credentials_no_app_engine(): + # test both with and without LEGACY_APPENGINE_RUNTIME setting + assert environment_vars.LEGACY_APPENGINE_RUNTIME not in os.environ + import sys - with mock.patch.dict("sys.modules"): - sys.modules["google.auth.app_engine"] = None + with mock.patch.dict(sys.modules, {"google.auth.app_engine": None}): + credentials, project_id = _default._get_gae_credentials() + assert credentials is None + assert project_id is None + + os.environ[environment_vars.LEGACY_APPENGINE_RUNTIME] = "python27" credentials, project_id = _default._get_gae_credentials() assert credentials is None assert project_id is None +@mock.patch.dict(os.environ) +@mock.patch.object(app_engine, "app_identity", new=None) def test__get_gae_credentials_no_apis(): - assert _default._get_gae_credentials() == (None, None) + # test both with and without LEGACY_APPENGINE_RUNTIME setting + assert environment_vars.LEGACY_APPENGINE_RUNTIME not in os.environ + + credentials, project_id = _default._get_gae_credentials() + assert credentials is None + assert project_id is None + + os.environ[environment_vars.LEGACY_APPENGINE_RUNTIME] = "python27" + credentials, project_id = _default._get_gae_credentials() + assert credentials is None + assert project_id is None @mock.patch(