From 7078d6fb28c291059fa9051b5a96c65e98eb7e38 Mon Sep 17 00:00:00 2001 From: Gene Wood Date: Thu, 23 Feb 2017 15:36:27 -0800 Subject: [PATCH 1/3] Adding persistent caching of temporary credentials --- botocore/credentials.py | 48 ++++++++++++++++++++++++- tests/__init__.py | 13 +++++++ tests/unit/test_credentials.py | 65 +++++++++++++++++++++++++++++++++- 3 files changed, 124 insertions(+), 2 deletions(-) diff --git a/botocore/credentials.py b/botocore/credentials.py index fcfe916cbf..326a624a10 100644 --- a/botocore/credentials.py +++ b/botocore/credentials.py @@ -17,6 +17,7 @@ import os import getpass import threading +import json import subprocess from collections import namedtuple from copy import deepcopy @@ -71,7 +72,7 @@ def create_credential_resolver(session): assume_role_provider = AssumeRoleProvider( load_config=lambda: session.full_config, client_creator=session.create_client, - cache={}, + cache=JSONFileCache(), profile_name=profile_name, credential_sourcer=CanonicalNameCredentialSourcer([ env_provider, container_provider, instance_metadata_provider @@ -178,6 +179,51 @@ def __call__(self): return _Refresher(actual_refresh) +class JSONFileCache(object): + """JSON file cache. + This provides a dict like interface that stores JSON serializable + objects. + The objects are serialized to JSON and stored in a file. These + values can be retrieved at a later time. + """ + + CACHE_DIR = os.path.expanduser(os.path.join('~', '.aws', 'cli', 'cache')) + + def __init__(self, working_dir=CACHE_DIR): + self._working_dir = working_dir + + def __contains__(self, cache_key): + actual_key = self._convert_cache_key(cache_key) + return os.path.isfile(actual_key) + + def __getitem__(self, cache_key): + """Retrieve value from a cache key.""" + actual_key = self._convert_cache_key(cache_key) + try: + with open(actual_key) as f: + return json.load(f) + except (OSError, ValueError, IOError): + raise KeyError(cache_key) + + def __setitem__(self, cache_key, value): + full_key = self._convert_cache_key(cache_key) + try: + file_content = json.dumps(value, default=_serialize_if_needed) + except (TypeError, ValueError): + raise ValueError("Value cannot be cached, must be " + "JSON serializable: %s" % value) + if not os.path.isdir(self._working_dir): + os.makedirs(self._working_dir) + with os.fdopen(os.open(full_key, + os.O_WRONLY | os.O_CREAT, 0o600), 'w') as f: + f.truncate() + f.write(file_content) + + def _convert_cache_key(self, cache_key): + full_path = os.path.join(self._working_dir, cache_key + '.json') + return full_path + + class Credentials(object): """ Holds the credentials needed to authenticate requests. diff --git a/tests/__init__.py b/tests/__init__.py index 74a2e4de19..7d74dd1678 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -57,6 +57,19 @@ def skip_unless_has_memory_collection(cls): return cls +def skip_if_windows(reason): + """Decorator to skip tests that should not be run on windows. + Example usage: + @skip_if_windows("Not valid") + def test_some_non_windows_stuff(self): + self.assertEqual(...) + """ + def decorator(func): + return unittest.skipIf( + platform.system() not in ['Darwin', 'Linux'], reason)(func) + return decorator + + def random_chars(num_chars): """Returns random hex characters. diff --git a/tests/unit/test_credentials.py b/tests/unit/test_credentials.py index 77a7e7120d..cf1c90aa18 100644 --- a/tests/unit/test_credentials.py +++ b/tests/unit/test_credentials.py @@ -15,6 +15,8 @@ import subprocess import mock import os +import tempfile +import shutil import json import copy @@ -29,7 +31,7 @@ from botocore.credentials import Credentials import botocore.exceptions import botocore.session -from tests import unittest, BaseEnvVar, IntegerRefresher +from tests import unittest, BaseEnvVar, IntegerRefresher, skip_if_windows # Passed to session to keep it from finding default config file @@ -2144,6 +2146,67 @@ def test_recursive_assume_role(self): ]) +class TestJSONCache(unittest.TestCase): + def setUp(self): + self.tempdir = tempfile.mkdtemp() + self.cache = credentials.JSONFileCache(self.tempdir) + + def tearDown(self): + shutil.rmtree(self.tempdir) + + def test_supports_contains_check(self): + # By default the cache is empty because we're + # using a new temp dir everytime. + self.assertTrue('mykey' not in self.cache) + + def test_add_key_and_contains_check(self): + self.cache['mykey'] = {'foo': 'bar'} + self.assertTrue('mykey' in self.cache) + + def test_added_key_can_be_retrieved(self): + self.cache['mykey'] = {'foo': 'bar'} + self.assertEqual(self.cache['mykey'], {'foo': 'bar'}) + + def test_only_accepts_json_serializable_data(self): + with self.assertRaises(ValueError): + # set()'s cannot be serialized to a JSON string. + self.cache['mykey'] = set() + + def test_can_override_existing_values(self): + self.cache['mykey'] = {'foo': 'bar'} + self.cache['mykey'] = {'baz': 'newvalue'} + self.assertEqual(self.cache['mykey'], {'baz': 'newvalue'}) + + def test_can_add_multiple_keys(self): + self.cache['mykey'] = {'foo': 'bar'} + self.cache['mykey2'] = {'baz': 'qux'} + self.assertEqual(self.cache['mykey'], {'foo': 'bar'}) + self.assertEqual(self.cache['mykey2'], {'baz': 'qux'}) + + def test_working_dir_does_not_exist(self): + working_dir = os.path.join(self.tempdir, 'foo') + cache = credentials.JSONFileCache(working_dir) + cache['foo'] = {'bar': 'baz'} + self.assertEqual(cache['foo'], {'bar': 'baz'}) + + def test_key_error_raised_when_cache_key_does_not_exist(self): + with self.assertRaises(KeyError): + self.cache['foo'] + + def test_file_is_truncated_before_writing(self): + self.cache['mykey'] = { + 'really long key in the cache': 'really long value in cache'} + # Now overwrite it with a smaller value. + self.cache['mykey'] = {'a': 'b'} + self.assertEqual(self.cache['mykey'], {'a': 'b'}) + + @skip_if_windows('File permissions tests not supported on Windows.') + def test_permissions_for_file_restricted(self): + self.cache['mykey'] = {'foo': 'bar'} + filename = os.path.join(self.tempdir, 'mykey.json') + self.assertEqual(os.stat(filename).st_mode & 0xFFF, 0o600) + + class TestRefreshLogic(unittest.TestCase): def test_mandatory_refresh_needed(self): creds = IntegerRefresher( From 56cef9cfedb3aa552bf8c427fb7c97e426b6b15c Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Wed, 13 Dec 2017 14:18:29 -0800 Subject: [PATCH 2/3] Accept cache as a factory variable --- botocore/credentials.py | 6 ++++-- tests/unit/test_credentials.py | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/botocore/credentials.py b/botocore/credentials.py index 326a624a10..f428d599de 100644 --- a/botocore/credentials.py +++ b/botocore/credentials.py @@ -48,7 +48,7 @@ ['access_key', 'secret_key', 'token']) -def create_credential_resolver(session): +def create_credential_resolver(session, cache=None): """Create a default credential resolver. This creates a pre-configured credential resolver @@ -61,6 +61,8 @@ def create_credential_resolver(session): config_file = session.get_config_variable('config_file') metadata_timeout = session.get_config_variable('metadata_service_timeout') num_attempts = session.get_config_variable('metadata_service_num_attempts') + if cache is None: + cache = {} env_provider = EnvProvider() container_provider = ContainerProvider() @@ -72,7 +74,7 @@ def create_credential_resolver(session): assume_role_provider = AssumeRoleProvider( load_config=lambda: session.full_config, client_creator=session.create_client, - cache=JSONFileCache(), + cache=cache, profile_name=profile_name, credential_sourcer=CanonicalNameCredentialSourcer([ env_provider, container_provider, instance_metadata_provider diff --git a/tests/unit/test_credentials.py b/tests/unit/test_credentials.py index cf1c90aa18..61b27b6b97 100644 --- a/tests/unit/test_credentials.py +++ b/tests/unit/test_credentials.py @@ -1319,6 +1319,20 @@ def test_env_provider_added_if_profile_from_env_set(self): self.assertTrue( any(isinstance(p, EnvProvider) for p in resolver.providers)) + def test_default_cache(self): + resolver = credentials.create_credential_resolver(self.session) + cache = resolver.get_provider('assume-role').cache + self.assertIsInstance(cache, dict) + self.assertEqual(cache, {}) + + def test_custom_cache(self): + custom_cache = credentials.JSONFileCache() + resolver = credentials.create_credential_resolver( + self.session, custom_cache + ) + cache = resolver.get_provider('assume-role').cache + self.assertIs(cache, custom_cache) + class TestCanonicalNameSourceProvider(BaseEnvVar): def setUp(self): From b915ca35dd7518d97f9fc48d974158126a575372 Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Thu, 14 Dec 2017 09:25:20 -0800 Subject: [PATCH 3/3] Move default cache dir --- .changes/next-release/enhancement-credentials-15604.json | 5 +++++ botocore/credentials.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changes/next-release/enhancement-credentials-15604.json diff --git a/.changes/next-release/enhancement-credentials-15604.json b/.changes/next-release/enhancement-credentials-15604.json new file mode 100644 index 0000000000..88b941d577 --- /dev/null +++ b/.changes/next-release/enhancement-credentials-15604.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "credentials", + "description": "Moved the JSONFileCache from the CLI into botocore so that it can be used without importing from the cli." +} diff --git a/botocore/credentials.py b/botocore/credentials.py index f428d599de..d846b1e506 100644 --- a/botocore/credentials.py +++ b/botocore/credentials.py @@ -189,7 +189,7 @@ class JSONFileCache(object): values can be retrieved at a later time. """ - CACHE_DIR = os.path.expanduser(os.path.join('~', '.aws', 'cli', 'cache')) + CACHE_DIR = os.path.expanduser(os.path.join('~', '.aws', 'boto', 'cache')) def __init__(self, working_dir=CACHE_DIR): self._working_dir = working_dir