From 41e1bd0be64e15a5e0c12b45bdf3dcde5fabf244 Mon Sep 17 00:00:00 2001 From: guang-yee Date: Mon, 8 Feb 2016 11:16:24 -0800 Subject: [PATCH] Support unscoped token request Make scope check optional for the "token issue" command as unscoped token is a valid Keystone V2/V3 API. Change-Id: Ie1cded4dbfdafd3a78c0ebdf89e3f66762509930 Closes-Bug: #1543214 --- openstackclient/api/auth.py | 11 +++++--- openstackclient/common/clientmanager.py | 22 ++++++++++++++-- openstackclient/identity/v2_0/token.py | 6 ++++- openstackclient/identity/v3/token.py | 3 +++ openstackclient/shell.py | 3 +++ .../tests/common/test_clientmanager.py | 25 +++++++++++++++++++ openstackclient/tests/identity/v2_0/fakes.py | 6 +++++ .../tests/identity/v2_0/test_token.py | 22 ++++++++++++++++ openstackclient/tests/identity/v3/fakes.py | 6 +++++ .../tests/identity/v3/test_token.py | 21 ++++++++++++++++ .../notes/bug-1543214-959aee7830db2b0d.yaml | 6 +++++ 11 files changed, 125 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/bug-1543214-959aee7830db2b0d.yaml diff --git a/openstackclient/api/auth.py b/openstackclient/api/auth.py index 66272e424..2cde7d400 100644 --- a/openstackclient/api/auth.py +++ b/openstackclient/api/auth.py @@ -135,8 +135,12 @@ def build_auth_params(auth_plugin_name, cmd_options): return (auth_plugin_class, auth_params) -def check_valid_auth_options(options, auth_plugin_name): - """Perform basic option checking, provide helpful error messages""" +def check_valid_auth_options(options, auth_plugin_name, required_scope=True): + """Perform basic option checking, provide helpful error messages. + + :param required_scope: indicate whether a scoped token is required + + """ msg = '' if auth_plugin_name.endswith('password'): @@ -146,7 +150,8 @@ def check_valid_auth_options(options, auth_plugin_name): if not options.auth.get('auth_url', None): msg += _('Set an authentication URL, with --os-auth-url,' ' OS_AUTH_URL or auth.auth_url\n') - if (not options.auth.get('project_id', None) and not + if (required_scope and not + options.auth.get('project_id', None) and not options.auth.get('domain_id', None) and not options.auth.get('domain_name', None) and not options.auth.get('project_name', None) and not diff --git a/openstackclient/common/clientmanager.py b/openstackclient/common/clientmanager.py index dce197251..48b3aca57 100644 --- a/openstackclient/common/clientmanager.py +++ b/openstackclient/common/clientmanager.py @@ -113,19 +113,35 @@ class ClientManager(object): root_logger = logging.getLogger('') LOG.setLevel(root_logger.getEffectiveLevel()) - def setup_auth(self): + # NOTE(gyee): use this flag to indicate whether auth setup has already + # been completed. If so, do not perform auth setup again. The reason + # we need this flag is that we want to be able to perform auth setup + # outside of auth_ref as auth_ref itself is a property. We can not + # retrofit auth_ref to optionally skip scope check. Some operations + # do not require a scoped token. In those cases, we call setup_auth + # prior to dereferrencing auth_ref. + self._auth_setup_completed = False + + def setup_auth(self, required_scope=True): """Set up authentication + :param required_scope: indicate whether a scoped token is required + This is deferred until authentication is actually attempted because it gets in the way of things that do not require auth. """ + if self._auth_setup_completed: + return + # If no auth type is named by the user, select one based on # the supplied options self.auth_plugin_name = auth.select_auth_plugin(self._cli_options) # Basic option checking to avoid unhelpful error messages - auth.check_valid_auth_options(self._cli_options, self.auth_plugin_name) + auth.check_valid_auth_options(self._cli_options, + self.auth_plugin_name, + required_scope=required_scope) # Horrible hack alert...must handle prompt for null password if # password auth is requested. @@ -180,6 +196,8 @@ class ClientManager(object): user_agent=USER_AGENT, ) + self._auth_setup_completed = True + return @property diff --git a/openstackclient/identity/v2_0/token.py b/openstackclient/identity/v2_0/token.py index db38fae8e..6a66a1c6d 100644 --- a/openstackclient/identity/v2_0/token.py +++ b/openstackclient/identity/v2_0/token.py @@ -24,6 +24,9 @@ from openstackclient.i18n import _ # noqa class IssueToken(command.ShowOne): """Issue new token""" + # scoped token is optional + required_scope = False + def get_parser(self, prog_name): parser = super(IssueToken, self).get_parser(prog_name) return parser @@ -31,7 +34,8 @@ class IssueToken(command.ShowOne): def take_action(self, parsed_args): token = self.app.client_manager.auth_ref.service_catalog.get_token() - token['project_id'] = token.pop('tenant_id') + if 'tenant_id' in token: + token['project_id'] = token.pop('tenant_id') return zip(*sorted(six.iteritems(token))) diff --git a/openstackclient/identity/v3/token.py b/openstackclient/identity/v3/token.py index 9ebd17995..5f131939c 100644 --- a/openstackclient/identity/v3/token.py +++ b/openstackclient/identity/v3/token.py @@ -164,6 +164,9 @@ class CreateRequestToken(command.ShowOne): class IssueToken(command.ShowOne): """Issue new token""" + # scoped token is optional + required_scope = False + def get_parser(self, prog_name): parser = super(IssueToken, self).get_parser(prog_name) return parser diff --git a/openstackclient/shell.py b/openstackclient/shell.py index 137446efb..dfec40af3 100644 --- a/openstackclient/shell.py +++ b/openstackclient/shell.py @@ -353,6 +353,9 @@ class OpenStackShell(app.App): cmd.__class__.__name__, ) if cmd.auth_required: + if hasattr(cmd, 'required_scope'): + # let the command decide whether we need a scoped token + self.client_manager.setup_auth(cmd.required_scope) # Trigger the Identity client to initialize self.client_manager.auth_ref return diff --git a/openstackclient/tests/common/test_clientmanager.py b/openstackclient/tests/common/test_clientmanager.py index 523f79a32..ef46f61c5 100644 --- a/openstackclient/tests/common/test_clientmanager.py +++ b/openstackclient/tests/common/test_clientmanager.py @@ -325,3 +325,28 @@ class TestClientManager(utils.TestCase): exc.CommandError, client_manager.setup_auth, ) + + @mock.patch('openstackclient.api.auth.check_valid_auth_options') + def test_client_manager_auth_setup_once(self, check_auth_options_func): + client_manager = clientmanager.ClientManager( + cli_options=FakeOptions( + auth=dict( + auth_url=fakes.AUTH_URL, + username=fakes.USERNAME, + password=fakes.PASSWORD, + project_name=fakes.PROJECT_NAME, + ), + ), + api_version=API_VERSION, + verify=False, + ) + self.assertFalse(client_manager._auth_setup_completed) + client_manager.setup_auth() + self.assertTrue(check_auth_options_func.called) + self.assertTrue(client_manager._auth_setup_completed) + + # now make sure we don't do auth setup the second time around + # by checking whether check_valid_auth_options() gets called again + check_auth_options_func.reset_mock() + client_manager.auth_ref + check_auth_options_func.assert_not_called() diff --git a/openstackclient/tests/identity/v2_0/fakes.py b/openstackclient/tests/identity/v2_0/fakes.py index 6688606a4..565606c16 100644 --- a/openstackclient/tests/identity/v2_0/fakes.py +++ b/openstackclient/tests/identity/v2_0/fakes.py @@ -80,6 +80,12 @@ TOKEN = { 'user_id': user_id, } +UNSCOPED_TOKEN = { + 'expires': token_expires, + 'id': token_id, + 'user_id': user_id, +} + endpoint_name = service_name endpoint_adminurl = 'https://admin.example.com/v2/UUID' endpoint_region = 'RegionOne' diff --git a/openstackclient/tests/identity/v2_0/test_token.py b/openstackclient/tests/identity/v2_0/test_token.py index 7687a063f..c90477f94 100644 --- a/openstackclient/tests/identity/v2_0/test_token.py +++ b/openstackclient/tests/identity/v2_0/test_token.py @@ -60,6 +60,28 @@ class TestTokenIssue(TestToken): ) self.assertEqual(datalist, data) + def test_token_issue_with_unscoped_token(self): + # make sure we return an unscoped token + self.sc_mock.get_token.return_value = identity_fakes.UNSCOPED_TOKEN + + arglist = [] + verifylist = [] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + # DisplayCommandBase.take_action() returns two tuples + columns, data = self.cmd.take_action(parsed_args) + + self.sc_mock.get_token.assert_called_with() + + collist = ('expires', 'id', 'user_id') + self.assertEqual(collist, columns) + datalist = ( + identity_fakes.token_expires, + identity_fakes.token_id, + identity_fakes.user_id, + ) + self.assertEqual(datalist, data) + class TestTokenRevoke(TestToken): diff --git a/openstackclient/tests/identity/v3/fakes.py b/openstackclient/tests/identity/v3/fakes.py index a06802c56..420604f18 100644 --- a/openstackclient/tests/identity/v3/fakes.py +++ b/openstackclient/tests/identity/v3/fakes.py @@ -244,6 +244,12 @@ TRUST = { token_expires = '2014-01-01T00:00:00Z' token_id = 'tttttttt-tttt-tttt-tttt-tttttttttttt' +UNSCOPED_TOKEN = { + 'expires': token_expires, + 'id': token_id, + 'user_id': user_id, +} + TOKEN_WITH_PROJECT_ID = { 'expires': token_expires, 'id': token_id, diff --git a/openstackclient/tests/identity/v3/test_token.py b/openstackclient/tests/identity/v3/test_token.py index b051aacbf..80c397bcc 100644 --- a/openstackclient/tests/identity/v3/test_token.py +++ b/openstackclient/tests/identity/v3/test_token.py @@ -85,6 +85,27 @@ class TestTokenIssue(TestToken): ) self.assertEqual(datalist, data) + def test_token_issue_with_unscoped(self): + arglist = [] + verifylist = [] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.sc_mock.get_token.return_value = \ + identity_fakes.UNSCOPED_TOKEN + + # DisplayCommandBase.take_action() returns two tuples + columns, data = self.cmd.take_action(parsed_args) + + self.sc_mock.get_token.assert_called_with() + + collist = ('expires', 'id', 'user_id') + self.assertEqual(collist, columns) + datalist = ( + identity_fakes.token_expires, + identity_fakes.token_id, + identity_fakes.user_id, + ) + self.assertEqual(datalist, data) + class TestTokenRevoke(TestToken): diff --git a/releasenotes/notes/bug-1543214-959aee7830db2b0d.yaml b/releasenotes/notes/bug-1543214-959aee7830db2b0d.yaml new file mode 100644 index 000000000..e228480bb --- /dev/null +++ b/releasenotes/notes/bug-1543214-959aee7830db2b0d.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + The ``token issue`` can now return an unscoped token. If a `project` or `domain` + target scope are not specified, an unscoped token will be returned. + [Bug `1543214 `_]