From f97578e3564696bd2216de3154bd2d46c6da3858 Mon Sep 17 00:00:00 2001 From: Jeff Casavant Date: Thu, 28 Feb 2019 11:02:17 -0500 Subject: [PATCH 1/4] If any auth env var is empty string, treat it as if it was unset --- botocore/credentials.py | 11 +++++++---- tests/unit/test_credentials.py | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/botocore/credentials.py b/botocore/credentials.py index 19b12b327b..51e301f0bc 100644 --- a/botocore/credentials.py +++ b/botocore/credentials.py @@ -921,7 +921,10 @@ def load(self): """ Search for credentials in explicit environment variables. """ - if self._mapping['access_key'] in self.environ: + + access_key_var = self._mapping['access_key'] + + if access_key_var in self.environ and not self.environ.get(access_key_var) == '': logger.info('Found credentials in environment variables.') fetcher = self._create_credentials_fetcher() credentials = fetcher(require_expiry=False) @@ -951,20 +954,20 @@ def fetch_credentials(require_expiry=True): credentials = {} access_key = environ.get(mapping['access_key']) - if access_key is None: + if access_key is None or access_key == '': raise PartialCredentialsError( provider=method, cred_var=mapping['access_key']) credentials['access_key'] = access_key secret_key = environ.get(mapping['secret_key']) - if secret_key is None: + if secret_key is None or secret_key == '': raise PartialCredentialsError( provider=method, cred_var=mapping['secret_key']) credentials['secret_key'] = secret_key token = None for token_env_var in mapping['token']: - if token_env_var in environ: + if token_env_var in environ and not environ[token_env_var] == '': token = environ[token_env_var] break credentials['token'] = token diff --git a/tests/unit/test_credentials.py b/tests/unit/test_credentials.py index 12caa4b09f..796cdab9ae 100644 --- a/tests/unit/test_credentials.py +++ b/tests/unit/test_credentials.py @@ -691,6 +691,16 @@ def test_envvars_not_found(self): creds = provider.load() self.assertIsNone(creds) + def test_envvars_empty_string(self): + environ = { + 'AWS_ACCESS_KEY_ID': '', + 'AWS_SECRET_ACCESS_KEY': '', + 'AWS_SECURITY_TOKEN': '', + } + provider = credentials.EnvProvider(environ) + creds = provider.load() + self.assertIsNone(creds) + def test_can_override_env_var_mapping(self): # We can change the env var provider to # use our specified env var names. @@ -765,6 +775,18 @@ def test_partial_creds_is_an_error(self): with self.assertRaises(botocore.exceptions.PartialCredentialsError): provider.load() + def test_partial_creds_is_an_error_empty_string(self): + # If the user provides an access key, they must also + # provide a secret key. Not doing so will generate an + # error. + environ = { + 'AWS_ACCESS_KEY_ID': 'foo', + 'AWS_SECRET_ACCESS_KEY': '', + } + provider = credentials.EnvProvider(environ) + with self.assertRaises(botocore.exceptions.PartialCredentialsError): + provider.load() + def test_missing_access_key_id_raises_error(self): expiry_time = datetime.now(tzlocal()) - timedelta(hours=1) environ = { From 01790d88d46729998d0831b85e21ff7715b1186d Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Tue, 16 Apr 2019 13:16:55 -0700 Subject: [PATCH 2/4] Simplify empty env var creds check --- botocore/credentials.py | 28 +++++++++++++++------------- tests/unit/test_credentials.py | 4 ++-- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/botocore/credentials.py b/botocore/credentials.py index 51e301f0bc..5f3cb3bf3a 100644 --- a/botocore/credentials.py +++ b/botocore/credentials.py @@ -922,9 +922,9 @@ def load(self): Search for credentials in explicit environment variables. """ - access_key_var = self._mapping['access_key'] + access_key = self.environ.get(self._mapping['access_key'], '') - if access_key_var in self.environ and not self.environ.get(access_key_var) == '': + if access_key: logger.info('Found credentials in environment variables.') fetcher = self._create_credentials_fetcher() credentials = fetcher(require_expiry=False) @@ -953,30 +953,32 @@ def _create_credentials_fetcher(self): def fetch_credentials(require_expiry=True): credentials = {} - access_key = environ.get(mapping['access_key']) - if access_key is None or access_key == '': + access_key = environ.get(mapping['access_key'], '') + if not access_key: raise PartialCredentialsError( provider=method, cred_var=mapping['access_key']) credentials['access_key'] = access_key - secret_key = environ.get(mapping['secret_key']) - if secret_key is None or secret_key == '': + secret_key = environ.get(mapping['secret_key'], '') + if not secret_key: raise PartialCredentialsError( provider=method, cred_var=mapping['secret_key']) credentials['secret_key'] = secret_key - token = None + credentials['token'] = None for token_env_var in mapping['token']: - if token_env_var in environ and not environ[token_env_var] == '': - token = environ[token_env_var] + token = environ.get(token_env_var, '') + if token: + credentials['token'] = token break - credentials['token'] = token - expiry_time = environ.get(mapping['expiry_time']) - if require_expiry and expiry_time is None: + credentials['expiry_time'] = None + expiry_time = environ.get(mapping['expiry_time'], '') + if expiry_time: + credentials['expiry_time'] = expiry_time + if require_expiry and not expiry_time: raise PartialCredentialsError( provider=method, cred_var=mapping['expiry_time']) - credentials['expiry_time'] = expiry_time return credentials diff --git a/tests/unit/test_credentials.py b/tests/unit/test_credentials.py index 796cdab9ae..19bf4dca2e 100644 --- a/tests/unit/test_credentials.py +++ b/tests/unit/test_credentials.py @@ -1845,7 +1845,7 @@ def test_external_id_provided(self): RoleArn='myrole', ExternalId='myid', RoleSessionName=mock.ANY) def test_assume_role_with_duration(self): - self.fake_config['profiles']['development']['duration_seconds'] = 7200 + self.fake_config['profiles']['development']['duration_seconds'] = 7200 response = { 'Credentials': { 'AccessKeyId': 'foo', @@ -1864,7 +1864,7 @@ def test_assume_role_with_duration(self): client = client_creator.return_value client.assume_role.assert_called_with( - RoleArn='myrole', RoleSessionName=mock.ANY, + RoleArn='myrole', RoleSessionName=mock.ANY, DurationSeconds=7200) def test_assume_role_with_bad_duration(self): From 562c28f164acc564cfd1a8aca305123db1cb9fd4 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Tue, 16 Apr 2019 13:20:13 -0700 Subject: [PATCH 3/4] Add changelog entry --- .../next-release/enhancement-EnvironmentVariables-90061.json | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/next-release/enhancement-EnvironmentVariables-90061.json diff --git a/.changes/next-release/enhancement-EnvironmentVariables-90061.json b/.changes/next-release/enhancement-EnvironmentVariables-90061.json new file mode 100644 index 0000000000..1e4cdbb16e --- /dev/null +++ b/.changes/next-release/enhancement-EnvironmentVariables-90061.json @@ -0,0 +1,5 @@ +{ + "category": "Environment Variables", + "type": "enhancement", + "description": "Ignore env var credentials is values are empty (`#1680 `__)" +} From c9bc7fba2cc095fe960b7d24f0e2383b38843dda Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Mon, 29 Apr 2019 14:51:49 -0700 Subject: [PATCH 4/4] Add missing test cases for empty expiry values --- tests/unit/test_credentials.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/unit/test_credentials.py b/tests/unit/test_credentials.py index 19bf4dca2e..ac9de35b6a 100644 --- a/tests/unit/test_credentials.py +++ b/tests/unit/test_credentials.py @@ -701,6 +701,38 @@ def test_envvars_empty_string(self): creds = provider.load() self.assertIsNone(creds) + def test_expiry_omitted_if_envvar_empty(self): + environ = { + 'AWS_ACCESS_KEY_ID': 'foo', + 'AWS_SECRET_ACCESS_KEY': 'bar', + 'AWS_SESSION_TOKEN': 'baz', + 'AWS_CREDENTIAL_EXPIRATION': '', + } + provider = credentials.EnvProvider(environ) + creds = provider.load() + # Because we treat empty env vars the same as not being provided, + # we should return static credentials and not a refreshable + # credential. + self.assertNotIsInstance(creds, credentials.RefreshableCredentials) + self.assertEqual(creds.access_key, 'foo') + self.assertEqual(creds.secret_key, 'bar') + self.assertEqual(creds.token, 'baz') + + def test_error_when_expiry_required_but_empty(self): + expiry_time = datetime.now(tzlocal()) - timedelta(hours=1) + environ = { + 'AWS_ACCESS_KEY_ID': 'foo', + 'AWS_SECRET_ACCESS_KEY': 'bar', + 'AWS_CREDENTIAL_EXPIRATION': expiry_time.isoformat(), + } + provider = credentials.EnvProvider(environ) + creds = provider.load() + + del environ['AWS_CREDENTIAL_EXPIRATION'] + + with self.assertRaises(botocore.exceptions.PartialCredentialsError): + creds.get_frozen_credentials() + def test_can_override_env_var_mapping(self): # We can change the env var provider to # use our specified env var names.