openstack/adjutant
Code Issues Proposed changes

Fix an issue with Adjutant's Invite process not checking inherited roles

Browse Source 
Adjutant's InviteUser action was not correctly checking inherited roles
which might allow someone to invite/create another user with roles outside
of those they inviting user can manage.

Change-Id: I1f45da4ce5ee6d1295a17767c432875c23106b15
Story: #2009326
This commit is contained in:
Adrian Turjak 2021-10-26 15:34:14 +13:00
parent bee1e42c9f
commit ab7f8115d4
2 changed files with 86 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
adjutant/actions/v1/base.py
View File

@ -291,8 +291,9 @@ class UserMixin(ResourceMixin):
def _validate_role_permissions(self):
keystone_user = self.action.task.keystone_user
# Role permissions check
requested_roles = set(list(self.roles) + list(self.inherited_roles))
if not self.are_roles_manageable(
user_roles=keystone_user["roles"], requested_roles=self.roles
user_roles=keystone_user["roles"], requested_roles=requested_roles
):
self.add_note("User does not have permission to edit role(s).")
return False

84
adjutant/actions/v1/tests/test_user_actions.py
View File

@ -374,6 +374,51 @@ class UserActionTests(AdjutantTestCase):
action.prepare()
self.assertFalse(action.valid)
def test_new_user_invalid_roles(self):
"""
Test that you can't add roles outside of managed roles.
Action should be invalid.
"""
project = fake_clients.FakeProject(name="test_project")
setup_identity_cache(projects=[project])
task = Task.objects.create(
keystone_user={
"roles": ["project_admin"],
"project_id": project.id,
"project_domain_id": "default",
}
)
data = {
"email": "test@example.com",
"project_id": project.id,
"roles": ["admin"],
"inherited_roles": [],
"domain_id": "default",
}
action = NewUserAction(data, task=task, order=1)
action.prepare()
self.assertFalse(action.valid)
data = {
"email": "test@example.com",
"project_id": project.id,
"roles": [],
"inherited_roles": ["admin"],
"domain_id": "default",
}
action = NewUserAction(data, task=task, order=1)
action.prepare()
self.assertFalse(action.valid)
def test_new_user_wrong_domain(self):
"""
Existing user, valid project, invalid domain.
@ -780,6 +825,18 @@ class UserActionTests(AdjutantTestCase):
role_name="project_admin",
user={"id": user.id},
),
fake_clients.FakeRoleAssignment(
scope={"project": {"id": project.id}},
role_name="member",
user={"id": user.id},
inherited=True,
),
fake_clients.FakeRoleAssignment(
scope={"project": {"id": project.id}},
role_name="project_admin",
user={"id": user.id},
inherited=True,
),
]
setup_identity_cache(
@ -813,6 +870,33 @@ class UserActionTests(AdjutantTestCase):
roles = fake_client._get_roles_as_names(user, project)
self.assertEqual(roles, ["member", "project_admin"])
task = Task.objects.create(
keystone_user={
"roles": ["project_mod"],
"project_id": project.id,
"project_domain_id": "default",
}
)
data = {
"domain_id": "default",
"user_id": user.id,
"project_id": project.id,
"roles": [],
"inherited_roles": ["project_mod"],
"remove": False,
}
action = EditUserRolesAction(data, task=task, order=1)
action.prepare()
self.assertEqual(action.valid, False)
fake_client = fake_clients.FakeManager()
roles = fake_client._get_roles_as_names(user, project)
self.assertEqual(roles, ["member", "project_admin"])
def test_edit_user_roles_modified_config(self):
"""
Tests that the role mappings do come from config and that they