From: Volker Theile Date: Fri, 30 Aug 2019 15:30:00 +0000 (+0200) Subject: mgr/dashboard: Check password complexity in Dashboard CLI commands X-Git-Tag: v15.1.0~651^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=6f0b3179b91a7c32d6da7ee750d778cc6e9a9289;p=ceph-ci.git mgr/dashboard: Check password complexity in Dashboard CLI commands - Refactor parts of the existing password complexity code. - Check password complexity when setting password via Dashboard CLI commands. - Add ability to force setting a password via CLI. This is useful in vstart environments or wherever it is necessary to disable the password complexity check. Signed-off-by: Volker Theile --- diff --git a/doc/mgr/dashboard.rst b/doc/mgr/dashboard.rst index d004f3d70fc..1dbe9e2b779 100644 --- a/doc/mgr/dashboard.rst +++ b/doc/mgr/dashboard.rst @@ -615,7 +615,7 @@ We provide a set of CLI commands to manage user accounts: - *Create User*:: - $ ceph dashboard ac-user-create [] [] [] [] [] + $ ceph dashboard ac-user-create [--force-password] [] [] [] [] [--enabled] - *Delete User*:: @@ -623,7 +623,7 @@ We provide a set of CLI commands to manage user accounts: - *Change Password*:: - $ ceph dashboard ac-user-set-password + $ ceph dashboard ac-user-set-password [--force-password] - *Change Password Hash*:: diff --git a/qa/tasks/mgr/dashboard/helper.py b/qa/tasks/mgr/dashboard/helper.py index 90e5d61dc19..98064962b67 100644 --- a/qa/tasks/mgr/dashboard/helper.py +++ b/qa/tasks/mgr/dashboard/helper.py @@ -35,7 +35,18 @@ class DashboardTestCase(MgrTestCase): AUTH_ROLES = ['administrator'] @classmethod - def create_user(cls, username, password, roles): + def create_user(cls, username, password, roles=None, force_password=True): + """ + :param username: The name of the user. + :type username: str + :param password: The password. + :type password: str + :param roles: A list of roles. + :type roles: list + :param force_password: Force the use of the specified password. This + will bypass the password complexity check. Defaults to 'True'. + :type force_password: bool + """ try: cls._ceph_cmd(['dashboard', 'ac-user-show', username]) cls._ceph_cmd(['dashboard', 'ac-user-delete', username]) @@ -43,28 +54,32 @@ class DashboardTestCase(MgrTestCase): if ex.exitstatus != 2: raise ex - cls._ceph_cmd(['dashboard', 'ac-user-create', username, password]) - - set_roles_args = ['dashboard', 'ac-user-set-roles', username] - for idx, role in enumerate(roles): - if isinstance(role, str): - set_roles_args.append(role) - else: - assert isinstance(role, dict) - rolename = 'test_role_{}'.format(idx) - try: - cls._ceph_cmd(['dashboard', 'ac-role-show', rolename]) - cls._ceph_cmd(['dashboard', 'ac-role-delete', rolename]) - except CommandFailedError as ex: - if ex.exitstatus != 2: - raise ex - cls._ceph_cmd(['dashboard', 'ac-role-create', rolename]) - for mod, perms in role.items(): - args = ['dashboard', 'ac-role-add-scope-perms', rolename, mod] - args.extend(perms) - cls._ceph_cmd(args) - set_roles_args.append(rolename) - cls._ceph_cmd(set_roles_args) + user_create_args = ['dashboard', 'ac-user-create', username, password] + if force_password: + user_create_args.append('--force-password') + cls._ceph_cmd(user_create_args) + + if roles: + set_roles_args = ['dashboard', 'ac-user-set-roles', username] + for idx, role in enumerate(roles): + if isinstance(role, str): + set_roles_args.append(role) + else: + assert isinstance(role, dict) + rolename = 'test_role_{}'.format(idx) + try: + cls._ceph_cmd(['dashboard', 'ac-role-show', rolename]) + cls._ceph_cmd(['dashboard', 'ac-role-delete', rolename]) + except CommandFailedError as ex: + if ex.exitstatus != 2: + raise ex + cls._ceph_cmd(['dashboard', 'ac-role-create', rolename]) + for mod, perms in role.items(): + args = ['dashboard', 'ac-role-add-scope-perms', rolename, mod] + args.extend(perms) + cls._ceph_cmd(args) + set_roles_args.append(rolename) + cls._ceph_cmd(set_roles_args) @classmethod def login(cls, username, password): @@ -368,6 +383,12 @@ class DashboardTestCase(MgrTestCase): log.info("command result: %s", res) return res + @classmethod + def _ceph_cmd_result(cls, cmd): + exitstatus = cls.mgr_cluster.mon_manager.raw_cluster_cmd_result(*cmd) + log.info("command exit status: %d", exitstatus) + return exitstatus + def set_config_key(self, key, value): self._ceph_cmd(['config-key', 'set', key, value]) diff --git a/qa/tasks/mgr/dashboard/test_auth.py b/qa/tasks/mgr/dashboard/test_auth.py index e58112dfe99..0fd5b3080c9 100644 --- a/qa/tasks/mgr/dashboard/test_auth.py +++ b/qa/tasks/mgr/dashboard/test_auth.py @@ -131,7 +131,8 @@ class AuthTest(DashboardTestCase): self._get("/api/host") self.assertStatus(200) time.sleep(1) - self._ceph_cmd(['dashboard', 'ac-user-set-password', 'user', 'user2']) + self._ceph_cmd(['dashboard', 'ac-user-set-password', '--force-password', + 'user', 'user2']) time.sleep(1) self._get("/api/host") self.assertStatus(401) diff --git a/qa/tasks/mgr/dashboard/test_user.py b/qa/tasks/mgr/dashboard/test_user.py index 29e182ebcb4..a66228539a8 100644 --- a/qa/tasks/mgr/dashboard/test_user.py +++ b/qa/tasks/mgr/dashboard/test_user.py @@ -8,7 +8,6 @@ from .helper import DashboardTestCase class UserTest(DashboardTestCase): - @classmethod def _create_user(cls, username=None, password=None, name=None, email=None, roles=None, enabled=True): data = {} @@ -179,62 +178,67 @@ class UserTest(DashboardTestCase): self.assertError(code='invalid_old_password', component='user') def test_change_password_as_old_password(self): - self.create_user('test1', 'mypassword10#', ['read-only']) + self.create_user('test1', 'mypassword10#', ['read-only'], force_password=False) self.login('test1', 'mypassword10#') self._post('/api/user/test1/change_password', { 'old_password': 'mypassword10#', 'new_password': 'mypassword10#' }) self.assertStatus(400) - self.assertError(code='pwd-must-not-be-last-one', component='user') + self.assertError('password_policy_validation_failed', 'user', + 'Password cannot be the same as the previous one.') self._reset_login_to_admin('test1') def test_change_password_contains_username(self): - self.create_user('test1', 'mypassword10#', ['read-only']) + self.create_user('test1', 'mypassword10#', ['read-only'], force_password=False) self.login('test1', 'mypassword10#') self._post('/api/user/test1/change_password', { 'old_password': 'mypassword10#', 'new_password': 'mypasstest1@#' }) self.assertStatus(400) - self.assertError(code='pwd-must-not-contain-username', component='user') + self.assertError('password_policy_validation_failed', 'user', + 'Password cannot contain username.') self._reset_login_to_admin('test1') def test_change_password_contains_forbidden_words(self): - self.create_user('test1', 'mypassword10#', ['read-only']) + self.create_user('test1', 'mypassword10#', ['read-only'], force_password=False) self.login('test1', 'mypassword10#') self._post('/api/user/test1/change_password', { 'old_password': 'mypassword10#', 'new_password': 'mypassOSD01' }) self.assertStatus(400) - self.assertError(code='pwd-must-not-contain-forbidden-keywords', component='user') + self.assertError('password_policy_validation_failed', 'user', + 'Password cannot contain keywords.') self._reset_login_to_admin('test1') def test_change_password_contains_sequential_characters(self): - self.create_user('test1', 'mypassword10#', ['read-only']) + self.create_user('test1', 'mypassword10#', ['read-only'], force_password=False) self.login('test1', 'mypassword10#') self._post('/api/user/test1/change_password', { 'old_password': 'mypassword10#', 'new_password': 'mypass123456!@$' }) self.assertStatus(400) - self.assertError(code='pwd-must-not-contain-sequential-chars', component='user') + self.assertError('password_policy_validation_failed', 'user', + 'Password cannot contain sequential characters.') self._reset_login_to_admin('test1') def test_change_password_contains_repetetive_characters(self): - self.create_user('test1', 'mypassword10#', ['read-only']) + self.create_user('test1', 'mypassword10#', ['read-only'], force_password=False) self.login('test1', 'mypassword10#') self._post('/api/user/test1/change_password', { 'old_password': 'mypassword10#', 'new_password': 'aaaaA1@!#' }) self.assertStatus(400) - self.assertError(code='pwd-must-not-contain-repetitive-chars', component='user') + self.assertError('password_policy_validation_failed', 'user', + 'Password cannot contain repetitive characters.') self._reset_login_to_admin('test1') def test_change_password(self): - self.create_user('test1', 'mypassword10#', ['read-only']) + self.create_user('test1', 'mypassword10#', ['read-only'], force_password=False) self.login('test1', 'mypassword10#') self._post('/api/user/test1/change_password', { 'old_password': 'mypassword10#', @@ -247,3 +251,42 @@ class UserTest(DashboardTestCase): self.assertError(code='invalid_credentials', component='auth') self.delete_user('test1') self.login('admin', 'admin') + + def test_create_user_password_cli(self): + exitcode = self._ceph_cmd_result(['dashboard', 'ac-user-create', + 'test1', 'mypassword10#']) + self.assertEqual(exitcode, 0) + self.delete_user('test1') + + def test_change_user_password_cli(self): + self.create_user('test2', 'foo_bar_10#', force_password=False) + exitcode = self._ceph_cmd_result(['dashboard', 'ac-user-set-password', + 'test2', 'foo_new-password01#']) + self.assertEqual(exitcode, 0) + self.delete_user('test2') + + def test_create_user_password_force_cli(self): + exitcode = self._ceph_cmd_result(['dashboard', 'ac-user-create', + '--force-password', 'test11', + 'bar']) + self.assertEqual(exitcode, 0) + self.delete_user('test11') + + def test_change_user_password_force_cli(self): + self.create_user('test22', 'foo_bar_10#', force_password=False) + exitcode = self._ceph_cmd_result(['dashboard', 'ac-user-set-password', + '--force-password', 'test22', + 'bar']) + self.assertEqual(exitcode, 0) + self.delete_user('test22') + + def test_create_user_password_cli_fail(self): + exitcode = self._ceph_cmd_result(['dashboard', 'ac-user-create', 'test3', 'foo']) + self.assertNotEqual(exitcode, 0) + + def test_change_user_password_cli_fail(self): + self.create_user('test4', 'x1z_tst+_10#', force_password=False) + exitcode = self._ceph_cmd_result(['dashboard', 'ac-user-set-password', + 'test4', 'bar']) + self.assertNotEqual(exitcode, 0) + self.delete_user('test4') diff --git a/src/pybind/mgr/dashboard/controllers/user.py b/src/pybind/mgr/dashboard/controllers/user.py index a75c99e033f..560471957af 100644 --- a/src/pybind/mgr/dashboard/controllers/user.py +++ b/src/pybind/mgr/dashboard/controllers/user.py @@ -6,40 +6,19 @@ import cherrypy from . import BaseController, ApiController, RESTController, Endpoint from .. import mgr from ..exceptions import DashboardException, UserAlreadyExists, \ - UserDoesNotExist + UserDoesNotExist, PasswordCheckException from ..security import Scope from ..services.access_control import SYSTEM_ROLES, PasswordCheck from ..services.auth import JwtManager -def check_password_complexity(password, username, old_password=None): - password_complexity = PasswordCheck(password, username, old_password) - if password_complexity.check_if_as_the_old_password(): - raise DashboardException(msg='Password cannot be the\ - same as the previous one.', - code='pwd-must-not-be-last-one', - component='user') - if password_complexity.check_if_contains_username(): - raise DashboardException(msg='Password cannot contain username.', - code='pwd-must-not-contain-username', - component='user') - if password_complexity.check_if_contains_forbidden_words(): - raise DashboardException(msg='Password cannot contain keywords.', - code='pwd-must-not-contain-forbidden-keywords', - component='user') - if password_complexity.check_if_repetetive_characters(): - raise DashboardException(msg='Password cannot contain repetitive \ - characters.', - code='pwd-must-not-contain-repetitive-chars', - component='user') - if password_complexity.check_if_sequential_characters(): - raise DashboardException(msg='Password cannot contain sequential \ - characters.', - code='pwd-must-not-contain-sequential-chars', - component='user') - if password_complexity.check_password_characters() < 10: - raise DashboardException(msg='Password is too weak.', - code='pwd-too-weak', +def validate_password_policy(password, username, old_password=None): + pw_check = PasswordCheck(password, username, old_password) + try: + pw_check.check_all() + except PasswordCheckException as ex: + raise DashboardException(msg=str(ex), + code='password_policy_validation_failed', component='user') @@ -84,7 +63,7 @@ class User(RESTController): if roles: user_roles = User._get_user_roles(roles) if password: - check_password_complexity(password, username) + validate_password_policy(password, username) try: user = mgr.ACCESS_CTRL_DB.create_user(username, password, name, email, enabled) @@ -124,7 +103,7 @@ class User(RESTController): if roles: user_roles = User._get_user_roles(roles) if password: - check_password_complexity(password, username) + validate_password_policy(password, username) user.set_password(password) user.name = name user.email = email @@ -152,6 +131,6 @@ class UserChangePassword(BaseController): raise DashboardException(msg='Invalid old password', code='invalid_old_password', component='user') - check_password_complexity(new_password, username, old_password) + validate_password_policy(new_password, username, old_password) user.set_password(new_password) mgr.ACCESS_CTRL_DB.save() diff --git a/src/pybind/mgr/dashboard/exceptions.py b/src/pybind/mgr/dashboard/exceptions.py index 83de90f0619..5d4badd7ff8 100644 --- a/src/pybind/mgr/dashboard/exceptions.py +++ b/src/pybind/mgr/dashboard/exceptions.py @@ -105,3 +105,7 @@ class RoleNotInUser(Exception): class GrafanaError(Exception): pass + + +class PasswordCheckException(Exception): + pass diff --git a/src/pybind/mgr/dashboard/services/access_control.py b/src/pybind/mgr/dashboard/services/access_control.py index d3d12730082..a5768a5c69e 100644 --- a/src/pybind/mgr/dashboard/services/access_control.py +++ b/src/pybind/mgr/dashboard/services/access_control.py @@ -20,7 +20,7 @@ from ..security import Scope, Permission from ..exceptions import RoleAlreadyExists, RoleDoesNotExist, ScopeNotValid, \ PermissionNotValid, RoleIsAssociatedWithUser, \ UserAlreadyExists, UserDoesNotExist, ScopeNotInRole, \ - RoleNotInUser + RoleNotInUser, PasswordCheckException # password hashing algorithm @@ -39,6 +39,14 @@ _P = Permission # short alias class PasswordCheck(object): def __init__(self, password, username, old_password=None): + """ + :param password: The new plain password. + :type password: str + :param username: The name of the user. + :type username: str + :param old_password: The old plain password. + :type old_password: str | None + """ self.password = password self.username = username self.old_password = old_password @@ -58,20 +66,20 @@ class PasswordCheck(object): big_letter_credit = 2 special_character_credit = 3 other_character_credit = 5 - for _ in self.password: - if _ in ascii_uppercase: + for ch in self.password: + if ch in ascii_uppercase: self.complexity_credits += big_letter_credit - elif _ in ascii_lowercase: + elif ch in ascii_lowercase: self.complexity_credits += small_letter_credit - elif _ in digits: + elif ch in digits: self.complexity_credits += digit_credit - elif _ in punctuation: + elif ch in punctuation: self.complexity_credits += special_character_credit else: self.complexity_credits += other_character_credit return self.complexity_credits - def check_if_as_the_old_password(self): + def check_is_old_password(self): return self.old_password and self.password == self.old_password def check_if_contains_username(self): @@ -82,18 +90,36 @@ class PasswordCheck(object): '|'.join(self.forbidden_words)) def check_if_sequential_characters(self): - for _ in range(1, len(self.password)-1): - if ord(self.password[_-1])+1 == ord(self.password[_])\ - == ord(self.password[_+1])-1: + for i in range(1, len(self.password) - 1): + if ord(self.password[i - 1]) + 1 == ord(self.password[i])\ + == ord(self.password[i + 1]) - 1: return True return False def check_if_repetetive_characters(self): - for _ in range(1, len(self.password)-1): - if self.password[_-1] == self.password[_] == self.password[_+1]: + for i in range(1, len(self.password) - 1): + if self.password[i - 1] == self.password[i] == self.password[i + 1]: return True return False + def check_all(self): + """ + Perform all password policy checks. + :raise PasswordCheckException: If a password policy check fails. + """ + if self.check_is_old_password(): + raise PasswordCheckException('Password cannot be the same as the previous one.') + if self.check_if_contains_username(): + raise PasswordCheckException('Password cannot contain username.') + if self.check_if_contains_forbidden_words(): + raise PasswordCheckException('Password cannot contain keywords.') + if self.check_if_repetetive_characters(): + raise PasswordCheckException('Password cannot contain repetitive characters.') + if self.check_if_sequential_characters(): + raise PasswordCheckException('Password cannot contain sequential characters.') + if self.check_password_characters() < 10: + raise PasswordCheckException('Password is too weak.') + class Role(object): def __init__(self, name, description=None, scope_permissions=None): @@ -613,10 +639,11 @@ def ac_user_show_cmd(_, username=None): 'name=rolename,type=CephString,req=false ' 'name=name,type=CephString,req=false ' 'name=email,type=CephString,req=false ' - 'name=enabled,type=CephBool,req=false', + 'name=enabled,type=CephBool,req=false ' + 'name=force_password,type=CephBool,req=false', 'Create a user') def ac_user_create_cmd(_, username, password=None, rolename=None, name=None, - email=None, enabled=True): + email=None, enabled=True, force_password=False): try: role = mgr.ACCESS_CTRL_DB.get_role(rolename) if rolename else None except RoleDoesNotExist as ex: @@ -625,7 +652,12 @@ def ac_user_create_cmd(_, username, password=None, rolename=None, name=None, role = SYSTEM_ROLES[rolename] try: + if not force_password: + pw_check = PasswordCheck(password, username) + pw_check.check_all() user = mgr.ACCESS_CTRL_DB.create_user(username, password, name, email, enabled) + except PasswordCheckException as ex: + return -errno.EINVAL, '', str(ex) except UserAlreadyExists as ex: return -errno.EEXIST, '', str(ex) @@ -748,15 +780,20 @@ def ac_user_del_roles_cmd(_, username, roles): @CLIWriteCommand('dashboard ac-user-set-password', 'name=username,type=CephString ' - 'name=password,type=CephString', + 'name=password,type=CephString ' + 'name=force_password,type=CephBool,req=false', 'Set user password') -def ac_user_set_password(_, username, password): +def ac_user_set_password(_, username, password, force_password=False): try: user = mgr.ACCESS_CTRL_DB.get_user(username) + if not force_password: + pw_check = PasswordCheck(password, user.name) + pw_check.check_all() user.set_password(password) - mgr.ACCESS_CTRL_DB.save() return 0, json.dumps(user.to_dict()), '' + except PasswordCheckException as ex: + return -errno.EINVAL, '', str(ex) except UserDoesNotExist as ex: return -errno.ENOENT, '', str(ex) diff --git a/src/pybind/mgr/dashboard/tests/test_access_control.py b/src/pybind/mgr/dashboard/tests/test_access_control.py index 39aa02101ef..337d5cb53d8 100644 --- a/src/pybind/mgr/dashboard/tests/test_access_control.py +++ b/src/pybind/mgr/dashboard/tests/test_access_control.py @@ -276,7 +276,7 @@ class AccessControlTest(unittest.TestCase, CLICommandTestMixin): rolename=rolename, password='admin', name='{} User'.format(username), email='{}@user.com'.format(username), - enabled=enabled) + enabled=enabled, force_password=True) pass_hash = password_hash('admin', user['password']) self.assertDictEqual(user, { @@ -316,7 +316,8 @@ class AccessControlTest(unittest.TestCase, CLICommandTestMixin): self.test_create_user() with self.assertRaises(CmdException) as ctx: - self.exec_cmd('ac-user-create', username='admin', password='admin') + self.exec_cmd('ac-user-create', username='admin', password='admin', + force_password=True) self.assertEqual(ctx.exception.retcode, -errno.EEXIST) self.assertEqual(str(ctx.exception), "User 'admin' already exists") @@ -329,7 +330,8 @@ class AccessControlTest(unittest.TestCase, CLICommandTestMixin): try: self.exec_cmd('ac-user-create', username='foo', rolename='dne_role', password='foopass', - name='foo User', email='foo@user.com') + name='foo User', email='foo@user.com', + force_password=True) except CmdException as e: self.assertEqual(e.retcode, -errno.ENOENT) @@ -349,7 +351,8 @@ class AccessControlTest(unittest.TestCase, CLICommandTestMixin): self.test_create_role() self.exec_cmd('ac-user-create', username='bar', rolename='test_role', password='barpass', - name='bar User', email='bar@user.com') + name='bar User', email='bar@user.com', + force_password=True) # validate db: # user 'foo' should not exist @@ -549,7 +552,7 @@ class AccessControlTest(unittest.TestCase, CLICommandTestMixin): def test_set_user_password(self): user_orig = self.test_create_user() user = self.exec_cmd('ac-user-set-password', username='admin', - password='newpass') + password='newpass', force_password=True) pass_hash = password_hash('newpass', user['password']) self.assertDictEqual(user, { 'username': 'admin', @@ -567,7 +570,7 @@ class AccessControlTest(unittest.TestCase, CLICommandTestMixin): def test_set_user_password_nonexistent_user(self): with self.assertRaises(CmdException) as ctx: self.exec_cmd('ac-user-set-password', username='admin', - password='newpass') + password='newpass', force_password=True) self.assertEqual(ctx.exception.retcode, -errno.ENOENT) self.assertEqual(str(ctx.exception), "User 'admin' does not exist") diff --git a/src/vstart.sh b/src/vstart.sh index abc3f676f9c..39afc6a8750 100755 --- a/src/vstart.sh +++ b/src/vstart.sh @@ -946,7 +946,7 @@ EOF debug echo 'waiting for mgr dashboard module to start' sleep 1 done - ceph_adm dashboard ac-user-create admin admin administrator + ceph_adm dashboard ac-user-create --force-password admin admin administrator if [ "$ssl" != "0" ]; then if ! ceph_adm dashboard create-self-signed-cert; then debug echo dashboard module not working correctly!