diff --git a/horizon/dashboards/syspanel/templates/syspanel/users/_update.html b/horizon/dashboards/syspanel/templates/syspanel/users/_update.html index 389dc4003..b324345f0 100644 --- a/horizon/dashboards/syspanel/templates/syspanel/users/_update.html +++ b/horizon/dashboards/syspanel/templates/syspanel/users/_update.html @@ -14,7 +14,7 @@

{% trans "Description" %}:

-

{% trans "From here you can edit the user by changing their username, email, password, and default project." %}

+

{% trans "From here you can edit the user's details, including their default project." %}

{% endblock %} diff --git a/horizon/dashboards/syspanel/users/forms.py b/horizon/dashboards/syspanel/users/forms.py index de5d6ad31..71de09426 100644 --- a/horizon/dashboards/syspanel/users/forms.py +++ b/horizon/dashboards/syspanel/users/forms.py @@ -39,6 +39,7 @@ class BaseUserForm(forms.SelfHandlingForm): super(BaseUserForm, self).__init__(*args, **kwargs) # Populate tenant choices tenant_choices = [('', _("Select a project"))] + for tenant in api.tenant_list(request, admin=True): if tenant.enabled: tenant_choices.append((tenant.id, tenant.name)) @@ -107,30 +108,41 @@ class UpdateUserForm(BaseUserForm): widget=forms.PasswordInput(render_value=False), regex=validators.password_validator(), required=False, - error_messages={'invalid': validators.password_validator_msg()}) + error_messages={'invalid': + validators.password_validator_msg()}) confirm_password = forms.CharField( label=_("Confirm Password"), widget=forms.PasswordInput(render_value=False), required=False) tenant_id = forms.ChoiceField(label=_("Primary Project")) + def __init__(self, request, *args, **kwargs): + super(UpdateUserForm, self).__init__(request, *args, **kwargs) + + if api.keystone_can_edit_user() == False: + for field in ('name', 'email', 'password', 'confirm_password'): + self.fields.pop(field) + def handle(self, request, data): failed, succeeded = [], [] + user_is_editable = api.keystone_can_edit_user() user = data.pop('id') - password = data.pop('password') tenant = data.pop('tenant_id') - # Discard the extra fields so we can pass kwargs to keystoneclient data.pop('method') - data.pop('confirm_password', None) - # Update user details - msg_bits = (_('name'), _('email')) - try: - api.keystone.user_update(request, user, **data) - succeeded.extend(msg_bits) - except: - failed.extend(msg_bits) - exceptions.handle(request, ignore=True) + if user_is_editable: + password = data.pop('password') + data.pop('confirm_password', None) + + if user_is_editable: + # Update user details + msg_bits = (_('name'), _('email')) + try: + api.keystone.user_update(request, user, **data) + succeeded.extend(msg_bits) + except: + failed.extend(msg_bits) + exceptions.handle(request, ignore=True) # Update default tenant msg_bits = (_('primary project'),) @@ -141,27 +153,23 @@ class UpdateUserForm(BaseUserForm): failed.append(msg_bits) exceptions.handle(request, ignore=True) - # If present, update password - # FIXME(gabriel): password change should be its own form and view - if password: - msg_bits = (_('password'),) - try: - api.user_update_password(request, user, password) - succeeded.extend(msg_bits) - except: - failed.extend(msg_bits) - exceptions.handle(request, ignore=True) + if user_is_editable: + # If present, update password + # FIXME(gabriel): password change should be its own form and view + if password: + msg_bits = (_('password'),) + try: + api.user_update_password(request, user, password) + succeeded.extend(msg_bits) + except: + failed.extend(msg_bits) + exceptions.handle(request, ignore=True) if succeeded: - succeeded = map(force_unicode, succeeded) - messages.success(request, - _('Updated %(attributes)s for "%(user)s".') - % {"user": data["name"], - "attributes": ", ".join(succeeded)}) + messages.success(request, _('User has been updated successfully.')) if failed: failed = map(force_unicode, failed) messages.error(request, - _('Unable to update %(attributes)s for "%(user)s".') - % {"user": data["name"], - "attributes": ", ".join(failed)}) + _('Unable to update %(attributes)s for the user.') + % {"attributes": ", ".join(failed)}) return shortcuts.redirect('horizon:syspanel:users:index') diff --git a/horizon/dashboards/syspanel/users/tables.py b/horizon/dashboards/syspanel/users/tables.py index fe51a3a40..b6a63fa7b 100644 --- a/horizon/dashboards/syspanel/users/tables.py +++ b/horizon/dashboards/syspanel/users/tables.py @@ -17,6 +17,11 @@ class CreateUserLink(tables.LinkAction): url = "horizon:syspanel:users:create" classes = ("ajax-modal", "btn-create") + def allowed(self, request, user): + if api.keystone_can_edit_user(): + return True + return False + class EditUserLink(tables.LinkAction): name = "edit" @@ -93,7 +98,8 @@ class DeleteUsersAction(tables.DeleteAction): data_type_plural = _("Users") def allowed(self, request, datum): - if datum and datum.id == request.user.id: + if not api.keystone_can_edit_user() or \ + (datum and datum.id == request.user.id): return False return True @@ -134,11 +140,6 @@ class UsersTable(tables.DataTable): class Meta: name = "users" verbose_name = _("Users") - if api.keystone_can_edit_user(): - row_actions = (EditUserLink, EnableUsersAction, DisableUsersAction, - DeleteUsersAction) - table_actions = (UserFilterAction, CreateUserLink, - DeleteUsersAction) - else: - row_actions = (EnableUsersAction, DisableUsersAction) - table_actions = (UserFilterAction,) + row_actions = (EditUserLink, EnableUsersAction, DisableUsersAction, + DeleteUsersAction) + table_actions = (UserFilterAction, CreateUserLink, DeleteUsersAction) diff --git a/horizon/dashboards/syspanel/users/tests.py b/horizon/dashboards/syspanel/users/tests.py index 531493f6d..8e7614a6c 100644 --- a/horizon/dashboards/syspanel/users/tests.py +++ b/horizon/dashboards/syspanel/users/tests.py @@ -33,22 +33,25 @@ USER_UPDATE_URL = reverse('horizon:syspanel:users:update', args=[1]) class UsersViewTests(test.BaseAdminViewTests): + @test.create_stubs({api.keystone: ('user_list',)}) def test_index(self): - self.mox.StubOutWithMock(api.keystone, 'user_list') api.keystone.user_list(IgnoreArg()).AndReturn(self.users.list()) + self.mox.ReplayAll() res = self.client.get(USERS_INDEX_URL) + self.assertTemplateUsed(res, 'syspanel/users/index.html') self.assertItemsEqual(res.context['table'].data, self.users.list()) - def test_create_user(self): + @test.create_stubs({api: ('user_create', + 'tenant_list', + 'add_tenant_user_role'), + api.keystone: ('get_default_role',)}) + def test_create(self): user = self.users.get(id="1") role = self.roles.first() - self.mox.StubOutWithMock(api, 'user_create') - self.mox.StubOutWithMock(api, 'tenant_list') - self.mox.StubOutWithMock(api.keystone, 'get_default_role') - self.mox.StubOutWithMock(api, 'add_tenant_user_role') + api.tenant_list(IgnoreArg(), admin=True).AndReturn(self.tenants.list()) api.user_create(IgnoreArg(), user.name, @@ -58,6 +61,7 @@ class UsersViewTests(test.BaseAdminViewTests): True).AndReturn(user) api.keystone.get_default_role(IgnoreArg()).AndReturn(role) api.add_tenant_user_role(IgnoreArg(), self.tenant.id, user.id, role.id) + self.mox.ReplayAll() formData = {'method': 'CreateUserForm', @@ -67,13 +71,16 @@ class UsersViewTests(test.BaseAdminViewTests): 'tenant_id': self.tenant.id, 'confirm_password': user.password} res = self.client.post(USER_CREATE_URL, formData) + self.assertNoFormErrors(res) self.assertMessageCount(success=1) - def test_create_user_password_mismatch(self): + @test.create_stubs({api: ('tenant_list',)}) + def test_create_with_password_mismatch(self): user = self.users.get(id="1") - self.mox.StubOutWithMock(api, 'tenant_list') + api.tenant_list(IgnoreArg(), admin=True).AndReturn(self.tenants.list()) + self.mox.ReplayAll() formData = {'method': 'CreateUserForm', @@ -84,13 +91,15 @@ class UsersViewTests(test.BaseAdminViewTests): 'confirm_password': "doesntmatch"} res = self.client.post(USER_CREATE_URL, formData) + self.assertFormError(res, "form", None, ['Passwords do not match.']) - def test_create_user_field_validation(self): + @test.create_stubs({api: ('tenant_list',)}) + def test_create_validation_for_password_too_short(self): user = self.users.get(id="1") - self.mox.StubOutWithMock(api, 'tenant_list') - api.tenant_list(IgnoreArg(), admin=True).AndReturn(self.tenants.list()) + api.tenant_list(IgnoreArg(), admin=True).AndReturn(self.tenants.list()) + self.mox.ReplayAll() # check password min-len verification @@ -102,27 +111,45 @@ class UsersViewTests(test.BaseAdminViewTests): 'confirm_password': 'four'} res = self.client.post(USER_CREATE_URL, formData) + self.assertFormError( res, "form", 'password', ['Password must be between 8 and 18 characters.']) - # check password max-len verification - formData['password'] = 'MoreThanEighteenChars' - formData['confirm_password'] = 'MoreThanEighteenChars' + @test.create_stubs({api: ('tenant_list',)}) + 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()) + + self.mox.ReplayAll() + + # check password min-len verification + formData = {'method': 'CreateUserForm', + 'name': user.name, + 'email': user.email, + 'password': 'MoreThanEighteenChars', + 'tenant_id': self.tenant.id, + 'confirm_password': 'MoreThanEighteenChars'} res = self.client.post(USER_CREATE_URL, formData) + self.assertFormError( res, "form", 'password', ['Password must be between 8 and 18 characters.']) - def test_update_user_field_validation(self): + @test.create_stubs({api: ('user_get', + 'tenant_list', + 'user_update_tenant', + 'user_update_password'), + api.keystone: ('user_update',)}) + def test_update(self): user = self.users.get(id="1") - self.mox.StubOutWithMock(api, 'tenant_list') - self.mox.StubOutWithMock(api, 'user_get') - self.mox.StubOutWithMock(api.keystone, 'user_update') - self.mox.StubOutWithMock(api, 'user_update_tenant') - self.mox.StubOutWithMock(api, 'user_update_password') + api.user_get(IsA(http.HttpRequest), '1', + admin=True).AndReturn(user) + api.tenant_list(IgnoreArg(), + admin=True).AndReturn(self.tenants.list()) api.keystone.user_update(IsA(http.HttpRequest), user.id, email=u'test@example.com', @@ -131,86 +158,143 @@ class UsersViewTests(test.BaseAdminViewTests): user.id, self.tenant.id).AndReturn(None) api.user_update_password(IsA(http.HttpRequest), - user.id, - IgnoreArg()).AndReturn(None) - api.user_get(IsA(http.HttpRequest), '1', admin=True).AndReturn(user) - api.tenant_list(IgnoreArg(), admin=True).AndReturn(self.tenants.list()) - api.user_get(IsA(http.HttpRequest), '1', admin=True).AndReturn(user) - api.tenant_list(IgnoreArg(), admin=True).AndReturn(self.tenants.list()) - api.user_get(IsA(http.HttpRequest), '1', admin=True).AndReturn(user) - api.tenant_list(IgnoreArg(), admin=True).AndReturn(self.tenants.list()) + user.id, + IgnoreArg()).AndReturn(None) + self.mox.ReplayAll() - user.tenantId = 1 formData = {'method': 'UpdateUserForm', - 'id': 1, + 'id': user.id, 'name': user.name, 'email': user.email, 'password': 'normalpwd', 'tenant_id': self.tenant.id, 'confirm_password': 'normalpwd'} - # check successful update res = self.client.post(USER_UPDATE_URL, formData) + self.assertNoFormErrors(res) - # check password min-len verification - formData['password'] = 'four' - formData['confirm_password'] = 'four' + @test.create_stubs({api: ('user_get', + 'tenant_list', + 'user_update_tenant', + 'keystone_can_edit_user')}) + def test_update_with_keystone_can_edit_user_false(self): + user = self.users.get(id="1") + + api.user_get(IsA(http.HttpRequest), + '1', + admin=True).AndReturn(user) + api.tenant_list(IgnoreArg(), admin=True).AndReturn(self.tenants.list()) + api.keystone_can_edit_user().AndReturn(False) + api.keystone_can_edit_user().AndReturn(False) + api.user_update_tenant(IsA(http.HttpRequest), + user.id, + self.tenant.id).AndReturn(None) + + self.mox.ReplayAll() + + formData = {'method': 'UpdateUserForm', + 'tenant_id': self.tenant.id, + 'id': user.id} res = self.client.post(USER_UPDATE_URL, formData) - self.assertFormError( - res, "form", 'password', - ['Password must be between 8 and 18 characters.']) - # check password max-len verification - formData['password'] = 'MoreThanEighteenChars' - formData['confirm_password'] = 'MoreThanEighteenChars' + self.assertNoFormErrors(res) + + @test.create_stubs({api: ('user_get', 'tenant_list')}) + def test_update_validation_for_password_too_short(self): + user = self.users.get(id="1") + + api.user_get(IsA(http.HttpRequest), '1', + admin=True).AndReturn(user) + api.tenant_list(IgnoreArg(), + admin=True).AndReturn(self.tenants.list()) + + self.mox.ReplayAll() + + formData = {'method': 'UpdateUserForm', + 'id': user.id, + 'name': user.name, + 'email': user.email, + 'password': 't', + 'tenant_id': self.tenant.id, + 'confirm_password': 't'} res = self.client.post(USER_UPDATE_URL, formData) - self.assertFormError( - res, "form", 'password', - ['Password must be between 8 and 18 characters.']) + self.assertFormError( + res, "form", 'password', + ['Password must be between 8 and 18 characters.']) + + @test.create_stubs({api: ('user_get', 'tenant_list')}) + def test_update_validation_for_password_too_long(self): + user = self.users.get(id="1") + + api.user_get(IsA(http.HttpRequest), '1', + admin=True).AndReturn(user) + api.tenant_list(IgnoreArg(), + admin=True).AndReturn(self.tenants.list()) + + self.mox.ReplayAll() + + formData = {'method': 'UpdateUserForm', + 'id': user.id, + 'name': user.name, + 'email': user.email, + 'password': 'ThisIsASuperLongPassword', + 'tenant_id': self.tenant.id, + 'confirm_password': 'ThisIsASuperLongPassword'} + + res = self.client.post(USER_UPDATE_URL, formData) + + self.assertFormError( + res, "form", 'password', + ['Password must be between 8 and 18 characters.']) + + @test.create_stubs({api.keystone: ('user_update_enabled', 'user_list')}) def test_enable_user(self): user = self.users.get(id="2") - self.mox.StubOutWithMock(api.keystone, 'user_update_enabled') - self.mox.StubOutWithMock(api.keystone, 'user_list') + api.keystone.user_list(IgnoreArg()).AndReturn(self.users.list()) api.keystone.user_update_enabled(IgnoreArg(), user.id, True).AndReturn(user) + self.mox.ReplayAll() formData = {'action': 'users__enable__%s' % user.id} res = self.client.post(USERS_INDEX_URL, formData) + self.assertRedirectsNoFollow(res, USERS_INDEX_URL) + @test.create_stubs({api.keystone: ('user_update_enabled', 'user_list')}) def test_disable_user(self): user = self.users.get(id="2") - self.mox.StubOutWithMock(api.keystone, 'user_update_enabled') - self.mox.StubOutWithMock(api.keystone, 'user_list') api.keystone.user_list(IgnoreArg()).AndReturn(self.users.list()) api.keystone.user_update_enabled(IgnoreArg(), user.id, False).AndReturn(user) + self.mox.ReplayAll() formData = {'action': 'users__disable__%s' % user.id} res = self.client.post(USERS_INDEX_URL, formData) + self.assertRedirectsNoFollow(res, USERS_INDEX_URL) + @test.create_stubs({api.keystone: ('user_update_enabled', 'user_list')}) def test_enable_disable_user_exception(self): user = self.users.get(id="2") - self.mox.StubOutWithMock(api.keystone, 'user_update_enabled') - self.mox.StubOutWithMock(api.keystone, 'user_list') + api.keystone.user_list(IgnoreArg()).AndReturn(self.users.list()) api_exception = keystone_exceptions.ClientException('apiException', message='apiException') api.keystone.user_update_enabled(IgnoreArg(), user.id, True).AndRaise(api_exception) + self.mox.ReplayAll() formData = {'action': 'users__enable__%s' % user.id} @@ -218,24 +302,30 @@ class UsersViewTests(test.BaseAdminViewTests): self.assertRedirectsNoFollow(res, USERS_INDEX_URL) - def test_shoot_yourself_in_the_foot(self): - self.mox.StubOutWithMock(api.keystone, 'user_list') - # Four times... one for each post and one for each followed redirect - api.keystone.user_list(IgnoreArg()).AndReturn(self.users.list()) - api.keystone.user_list(IgnoreArg()).AndReturn(self.users.list()) - api.keystone.user_list(IgnoreArg()).AndReturn(self.users.list()) - api.keystone.user_list(IgnoreArg()).AndReturn(self.users.list()) + @test.create_stubs({api.keystone: ('user_list',)}) + def test_disabling_current_user(self): + for i in range(0, 2): + api.keystone.user_list(IgnoreArg()).AndReturn(self.users.list()) self.mox.ReplayAll() formData = {'action': 'users__disable__%s' % self.request.user.id} res = self.client.post(USERS_INDEX_URL, formData, follow=True) + self.assertEqual(list(res.context['messages'])[0].message, u'You cannot disable the user you are currently ' u'logged in as.') + @test.create_stubs({api.keystone: ('user_list',)}) + def test_delete_user_with_improper_permissions(self): + for i in range(0, 2): + api.keystone.user_list(IgnoreArg()).AndReturn(self.users.list()) + + self.mox.ReplayAll() + formData = {'action': 'users__delete__%s' % self.request.user.id} res = self.client.post(USERS_INDEX_URL, formData, follow=True) + self.assertEqual(list(res.context['messages'])[0].message, u'You do not have permission to delete user: %s' % self.request.user.username) diff --git a/horizon/test.py b/horizon/test.py index d56c0f7e7..c12988d85 100644 --- a/horizon/test.py +++ b/horizon/test.py @@ -27,6 +27,7 @@ from django.conf import settings from django.contrib.messages.storage import default_storage from django.core.handlers import wsgi from django.test.client import RequestFactory +from functools import wraps from glanceclient.v1 import client as glance_client from keystoneclient.v2_0 import client as keystone_client from novaclient.v1_1 import client as nova_client @@ -48,6 +49,28 @@ from .time import utcnow wsgi.WSGIRequest.__repr__ = lambda self: "" +def create_stubs(stubs_to_create={}): + if not isinstance(stubs_to_create, dict): + raise TypeError, ("create_stub must be passed a dict, but a %s was " \ + "given." % type(stubs_to_create).__name__) + + def inner_stub_out(fn): + @wraps(fn) + def instance_stub_out(self): + for key in stubs_to_create: + if not (isinstance(stubs_to_create[key], tuple) or \ + isinstance(stubs_to_create[key], list)): + raise TypeError, ("The values of the create_stub " \ + "dict must be lists or tuples, but is a %s." % + type(stubs_to_create[key]).__name__) + + for value in stubs_to_create[key]: + self.mox.StubOutWithMock(key, value) + return fn(self) + return instance_stub_out + return inner_stub_out + + class RequestFactoryWithMessages(RequestFactory): def get(self, *args, **kwargs): req = super(RequestFactoryWithMessages, self).get(*args, **kwargs) diff --git a/horizon/tests/test_data/keystone_data.py b/horizon/tests/test_data/keystone_data.py index 180b10f6d..c2f6507d3 100644 --- a/horizon/tests/test_data/keystone_data.py +++ b/horizon/tests/test_data/keystone_data.py @@ -104,11 +104,11 @@ def data(TEST): 'email': 'test@example.com', 'password': 'password', 'token': 'test_token'} - user = users.User(users.UserManager, user_dict) + user = users.User(users.UserManager(None), user_dict) user_dict.update({'id': "2", 'name': 'user_two', 'email': 'two@example.com'}) - user2 = users.User(users.UserManager, user_dict) + user2 = users.User(users.UserManager(None), user_dict) TEST.users.add(user, user2) TEST.user = user # Your "current" user TEST.user.service_catalog = SERVICE_CATALOG