From 68eed64a02bea4a3adb4c8761f8d8bc4f55e5ecd Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Wed, 22 May 2013 17:00:53 -0400 Subject: [PATCH] Fix optional keyring support, add basic keyring tests Commit 06d9437e8388b369546d760607f17cb5022750e9 made using a keyring optional by adding a function to handle the imports. The problem is that import needs to be available at a global level for it to be usable in other parts of the code. The function is replaced by a top-level try/except. Fixing the imports isn't enough to get keyring support working because in get_auth_ref_from_keyring() the wrong token was being used when evaluating token expiration. It was using the token already in the HTTPClient object and not the one returned from the keyring. Some simple tests were added to prevent future regressions. These tests will be skipped if the keyring or pickle packages are not installed. Change-Id: I1fe2c9e5cdf275df2047018368da2e4b3d2d6de2 Fixes: bug #1183072 --- keystoneclient/client.py | 28 ++++----- tests/test_keyring.py | 126 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 16 deletions(-) create mode 100644 tests/test_keyring.py diff --git a/keystoneclient/client.py b/keystoneclient/client.py index 5b2f1253a..9c008556b 100644 --- a/keystoneclient/client.py +++ b/keystoneclient/client.py @@ -10,11 +10,17 @@ OpenStack Client interface. Handles the REST calls and responses. import copy import logging -import sys import urlparse import requests +try: + import keyring + import pickle +except ImportError: + keyring = None + pickle = None + try: import json except ImportError: @@ -33,19 +39,6 @@ from keystoneclient import exceptions _logger = logging.getLogger(__name__) -def try_import_keyring(): - try: - import keyring # noqa - import pickle # noqa - return True - except ImportError: - if (hasattr(sys.stderr, 'isatty') and sys.stderr.isatty()): - print >> sys.stderr, 'Failed to load keyring modules.' - else: - _logger.warning('Failed to load keyring modules.') - return False - - class HTTPClient(object): USER_AGENT = 'python-keystoneclient' @@ -121,7 +114,9 @@ class HTTPClient(object): requests.logging.getLogger(requests.__name__).addHandler(ch) # keyring setup - self.use_keyring = use_keyring and try_import_keyring() + if use_keyring and keyring is None: + _logger.warning('Failed to load keyring modules.') + self.use_keyring = use_keyring and keyring is not None self.force_new_token = force_new_token self.stale_duration = stale_duration or access.STALE_TOKEN_DURATION self.stale_duration = int(self.stale_duration) @@ -233,6 +228,7 @@ class HTTPClient(object): Otherwise, (keyring_key, None) is returned. :returns: (keyring_key, auth_ref) or (keyring_key, None) + :returns: or (None, None) if use_keyring is not set in the object """ keyring_key = None @@ -246,7 +242,7 @@ class HTTPClient(object): keyring_key) if auth_ref: auth_ref = pickle.loads(auth_ref) - if self.auth_ref.will_expire_soon(self.stale_duration): + if auth_ref.will_expire_soon(self.stale_duration): # token has expired, don't use it auth_ref = None except Exception as e: diff --git a/tests/test_keyring.py b/tests/test_keyring.py new file mode 100644 index 000000000..9ac8d7522 --- /dev/null +++ b/tests/test_keyring.py @@ -0,0 +1,126 @@ +import datetime +import unittest + +from keystoneclient import access +from keystoneclient import client +from keystoneclient.openstack.common import timeutils +from tests import client_fixtures +from tests import utils + +try: + import keyring # noqa + import pickle # noqa +except ImportError: + keyring = None + +PROJECT_SCOPED_TOKEN = client_fixtures.PROJECT_SCOPED_TOKEN + +# These mirror values from PROJECT_SCOPED_TOKEN +USERNAME = 'exampleuser' +AUTH_URL = 'http://public.com:5000/v2.0' +TOKEN = '04c7d5ffaeef485f9dc69c06db285bdb' + +PASSWORD = 'password' +TENANT = 'tenant' +TENANT_ID = 'tenant_id' + + +class KeyringTest(utils.TestCase): + + @classmethod + def setUpClass(cls): + super(KeyringTest, cls).setUpClass() + + if keyring is None: + raise unittest.SkipTest( + 'optional package keyring or pickle is not installed') + + def setUp(self): + class MemoryKeyring(keyring.backend.KeyringBackend): + """Simple memory keyring with support for multiple keys""" + def __init__(self): + self.passwords = {} + + def supported(self): + return 1 + + def get_password(self, service, username): + key = username + '@' + service + if key not in self.passwords: + return None + return self.passwords[key] + + def set_password(self, service, username, password): + key = username + '@' + service + self.passwords[key] = password + + super(KeyringTest, self).setUp() + keyring.set_keyring(MemoryKeyring()) + + def test_no_keyring_key(self): + """ + Ensure that we get no value back if we don't have use_keyring + set in the client. + """ + cl = client.HTTPClient(username=USERNAME, password=PASSWORD, + tenant_id=TENANT_ID, auth_url=AUTH_URL) + + (keyring_key, auth_ref) = cl.get_auth_ref_from_keyring(AUTH_URL, + USERNAME, + TENANT, + TENANT_ID, + TOKEN) + + self.assertIsNone(keyring_key) + self.assertIsNone(auth_ref) + + def test_build_keyring_key(self): + cl = client.HTTPClient(username=USERNAME, password=PASSWORD, + tenant_id=TENANT_ID, auth_url=AUTH_URL) + + keyring_key = cl._build_keyring_key(AUTH_URL, USERNAME, + TENANT, TENANT_ID, + TOKEN) + + self.assertEqual(keyring_key, + '%s/%s/%s/%s/%s' % + (AUTH_URL, USERNAME, TENANT, TENANT_ID, TOKEN)) + + def test_set_and_get_keyring_expired(self): + cl = client.HTTPClient(username=USERNAME, password=PASSWORD, + tenant_id=TENANT_ID, auth_url=AUTH_URL, + use_keyring=True) + keyring_key = cl._build_keyring_key(AUTH_URL, USERNAME, + TENANT, TENANT_ID, + TOKEN) + + cl.auth_ref = access.AccessInfo(PROJECT_SCOPED_TOKEN['access']) + expired = timeutils.utcnow() - datetime.timedelta(minutes=30) + cl.auth_ref['token']['expires'] = timeutils.isotime(expired) + cl.store_auth_ref_into_keyring(keyring_key) + (keyring_key, auth_ref) = cl.get_auth_ref_from_keyring(AUTH_URL, + USERNAME, + TENANT, + TENANT_ID, + TOKEN) + self.assertIsNone(auth_ref) + + def test_set_and_get_keyring(self): + cl = client.HTTPClient(username=USERNAME, password=PASSWORD, + tenant_id=TENANT_ID, auth_url=AUTH_URL, + use_keyring=True) + keyring_key = cl._build_keyring_key(AUTH_URL, USERNAME, + TENANT, TENANT_ID, + TOKEN) + + cl.auth_ref = access.AccessInfo(PROJECT_SCOPED_TOKEN['access']) + expires = timeutils.utcnow() + datetime.timedelta(minutes=30) + cl.auth_ref['token']['expires'] = timeutils.isotime(expires) + cl.store_auth_ref_into_keyring(keyring_key) + (keyring_key, auth_ref) = cl.get_auth_ref_from_keyring(AUTH_URL, + USERNAME, + TENANT, + TENANT_ID, + TOKEN) + self.assertEqual(auth_ref.auth_token, TOKEN) + self.assertEqual(auth_ref.username, USERNAME)