openstack/adjutant
Code Issues Proposed changes

better manageable roles

Browse Source 
* A user cannot edit another user unless all target user
  roles are a subset of editor manageable roles, not just
  the roles that are being edited.
* User list now returns 'manageable' boolean to indicate
  if your logged in user can edit roles of the listed user.
* Fixing an issue with mocking and test code not actually
  returning correct values for 'user_list'.
* Editing ROLES_MAPPING in settings to match default conf.

Change-Id: I3720386b6dfe4465a50ded3e21b5b577f1340dbf
This commit is contained in:
adrian-turjak 2017-01-24 15:01:45 +13:00
parent 6cdbb8eef1
commit 8a2f2f2107
6 changed files with 120 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
stacktask/actions/v1/tests/test_user_actions.py
View File

@ -668,3 +668,46 @@ class UserActionTests(TestCase):
self.assertEquals(action.valid, True)
self.assertEquals(project.roles[user.id], ['_member_'])
def test_edit_user_roles_can_manage_all(self):
"""
Confirm that you cannot edit a user unless all their roles
can be managed by you.
"""
user = mock.Mock()
user.id = 'user_id'
user.name = "test@example.com"
user.email = "test@example.com"
user.domain = 'default'
project = mock.Mock()
project.id = 'test_project_id'
project.name = 'test_project'
project.domain = 'default'
project.roles = {user.id: ['_member_', 'project_admin']}
setup_temp_cache({'test_project': project}, {user.id: user})
task = Task.objects.create(
ip_address="0.0.0.0",
keystone_user={
'roles': ['project_mod'],
'project_id': 'test_project_id',
'project_domain_id': 'default',
})
data = {
'domain_id': 'default',
'user_id': 'user_id',
'project_id': 'test_project_id',
'roles': ['project_mod'],
'remove': False
}
action = EditUserRolesAction(data, task=task, order=1)
action.pre_approve()
self.assertEquals(action.valid, False)
self.assertEquals(
project.roles[user.id], ['_member_', 'project_admin'])

10
stacktask/actions/v1/users.py
View File

@ -244,6 +244,16 @@ class EditUserRolesAction(UserIdAction, ProjectMixin, UserMixin):
# user roles
current_roles = id_manager.get_roles(user, project)
current_role_names = {role.name for role in current_roles}
# NOTE(adriant): Only allow someone to edit roles if all roles from
# the target user can be managed by editor.
can_manage_roles = user_store.get_managable_roles(
self.action.task.keystone_user['roles'])
if not set(can_manage_roles).issuperset(current_role_names):
self.add_note(
'Not all target user roles are manageable.')
return False
if self.remove:
remaining = set(current_role_names) & set(self.roles)
if not remaining:

19
stacktask/api/v1/openstack.py
View File

@ -37,6 +37,9 @@ class UserList(tasks.InviteUser):
project_id = request.keystone_user['project_id']
project = id_manager.get_project(project_id)
can_manage_roles = user_store.get_managable_roles(
request.keystone_user['roles'])
active_emails = set()
for user in id_manager.list_users(project):
skip = False
@ -53,13 +56,15 @@ class UserList(tasks.InviteUser):
enabled = getattr(user, 'enabled')
user_status = 'Active' if enabled else 'Account Disabled'
active_emails.add(email)
user_list.append({'id': user.id,
'name': user.name,
'email': email,
'roles': roles,
'cohort': 'Member',
'status': user_status
})
user_list.append({
'id': user.id,
'name': user.name,
'email': email,
'roles': roles,
'cohort': 'Member',
'status': user_status,
'manageable': set(can_manage_roles).issuperset(roles),
})
# Get my active tasks for this project:
project_tasks = models.Task.objects.filter(

5
stacktask/api/v1/tests/__init__.py
View File

@ -104,14 +104,15 @@ class FakeManager(object):
roles = temp_cache['projects'][project.name].roles
users = []
for user_id, roles in roles.iteritems():
for user_id, user_roles in roles.iteritems():
user = self.get_user(user_id)
user.roles = []
for role in roles:
for role in user_roles:
r = mock.Mock()
r.name = role
user.roles.append(r)
users.append(user)
return users
def create_user(self, name, password, email, created_on,

50
stacktask/api/v1/tests/test_api_openstack.py
View File

@ -107,6 +107,56 @@ class OpenstackAPITests(APITestCase):
response = self.client.get(url, headers=headers)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(len(response.data['users']), 2)
def test_user_list_managable(self):
"""
Confirm that the manageable value is set correctly.
"""
user = mock.Mock()
user.id = 'user_id_1'
user.name = "test1@example.com"
user.email = "test1@example.com"
user.domain = 'default'
user2 = mock.Mock()
user2.id = 'user_id_2'
user2.name = "test2@example.com"
user2.email = "test2@example.com"
user2.domain = 'default'
project = mock.Mock()
project.id = 'test_project_id'
project.name = 'test_project'
project.domain = 'default'
project.roles = {
user.id: ['_member_', 'project_admin'],
user2.id: ['_member_', 'project_mod']}
setup_temp_cache(
{'test_project': project},
{user.id: user, user2.id: user2})
url = "/v1/openstack/users"
headers = {
'project_name': "test_project",
'project_id': "test_project_id",
'roles': "_member_,project_mod",
'username': "test@example.com",
'user_id': "test_user_id",
'authenticated': True
}
url = "/v1/openstack/users"
response = self.client.get(url, headers=headers)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(len(response.data['users']), 2)
for st_user in response.data['users']:
if st_user['id'] == user.id:
self.assertFalse(st_user['manageable'])
if st_user['id'] == user2.id:
self.assertTrue(st_user['manageable'])
def test_force_reset_password(self):
"""

4
stacktask/test_settings.py
View File

@ -234,10 +234,10 @@ ROLES_MAPPING = {
'project_admin', 'project_mod', '_member_', 'heat_stack_owner'
],
'project_admin': [
'project_mod', '_member_', 'heat_stack_owner'
'project_mod', '_member_', 'heat_stack_owner', 'project_admin',
],
'project_mod': [
'_member_', 'heat_stack_owner'
'_member_', 'heat_stack_owner', 'project_mod',
],
}