diff --git a/openstack_dashboard/dashboards/identity/users/forms.py b/openstack_dashboard/dashboards/identity/users/forms.py index 019d299cf3..8e794a4e3d 100644 --- a/openstack_dashboard/dashboards/identity/users/forms.py +++ b/openstack_dashboard/dashboards/identity/users/forms.py @@ -133,6 +133,9 @@ class CreateUserForm(PasswordMixin, BaseUserForm, AddExtraColumnMixIn): enabled = forms.BooleanField(label=_("Enabled"), required=False, initial=True) + lock_password = forms.BooleanField(label=_("Lock password"), + required=False, + initial=False) def __init__(self, *args, **kwargs): roles = kwargs.pop('roles') @@ -141,7 +144,7 @@ class CreateUserForm(PasswordMixin, BaseUserForm, AddExtraColumnMixIn): ordering = ["domain_id", "domain_name", "name", "description", "email", "password", "confirm_password", "project", "role_id", - "enabled"] + "enabled", "lock_password"] self.add_extra_fields(ordering) self.fields = collections.OrderedDict( (key, self.fields[key]) for key in ordering) @@ -178,6 +181,10 @@ class CreateUserForm(PasswordMixin, BaseUserForm, AddExtraColumnMixIn): else: kwargs = {} + if "lock_password" in data: + kwargs.update({'options': + {'lock_password': data['lock_password']}}) + new_user = \ api.keystone.user_create(request, name=data['name'], @@ -235,6 +242,10 @@ class UpdateUserForm(BaseUserForm, AddExtraColumnMixIn): project = forms.ThemableChoiceField(label=_("Primary Project"), required=PROJECT_REQUIRED) + lock_password = forms.BooleanField(label=_("Lock password"), + required=False, + initial=False) + def __init__(self, request, *args, **kwargs): super(UpdateUserForm, self).__init__(request, *args, **kwargs) self.add_extra_fields() @@ -261,6 +272,10 @@ class UpdateUserForm(BaseUserForm, AddExtraColumnMixIn): if 'description' not in self.changed_data: data.pop('description') + if 'lock_password' in data: + data.update({'options': {'lock_password': data['lock_password']}}) + data.pop('lock_password') + try: if "email" in data: data['email'] = data['email'] diff --git a/openstack_dashboard/dashboards/identity/users/tables.py b/openstack_dashboard/dashboards/identity/users/tables.py index fcf2cf9d60..8a9a59c900 100644 --- a/openstack_dashboard/dashboards/identity/users/tables.py +++ b/openstack_dashboard/dashboards/identity/users/tables.py @@ -64,7 +64,9 @@ class ChangePasswordLink(policy.PolicyTargetMixin, tables.LinkAction): ("target.user.domain_id", "domain_id")) def allowed(self, request, user): - return api.keystone.keystone_can_edit_user() + options = getattr(user, "options", {}) + lock_password = options.get("lock_password", False) + return not lock_password and api.keystone.keystone_can_edit_user() class ToggleEnabled(policy.PolicyTargetMixin, tables.BatchAction): diff --git a/openstack_dashboard/dashboards/identity/users/templates/users/_detail_overview.html b/openstack_dashboard/dashboards/identity/users/templates/users/_detail_overview.html index 2f031ecd06..b910b67dce 100644 --- a/openstack_dashboard/dashboards/identity/users/templates/users/_detail_overview.html +++ b/openstack_dashboard/dashboards/identity/users/templates/users/_detail_overview.html @@ -20,6 +20,8 @@
{{ user.enabled|yesno|capfirst }}
{% trans "Password Expires At" %}
{{ user.password_expires_at }}
+
{% trans "Lock password" %}
+
{{ lock_password|yesno:"yes,no"|capfirst }}
{% trans "Primary Project" %}
{% if user.project_id %}
diff --git a/openstack_dashboard/dashboards/identity/users/tests.py b/openstack_dashboard/dashboards/identity/users/tests.py index 2c1edd7174..f93d6eb8a7 100644 --- a/openstack_dashboard/dashboards/identity/users/tests.py +++ b/openstack_dashboard/dashboards/identity/users/tests.py @@ -152,9 +152,11 @@ class UsersViewTests(test.BaseAdminViewTests): kwargs = {'phone_num': phone_number} self.mock_user_create.assert_called_once_with( - test.IsHttpRequest(), name=user.name, description=user.description, - email=user.email, password=user.password, project=self.tenant.id, - enabled=True, domain=domain_id, **kwargs) + test.IsHttpRequest(), name=user.name, + description=user.description, email=user.email, + password=user.password, project=self.tenant.id, + enabled=True, domain=domain_id, options={'lock_password': False}, + **kwargs) self.mock_role_list.assert_called_once_with(test.IsHttpRequest()) self.mock_get_default_role.assert_called_once_with( test.IsHttpRequest()) @@ -229,7 +231,8 @@ class UsersViewTests(test.BaseAdminViewTests): password=user.password, project=self.tenant.id, enabled=True, - domain=domain_id) + domain=domain_id, + options={'lock_password': False}) self.mock_role_list.assert_called_once_with(test.IsHttpRequest()) self.mock_get_default_role.assert_called_once_with( test.IsHttpRequest()) @@ -394,6 +397,71 @@ class UsersViewTests(test.BaseAdminViewTests): self.mock_get_default_role, 2, mock.call(test.IsHttpRequest())) + @test.create_mocks({api.keystone: ('user_create', + 'get_default_domain', + 'tenant_list', + 'add_tenant_user_role', + 'get_default_role', + 'roles_for_user', + 'role_list')}) + def test_create_lock_pass(self): + user = self.users.get(id="1") + domain = self._get_default_domain() + domain_id = user.domain_id + + role = self.roles.first() + + self.mock_get_default_domain.return_value = domain + self.mock_tenant_list.return_value = [self.tenants.list(), False] + + self.mock_user_create.return_value = user + self.mock_role_list.return_value = self.roles.list() + self.mock_get_default_role.return_value = role + self.mock_roles_for_user.return_value = [] + self.mock_add_tenant_user_role.return_value = None + + formData = {'method': 'CreateUserForm', + 'domain_id': domain_id, + 'name': user.name, + 'description': user.description, + 'email': user.email, + 'password': user.password, + 'project': self.tenant.id, + 'role_id': self.roles.first().id, + 'enabled': True, + 'confirm_password': user.password, + 'lock_password': True} + res = self.client.post(USER_CREATE_URL, formData) + + self.assertNoFormErrors(res) + self.assertMessageCount(success=1) + + self.mock_get_default_domain.assert_has_calls([ + mock.call(test.IsHttpRequest()), + mock.call(test.IsHttpRequest(), False), + ]) + self.assertEqual(2, self.mock_get_default_domain.call_count) + + if api.keystone.VERSIONS.active >= 3: + self.mock_tenant_list.assert_called_once_with( + test.IsHttpRequest(), domain=domain.id) + else: + self.mock_tenant_list.assert_called_once_with( + test.IsHttpRequest(), user=None) + + self.mock_user_create.assert_called_once_with( + test.IsHttpRequest(), name=user.name, + options={'lock_password': True}, description=user.description, + email=user.email, password=user.password, project=self.tenant.id, + enabled=True, domain=domain.id) + self.mock_role_list.assert_called_once_with(test.IsHttpRequest()) + self.mock_get_default_role.assert_called_once_with( + test.IsHttpRequest()) + self.mock_roles_for_user.assert_called_once_with( + test.IsHttpRequest(), user.id, self.tenant.id) + self.mock_add_tenant_user_role.assert_called_once_with( + test.IsHttpRequest(), self.tenant.id, user.id, role.id) + @override_settings(USER_TABLE_EXTRA_INFO={'phone_num': 'Phone Number'}) @test.create_mocks({api.keystone: ('user_get', 'domain_get', @@ -435,6 +503,8 @@ class UsersViewTests(test.BaseAdminViewTests): self.mock_user_update.assert_called_once_with(test.IsHttpRequest(), user.id, email=user.email, + options={'lock_password': + False}, name=user.name, **kwargs) @@ -477,6 +547,8 @@ class UsersViewTests(test.BaseAdminViewTests): self.mock_user_update.assert_called_once_with(test.IsHttpRequest(), user.id, email=user.email, + options={'lock_password': + False}, name=user.name, project=new_project_id) @@ -518,6 +590,8 @@ class UsersViewTests(test.BaseAdminViewTests): self.mock_user_update.assert_called_once_with(test.IsHttpRequest(), user.id, email=user.email or "", + options={'lock_password': + False}, name=user.name, project=self.tenant.id) @@ -545,6 +619,40 @@ class UsersViewTests(test.BaseAdminViewTests): self.assertNoFormErrors(res) self.assertMessageCount(error=1) + self.mock_user_get.assert_called_once_with(test.IsHttpRequest(), '1', + admin=True) + self.mock_domain_get.assert_called_once_with(test.IsHttpRequest(), + domain_id) + self.mock_tenant_list.assert_called_once_with( + test.IsHttpRequest(), domain=domain.id) + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_keystone_can_edit_user, 2, + mock.call()) + + @test.create_mocks({api.keystone: ('user_get', + 'domain_get', + 'tenant_list', + 'keystone_can_edit_user', )}) + def test_update_with_locked_password(self): + user = self.users.get(id="6") + domain_id = user.domain_id + domain = self.domains.get(id=domain_id) + + self.mock_user_get.return_value = user + self.mock_domain_get.return_value = domain + self.mock_tenant_list.return_value = [self.tenants.list(), False] + self.mock_keystone_can_edit_user.return_value = False + + formData = {'method': 'UpdateUserForm', + 'id': user.id, + 'name': user.name, + 'project': self.tenant.id, } + + res = self.client.post(USER_UPDATE_URL, formData) + + self.assertNoFormErrors(res) + self.assertMessageCount(error=1) + self.mock_user_get.assert_called_once_with(test.IsHttpRequest(), '1', admin=True) self.mock_domain_get.assert_called_once_with(test.IsHttpRequest(), @@ -578,8 +686,33 @@ class UsersViewTests(test.BaseAdminViewTests): self.assertNoFormErrors(res) - self.mock_user_get.assert_called_once_with(test.IsHttpRequest(), '1', - admin=False) + self.mock_user_get.assert_called_with(test.IsHttpRequest(), '1', + admin=False) + self.assertEqual(self.mock_user_get.call_count, 1) + self.mock_user_update_password.assert_called_once_with( + test.IsHttpRequest(), user.id, test_password, admin=False) + + @test.create_mocks({api.keystone: ('user_get', + 'user_update_password')}) + def test_change_locked_password(self): + user = self.users.get(id="6") + test_password = 'normalpwd' + + self.mock_user_get.return_value = user + self.mock_user_update_password.return_value = None + + formData = {'method': 'ChangePasswordForm', + 'id': user.id, + 'name': user.name, + 'password': test_password, + 'confirm_password': test_password} + + res = self.client.post(USER_CHANGE_PASSWORD_URL, formData) + self.assertRedirectsNoFollow(res, USERS_INDEX_URL) + + self.mock_user_get.assert_called_with(test.IsHttpRequest(), '1', + admin=False) + self.assertEqual(self.mock_user_get.call_count, 1) self.mock_user_update_password.assert_called_once_with( test.IsHttpRequest(), user.id, test_password, admin=False) @@ -605,8 +738,9 @@ class UsersViewTests(test.BaseAdminViewTests): self.assertFormError(res, "form", None, ['The admin password is incorrect.']) - self.mock_user_get.assert_called_once_with(test.IsHttpRequest(), '1', - admin=False) + self.mock_user_get.assert_called_with(test.IsHttpRequest(), '1', + admin=False) + self.assertEqual(self.mock_user_get.call_count, 1) self.mock_user_verify_admin_password.assert_called_once_with( test.IsHttpRequest(), admin_password) @@ -1214,6 +1348,8 @@ class UsersViewTests(test.BaseAdminViewTests): self.mock_user_update.assert_called_once_with(test.IsHttpRequest(), user.id, email=user.email, + options={'lock_password': + False}, name=user.name, description='changed') diff --git a/openstack_dashboard/dashboards/identity/users/views.py b/openstack_dashboard/dashboards/identity/users/views.py index 3d1b808ba7..4abb538a24 100644 --- a/openstack_dashboard/dashboards/identity/users/views.py +++ b/openstack_dashboard/dashboards/identity/users/views.py @@ -130,6 +130,7 @@ class UpdateView(forms.ModalFormView): def get_initial(self): user = self.get_object() + options = getattr(user, "options", {}) domain_id = getattr(user, "domain_id", None) domain_name = '' # Retrieve the domain name where the project belongs @@ -154,7 +155,8 @@ class UpdateView(forms.ModalFormView): 'name': user.name, 'project': user.project_id, 'email': getattr(user, 'email', None), - 'description': getattr(user, 'description', None)} + 'description': getattr(user, 'description', None), + 'lock_password': options.get("lock_password", False)} if api.keystone.VERSIONS.active >= 3: for key in settings.USER_TABLE_EXTRA_INFO: data[key] = getattr(user, key, None) @@ -209,6 +211,8 @@ class DetailView(tabs.TabView): context["user"] = user context["url"] = self.get_redirect_url() context["actions"] = table.render_row_actions(user) + options = getattr(user, "options", {}) + context["lock_password"] = options.get("lock_password", False) return context @memoized.memoized_method diff --git a/openstack_dashboard/dashboards/settings/password/forms.py b/openstack_dashboard/dashboards/settings/password/forms.py index 2497f33ad8..719429f77e 100644 --- a/openstack_dashboard/dashboards/settings/password/forms.py +++ b/openstack_dashboard/dashboards/settings/password/forms.py @@ -59,6 +59,13 @@ class PasswordForm(forms.SelfHandlingForm): @sensitive_variables('data') def handle(self, request, data): user_is_editable = api.keystone.keystone_can_edit_user() + user_id = request.user.id + user = api.keystone.user_get(self.request, user_id, admin=False) + options = getattr(user, "options", {}) + lock_password = options.get("lock_password", False) + if lock_password: + messages.error(request, _('Password is locked.')) + return False if user_is_editable: try: diff --git a/openstack_dashboard/dashboards/settings/password/tests.py b/openstack_dashboard/dashboards/settings/password/tests.py index 9d1d9e4591..31a041022c 100644 --- a/openstack_dashboard/dashboards/settings/password/tests.py +++ b/openstack_dashboard/dashboards/settings/password/tests.py @@ -29,9 +29,11 @@ INDEX_URL = reverse('horizon:settings:password:index') class ChangePasswordTests(test.TestCase): - @mock.patch.object(api.keystone, 'user_update_own_password') - def test_change_password(self, mock_user_update_own_password): - mock_user_update_own_password.return_value = None + @test.create_mocks({ + api.keystone: ['user_update_own_password', 'user_get']}) + def test_change_password(self): + self.mock_user_update_own_password.return_value = None + self.mock_user_get.return_value = mock.Mock(options={}) formData = {'method': 'PasswordForm', 'current_password': 'oldpwd', @@ -40,22 +42,17 @@ class ChangePasswordTests(test.TestCase): res = self.client.post(INDEX_URL, formData) self.assertNoFormErrors(res) - mock_user_update_own_password.assert_called_once_with( + self.mock_user_get.assert_called_once_with(test.IsHttpRequest(), '1', + admin=False) + self.mock_user_update_own_password.assert_called_once_with( test.IsHttpRequest(), 'oldpwd', 'normalpwd') - def test_change_validation_passwords_not_matching(self): - formData = {'method': 'PasswordForm', - 'current_password': 'currpasswd', - 'new_password': 'testpassword', - 'confirm_password': 'doesnotmatch'} - res = self.client.post(INDEX_URL, formData) + @test.create_mocks({ + api.keystone: ['user_update_own_password', 'user_get']}) + def test_change_password_sets_logout_reason(self): - self.assertFormError(res, "form", None, ['Passwords do not match.']) - - @mock.patch.object(api.keystone, 'user_update_own_password') - def test_change_password_sets_logout_reason(self, - mock_user_update_own_password): - mock_user_update_own_password.return_value = None + self.mock_user_update_own_password.return_value = None + self.mock_user_get.return_value = mock.Mock(options={}) formData = {'method': 'PasswordForm', 'current_password': 'oldpwd', @@ -73,5 +70,30 @@ class ChangePasswordTests(test.TestCase): redirect_response = res.client.get(path, http.QueryDict(query)) self.assertRedirectsNoFollow(redirect_response, settings.LOGIN_URL) - mock_user_update_own_password.assert_called_once_with( + self.mock_user_get.assert_called_once_with(test.IsHttpRequest(), '1', + admin=False) + self.mock_user_update_own_password.assert_called_once_with( test.IsHttpRequest(), 'oldpwd', 'normalpwd') + + def test_change_validation_passwords_not_matching(self): + formData = {'method': 'PasswordForm', + 'current_password': 'currpasswd', + 'new_password': 'testpassword', + 'confirm_password': 'doesnotmatch'} + res = self.client.post(INDEX_URL, formData) + + self.assertFormError(res, "form", None, ['Passwords do not match.']) + + @test.create_mocks({ + api.keystone: ['user_get']}) + def test_lock_password(self): + self.mock_user_get.return_value = mock.Mock( + options={'lock_password': True}) + formData = {'method': 'PasswordForm', + 'current_password': 'oldpwd', + 'new_password': 'normalpwd', + 'confirm_password': 'normalpwd'} + res = self.client.post(INDEX_URL, formData) + self.assertNoFormErrors(res) + self.mock_user_get.assert_called_once_with(test.IsHttpRequest(), '1', + admin=False) diff --git a/openstack_dashboard/test/test_data/keystone_data.py b/openstack_dashboard/test/test_data/keystone_data.py index f13636b9ea..9e2902d53f 100644 --- a/openstack_dashboard/test/test_data/keystone_data.py +++ b/openstack_dashboard/test/test_data/keystone_data.py @@ -217,7 +217,18 @@ def data(TEST): 'enabled': True, 'domain_id': "1"} user5 = users.User(None, user_dict) - TEST.users.add(user, user2, user3, user4, user5) + user_dict = {'id': "6", + 'name': 'user_six', + 'description': 'test_description', + 'email': 'test@example.com', + 'password': 'password', + 'token': 'test_token', + 'project_id': '1', + 'enabled': True, + 'domain_id': "1", + 'lock_password': True} + user6 = users.User(None, user_dict) + TEST.users.add(user, user2, user3, user4, user5, user6) TEST.user = user # Your "current" user TEST.user.service_catalog = copy.deepcopy(SERVICE_CATALOG) diff --git a/releasenotes/notes/lock-password-d87b63f361d4bdc1.yaml b/releasenotes/notes/lock-password-d87b63f361d4bdc1.yaml new file mode 100644 index 0000000000..b1d5b39295 --- /dev/null +++ b/releasenotes/notes/lock-password-d87b63f361d4bdc1.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Added support for Keystone locking user option. Locked user can't change + own password using the self-service password change API. By default, users + are unlocked and can change their own passwords. In case of older Keystone + not supporting this feature, all users treated as unlocked.