diff --git a/stacktask/actions/v1/tests/test_user_actions.py b/stacktask/actions/v1/tests/test_user_actions.py index 61eade3..6e80e1d 100644 --- a/stacktask/actions/v1/tests/test_user_actions.py +++ b/stacktask/actions/v1/tests/test_user_actions.py @@ -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']) diff --git a/stacktask/actions/v1/users.py b/stacktask/actions/v1/users.py index cb16898..cf228ee 100644 --- a/stacktask/actions/v1/users.py +++ b/stacktask/actions/v1/users.py @@ -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: diff --git a/stacktask/api/v1/openstack.py b/stacktask/api/v1/openstack.py index f14bf4a..d0ce927 100644 --- a/stacktask/api/v1/openstack.py +++ b/stacktask/api/v1/openstack.py @@ -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( diff --git a/stacktask/api/v1/tests/__init__.py b/stacktask/api/v1/tests/__init__.py index d782c2d..eef7e2f 100644 --- a/stacktask/api/v1/tests/__init__.py +++ b/stacktask/api/v1/tests/__init__.py @@ -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, diff --git a/stacktask/api/v1/tests/test_api_openstack.py b/stacktask/api/v1/tests/test_api_openstack.py index b2eac57..95f0add 100644 --- a/stacktask/api/v1/tests/test_api_openstack.py +++ b/stacktask/api/v1/tests/test_api_openstack.py @@ -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): """ diff --git a/stacktask/test_settings.py b/stacktask/test_settings.py index 855bf11..597d561 100644 --- a/stacktask/test_settings.py +++ b/stacktask/test_settings.py @@ -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', ], }