From 167cf11e825af95fe40c1daefdb6095c791a3ee5 Mon Sep 17 00:00:00 2001 From: Pavlo Shchelokovskyy Date: Wed, 13 Jul 2022 15:09:34 +0300 Subject: [PATCH] Add authorization_ttl for identity providers this is supported since Ussuri (Keystone API version 3.14) but was lacking from openstackclient. Change-Id: Ifac818b9a4eff66d9a68455ada1ddfe67cb46b3b --- .../identity/v3/identity_provider.py | 45 ++++- .../identity/v3/test_identity_provider.py | 170 ++++++++++++++++++ .../notes/idp-auth-ttl-6632df5db65a3bdd.yaml | 9 + 3 files changed, 223 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/idp-auth-ttl-6632df5db65a3bdd.yaml diff --git a/openstackclient/identity/v3/identity_provider.py b/openstackclient/identity/v3/identity_provider.py index 7307cea00c..19a6214487 100644 --- a/openstackclient/identity/v3/identity_provider.py +++ b/openstackclient/identity/v3/identity_provider.py @@ -63,6 +63,16 @@ class CreateIdentityProvider(command.ShowOne): 'specified, a domain will be created automatically. ' '(Name or ID)'), ) + parser.add_argument( + '--authorization-ttl', + metavar='', + type=int, + help=_('Time to keep the role assignments for users ' + 'authenticating via this identity provider. ' + 'When not provided, global default configured in the ' + 'Identity service will be used. ' + 'Available since Identity API version 3.14 (Ussuri).'), + ) enable_identity_provider = parser.add_mutually_exclusive_group() enable_identity_provider.add_argument( '--enable', @@ -95,12 +105,23 @@ class CreateIdentityProvider(command.ShowOne): domain_id = common.find_domain(identity_client, parsed_args.domain).id + # TODO(pas-ha) actually check for 3.14 microversion + kwargs = {} + auth_ttl = parsed_args.authorization_ttl + if auth_ttl is not None: + if auth_ttl < 0: + msg = (_("%(param)s must be positive integer or zero." + ) % {"param": "authorization-ttl"}) + raise exceptions.CommandError(msg) + kwargs['authorization_ttl'] = auth_ttl + idp = identity_client.federation.identity_providers.create( id=parsed_args.identity_provider_id, remote_ids=remote_ids, description=parsed_args.description, domain_id=domain_id, - enabled=parsed_args.enabled) + enabled=parsed_args.enabled, + **kwargs) idp._info.pop('links', None) remote_ids = format_columns.ListColumn(idp._info.pop('remote_ids', [])) @@ -205,6 +226,14 @@ class SetIdentityProvider(command.Command): help=_('Name of a file that contains many remote IDs to associate ' 'with the identity provider, one per line'), ) + parser.add_argument( + '--authorization-ttl', + metavar='', + type=int, + help=_('Time to keep the role assignments for users ' + 'authenticating via this identity provider. ' + 'Available since Identity API version 3.14 (Ussuri).'), + ) enable_identity_provider = parser.add_mutually_exclusive_group() enable_identity_provider.add_argument( '--enable', @@ -241,6 +270,20 @@ class SetIdentityProvider(command.Command): if parsed_args.remote_id_file or parsed_args.remote_id: kwargs['remote_ids'] = remote_ids + # TODO(pas-ha) actually check for 3.14 microversion + # TODO(pas-ha) make it possible to reset authorization_ttl + # back to None value. + # Currently not possible as filter_kwargs decorator in + # keystoneclient/base.py explicitly drops the None-valued keys + # from kwargs, and 'update' method is wrapped in this decorator. + auth_ttl = parsed_args.authorization_ttl + if auth_ttl is not None: + if auth_ttl < 0: + msg = (_("%(param)s must be positive integer or zero." + ) % {"param": "authorization-ttl"}) + raise exceptions.CommandError(msg) + kwargs['authorization_ttl'] = auth_ttl + federation_client.identity_providers.update( parsed_args.identity_provider, **kwargs diff --git a/openstackclient/tests/unit/identity/v3/test_identity_provider.py b/openstackclient/tests/unit/identity/v3/test_identity_provider.py index 1a9a799123..480bae596c 100644 --- a/openstackclient/tests/unit/identity/v3/test_identity_provider.py +++ b/openstackclient/tests/unit/identity/v3/test_identity_provider.py @@ -15,9 +15,12 @@ import copy from unittest import mock +from osc_lib import exceptions + from openstackclient.identity.v3 import identity_provider from openstackclient.tests.unit import fakes from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes +from openstackclient.tests.unit import utils as test_utils class TestIdentityProvider(identity_fakes.TestFederatedIdentity): @@ -308,6 +311,86 @@ class TestIdentityProviderCreate(TestIdentityProvider): self.assertEqual(self.columns, columns) self.assertCountEqual(self.datalist, data) + def test_create_identity_provider_authttl_positive(self): + arglist = [ + '--authorization-ttl', '60', + identity_fakes.idp_id, + ] + verifylist = [ + ('identity_provider_id', identity_fakes.idp_id), + ('authorization_ttl', 60), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'remote_ids': None, + 'description': None, + 'domain_id': None, + 'enabled': True, + 'authorization_ttl': 60, + } + + self.identity_providers_mock.create.assert_called_with( + id=identity_fakes.idp_id, + **kwargs + ) + + self.assertEqual(self.columns, columns) + self.assertCountEqual(self.datalist, data) + + def test_create_identity_provider_authttl_zero(self): + arglist = [ + '--authorization-ttl', '0', + identity_fakes.idp_id, + ] + verifylist = [ + ('identity_provider_id', identity_fakes.idp_id), + ('authorization_ttl', 0), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'remote_ids': None, + 'description': None, + 'domain_id': None, + 'enabled': True, + 'authorization_ttl': 0, + } + + self.identity_providers_mock.create.assert_called_with( + id=identity_fakes.idp_id, + **kwargs + ) + + self.assertEqual(self.columns, columns) + self.assertCountEqual(self.datalist, data) + + def test_create_identity_provider_authttl_negative(self): + arglist = [ + '--authorization-ttl', '-60', + identity_fakes.idp_id, + ] + verifylist = [ + ('identity_provider_id', identity_fakes.idp_id), + ('authorization_ttl', -60), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.assertRaises(exceptions.CommandError, self.cmd.take_action, + parsed_args) + + def test_create_identity_provider_authttl_not_int(self): + arglist = [ + '--authorization-ttl', 'spam', + identity_fakes.idp_id, + ] + verifylist = [] + self.assertRaises(test_utils.ParserException, self.check_parser, + self.cmd, arglist, verifylist) + class TestIdentityProviderDelete(TestIdentityProvider): @@ -678,6 +761,93 @@ class TestIdentityProviderSet(TestIdentityProvider): self.cmd.take_action(parsed_args) + def test_identity_provider_set_authttl_positive(self): + def prepare(self): + """Prepare fake return objects before the test is executed""" + updated_idp = copy.deepcopy(identity_fakes.IDENTITY_PROVIDER) + updated_idp['authorization_ttl'] = 60 + resources = fakes.FakeResource( + None, + updated_idp, + loaded=True + ) + self.identity_providers_mock.update.return_value = resources + + prepare(self) + arglist = [ + '--authorization-ttl', '60', + identity_fakes.idp_id + ] + verifylist = [ + ('identity_provider', identity_fakes.idp_id), + ('enable', False), + ('disable', False), + ('remote_id', None), + ('authorization_ttl', 60), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + self.identity_providers_mock.update.assert_called_with( + identity_fakes.idp_id, + authorization_ttl=60, + ) + + def test_identity_provider_set_authttl_zero(self): + def prepare(self): + """Prepare fake return objects before the test is executed""" + updated_idp = copy.deepcopy(identity_fakes.IDENTITY_PROVIDER) + updated_idp['authorization_ttl'] = 0 + resources = fakes.FakeResource( + None, + updated_idp, + loaded=True + ) + self.identity_providers_mock.update.return_value = resources + + prepare(self) + arglist = [ + '--authorization-ttl', '0', + identity_fakes.idp_id + ] + verifylist = [ + ('identity_provider', identity_fakes.idp_id), + ('enable', False), + ('disable', False), + ('remote_id', None), + ('authorization_ttl', 0), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + self.identity_providers_mock.update.assert_called_with( + identity_fakes.idp_id, + authorization_ttl=0, + ) + + def test_identity_provider_set_authttl_negative(self): + arglist = [ + '--authorization-ttl', '-1', + identity_fakes.idp_id + ] + verifylist = [ + ('identity_provider', identity_fakes.idp_id), + ('enable', False), + ('disable', False), + ('remote_id', None), + ('authorization_ttl', -1), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.assertRaises(exceptions.CommandError, self.cmd.take_action, + parsed_args) + + def test_identity_provider_set_authttl_not_int(self): + arglist = [ + '--authorization-ttl', 'spam', + identity_fakes.idp_id + ] + verifylist = [] + self.assertRaises(test_utils.ParserException, self.check_parser, + self.cmd, arglist, verifylist) + class TestIdentityProviderShow(TestIdentityProvider): diff --git a/releasenotes/notes/idp-auth-ttl-6632df5db65a3bdd.yaml b/releasenotes/notes/idp-auth-ttl-6632df5db65a3bdd.yaml new file mode 100644 index 0000000000..dfe19c7ad0 --- /dev/null +++ b/releasenotes/notes/idp-auth-ttl-6632df5db65a3bdd.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + ``identity provider create`` and ``identity provider set`` commands now + accept the ``--authorization-ttl `` argument, with ```` + being a non-negative integer. + + See `note `_ + in Keystone documentations for more details on the meaning of this option.