From 4198cbf87d364227f6f033ef7105dc0302951be1 Mon Sep 17 00:00:00 2001 From: Cindy Lu Date: Tue, 24 Feb 2015 15:23:50 -0800 Subject: [PATCH] Double check admin password when update user password Add a new setting to enable a new textbox field on the Change Password form that will double check the logged-in Admin user's password. For example if the logged-in admin leaves an unattended computer, someone can change the password of the logged in user successfully. Co-Authored-By: Steve Martinelli Change-Id: Icafa8ce8ab30ec43d3f6419a77118a634a163870 Closes-Bug: #1387372 --- doc/source/topics/settings.rst | 11 ++++++++ openstack_dashboard/api/keystone.py | 21 ++++++++++++++ .../dashboards/identity/users/forms.py | 21 ++++++++++++-- .../dashboards/identity/users/tests.py | 28 +++++++++++++++++++ .../local/local_settings.py.example | 5 ++++ 5 files changed, 83 insertions(+), 3 deletions(-) diff --git a/doc/source/topics/settings.rst b/doc/source/topics/settings.rst index 5b18d3ed76..28ceac6fb1 100755 --- a/doc/source/topics/settings.rst +++ b/doc/source/topics/settings.rst @@ -379,6 +379,17 @@ This setting sets the maximum number of items displayed in a dropdown. Dropdowns that limit based on this value need to support a way to observe the entire list. +``ENFORCE_PASSWORD_CHECK`` +-------------------------- + +.. versionadded:: 2015.1(Kilo) + +Default: ``False`` + +This setting will display an 'Admin Password' field on the Change Password +form to verify that it is indeed the admin logged-in who wants to change +the password. + ``IMAGES_LIST_FILTER_TENANTS`` ------------------------------ diff --git a/openstack_dashboard/api/keystone.py b/openstack_dashboard/api/keystone.py index 33756999d9..da726d1c35 100644 --- a/openstack_dashboard/api/keystone.py +++ b/openstack_dashboard/api/keystone.py @@ -397,6 +397,27 @@ def user_update_password(request, user, password, admin=True): return manager.update(user, password=password) +def user_verify_admin_password(request, admin_password): + # attempt to create a new client instance with admin password to + # verify if it's correct. + client = keystone_client_v2 if VERSIONS.active < 3 else keystone_client_v3 + try: + endpoint = _get_endpoint_url(request, 'internalURL') + insecure = getattr(settings, 'OPENSTACK_SSL_NO_VERIFY', False) + cacert = getattr(settings, 'OPENSTACK_SSL_CACERT', None) + client.Client( + username=request.user.username, + password=admin_password, + insecure=insecure, + cacert=cacert, + auth_url=endpoint + ) + return True + except Exception: + exceptions.handle(request, ignore=True) + return False + + def user_update_own_password(request, origpassword, password): client = keystoneclient(request, admin=False) client.user_id = request.user.id diff --git a/openstack_dashboard/dashboards/identity/users/forms.py b/openstack_dashboard/dashboards/identity/users/forms.py index a0659b255f..84c5730f9c 100644 --- a/openstack_dashboard/dashboards/identity/users/forms.py +++ b/openstack_dashboard/dashboards/identity/users/forms.py @@ -18,6 +18,7 @@ import logging +from django.conf import settings from django.forms import ValidationError # noqa from django import http from django.utils.translation import ugettext_lazy as _ @@ -223,17 +224,31 @@ class ChangePasswordForm(PasswordMixin, forms.SelfHandlingForm): def __init__(self, request, *args, **kwargs): super(ChangePasswordForm, self).__init__(request, *args, **kwargs) - # Reorder form fields from multiple inheritance - self.fields.keyOrder = ["id", "name", "password", "confirm_password"] + if getattr(settings, 'ENFORCE_PASSWORD_CHECK', False): + self.fields["admin_password"] = forms.CharField( + label=_("Admin Password"), + widget=forms.PasswordInput(render_value=False)) + # Reorder form fields from multiple inheritance + self.fields.keyOrder = ["id", "name", "admin_password", + "password", "confirm_password"] - @sensitive_variables('data', 'password') + @sensitive_variables('data', 'password', 'admin_password') def handle(self, request, data): user_id = data.pop('id') password = data.pop('password') + admin_password = None # Throw away the password confirmation, we're done with it. data.pop('confirm_password', None) + # Verify admin password before changing user password + if getattr(settings, 'ENFORCE_PASSWORD_CHECK', False): + admin_password = data.pop('admin_password') + if not api.keystone.user_verify_admin_password(request, + admin_password): + self.api_error(_('The admin password is incorrect.')) + return False + try: response = api.keystone.user_update_password( request, user_id, password) diff --git a/openstack_dashboard/dashboards/identity/users/tests.py b/openstack_dashboard/dashboards/identity/users/tests.py index a1e0ef998a..6331371761 100644 --- a/openstack_dashboard/dashboards/identity/users/tests.py +++ b/openstack_dashboard/dashboards/identity/users/tests.py @@ -20,6 +20,7 @@ from socket import timeout as socket_timeout # noqa from django.core.urlresolvers import reverse from django import http +from django.test.utils import override_settings from mox import IgnoreArg # noqa from mox import IsA # noqa @@ -409,6 +410,33 @@ class UsersViewTests(test.BaseAdminViewTests): self.assertNoFormErrors(res) + @test.create_stubs({api.keystone: ('user_get', + 'user_verify_admin_password')}) + @override_settings(ENFORCE_PASSWORD_CHECK=True) + def test_change_password_validation_for_admin_password(self): + user = self.users.get(id="1") + test_password = 'normalpwd' + admin_password = 'secret' + + api.keystone.user_get(IsA(http.HttpRequest), '1', + admin=True).AndReturn(user) + api.keystone.user_verify_admin_password( + IsA(http.HttpRequest), admin_password).AndReturn(None) + + self.mox.ReplayAll() + + formData = {'method': 'ChangePasswordForm', + 'id': user.id, + 'name': user.name, + 'password': test_password, + 'confirm_password': test_password, + 'admin_password': admin_password} + + res = self.client.post(USER_CHANGE_PASSWORD_URL, formData) + + self.assertFormError(res, "form", None, + ['The admin password is incorrect.']) + @test.create_stubs({api.keystone: ('user_get',)}) def test_update_validation_for_password_too_short(self): user = self.users.get(id="1") diff --git a/openstack_dashboard/local/local_settings.py.example b/openstack_dashboard/local/local_settings.py.example index 27a61b0736..1f0b89983d 100644 --- a/openstack_dashboard/local/local_settings.py.example +++ b/openstack_dashboard/local/local_settings.py.example @@ -278,6 +278,11 @@ TIME_ZONE = "UTC" # 'reverse': False, #} +# Set this to True to display an 'Admin Password' field on the Change Password +# form to verify that it is indeed the admin logged-in who wants to change +# the password. +# ENFORCE_PASSWORD_CHECK = False + # The Horizon Policy Enforcement engine uses these values to load per service # policy rule files. The content of these files should match the files the # OpenStack services are using to determine role based access control in the