From 29aa87b1bf2878bc8c5a413226340d465339bf57 Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Wed, 22 Oct 2014 17:22:05 +0200 Subject: [PATCH] Use real discovery object in auth_token middleware. Auth token middleware has always done its own version of discovery. It is slightly more lenient than the one in keystoneclient however we should use the standard tools for these jobs. Move the handling of this discovery into the plugin. This is inline with how auth plugins work and will make it easy to replace with other plugins. Change-Id: I3c4ff5bed1aa76dfff8e445de5a384c3f7992bbe --- keystonemiddleware/auth_token.py | 112 +++++++++--------- .../tests/test_auth_token_middleware.py | 28 ++++- 2 files changed, 78 insertions(+), 62 deletions(-) diff --git a/keystonemiddleware/auth_token.py b/keystonemiddleware/auth_token.py index c62420cf..1f1eba67 100644 --- a/keystonemiddleware/auth_token.py +++ b/keystonemiddleware/auth_token.py @@ -185,6 +185,7 @@ from keystoneclient.auth.identity import base as base_identity from keystoneclient.auth.identity import v2 from keystoneclient.auth import token_endpoint from keystoneclient.common import cms +from keystoneclient import discover from keystoneclient import exceptions from keystoneclient import session from oslo.config import cfg @@ -569,24 +570,60 @@ class _AuthTokenPlugin(auth.BaseAuthPlugin): tenant_name=admin_tenant_name) self._LOG = log + self._discover = None def get_token(self, *args, **kwargs): return self._plugin.get_token(*args, **kwargs) def get_endpoint(self, session, interface=None, version=None, **kwargs): + """Return an endpoint for the client. + + There are no required keyword arguments to ``get_endpoint`` as a plugin + implementation should use best effort with the information available to + determine the endpoint. + + :param Session session: The session object that the auth_plugin + belongs to. + :param tuple version: The version number required for this endpoint. + :param str interface: what visibility the endpoint should have. + + :returns: The base URL that will be used to talk to the required + service or None if not available. + :rtype: string + """ if interface == auth.AUTH_INTERFACE: return self._identity_uri - url = self._identity_uri + if not version: + # NOTE(jamielennox): This plugin can only be used within auth_token + # and auth_token will always provide version= with requests. + return None - if version and version[0] == 2: - url += '/v2.0' - elif version and version[0] == 3: - url += '/v3' - else: - raise exceptions.EndpointNotFound() + if not self._discover: + self._discover = discover.Discover(session, + auth_url=self._identity_uri, + authenticated=False) - return url + if not self._discover.url_for(version): + # NOTE(jamielennox): The requested version is not supported by the + # identity server. + return None + + # NOTE(jamielennox): for backwards compatibility here we don't + # actually use the URL from discovery we hack it up instead. :( + # NOTE(jamielennox): matching the string version here matches the value + # from _LIST_OF_VERSIONS_TO_ATTEMPT. We can use the tuple only match + # in the future. + if version == 'v2.0' or version[0] == 2: + return '%s/v2.0' % self._identity_uri + elif version == 'v3.0' or version[0] == 3: + return '%s/v3' % self._identity_uri + + # NOTE(jamielennox): This plugin will only get called from auth_token + # middleware. The middleware should never request a version that the + # plugin doesn't know how to handle. + msg = _('Invalid version asked for in auth_token plugin') + raise NotImplementedError(msg) def invalidate(self): return self._plugin.invalidate() @@ -1536,57 +1573,20 @@ class _IdentityServer(object): # as possible in the way of letting auth_token talk to the # server. if self._req_auth_version: - version_to_use = self._req_auth_version self._LOG.info(_LI('Auth Token proceeding with requested %s apis'), - version_to_use) - else: - version_to_use = None - versions_supported_by_server = self._get_supported_versions() - if versions_supported_by_server: - for version in _LIST_OF_VERSIONS_TO_ATTEMPT: - if version in versions_supported_by_server: - version_to_use = version - break - if version_to_use: + self._req_auth_version) + return self._req_auth_version + + for auth_version in _LIST_OF_VERSIONS_TO_ATTEMPT: + if self._adapter.get_endpoint(version=auth_version): self._LOG.info(_LI('Auth Token confirmed use of %s apis'), - version_to_use) - else: - self._LOG.error( - _LE('Attempted versions [%(attempt)s] not in list ' - 'supported by server [%(supported)s]'), - {'attempt': ', '.join(_LIST_OF_VERSIONS_TO_ATTEMPT), - 'supported': ', '.join(versions_supported_by_server)}) - raise ServiceError(_('No compatible apis supported by server')) - return version_to_use + auth_version) + return auth_version - def _get_supported_versions(self): - versions = [] - response, data = self._json_request( - 'GET', '/', - authenticated=False, - endpoint_filter={'interface': auth.AUTH_INTERFACE}) - if response.status_code == 501: - self._LOG.warning(_LW('Old identity server found...assuming v2.0')) - versions.append('v2.0') - elif response.status_code != 300: - self._LOG.error( - _LE('Unable to get version info from identity server: %s'), - response.status_code) - raise ServiceError( - _('Unable to get version info from identity server')) - else: - try: - for version in data['versions']['values']: - versions.append(version['id']) - except KeyError: - self._LOG.error( - _LE('Invalid version response format from server')) - raise ServiceError(_('Unable to parse version response ' - 'from identity server.')) - - self._LOG.debug('Server reports support for api versions: %s', - ', '.join(versions)) - return versions + self._LOG.error(_LE('No attempted versions [%s] supported by server') % + ', '.join(_LIST_OF_VERSIONS_TO_ATTEMPT)) + msg = _('No compatible apis supported by server') + raise ServiceError(msg) def _json_request(self, method, path, **kwargs): """HTTP request helper used to make json requests. diff --git a/keystonemiddleware/tests/test_auth_token_middleware.py b/keystonemiddleware/tests/test_auth_token_middleware.py index dbc3f673..60fbf170 100644 --- a/keystonemiddleware/tests/test_auth_token_middleware.py +++ b/keystonemiddleware/tests/test_auth_token_middleware.py @@ -360,7 +360,7 @@ class DiabloAuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest, expected_env=expected_env) self.requests.register_uri('GET', - "%s/" % BASE_URI, + BASE_URI, json=VERSION_LIST_v2, status_code=300) @@ -1385,6 +1385,12 @@ class V2CertDownloadMiddlewareTest(BaseAuthTokenMiddlewareTest, 'signing_dir': self.cert_dir, 'auth_version': self.auth_version, } + + self.requests.register_uri('GET', + BASE_URI, + json=VERSION_LIST_v3, + status_code=300) + self.set_middleware(conf=conf) # Usually we supply a signed_dir with pre-installed certificates, @@ -1433,9 +1439,14 @@ class V2CertDownloadMiddlewareTest(BaseAuthTokenMiddlewareTest, self.conf['auth_port'] = '1234' self.conf['auth_admin_prefix'] = '/newadmin/' - ca_url = "%s/newadmin%s" % (BASE_HOST, self.ca_path) - signing_url = "%s/newadmin%s" % (BASE_HOST, self.signing_path) + base_url = '%s/newadmin' % BASE_HOST + ca_url = "%s%s" % (base_url, self.ca_path) + signing_url = "%s%s" % (base_url, self.signing_path) + self.requests.register_uri('GET', + base_url, + json=VERSION_LIST_v3, + status_code=300) self.requests.register_uri('GET', ca_url, text='FAKECA') self.requests.register_uri('GET', signing_url, text='FAKECERT') @@ -1456,6 +1467,11 @@ class V2CertDownloadMiddlewareTest(BaseAuthTokenMiddlewareTest, ca_url = "%s%s" % (BASE_HOST, self.ca_path) signing_url = "%s%s" % (BASE_HOST, self.signing_path) + + self.requests.register_uri('GET', + BASE_HOST, + json=VERSION_LIST_v3, + status_code=300) self.requests.register_uri('GET', ca_url, text='FAKECA') self.requests.register_uri('GET', signing_url, text='FAKECERT') @@ -1528,7 +1544,7 @@ class v2AuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest, } self.requests.register_uri('GET', - "%s/" % BASE_URI, + BASE_URI, json=VERSION_LIST_v2, status_code=300) @@ -2282,7 +2298,7 @@ class v2CompositeAuthTests(BaseAuthTokenMiddlewareTest, } self.requests.register_uri('GET', - "%s/" % BASE_URI, + BASE_URI, json=VERSION_LIST_v2, status_code=300) @@ -2339,7 +2355,7 @@ class v3CompositeAuthTests(BaseAuthTokenMiddlewareTest, } self.requests.register_uri('GET', - "%s" % BASE_URI, + BASE_URI, json=VERSION_LIST_v3, status_code=300)