From 24748ac201d2f00e9d51821843c01b3d83198633 Mon Sep 17 00:00:00 2001 From: Gabriel Hurley Date: Mon, 11 Jun 2012 19:29:12 -0700 Subject: [PATCH] Clarifying user roles in various places. * Adds a dropdown selector for the role you wish to grant a user on the default project during user creation. Fixes bug 999380. * Adds a "roles" column to the "Modify Users" table so you can see which roles users have been granted on a project. Fixes bug 1011919. * General table cleanup for readability. Change-Id: Ic3f36903c78b52c70f66680f3956845e28656fd8 --- horizon/api/keystone.py | 6 +++++ .../dashboards/syspanel/projects/tables.py | 23 ++++++++++++++-- horizon/dashboards/syspanel/projects/tests.py | 5 ++++ horizon/dashboards/syspanel/users/forms.py | 17 +++++++----- horizon/dashboards/syspanel/users/tables.py | 2 +- horizon/dashboards/syspanel/users/tests.py | 26 ++++++++++++++++--- horizon/dashboards/syspanel/users/views.py | 22 +++++++++++++--- 7 files changed, 84 insertions(+), 17 deletions(-) diff --git a/horizon/api/keystone.py b/horizon/api/keystone.py index 333afc1f65..bd4e9feafe 100644 --- a/horizon/api/keystone.py +++ b/horizon/api/keystone.py @@ -262,6 +262,11 @@ def role_list(request): return keystoneclient(request, admin=True).roles.list() +def roles_for_user(request, user, project): + return keystoneclient(request, admin=True).roles.roles_for_user(user, + project) + + def add_tenant_user_role(request, tenant_id, user_id, role_id): """ Adds a role for a user on a tenant. """ return keystoneclient(request, admin=True).roles.add_user_role(user_id, @@ -289,6 +294,7 @@ def get_default_role(request): try: roles = keystoneclient(request, admin=True).roles.list() except: + roles = [] exceptions.handle(request) for role in roles: if role.id == default or role.name == default: diff --git a/horizon/dashboards/syspanel/projects/tables.py b/horizon/dashboards/syspanel/projects/tables.py index 3b6b81fd3d..8da1a61787 100644 --- a/horizon/dashboards/syspanel/projects/tables.py +++ b/horizon/dashboards/syspanel/projects/tables.py @@ -4,6 +4,7 @@ from django.core.urlresolvers import reverse from django.utils.translation import ugettext_lazy as _ from horizon import api +from horizon import exceptions from horizon import tables from ..users.tables import UsersTable @@ -70,16 +71,16 @@ class TenantFilterAction(tables.FilterAction): class TenantsTable(tables.DataTable): - id = tables.Column('id', verbose_name=_('Id')) name = tables.Column('name', verbose_name=_('Name')) description = tables.Column(lambda obj: getattr(obj, 'description', None), verbose_name=_('Description')) + id = tables.Column('id', verbose_name=_('Project ID')) enabled = tables.Column('enabled', verbose_name=_('Enabled'), status=True) class Meta: name = "tenants" verbose_name = _("Projects") - row_actions = (EditLink, UsageLink, ViewMembersLink, ModifyQuotasLink, + row_actions = (ViewMembersLink, EditLink, UsageLink, ModifyQuotasLink, DeleteTenantsAction) table_actions = (TenantFilterAction, CreateLink, DeleteTenantsAction) @@ -97,12 +98,29 @@ class RemoveUserAction(tables.BatchAction): api.keystone.remove_tenant_user(request, tenant_id, user_id) +class ProjectUserRolesColumn(tables.Column): + def get_raw_data(self, user): + request = self.table._meta.request + try: + roles = api.keystone.roles_for_user(request, + user.id, + self.table.kwargs["tenant_id"]) + except: + roles = [] + exceptions.handle(request, + _("Unable to retrieve role information.")) + return ", ".join([role.name for role in roles]) + + class TenantUsersTable(UsersTable): + roles = ProjectUserRolesColumn("roles", verbose_name=_("Roles")) + class Meta: name = "tenant_users" verbose_name = _("Users For Project") table_actions = (RemoveUserAction,) row_actions = (RemoveUserAction,) + columns = ("name", "email", "id", "roles", "enabled") class AddUserAction(tables.LinkAction): @@ -122,3 +140,4 @@ class AddUsersTable(UsersTable): verbose_name = _("Add New Users") table_actions = () row_actions = (AddUserAction,) + columns = ("name", "email", "id", "enabled") diff --git a/horizon/dashboards/syspanel/projects/tests.py b/horizon/dashboards/syspanel/projects/tests.py index 30d45579fd..2376fc20ca 100644 --- a/horizon/dashboards/syspanel/projects/tests.py +++ b/horizon/dashboards/syspanel/projects/tests.py @@ -67,12 +67,17 @@ class TenantsViewTests(test.BaseAdminViewTests): def test_modify_users(self): self.mox.StubOutWithMock(api.keystone, 'tenant_get') self.mox.StubOutWithMock(api.keystone, 'user_list') + self.mox.StubOutWithMock(api.keystone, 'roles_for_user') api.keystone.tenant_get(IgnoreArg(), self.tenant.id, admin=True) \ .AndReturn(self.tenant) api.keystone.user_list(IsA(http.HttpRequest)) \ .AndReturn(self.users.list()) api.keystone.user_list(IsA(http.HttpRequest), self.tenant.id) \ .AndReturn([self.user]) + api.keystone.roles_for_user(IsA(http.HttpRequest), + self.user.id, + self.tenant.id) \ + .AndReturn(self.roles.list()) self.mox.ReplayAll() url = reverse('horizon:syspanel:projects:users', args=(self.tenant.id,)) diff --git a/horizon/dashboards/syspanel/users/forms.py b/horizon/dashboards/syspanel/users/forms.py index d4bcacde1d..85c11f7885 100644 --- a/horizon/dashboards/syspanel/users/forms.py +++ b/horizon/dashboards/syspanel/users/forms.py @@ -72,6 +72,13 @@ class CreateUserForm(BaseUserForm): required=False, widget=forms.PasswordInput(render_value=False)) tenant_id = forms.ChoiceField(label=_("Primary Project")) + role_id = forms.ChoiceField(label=_("Role")) + + def __init__(self, *args, **kwargs): + roles = kwargs.pop('roles') + super(CreateUserForm, self).__init__(*args, **kwargs) + role_choices = [(role.id, role.name) for role in roles] + self.fields['role_id'].choices = role_choices # We have to protect the entire "data" dict because it contains the # password and confirm_password strings. @@ -89,12 +96,10 @@ class CreateUserForm(BaseUserForm): _('User "%s" was successfully created.') % data['name']) try: - default_role = api.keystone.get_default_role(request) - if default_role: - api.add_tenant_user_role(request, - data['tenant_id'], - new_user.id, - default_role.id) + api.add_tenant_user_role(request, + data['tenant_id'], + new_user.id, + data['role_id']) except: exceptions.handle(request, _('Unable to add user to primary project.')) diff --git a/horizon/dashboards/syspanel/users/tables.py b/horizon/dashboards/syspanel/users/tables.py index 0dbd70333d..9db4a15ade 100644 --- a/horizon/dashboards/syspanel/users/tables.py +++ b/horizon/dashboards/syspanel/users/tables.py @@ -103,12 +103,12 @@ class UsersTable(tables.DataTable): ("true", True), ("false", False) ) - id = tables.Column('id', verbose_name=_('ID')) name = tables.Column('name', verbose_name=_('User Name')) email = tables.Column('email', verbose_name=_('Email')) # Default tenant is not returned from Keystone currently. #default_tenant = tables.Column('default_tenant', # verbose_name=_('Default Project')) + id = tables.Column('id', verbose_name=_('User ID')) enabled = tables.Column('enabled', verbose_name=_('Enabled'), status=True, status_choices=STATUS_CHOICES, diff --git a/horizon/dashboards/syspanel/users/tests.py b/horizon/dashboards/syspanel/users/tests.py index 3875b4dfc7..ef630a11f2 100644 --- a/horizon/dashboards/syspanel/users/tests.py +++ b/horizon/dashboards/syspanel/users/tests.py @@ -46,7 +46,8 @@ class UsersViewTests(test.BaseAdminViewTests): @test.create_stubs({api: ('user_create', 'tenant_list', 'add_tenant_user_role'), - api.keystone: ('get_default_role',)}) + api.keystone: ('get_default_role', + 'role_list')}) def test_create(self): user = self.users.get(id="1") role = self.roles.first() @@ -58,6 +59,7 @@ class UsersViewTests(test.BaseAdminViewTests): user.password, self.tenant.id, True).AndReturn(user) + api.keystone.role_list(IgnoreArg()).AndReturn(self.roles.list()) api.keystone.get_default_role(IgnoreArg()).AndReturn(role) api.add_tenant_user_role(IgnoreArg(), self.tenant.id, user.id, role.id) @@ -68,17 +70,22 @@ class UsersViewTests(test.BaseAdminViewTests): 'email': user.email, 'password': user.password, 'tenant_id': self.tenant.id, + 'role_id': self.roles.first().id, 'confirm_password': user.password} res = self.client.post(USER_CREATE_URL, formData) self.assertNoFormErrors(res) self.assertMessageCount(success=1) - @test.create_stubs({api: ('tenant_list',)}) + @test.create_stubs({api: ('tenant_list',), + api.keystone: ('role_list', 'get_default_role')}) def test_create_with_password_mismatch(self): user = self.users.get(id="1") api.tenant_list(IgnoreArg(), admin=True).AndReturn(self.tenants.list()) + api.keystone.role_list(IgnoreArg()).AndReturn(self.roles.list()) + api.keystone.get_default_role(IgnoreArg()) \ + .AndReturn(self.roles.first()) self.mox.ReplayAll() @@ -87,17 +94,22 @@ class UsersViewTests(test.BaseAdminViewTests): 'email': user.email, 'password': user.password, 'tenant_id': self.tenant.id, + 'role_id': self.roles.first().id, 'confirm_password': "doesntmatch"} res = self.client.post(USER_CREATE_URL, formData) self.assertFormError(res, "form", None, ['Passwords do not match.']) - @test.create_stubs({api: ('tenant_list',)}) + @test.create_stubs({api: ('tenant_list',), + api.keystone: ('role_list', 'get_default_role')}) def test_create_validation_for_password_too_short(self): user = self.users.get(id="1") api.tenant_list(IgnoreArg(), admin=True).AndReturn(self.tenants.list()) + api.keystone.role_list(IgnoreArg()).AndReturn(self.roles.list()) + api.keystone.get_default_role(IgnoreArg()) \ + .AndReturn(self.roles.first()) self.mox.ReplayAll() @@ -107,6 +119,7 @@ class UsersViewTests(test.BaseAdminViewTests): 'email': user.email, 'password': 'four', 'tenant_id': self.tenant.id, + 'role_id': self.roles.first().id, 'confirm_password': 'four'} res = self.client.post(USER_CREATE_URL, formData) @@ -115,11 +128,15 @@ class UsersViewTests(test.BaseAdminViewTests): res, "form", 'password', ['Password must be between 8 and 18 characters.']) - @test.create_stubs({api: ('tenant_list',)}) + @test.create_stubs({api: ('tenant_list',), + api.keystone: ('role_list', 'get_default_role')}) def test_create_validation_for_password_too_long(self): user = self.users.get(id="1") api.tenant_list(IgnoreArg(), admin=True).AndReturn(self.tenants.list()) + api.keystone.role_list(IgnoreArg()).AndReturn(self.roles.list()) + api.keystone.get_default_role(IgnoreArg()) \ + .AndReturn(self.roles.first()) self.mox.ReplayAll() @@ -129,6 +146,7 @@ class UsersViewTests(test.BaseAdminViewTests): 'email': user.email, 'password': 'MoreThanEighteenChars', 'tenant_id': self.tenant.id, + 'role_id': self.roles.first().id, 'confirm_password': 'MoreThanEighteenChars'} res = self.client.post(USER_CREATE_URL, formData) diff --git a/horizon/dashboards/syspanel/users/views.py b/horizon/dashboards/syspanel/users/views.py index 9255b96c13..5f3be2db8d 100644 --- a/horizon/dashboards/syspanel/users/views.py +++ b/horizon/dashboards/syspanel/users/views.py @@ -18,7 +18,7 @@ # License for the specific language governing permissions and limitations # under the License. -import logging +import operator from django.core.urlresolvers import reverse from django.utils.decorators import method_decorator @@ -33,9 +33,6 @@ from .forms import CreateUserForm, UpdateUserForm from .tables import UsersTable -LOG = logging.getLogger(__name__) - - class IndexView(tables.DataTableView): table_class = UsersTable template_name = 'syspanel/users/index.html' @@ -85,3 +82,20 @@ class CreateView(forms.ModalFormView): 'confirm_password')) def dispatch(self, *args, **kwargs): return super(CreateView, self).dispatch(*args, **kwargs) + + def get_form_kwargs(self): + kwargs = super(CreateView, self).get_form_kwargs() + try: + roles = api.keystone.role_list(self.request) + except: + redirect = reverse("horizon:syspanel:users:index") + exceptions.handle(self.request, + _("Unable to retrieve user roles."), + redirect=redirect) + roles.sort(key=operator.attrgetter("id")) + kwargs['roles'] = roles + return kwargs + + def get_initial(self): + default_role = api.keystone.get_default_role(self.request) + return {'role_id': getattr(default_role, "id", None)}