Skip to content

Commit 95cc05b

Browse files
Zuulopenstack-gerrit
Zuul
authored and
openstack-gerrit
committed
Merge "Bypass user and group verification in RemoveRole"
2 parents b147764 + e246732 commit 95cc05b

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
@@ -64,59 +64,61 @@ def _add_identity_and_resource_options_to_parser(parser):
6464

6565

6666
def _process_identity_and_resource_options(parsed_args,
67-
identity_client_manager):
67+
identity_client_manager,
68+
validate_actor_existence=True):
69+
70+
def _find_user():
71+
try:
72+
return common.find_user(
73+
identity_client_manager,
74+
parsed_args.user,
75+
parsed_args.user_domain
76+
).id
77+
except exceptions.CommandError:
78+
if not validate_actor_existence:
79+
return parsed_args.user
80+
raise
81+
82+
def _find_group():
83+
try:
84+
return common.find_group(
85+
identity_client_manager,
86+
parsed_args.group,
87+
parsed_args.group_domain
88+
).id
89+
except exceptions.CommandError:
90+
if not validate_actor_existence:
91+
return parsed_args.group
92+
raise
93+
6894
kwargs = {}
6995
if parsed_args.user and parsed_args.system:
70-
kwargs['user'] = common.find_user(
71-
identity_client_manager,
72-
parsed_args.user,
73-
parsed_args.user_domain,
74-
).id
96+
kwargs['user'] = _find_user()
7597
kwargs['system'] = parsed_args.system
7698
elif parsed_args.user and parsed_args.domain:
77-
kwargs['user'] = common.find_user(
78-
identity_client_manager,
79-
parsed_args.user,
80-
parsed_args.user_domain,
81-
).id
99+
kwargs['user'] = _find_user()
82100
kwargs['domain'] = common.find_domain(
83101
identity_client_manager,
84102
parsed_args.domain,
85103
).id
86104
elif parsed_args.user and parsed_args.project:
87-
kwargs['user'] = common.find_user(
88-
identity_client_manager,
89-
parsed_args.user,
90-
parsed_args.user_domain,
91-
).id
105+
kwargs['user'] = _find_user()
92106
kwargs['project'] = common.find_project(
93107
identity_client_manager,
94108
parsed_args.project,
95109
parsed_args.project_domain,
96110
).id
97111
elif parsed_args.group and parsed_args.system:
98-
kwargs['group'] = common.find_group(
99-
identity_client_manager,
100-
parsed_args.group,
101-
parsed_args.group_domain,
102-
).id
112+
kwargs['group'] = _find_group()
103113
kwargs['system'] = parsed_args.system
104114
elif parsed_args.group and parsed_args.domain:
105-
kwargs['group'] = common.find_group(
106-
identity_client_manager,
107-
parsed_args.group,
108-
parsed_args.group_domain,
109-
).id
115+
kwargs['group'] = _find_group()
110116
kwargs['domain'] = common.find_domain(
111117
identity_client_manager,
112118
parsed_args.domain,
113119
).id
114120
elif parsed_args.group and parsed_args.project:
115-
kwargs['group'] = common.find_group(
116-
identity_client_manager,
117-
parsed_args.group,
118-
parsed_args.group_domain,
119-
).id
121+
kwargs['group'] = _find_group()
120122
kwargs['project'] = common.find_project(
121123
identity_client_manager,
122124
parsed_args.project,
@@ -340,7 +342,9 @@ def take_action(self, parsed_args):
340342
)
341343

342344
kwargs = _process_identity_and_resource_options(
343-
parsed_args, self.app.client_manager.identity)
345+
parsed_args, self.app.client_manager.identity,
346+
validate_actor_existence=False
347+
)
344348
identity_client.roles.revoke(role.id, **kwargs)
345349

346350

‎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
@@ -846,6 +847,47 @@ def test_role_remove_user_system(self):
846847
)
847848
self.assertIsNone(result)
848849

850+
@mock.patch.object(common, 'find_user')
851+
def test_role_remove_non_existent_user_system(self, find_mock):
852+
# Simulate the user not being in keystone, the client should gracefully
853+
# handle this exception and send the request to remove the role since
854+
# keystone supports removing role assignments with non-existent actors
855+
# (e.g., users or groups).
856+
find_mock.side_effect = exceptions.CommandError
857+
858+
arglist = [
859+
'--user', identity_fakes.user_id,
860+
'--system', 'all',
861+
identity_fakes.role_name
862+
]
863+
if self._is_inheritance_testcase():
864+
arglist.append('--inherited')
865+
verifylist = [
866+
('user', identity_fakes.user_id),
867+
('group', None),
868+
('system', 'all'),
869+
('domain', None),
870+
('project', None),
871+
('role', identity_fakes.role_name),
872+
('inherited', self._is_inheritance_testcase()),
873+
]
874+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
875+
876+
result = self.cmd.take_action(parsed_args)
877+
878+
# Set expected values
879+
kwargs = {
880+
'user': identity_fakes.user_id,
881+
'system': 'all',
882+
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
883+
}
884+
# RoleManager.revoke(role, user=, group=, domain=, project=)
885+
self.roles_mock.revoke.assert_called_with(
886+
identity_fakes.role_id,
887+
**kwargs
888+
)
889+
self.assertIsNone(result)
890+
849891
def test_role_remove_user_domain(self):
850892
arglist = [
851893
'--user', identity_fakes.user_name,
@@ -879,6 +921,46 @@ def test_role_remove_user_domain(self):
879921
)
880922
self.assertIsNone(result)
881923

924+
@mock.patch.object(common, 'find_user')
925+
def test_role_remove_non_existent_user_domain(self, find_mock):
926+
# Simulate the user not being in keystone, the client the gracefully
927+
# handle this exception and send the request to remove the role since
928+
# keystone will validate.
929+
find_mock.side_effect = exceptions.CommandError
930+
931+
arglist = [
932+
'--user', identity_fakes.user_id,
933+
'--domain', identity_fakes.domain_name,
934+
identity_fakes.role_name
935+
]
936+
if self._is_inheritance_testcase():
937+
arglist.append('--inherited')
938+
verifylist = [
939+
('user', identity_fakes.user_id),
940+
('group', None),
941+
('system', None),
942+
('domain', identity_fakes.domain_name),
943+
('project', None),
944+
('role', identity_fakes.role_name),
945+
('inherited', self._is_inheritance_testcase()),
946+
]
947+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
948+
949+
result = self.cmd.take_action(parsed_args)
950+
951+
# Set expected values
952+
kwargs = {
953+
'user': identity_fakes.user_id,
954+
'domain': identity_fakes.domain_id,
955+
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
956+
}
957+
# RoleManager.revoke(role, user=, group=, domain=, project=)
958+
self.roles_mock.revoke.assert_called_with(
959+
identity_fakes.role_id,
960+
**kwargs
961+
)
962+
self.assertIsNone(result)
963+
882964
def test_role_remove_user_project(self):
883965
arglist = [
884966
'--user', identity_fakes.user_name,
@@ -912,6 +994,46 @@ def test_role_remove_user_project(self):
912994
)
913995
self.assertIsNone(result)
914996

997+
@mock.patch.object(common, 'find_user')
998+
def test_role_remove_non_existent_user_project(self, find_mock):
999+
# Simulate the user not being in keystone, the client the gracefully
1000+
# handle this exception and send the request to remove the role since
1001+
# keystone will validate.
1002+
find_mock.side_effect = exceptions.CommandError
1003+
1004+
arglist = [
1005+
'--user', identity_fakes.user_id,
1006+
'--project', identity_fakes.project_name,
1007+
identity_fakes.role_name
1008+
]
1009+
if self._is_inheritance_testcase():
1010+
arglist.append('--inherited')
1011+
verifylist = [
1012+
('user', identity_fakes.user_id),
1013+
('group', None),
1014+
('system', None),
1015+
('domain', None),
1016+
('project', identity_fakes.project_name),
1017+
('role', identity_fakes.role_name),
1018+
('inherited', self._is_inheritance_testcase()),
1019+
]
1020+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
1021+
1022+
result = self.cmd.take_action(parsed_args)
1023+
1024+
# Set expected values
1025+
kwargs = {
1026+
'user': identity_fakes.user_id,
1027+
'project': identity_fakes.project_id,
1028+
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
1029+
}
1030+
# RoleManager.revoke(role, user=, group=, domain=, project=)
1031+
self.roles_mock.revoke.assert_called_with(
1032+
identity_fakes.role_id,
1033+
**kwargs
1034+
)
1035+
self.assertIsNone(result)
1036+
9151037
def test_role_remove_group_system(self):
9161038
arglist = [
9171039
'--group', identity_fakes.group_name,
@@ -947,6 +1069,46 @@ def test_role_remove_group_system(self):
9471069
)
9481070
self.assertIsNone(result)
9491071

1072+
@mock.patch.object(common, 'find_group')
1073+
def test_role_remove_non_existent_group_system(self, find_mock):
1074+
# Simulate the user not being in keystone, the client the gracefully
1075+
# handle this exception and send the request to remove the role since
1076+
# keystone will validate.
1077+
find_mock.side_effect = exceptions.CommandError
1078+
1079+
arglist = [
1080+
'--group', identity_fakes.group_id,
1081+
'--system', 'all',
1082+
identity_fakes.role_name
1083+
]
1084+
if self._is_inheritance_testcase():
1085+
arglist.append('--inherited')
1086+
verifylist = [
1087+
('user', None),
1088+
('group', identity_fakes.group_id),
1089+
('system', 'all'),
1090+
('domain', None),
1091+
('project', None),
1092+
('role', identity_fakes.role_name),
1093+
('inherited', self._is_inheritance_testcase()),
1094+
]
1095+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
1096+
1097+
result = self.cmd.take_action(parsed_args)
1098+
1099+
# Set expected values
1100+
kwargs = {
1101+
'group': identity_fakes.group_id,
1102+
'system': 'all',
1103+
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
1104+
}
1105+
# RoleManager.revoke(role, user=, group=, domain=, project=)
1106+
self.roles_mock.revoke.assert_called_with(
1107+
identity_fakes.role_id,
1108+
**kwargs
1109+
)
1110+
self.assertIsNone(result)
1111+
9501112
def test_role_remove_group_domain(self):
9511113
arglist = [
9521114
'--group', identity_fakes.group_name,
@@ -981,6 +1143,46 @@ def test_role_remove_group_domain(self):
9811143
)
9821144
self.assertIsNone(result)
9831145

1146+
@mock.patch.object(common, 'find_group')
1147+
def test_role_remove_non_existent_group_domain(self, find_mock):
1148+
# Simulate the user not being in keystone, the client the gracefully
1149+
# handle this exception and send the request to remove the role since
1150+
# keystone will validate.
1151+
find_mock.side_effect = exceptions.CommandError
1152+
1153+
arglist = [
1154+
'--group', identity_fakes.group_id,
1155+
'--domain', identity_fakes.domain_name,
1156+
identity_fakes.role_name
1157+
]
1158+
if self._is_inheritance_testcase():
1159+
arglist.append('--inherited')
1160+
verifylist = [
1161+
('user', None),
1162+
('group', identity_fakes.group_id),
1163+
('system', None),
1164+
('domain', identity_fakes.domain_name),
1165+
('project', None),
1166+
('role', identity_fakes.role_name),
1167+
('inherited', self._is_inheritance_testcase()),
1168+
]
1169+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
1170+
1171+
result = self.cmd.take_action(parsed_args)
1172+
1173+
# Set expected values
1174+
kwargs = {
1175+
'group': identity_fakes.group_id,
1176+
'domain': identity_fakes.domain_id,
1177+
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
1178+
}
1179+
# RoleManager.revoke(role, user=, group=, domain=, project=)
1180+
self.roles_mock.revoke.assert_called_with(
1181+
identity_fakes.role_id,
1182+
**kwargs
1183+
)
1184+
self.assertIsNone(result)
1185+
9841186
def test_role_remove_group_project(self):
9851187
arglist = [
9861188
'--group', identity_fakes.group_name,
@@ -1014,6 +1216,46 @@ def test_role_remove_group_project(self):
10141216
)
10151217
self.assertIsNone(result)
10161218

1219+
@mock.patch.object(common, 'find_group')
1220+
def test_role_remove_non_existent_group_project(self, find_mock):
1221+
# Simulate the user not being in keystone, the client the gracefully
1222+
# handle this exception and send the request to remove the role since
1223+
# keystone will validate.
1224+
find_mock.side_effect = exceptions.CommandError
1225+
1226+
arglist = [
1227+
'--group', identity_fakes.group_id,
1228+
'--project', identity_fakes.project_name,
1229+
identity_fakes.role_name
1230+
]
1231+
if self._is_inheritance_testcase():
1232+
arglist.append('--inherited')
1233+
verifylist = [
1234+
('user', None),
1235+
('group', identity_fakes.group_id),
1236+
('system', None),
1237+
('domain', None),
1238+
('project', identity_fakes.project_name),
1239+
('role', identity_fakes.role_name),
1240+
('inherited', self._is_inheritance_testcase()),
1241+
]
1242+
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
1243+
1244+
result = self.cmd.take_action(parsed_args)
1245+
1246+
# Set expected values
1247+
kwargs = {
1248+
'group': identity_fakes.group_id,
1249+
'project': identity_fakes.project_id,
1250+
'os_inherit_extension_inherited': self._is_inheritance_testcase(),
1251+
}
1252+
# RoleManager.revoke(role, user=, group=, domain=, project=)
1253+
self.roles_mock.revoke.assert_called_with(
1254+
identity_fakes.role_id,
1255+
**kwargs
1256+
)
1257+
self.assertIsNone(result)
1258+
10171259
def test_role_remove_domain_role_on_group_domain(self):
10181260
self.roles_mock.get.return_value = fakes.FakeResource(
10191261
None,

0 commit comments

Comments
 (0)