Skip to content

Commit 54bf2c0

Browse files
Zuulopenstack-gerrit
Zuul
authored and
openstack-gerrit
committed
Merge "Bypass user and group verification in RemoveRole" into stable/train
2 parents 30fe947 + 2e0a0f1 commit 54bf2c0

3 files changed

Lines changed: 285 additions & 32 deletions

File tree

‎openstackclient/identity/v3/role.py‎

Lines changed: 36 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -65,59 +65,61 @@ def _add_identity_and_resource_options_to_parser(parser):
6565

6666

6767
def _process_identity_and_resource_options(parsed_args,
68-
identity_client_manager):
68+
identity_client_manager,
69+
validate_actor_existence=True):
70+
71+
def _find_user():
72+
try:
73+
return common.find_user(
74+
identity_client_manager,
75+
parsed_args.user,
76+
parsed_args.user_domain
77+
).id
78+
except exceptions.CommandError:
79+
if not validate_actor_existence:
80+
return parsed_args.user
81+
raise
82+
83+
def _find_group():
84+
try:
85+
return common.find_group(
86+
identity_client_manager,
87+
parsed_args.group,
88+
parsed_args.group_domain
89+
).id
90+
except exceptions.CommandError:
91+
if not validate_actor_existence:
92+
return parsed_args.group
93+
raise
94+
6995
kwargs = {}
7096
if parsed_args.user and parsed_args.system:
71-
kwargs['user'] = common.find_user(
72-
identity_client_manager,
73-
parsed_args.user,
74-
parsed_args.user_domain,
75-
).id
97+
kwargs['user'] = _find_user()
7698
kwargs['system'] = parsed_args.system
7799
elif parsed_args.user and parsed_args.domain:
78-
kwargs['user'] = common.find_user(
79-
identity_client_manager,
80-
parsed_args.user,
81-
parsed_args.user_domain,
82-
).id
100+
kwargs['user'] = _find_user()
83101
kwargs['domain'] = common.find_domain(
84102
identity_client_manager,
85103
parsed_args.domain,
86104
).id
87105
elif parsed_args.user and parsed_args.project:
88-
kwargs['user'] = common.find_user(
89-
identity_client_manager,
90-
parsed_args.user,
91-
parsed_args.user_domain,
92-
).id
106+
kwargs['user'] = _find_user()
93107
kwargs['project'] = common.find_project(
94108
identity_client_manager,
95109
parsed_args.project,
96110
parsed_args.project_domain,
97111
).id
98112
elif parsed_args.group and parsed_args.system:
99-
kwargs['group'] = common.find_group(
100-
identity_client_manager,
101-
parsed_args.group,
102-
parsed_args.group_domain,
103-
).id
113+
kwargs['group'] = _find_group()
104114
kwargs['system'] = parsed_args.system
105115
elif parsed_args.group and parsed_args.domain:
106-
kwargs['group'] = common.find_group(
107-
identity_client_manager,
108-
parsed_args.group,
109-
parsed_args.group_domain,
110-
).id
116+
kwargs['group'] = _find_group()
111117
kwargs['domain'] = common.find_domain(
112118
identity_client_manager,
113119
parsed_args.domain,
114120
).id
115121
elif parsed_args.group and parsed_args.project:
116-
kwargs['group'] = common.find_group(
117-
identity_client_manager,
118-
parsed_args.group,
119-
parsed_args.group_domain,
120-
).id
122+
kwargs['group'] = _find_group()
121123
kwargs['project'] = common.find_project(
122124
identity_client_manager,
123125
parsed_args.project,
@@ -332,7 +334,9 @@ def take_action(self, parsed_args):
332334
)
333335

334336
kwargs = _process_identity_and_resource_options(
335-
parsed_args, self.app.client_manager.identity)
337+
parsed_args, self.app.client_manager.identity,
338+
validate_actor_existence=False
339+
)
336340
identity_client.roles.revoke(role.id, **kwargs)
337341

338342

‎openstackclient/tests/unit/identity/v3/test_role.py‎

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from osc_lib import exceptions
2020
from osc_lib import utils
2121

22+
from openstackclient.identity import common
2223
from openstackclient.identity.v3 import role
2324
from openstackclient.tests.unit import fakes
2425
from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes
@@ -708,6 +709,47 @@ def test_role_remove_user_system(self):
708709
)
709710
self.assertIsNone(result)
710711

712+
@mock.patch.object(common, 'find_user')
713+
def test_role_remove_non_existent_user_system(self, find_mock):
714+
# Simulate the user not being in keystone, the client should gracefully
715+
# handle this exception and send the request to remove the role since
716+
# keystone supports removing role assignments with non-existent actors
717+
# (e.g., users or groups).
718+
find_mock.side_effect = exceptions.CommandError
719+
720+
arglist = [
721+
'--user', identity_fakes.user_id,
722+
'--system', 'all',
723+
identity_fakes.role_name
724+
]
725+
if self._is_inheritance_testcase():
726+
arglist.append('--inherited')
727+
verifylist = [
728+
('user', identity_fakes.user_id),
729+
('group', None),
730+
('system', 'all'),
731+
('domain', None),
732+
('project', None),
733+
('role', identity_fakes.role_name),
734+
('inherited', self._is_inheritance_testcase()),
735+
]
736+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
737+
738+
result = self.cmd.take_action(parsed_args)
739+
740+
# Set expected values
741+
kwargs = {
742+
'user': identity_fakes.user_id,
743+
'system': 'all',
744+
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
745+
}
746+
# RoleManager.revoke(role, user=, group=, domain=, project=)
747+
self.roles_mock.revoke.assert_called_with(
748+
identity_fakes.role_id,
749+
**kwargs
750+
)
751+
self.assertIsNone(result)
752+
711753
def test_role_remove_user_domain(self):
712754
arglist = [
713755
'--user', identity_fakes.user_name,
@@ -741,6 +783,46 @@ def test_role_remove_user_domain(self):
741783
)
742784
self.assertIsNone(result)
743785

786+
@mock.patch.object(common, 'find_user')
787+
def test_role_remove_non_existent_user_domain(self, find_mock):
788+
# Simulate the user not being in keystone, the client the gracefully
789+
# handle this exception and send the request to remove the role since
790+
# keystone will validate.
791+
find_mock.side_effect = exceptions.CommandError
792+
793+
arglist = [
794+
'--user', identity_fakes.user_id,
795+
'--domain', identity_fakes.domain_name,
796+
identity_fakes.role_name
797+
]
798+
if self._is_inheritance_testcase():
799+
arglist.append('--inherited')
800+
verifylist = [
801+
('user', identity_fakes.user_id),
802+
('group', None),
803+
('system', None),
804+
('domain', identity_fakes.domain_name),
805+
('project', None),
806+
('role', identity_fakes.role_name),
807+
('inherited', self._is_inheritance_testcase()),
808+
]
809+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
810+
811+
result = self.cmd.take_action(parsed_args)
812+
813+
# Set expected values
814+
kwargs = {
815+
'user': identity_fakes.user_id,
816+
'domain': identity_fakes.domain_id,
817+
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
818+
}
819+
# RoleManager.revoke(role, user=, group=, domain=, project=)
820+
self.roles_mock.revoke.assert_called_with(
821+
identity_fakes.role_id,
822+
**kwargs
823+
)
824+
self.assertIsNone(result)
825+
744826
def test_role_remove_user_project(self):
745827
arglist = [
746828
'--user', identity_fakes.user_name,
@@ -774,6 +856,46 @@ def test_role_remove_user_project(self):
774856
)
775857
self.assertIsNone(result)
776858

859+
@mock.patch.object(common, 'find_user')
860+
def test_role_remove_non_existent_user_project(self, find_mock):
861+
# Simulate the user not being in keystone, the client the gracefully
862+
# handle this exception and send the request to remove the role since
863+
# keystone will validate.
864+
find_mock.side_effect = exceptions.CommandError
865+
866+
arglist = [
867+
'--user', identity_fakes.user_id,
868+
'--project', identity_fakes.project_name,
869+
identity_fakes.role_name
870+
]
871+
if self._is_inheritance_testcase():
872+
arglist.append('--inherited')
873+
verifylist = [
874+
('user', identity_fakes.user_id),
875+
('group', None),
876+
('system', None),
877+
('domain', None),
878+
('project', identity_fakes.project_name),
879+
('role', identity_fakes.role_name),
880+
('inherited', self._is_inheritance_testcase()),
881+
]
882+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
883+
884+
result = self.cmd.take_action(parsed_args)
885+
886+
# Set expected values
887+
kwargs = {
888+
'user': identity_fakes.user_id,
889+
'project': identity_fakes.project_id,
890+
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
891+
}
892+
# RoleManager.revoke(role, user=, group=, domain=, project=)
893+
self.roles_mock.revoke.assert_called_with(
894+
identity_fakes.role_id,
895+
**kwargs
896+
)
897+
self.assertIsNone(result)
898+
777899
def test_role_remove_group_system(self):
778900
arglist = [
779901
'--group', identity_fakes.group_name,
@@ -809,6 +931,46 @@ def test_role_remove_group_system(self):
809931
)
810932
self.assertIsNone(result)
811933

934+
@mock.patch.object(common, 'find_group')
935+
def test_role_remove_non_existent_group_system(self, find_mock):
936+
# Simulate the user not being in keystone, the client the gracefully
937+
# handle this exception and send the request to remove the role since
938+
# keystone will validate.
939+
find_mock.side_effect = exceptions.CommandError
940+
941+
arglist = [
942+
'--group', identity_fakes.group_id,
943+
'--system', 'all',
944+
identity_fakes.role_name
945+
]
946+
if self._is_inheritance_testcase():
947+
arglist.append('--inherited')
948+
verifylist = [
949+
('user', None),
950+
('group', identity_fakes.group_id),
951+
('system', 'all'),
952+
('domain', None),
953+
('project', None),
954+
('role', identity_fakes.role_name),
955+
('inherited', self._is_inheritance_testcase()),
956+
]
957+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
958+
959+
result = self.cmd.take_action(parsed_args)
960+
961+
# Set expected values
962+
kwargs = {
963+
'group': identity_fakes.group_id,
964+
'system': 'all',
965+
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
966+
}
967+
# RoleManager.revoke(role, user=, group=, domain=, project=)
968+
self.roles_mock.revoke.assert_called_with(
969+
identity_fakes.role_id,
970+
**kwargs
971+
)
972+
self.assertIsNone(result)
973+
812974
def test_role_remove_group_domain(self):
813975
arglist = [
814976
'--group', identity_fakes.group_name,
@@ -843,6 +1005,46 @@ def test_role_remove_group_domain(self):
8431005
)
8441006
self.assertIsNone(result)
8451007

1008+
@mock.patch.object(common, 'find_group')
1009+
def test_role_remove_non_existent_group_domain(self, find_mock):
1010+
# Simulate the user not being in keystone, the client the gracefully
1011+
# handle this exception and send the request to remove the role since
1012+
# keystone will validate.
1013+
find_mock.side_effect = exceptions.CommandError
1014+
1015+
arglist = [
1016+
'--group', identity_fakes.group_id,
1017+
'--domain', identity_fakes.domain_name,
1018+
identity_fakes.role_name
1019+
]
1020+
if self._is_inheritance_testcase():
1021+
arglist.append('--inherited')
1022+
verifylist = [
1023+
('user', None),
1024+
('group', identity_fakes.group_id),
1025+
('system', None),
1026+
('domain', identity_fakes.domain_name),
1027+
('project', None),
1028+
('role', identity_fakes.role_name),
1029+
('inherited', self._is_inheritance_testcase()),
1030+
]
1031+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
1032+
1033+
result = self.cmd.take_action(parsed_args)
1034+
1035+
# Set expected values
1036+
kwargs = {
1037+
'group': identity_fakes.group_id,
1038+
'domain': identity_fakes.domain_id,
1039+
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
1040+
}
1041+
# RoleManager.revoke(role, user=, group=, domain=, project=)
1042+
self.roles_mock.revoke.assert_called_with(
1043+
identity_fakes.role_id,
1044+
**kwargs
1045+
)
1046+
self.assertIsNone(result)
1047+
8461048
def test_role_remove_group_project(self):
8471049
arglist = [
8481050
'--group', identity_fakes.group_name,
@@ -876,6 +1078,46 @@ def test_role_remove_group_project(self):
8761078
)
8771079
self.assertIsNone(result)
8781080

1081+
@mock.patch.object(common, 'find_group')
1082+
def test_role_remove_non_existent_group_project(self, find_mock):
1083+
# Simulate the user not being in keystone, the client the gracefully
1084+
# handle this exception and send the request to remove the role since
1085+
# keystone will validate.
1086+
find_mock.side_effect = exceptions.CommandError
1087+
1088+
arglist = [
1089+
'--group', identity_fakes.group_id,
1090+
'--project', identity_fakes.project_name,
1091+
identity_fakes.role_name
1092+
]
1093+
if self._is_inheritance_testcase():
1094+
arglist.append('--inherited')
1095+
verifylist = [
1096+
('user', None),
1097+
('group', identity_fakes.group_id),
1098+
('system', None),
1099+
('domain', None),
1100+
('project', identity_fakes.project_name),
1101+
('role', identity_fakes.role_name),
1102+
('inherited', self._is_inheritance_testcase()),
1103+
]
1104+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
1105+
1106+
result = self.cmd.take_action(parsed_args)
1107+
1108+
# Set expected values
1109+
kwargs = {
1110+
'group': identity_fakes.group_id,
1111+
'project': identity_fakes.project_id,
1112+
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
1113+
}
1114+
# RoleManager.revoke(role, user=, group=, domain=, project=)
1115+
self.roles_mock.revoke.assert_called_with(
1116+
identity_fakes.role_id,
1117+
**kwargs
1118+
)
1119+
self.assertIsNone(result)
1120+
8791121
def test_role_remove_domain_role_on_group_domain(self):
8801122
self.roles_mock.get.return_value = fakes.FakeResource(
8811123
None,

0 commit comments

Comments
 (0)