From 320fa59f13c9b31c96409f5fb38ecd76920e119e Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Fri, 21 Mar 2014 14:39:13 +1000 Subject: [PATCH] Remove _factory methods from auth plugins This was a simple factory that would give compatibility for the existing client to load up the appropriate auth plugin. A more robust plugin loading mechanism is coming for this and having it available encourages other auth plugins that they should be using that where they shouldn't. Just remove it from the auth plugin class. It shouldn't be used by anyone else so lets keep it on the client objects. Blueprint: plugin-params Change-Id: I0618b646f302300d41c7dd7153a1c0bdc237a745 --- keystoneclient/auth/identity/v2.py | 27 ------------ keystoneclient/auth/identity/v3.py | 27 ------------ keystoneclient/tests/auth/test_identity_v2.py | 4 -- keystoneclient/tests/auth/test_identity_v3.py | 4 -- keystoneclient/tests/v2_0/test_client.py | 6 +++ keystoneclient/tests/v3/test_client.py | 6 +++ keystoneclient/v2_0/client.py | 24 ++++++---- keystoneclient/v3/client.py | 44 ++++++++++++------- 8 files changed, 57 insertions(+), 85 deletions(-) diff --git a/keystoneclient/auth/identity/v2.py b/keystoneclient/auth/identity/v2.py index f362e8827..a421c6726 100644 --- a/keystoneclient/auth/identity/v2.py +++ b/keystoneclient/auth/identity/v2.py @@ -23,33 +23,6 @@ from keystoneclient import utils @six.add_metaclass(abc.ABCMeta) class Auth(base.BaseIdentityPlugin): - @staticmethod - def _factory(auth_url, **kwargs): - """Construct a plugin appropriate to your available arguments. - - This function should only be used for loading authentication from a - config file or other source where you do not know the type of plugin - that is required. - - If you know the style of authorization you require then you should - construct that plugin directly. - - :raises NoMatchingPlugin: if a plugin cannot be constructed. - - return Auth: a plugin that can be passed to a session. - """ - username = kwargs.pop('username', None) - password = kwargs.pop('password', None) - token = kwargs.pop('token', None) - - if token: - return Token(auth_url, token, **kwargs) - elif username and password: - return Password(auth_url, username, password, **kwargs) - - msg = 'A username and password or token is required.' - raise exceptions.NoMatchingPlugin(msg) - @utils.positional() def __init__(self, auth_url, trust_id=None, diff --git a/keystoneclient/auth/identity/v3.py b/keystoneclient/auth/identity/v3.py index 8df85fec3..50288a3ab 100644 --- a/keystoneclient/auth/identity/v3.py +++ b/keystoneclient/auth/identity/v3.py @@ -115,33 +115,6 @@ class Auth(base.BaseIdentityPlugin): return access.AccessInfoV3(resp.headers['X-Subject-Token'], **resp_data) - @staticmethod - def _factory(auth_url, **kwargs): - """Construct a plugin appropriate to your available arguments. - - This function is intended as a convenience and backwards compatibility. - If you know the style of authorization you require then you should - construct that plugin directly. - """ - - methods = [] - - # NOTE(jamielennox): kwargs extraction is outside the if statement to - # clear up additional args that might be passed but not valid for type. - method_kwargs = PasswordMethod._extract_kwargs(kwargs) - if method_kwargs.get('password'): - methods.append(PasswordMethod(**method_kwargs)) - - method_kwargs = TokenMethod._extract_kwargs(kwargs) - if method_kwargs.get('token'): - methods.append(TokenMethod(**method_kwargs)) - - if not methods: - msg = 'A username and password or token is required.' - raise exceptions.AuthorizationFailure(msg) - - return Auth(auth_url, methods, **kwargs) - @six.add_metaclass(abc.ABCMeta) class AuthMethod(object): diff --git a/keystoneclient/tests/auth/test_identity_v2.py b/keystoneclient/tests/auth/test_identity_v2.py index 4fbe6a625..f5578beac 100644 --- a/keystoneclient/tests/auth/test_identity_v2.py +++ b/keystoneclient/tests/auth/test_identity_v2.py @@ -142,10 +142,6 @@ class V2IdentityPlugin(utils.TestCase): self.assertRequestHeaderEqual('Accept', 'application/json') self.assertEqual(s.auth.auth_ref.auth_token, self.TEST_TOKEN) - def test_missing_auth_params(self): - self.assertRaises(exceptions.NoMatchingPlugin, v2.Auth._factory, - self.TEST_URL) - @httpretty.activate def test_with_trust_id(self): self.stub_auth(json=self.TEST_RESPONSE_DICT) diff --git a/keystoneclient/tests/auth/test_identity_v3.py b/keystoneclient/tests/auth/test_identity_v3.py index f271992cc..e17dc00fe 100644 --- a/keystoneclient/tests/auth/test_identity_v3.py +++ b/keystoneclient/tests/auth/test_identity_v3.py @@ -219,10 +219,6 @@ class V3IdentityPlugin(utils.TestCase): self.assertRequestHeaderEqual('Accept', 'application/json') self.assertEqual(s.auth.auth_ref.auth_token, self.TEST_TOKEN) - def test_missing_auth_params(self): - self.assertRaises(exceptions.AuthorizationFailure, v3.Auth._factory, - self.TEST_URL) - @httpretty.activate def test_with_expired(self): self.stub_auth(json=self.TEST_RESPONSE_DICT) diff --git a/keystoneclient/tests/v2_0/test_client.py b/keystoneclient/tests/v2_0/test_client.py index 294950aa0..fac0a5ba9 100644 --- a/keystoneclient/tests/v2_0/test_client.py +++ b/keystoneclient/tests/v2_0/test_client.py @@ -154,3 +154,9 @@ class KeystoneClientTest(utils.TestCase): region_name='South') self.assertEqual(cl.service_catalog.url_for(service_type='image'), 'https://image.south.host/v1/') + + def test_client_without_auth_params(self): + self.assertRaises(exceptions.AuthorizationFailure, + client.Client, + tenant_name='exampleproject', + auth_url=self.TEST_URL) diff --git a/keystoneclient/tests/v3/test_client.py b/keystoneclient/tests/v3/test_client.py index d225b95ef..cef8fbabe 100644 --- a/keystoneclient/tests/v3/test_client.py +++ b/keystoneclient/tests/v3/test_client.py @@ -199,3 +199,9 @@ class KeystoneClientTest(utils.TestCase): region_name='South') self.assertEqual(cl.service_catalog.url_for(service_type='image'), 'http://glance.south.host/glanceapi/public') + + def test_client_without_auth_params(self): + self.assertRaises(exceptions.AuthorizationFailure, + client.Client, + project_name='exampleproject', + auth_url=self.TEST_URL) diff --git a/keystoneclient/v2_0/client.py b/keystoneclient/v2_0/client.py index 8dcd778c7..0d4e68509 100644 --- a/keystoneclient/v2_0/client.py +++ b/keystoneclient/v2_0/client.py @@ -153,6 +153,9 @@ class Client(httpclient.HTTPClient): **kwargs): """Authenticate against the v2 Identity API. + If a token is provided it will be used in preference over username and + password. + :returns: access.AccessInfo if authentication was successful. :raises: AuthorizationFailure if unable to authenticate or validate the existing authorization token @@ -161,15 +164,20 @@ class Client(httpclient.HTTPClient): if auth_url is None: raise ValueError("Cannot authenticate without an auth_url") - a = v2_auth.Auth._factory(auth_url, - username=username, - password=password, - token=token, - trust_id=trust_id, - tenant_id=project_id or tenant_id, - tenant_name=project_name or tenant_name) + new_kwargs = {'trust_id': trust_id, + 'tenant_id': project_id or tenant_id, + 'tenant_name': project_name or tenant_name} - return a.get_auth_ref(self.session) + if token: + plugin = v2_auth.Token(auth_url, token, **new_kwargs) + elif username and password: + plugin = v2_auth.Password(auth_url, username, password, + **new_kwargs) + else: + msg = 'A username and password or token is required.' + raise exceptions.AuthorizationFailure(msg) + + return plugin.get_auth_ref(self.session) except (exceptions.AuthorizationFailure, exceptions.Unauthorized): _logger.debug("Authorization Failed.") raise diff --git a/keystoneclient/v3/client.py b/keystoneclient/v3/client.py index dc9bf9a22..995523019 100644 --- a/keystoneclient/v3/client.py +++ b/keystoneclient/v3/client.py @@ -151,6 +151,9 @@ class Client(httpclient.HTTPClient): **kwargs): """Authenticate against the v3 Identity API. + If password and token methods are both provided then both methods will + be used in the request. + :returns: access.AccessInfo if authentication was successful. :raises: AuthorizationFailure if unable to authenticate or validate the existing authorization token @@ -161,22 +164,33 @@ class Client(httpclient.HTTPClient): if auth_url is None: raise ValueError("Cannot authenticate without an auth_url") - a = v3_auth.Auth._factory(auth_url, - username=username, - password=password, - token=token, - trust_id=trust_id, - user_id=user_id, - domain_id=domain_id, - domain_name=domain_name, - user_domain_id=user_domain_id, - user_domain_name=user_domain_name, - project_id=project_id, - project_name=project_name, - project_domain_id=project_domain_id, - project_domain_name=project_domain_name) + auth_methods = [] - return a.get_auth_ref(self.session) + if token: + auth_methods.append(v3_auth.TokenMethod(token=token)) + + if password: + m = v3_auth.PasswordMethod(user_id=user_id, + username=username, + user_domain_id=user_domain_id, + user_domain_name=user_domain_name, + password=password) + auth_methods.append(m) + + if not auth_methods: + msg = 'A user and password or token is required.' + raise exceptions.AuthorizationFailure(msg) + + plugin = v3_auth.Auth(auth_url, auth_methods, + trust_id=trust_id, + domain_id=domain_id, + domain_name=domain_name, + project_id=project_id, + project_name=project_name, + project_domain_id=project_domain_id, + project_domain_name=project_domain_name) + + return plugin.get_auth_ref(self.session) except (exceptions.AuthorizationFailure, exceptions.Unauthorized): _logger.debug('Authorization failed.') raise