From 70ab3f9dd56a638cdff516ca85baa5ebd64c888b Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Wed, 21 Aug 2019 17:38:29 -0700 Subject: [PATCH] Add support for app cred access rules This commit introduces the --access-rules option for 'application credential create' as well as new 'access rule' commands for listing, showing, and deleting access rules. bp whitelist-extension-for-app-creds Change-Id: I04834b2874ec2a70da456a380b5bef03a392effa --- .../cli/command-objects/access-rules.rst | 61 ++++++ .../application-credentials.rst | 7 + lower-constraints.txt | 2 +- openstackclient/identity/v3/access_rule.py | 118 ++++++++++++ .../identity/v3/application_credential.py | 27 +++ .../tests/unit/identity/v3/fakes.py | 33 +++- .../unit/identity/v3/test_access_rule.py | 174 ++++++++++++++++++ .../v3/test_application_credential.py | 125 ++++++++++++- ...ension-for-app-creds-9afd5009b374190b.yaml | 6 + requirements.txt | 2 +- setup.cfg | 4 + 11 files changed, 548 insertions(+), 11 deletions(-) create mode 100644 doc/source/cli/command-objects/access-rules.rst create mode 100644 openstackclient/identity/v3/access_rule.py create mode 100644 openstackclient/tests/unit/identity/v3/test_access_rule.py create mode 100644 releasenotes/notes/bp-whitelist-extension-for-app-creds-9afd5009b374190b.yaml diff --git a/doc/source/cli/command-objects/access-rules.rst b/doc/source/cli/command-objects/access-rules.rst new file mode 100644 index 0000000000..bc8458283f --- /dev/null +++ b/doc/source/cli/command-objects/access-rules.rst @@ -0,0 +1,61 @@ +=========== +access rule +=========== + +Identity v3 + +Access rules are fine-grained permissions for application credentials. An access +rule comprises of a service type, a request path, and a request method. Access +rules may only be created as attributes of application credentials, but they may +be viewed and deleted independently. + + +access rule delete +------------------ + +Delete access rule(s) + +.. program:: access rule delete +.. code:: bash + + openstack access rule delete [ ...] + +.. describe:: + + Access rule(s) to delete (ID) + +access rule list +---------------- + +List access rules + +.. program:: access rule list +.. code:: bash + + openstack access rule list + [--user ] + [--user-domain ] + +.. option:: --user + + User whose access rules to list (name or ID). If not provided, looks up the + current user's access rules. + +.. option:: --user-domain + + Domain the user belongs to (name or ID). This can be + used in case collisions between user names exist. + +access rule show +--------------------------- + +Display access rule details + +.. program:: access rule show +.. code:: bash + + openstack access rule show + +.. describe:: + + Access rule to display (ID) diff --git a/doc/source/cli/command-objects/application-credentials.rst b/doc/source/cli/command-objects/application-credentials.rst index 2a1fbff25e..047f5ab661 100644 --- a/doc/source/cli/command-objects/application-credentials.rst +++ b/doc/source/cli/command-objects/application-credentials.rst @@ -22,6 +22,7 @@ Create new application credential [--expiration ] [--description ] [--restricted|--unrestricted] + [--access-rules ] .. option:: --secret @@ -52,6 +53,12 @@ Create new application credential Prohibit application credential from creating and deleting other application credentials and trusts (this is the default behavior) +.. option:: --access-rules + + Either a string or file path containing a JSON-formatted list of access + rules, each containing a request method, path, and service, for example + '[{"method": "GET", "path": "/v2.1/servers", "service": "compute"}]' + .. describe:: Name of the application credential diff --git a/lower-constraints.txt b/lower-constraints.txt index 91a543a617..0e00a11488 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -95,7 +95,7 @@ python-heatclient==1.10.0 python-ironic-inspector-client==1.5.0 python-ironicclient==2.3.0 python-karborclient==0.6.0 -python-keystoneclient==3.17.0 +python-keystoneclient==3.22.0 python-mimeparse==1.6.0 python-mistralclient==3.1.0 python-muranoclient==0.8.2 diff --git a/openstackclient/identity/v3/access_rule.py b/openstackclient/identity/v3/access_rule.py new file mode 100644 index 0000000000..d96b44daf9 --- /dev/null +++ b/openstackclient/identity/v3/access_rule.py @@ -0,0 +1,118 @@ +# Copyright 2019 SUSE LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +"""Identity v3 Access Rule action implementations""" + +import logging + +from osc_lib.command import command +from osc_lib import exceptions +from osc_lib import utils +import six + +from openstackclient.i18n import _ +from openstackclient.identity import common + + +LOG = logging.getLogger(__name__) + + +class DeleteAccessRule(command.Command): + _description = _("Delete access rule(s)") + + def get_parser(self, prog_name): + parser = super(DeleteAccessRule, self).get_parser(prog_name) + parser.add_argument( + 'access_rule', + metavar='', + nargs="+", + help=_('Application credentials(s) to delete (name or ID)'), + ) + return parser + + def take_action(self, parsed_args): + identity_client = self.app.client_manager.identity + + errors = 0 + for ac in parsed_args.access_rule: + try: + access_rule = utils.find_resource( + identity_client.access_rules, ac) + identity_client.access_rules.delete(access_rule.id) + except Exception as e: + errors += 1 + LOG.error(_("Failed to delete access rule with " + "ID '%(ac)s': %(e)s"), + {'ac': ac, 'e': e}) + + if errors > 0: + total = len(parsed_args.access_rule) + msg = (_("%(errors)s of %(total)s access rules failed " + "to delete.") % {'errors': errors, 'total': total}) + raise exceptions.CommandError(msg) + + +class ListAccessRule(command.Lister): + _description = _("List access rules") + + def get_parser(self, prog_name): + parser = super(ListAccessRule, self).get_parser(prog_name) + parser.add_argument( + '--user', + metavar='', + help=_('User whose access rules to list (name or ID)'), + ) + common.add_user_domain_option_to_parser(parser) + return parser + + def take_action(self, parsed_args): + identity_client = self.app.client_manager.identity + if parsed_args.user: + user_id = common.find_user(identity_client, + parsed_args.user, + parsed_args.user_domain).id + else: + user_id = None + + columns = ('ID', 'Service', 'Method', 'Path') + data = identity_client.access_rules.list( + user=user_id) + return (columns, + (utils.get_item_properties( + s, columns, + formatters={}, + ) for s in data)) + + +class ShowAccessRule(command.ShowOne): + _description = _("Display access rule details") + + def get_parser(self, prog_name): + parser = super(ShowAccessRule, self).get_parser(prog_name) + parser.add_argument( + 'access_rule', + metavar='', + help=_('Application credential to display (name or ID)'), + ) + return parser + + def take_action(self, parsed_args): + identity_client = self.app.client_manager.identity + access_rule = utils.find_resource(identity_client.access_rules, + parsed_args.access_rule) + + access_rule._info.pop('links', None) + + return zip(*sorted(six.iteritems(access_rule._info))) diff --git a/openstackclient/identity/v3/application_credential.py b/openstackclient/identity/v3/application_credential.py index ea0b30cdbb..a208985624 100644 --- a/openstackclient/identity/v3/application_credential.py +++ b/openstackclient/identity/v3/application_credential.py @@ -16,6 +16,7 @@ """Identity v3 Application Credential action implementations""" import datetime +import json import logging from osc_lib.command import command @@ -79,6 +80,17 @@ class CreateApplicationCredential(command.ShowOne): ' other application credentials and trusts (this is the' ' default behavior)'), ) + parser.add_argument( + '--access-rules', + metavar='', + help=_('Either a string or file path containing a JSON-formatted ' + 'list of access rules, each containing a request method, ' + 'path, and service, for example ' + '\'[{"method": "GET", ' + '"path": "/v2.1/servers", ' + '"service": "compute"}]\''), + + ) return parser def take_action(self, parsed_args): @@ -105,6 +117,20 @@ class CreateApplicationCredential(command.ShowOne): else: unrestricted = parsed_args.unrestricted + if parsed_args.access_rules: + try: + access_rules = json.loads(parsed_args.access_rules) + except ValueError: + try: + with open(parsed_args.access_rules) as f: + access_rules = json.load(f) + except IOError: + raise exceptions.CommandError( + _("Access rules is not valid JSON string or file does" + " not exist.")) + else: + access_rules = None + app_cred_manager = identity_client.application_credentials application_credential = app_cred_manager.create( parsed_args.name, @@ -113,6 +139,7 @@ class CreateApplicationCredential(command.ShowOne): description=parsed_args.description, secret=parsed_args.secret, unrestricted=unrestricted, + access_rules=access_rules, ) application_credential._info.pop('links', None) diff --git a/openstackclient/tests/unit/identity/v3/fakes.py b/openstackclient/tests/unit/identity/v3/fakes.py index c394ab8237..fc4a48e37f 100644 --- a/openstackclient/tests/unit/identity/v3/fakes.py +++ b/openstackclient/tests/unit/identity/v3/fakes.py @@ -470,6 +470,14 @@ app_cred_description = 'app credential for testing' app_cred_expires = datetime.datetime(2022, 1, 1, 0, 0) app_cred_expires_str = app_cred_expires.strftime('%Y-%m-%dT%H:%M:%S%z') app_cred_secret = 'moresecuresecret' +app_cred_access_rules = ( + '[{"path": "/v2.1/servers", "method": "GET", "service": "compute"}]' +) +app_cred_access_rules_path = '/tmp/access_rules.json' +access_rule_id = 'access-rule-id' +access_rule_service = 'compute' +access_rule_path = '/v2.1/servers' +access_rule_method = 'GET' APP_CRED_BASIC = { 'id': app_cred_id, 'name': app_cred_name, @@ -478,7 +486,8 @@ APP_CRED_BASIC = { 'description': None, 'expires_at': None, 'unrestricted': False, - 'secret': app_cred_secret + 'secret': app_cred_secret, + 'access_rules': None } APP_CRED_OPTIONS = { 'id': app_cred_id, @@ -488,7 +497,25 @@ APP_CRED_OPTIONS = { 'description': app_cred_description, 'expires_at': app_cred_expires_str, 'unrestricted': False, - 'secret': app_cred_secret + 'secret': app_cred_secret, + 'access_rules': None, +} +ACCESS_RULE = { + 'id': access_rule_id, + 'service': access_rule_service, + 'path': access_rule_path, + 'method': access_rule_method, +} +APP_CRED_ACCESS_RULES = { + 'id': app_cred_id, + 'name': app_cred_name, + 'project_id': project_id, + 'roles': app_cred_role, + 'description': None, + 'expires_at': None, + 'unrestricted': False, + 'secret': app_cred_secret, + 'access_rules': app_cred_access_rules } registered_limit_id = 'registered-limit-id' @@ -625,6 +652,8 @@ class FakeIdentityv3Client(object): self.application_credentials = mock.Mock() self.application_credentials.resource_class = fakes.FakeResource(None, {}) + self.access_rules = mock.Mock() + self.access_rules.resource_class = fakes.FakeResource(None, {}) self.inference_rules = mock.Mock() self.inference_rules.resource_class = fakes.FakeResource(None, {}) self.registered_limits = mock.Mock() diff --git a/openstackclient/tests/unit/identity/v3/test_access_rule.py b/openstackclient/tests/unit/identity/v3/test_access_rule.py new file mode 100644 index 0000000000..f8b6093a6c --- /dev/null +++ b/openstackclient/tests/unit/identity/v3/test_access_rule.py @@ -0,0 +1,174 @@ +# Copyright 2019 SUSE LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +import copy + +import mock +from osc_lib import exceptions +from osc_lib import utils + +from openstackclient.identity.v3 import access_rule +from openstackclient.tests.unit import fakes +from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes + + +class TestAccessRule(identity_fakes.TestIdentityv3): + + def setUp(self): + super(TestAccessRule, self).setUp() + + identity_manager = self.app.client_manager.identity + self.access_rules_mock = identity_manager.access_rules + self.access_rules_mock.reset_mock() + self.roles_mock = identity_manager.roles + self.roles_mock.reset_mock() + + +class TestAccessRuleDelete(TestAccessRule): + + def setUp(self): + super(TestAccessRuleDelete, self).setUp() + + # This is the return value for utils.find_resource() + self.access_rules_mock.get.return_value = fakes.FakeResource( + None, + copy.deepcopy(identity_fakes.ACCESS_RULE), + loaded=True, + ) + self.access_rules_mock.delete.return_value = None + + # Get the command object to test + self.cmd = access_rule.DeleteAccessRule( + self.app, None) + + def test_access_rule_delete(self): + arglist = [ + identity_fakes.access_rule_id, + ] + verifylist = [ + ('access_rule', [identity_fakes.access_rule_id]) + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + self.access_rules_mock.delete.assert_called_with( + identity_fakes.access_rule_id, + ) + self.assertIsNone(result) + + @mock.patch.object(utils, 'find_resource') + def test_delete_multi_access_rules_with_exception(self, find_mock): + find_mock.side_effect = [self.access_rules_mock.get.return_value, + exceptions.CommandError] + arglist = [ + identity_fakes.access_rule_id, + 'nonexistent_access_rule', + ] + verifylist = [ + ('access_rule', arglist), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + try: + self.cmd.take_action(parsed_args) + self.fail('CommandError should be raised.') + except exceptions.CommandError as e: + self.assertEqual('1 of 2 access rules failed to' + ' delete.', str(e)) + + find_mock.assert_any_call(self.access_rules_mock, + identity_fakes.access_rule_id) + find_mock.assert_any_call(self.access_rules_mock, + 'nonexistent_access_rule') + + self.assertEqual(2, find_mock.call_count) + self.access_rules_mock.delete.assert_called_once_with( + identity_fakes.access_rule_id) + + +class TestAccessRuleList(TestAccessRule): + + def setUp(self): + super(TestAccessRuleList, self).setUp() + + self.access_rules_mock.list.return_value = [ + fakes.FakeResource( + None, + copy.deepcopy(identity_fakes.ACCESS_RULE), + loaded=True, + ), + ] + + # Get the command object to test + self.cmd = access_rule.ListAccessRule(self.app, None) + + def test_access_rule_list(self): + arglist = [] + verifylist = [] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.access_rules_mock.list.assert_called_with(user=None) + + collist = ('ID', 'Service', 'Method', 'Path') + self.assertEqual(collist, columns) + datalist = (( + identity_fakes.access_rule_id, + identity_fakes.access_rule_service, + identity_fakes.access_rule_method, + identity_fakes.access_rule_path, + ), ) + self.assertEqual(datalist, tuple(data)) + + +class TestAccessRuleShow(TestAccessRule): + + def setUp(self): + super(TestAccessRuleShow, self).setUp() + + self.access_rules_mock.get.return_value = fakes.FakeResource( + None, + copy.deepcopy(identity_fakes.ACCESS_RULE), + loaded=True, + ) + + # Get the command object to test + self.cmd = access_rule.ShowAccessRule(self.app, None) + + def test_access_rule_show(self): + arglist = [ + identity_fakes.access_rule_id, + ] + verifylist = [ + ('access_rule', identity_fakes.access_rule_id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.access_rules_mock.get.assert_called_with( + identity_fakes.access_rule_id) + + collist = ('id', 'method', 'path', 'service') + self.assertEqual(collist, columns) + datalist = ( + identity_fakes.access_rule_id, + identity_fakes.access_rule_method, + identity_fakes.access_rule_path, + identity_fakes.access_rule_service, + ) + self.assertEqual(datalist, data) diff --git a/openstackclient/tests/unit/identity/v3/test_application_credential.py b/openstackclient/tests/unit/identity/v3/test_application_credential.py index 163aae9db9..24bafc9f5f 100644 --- a/openstackclient/tests/unit/identity/v3/test_application_credential.py +++ b/openstackclient/tests/unit/identity/v3/test_application_credential.py @@ -14,6 +14,7 @@ # import copy +import json from unittest import mock from osc_lib import exceptions @@ -79,16 +80,18 @@ class TestApplicationCredentialCreate(TestApplicationCredential): 'expires_at': None, 'description': None, 'unrestricted': False, + 'access_rules': None, } self.app_creds_mock.create.assert_called_with( name, **kwargs ) - collist = ('description', 'expires_at', 'id', 'name', 'project_id', - 'roles', 'secret', 'unrestricted') + collist = ('access_rules', 'description', 'expires_at', 'id', 'name', + 'project_id', 'roles', 'secret', 'unrestricted') self.assertEqual(collist, columns) datalist = ( + None, None, None, identity_fakes.app_cred_id, @@ -135,17 +138,19 @@ class TestApplicationCredentialCreate(TestApplicationCredential): 'roles': [identity_fakes.role_id], 'expires_at': identity_fakes.app_cred_expires, 'description': 'credential for testing', - 'unrestricted': False + 'unrestricted': False, + 'access_rules': None, } self.app_creds_mock.create.assert_called_with( name, **kwargs ) - collist = ('description', 'expires_at', 'id', 'name', 'project_id', - 'roles', 'secret', 'unrestricted') + collist = ('access_rules', 'description', 'expires_at', 'id', 'name', + 'project_id', 'roles', 'secret', 'unrestricted') self.assertEqual(collist, columns) datalist = ( + None, identity_fakes.app_cred_description, identity_fakes.app_cred_expires_str, identity_fakes.app_cred_id, @@ -157,6 +162,111 @@ class TestApplicationCredentialCreate(TestApplicationCredential): ) self.assertEqual(datalist, data) + def test_application_credential_create_with_access_rules_string(self): + name = identity_fakes.app_cred_name + self.app_creds_mock.create.return_value = fakes.FakeResource( + None, + copy.deepcopy(identity_fakes.APP_CRED_ACCESS_RULES), + loaded=True, + ) + + arglist = [ + name, + '--access-rules', identity_fakes.app_cred_access_rules, + ] + verifylist = [ + ('name', identity_fakes.app_cred_name), + ('access_rules', identity_fakes.app_cred_access_rules), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'secret': None, + 'roles': [], + 'expires_at': None, + 'description': None, + 'unrestricted': False, + 'access_rules': json.loads(identity_fakes.app_cred_access_rules) + } + self.app_creds_mock.create.assert_called_with( + name, + **kwargs + ) + + collist = ('access_rules', 'description', 'expires_at', 'id', 'name', + 'project_id', 'roles', 'secret', 'unrestricted') + self.assertEqual(collist, columns) + datalist = ( + identity_fakes.app_cred_access_rules, + None, + None, + identity_fakes.app_cred_id, + identity_fakes.app_cred_name, + identity_fakes.project_id, + identity_fakes.role_name, + identity_fakes.app_cred_secret, + False, + ) + self.assertEqual(datalist, data) + + @mock.patch('openstackclient.identity.v3.application_credential.json.load') + @mock.patch('openstackclient.identity.v3.application_credential.open') + def test_application_credential_create_with_access_rules_file( + self, _, mock_json_load): + mock_json_load.return_value = identity_fakes.app_cred_access_rules + + name = identity_fakes.app_cred_name + self.app_creds_mock.create.return_value = fakes.FakeResource( + None, + copy.deepcopy(identity_fakes.APP_CRED_ACCESS_RULES), + loaded=True, + ) + + arglist = [ + name, + '--access-rules', identity_fakes.app_cred_access_rules_path, + ] + verifylist = [ + ('name', identity_fakes.app_cred_name), + ('access_rules', identity_fakes.app_cred_access_rules_path), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'secret': None, + 'roles': [], + 'expires_at': None, + 'description': None, + 'unrestricted': False, + 'access_rules': identity_fakes.app_cred_access_rules + } + self.app_creds_mock.create.assert_called_with( + name, + **kwargs + ) + + collist = ('access_rules', 'description', 'expires_at', 'id', 'name', + 'project_id', 'roles', 'secret', 'unrestricted') + self.assertEqual(collist, columns) + datalist = ( + identity_fakes.app_cred_access_rules, + None, + None, + identity_fakes.app_cred_id, + identity_fakes.app_cred_name, + identity_fakes.project_id, + identity_fakes.role_name, + identity_fakes.app_cred_secret, + False, + ) + self.assertEqual(datalist, data) + class TestApplicationCredentialDelete(TestApplicationCredential): @@ -293,10 +403,11 @@ class TestApplicationCredentialShow(TestApplicationCredential): self.app_creds_mock.get.assert_called_with(identity_fakes.app_cred_id) - collist = ('description', 'expires_at', 'id', 'name', 'project_id', - 'roles', 'secret', 'unrestricted') + collist = ('access_rules', 'description', 'expires_at', 'id', 'name', + 'project_id', 'roles', 'secret', 'unrestricted') self.assertEqual(collist, columns) datalist = ( + None, None, None, identity_fakes.app_cred_id, diff --git a/releasenotes/notes/bp-whitelist-extension-for-app-creds-9afd5009b374190b.yaml b/releasenotes/notes/bp-whitelist-extension-for-app-creds-9afd5009b374190b.yaml new file mode 100644 index 0000000000..afa6e212e0 --- /dev/null +++ b/releasenotes/notes/bp-whitelist-extension-for-app-creds-9afd5009b374190b.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + [`blueprint whitelist-extension-for-app-creds `_] + Added support for creating access rules as an attribute of application + credentials as well as for listing, showing, and deleting access rules. diff --git a/requirements.txt b/requirements.txt index 08777b5fef..f7e2cecadb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,6 +12,6 @@ osc-lib>=2.0.0 # Apache-2.0 oslo.i18n>=3.15.3 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0 python-glanceclient>=2.8.0 # Apache-2.0 -python-keystoneclient>=3.17.0 # Apache-2.0 +python-keystoneclient>=3.22.0 # Apache-2.0 python-novaclient>=15.1.0 # Apache-2.0 python-cinderclient>=3.3.0 # Apache-2.0 diff --git a/setup.cfg b/setup.cfg index 2d6422787f..fcc9700715 100644 --- a/setup.cfg +++ b/setup.cfg @@ -197,6 +197,10 @@ openstack.identity.v2 = openstack.identity.v3 = access_token_create = openstackclient.identity.v3.token:CreateAccessToken + access_rule_delete = openstackclient.identity.v3.access_rule:DeleteAccessRule + access_rule_list = openstackclient.identity.v3.access_rule:ListAccessRule + access_rule_show = openstackclient.identity.v3.access_rule:ShowAccessRule + application_credential_create = openstackclient.identity.v3.application_credential:CreateApplicationCredential application_credential_delete = openstackclient.identity.v3.application_credential:DeleteApplicationCredential application_credential_list = openstackclient.identity.v3.application_credential:ListApplicationCredential