From: Volker Theile Date: Thu, 5 Dec 2019 14:40:54 +0000 (+0100) Subject: mgr/dashboard: Process password complexity checks immediately X-Git-Tag: v15.1.0~236^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=01c4bcadf6ee381e4947266c3b3eeebd0c0a9904;p=ceph.git mgr/dashboard: Process password complexity checks immediately - Add 'api/user/validate_password' endpoint to check if a password meets the password policy. A new controller has to be added for 'api/user' which has NO security scope, otherwise it wouldn't be possible for users without USER privileges to call the endpoint. - Add Angular async validator to check if the entered password meets the policy. Fixes: https://tracker.ceph.com/issues/43088 Signed-off-by: Volker Theile --- diff --git a/qa/tasks/mgr/dashboard/test_user.py b/qa/tasks/mgr/dashboard/test_user.py index 115e554ec11..f1dd9f15c8f 100644 --- a/qa/tasks/mgr/dashboard/test_user.py +++ b/qa/tasks/mgr/dashboard/test_user.py @@ -6,7 +6,7 @@ import time from datetime import datetime, timedelta -from .helper import DashboardTestCase +from .helper import DashboardTestCase, JObj, JLeaf class UserTest(DashboardTestCase): @@ -389,3 +389,81 @@ class UserTest(DashboardTestCase): self._delete('/api/user/user1') self._ceph_cmd(['dashboard', 'set-user-pwd-expiration-span', '0']) + + def test_validate_password_weak(self): + data = self._post('/api/user/validate_password', { + 'password': 'mypassword1' + }) + self.assertStatus(200) + self.assertSchema(data, JObj(sub_elems={ + 'valid': JLeaf(bool), + 'credits': JLeaf(int), + 'valuation': JLeaf(str) + })) + self.assertTrue(data['valid']) + self.assertEqual(data['credits'], 11) + self.assertEqual(data['valuation'], 'Weak') + + def test_validate_password_ok(self): + data = self._post('/api/user/validate_password', { + 'password': 'mypassword1!@' + }) + self.assertStatus(200) + self.assertTrue(data['valid']) + self.assertEqual(data['credits'], 17) + self.assertEqual(data['valuation'], 'OK') + + def test_validate_password_strong(self): + data = self._post('/api/user/validate_password', { + 'password': 'testpassword0047!@' + }) + self.assertStatus(200) + self.assertTrue(data['valid']) + self.assertEqual(data['credits'], 22) + self.assertEqual(data['valuation'], 'Strong') + + def test_validate_password_very_strong(self): + data = self._post('/api/user/validate_password', { + 'password': 'testpassword#!$!@$' + }) + self.assertStatus(200) + self.assertTrue(data['valid']) + self.assertEqual(data['credits'], 30) + self.assertEqual(data['valuation'], 'Very strong') + + def test_validate_password_fail(self): + data = self._post('/api/user/validate_password', { + 'password': 'foo' + }) + self.assertStatus(200) + self.assertFalse(data['valid']) + self.assertEqual(data['credits'], 0) + self.assertEqual(data['valuation'], 'Password is too weak.') + + def test_validate_password_fail_name(self): + data = self._post('/api/user/validate_password', { + 'password': 'x1zhugo_10', + 'username': 'hugo' + }) + self.assertStatus(200) + self.assertFalse(data['valid']) + self.assertEqual(data['credits'], 0) + self.assertEqual(data['valuation'], 'Password cannot contain username.') + + def test_validate_password_fail_oldpwd(self): + data = self._post('/api/user/validate_password', { + 'password': 'x1zt-st10', + 'old_password': 'x1zt-st10' + }) + self.assertStatus(200) + self.assertFalse(data['valid']) + self.assertEqual(data['credits'], 0) + self.assertEqual(data['valuation'], 'Password cannot be the same as the previous one.') + + @DashboardTestCase.RunAs('test', 'test', [{'user': ['read', 'delete']}]) + def test_validate_password_invalid_permissions(self): + self._post('/api/user/validate_password', { + 'password': 'foo' + }) + self.assertStatus(403) + self.assertError(code='invalid_credentials', component='auth') diff --git a/src/pybind/mgr/dashboard/controllers/__init__.py b/src/pybind/mgr/dashboard/controllers/__init__.py index 114f4b503cf..e539008eb52 100644 --- a/src/pybind/mgr/dashboard/controllers/__init__.py +++ b/src/pybind/mgr/dashboard/controllers/__init__.py @@ -913,20 +913,32 @@ def _set_func_permissions(func, permissions): def ReadPermission(func): # noqa: N802 + """ + :raises PermissionNotValid: If the permission is missing. + """ _set_func_permissions(func, Permission.READ) return func def CreatePermission(func): # noqa: N802 + """ + :raises PermissionNotValid: If the permission is missing. + """ _set_func_permissions(func, Permission.CREATE) return func def DeletePermission(func): # noqa: N802 + """ + :raises PermissionNotValid: If the permission is missing. + """ _set_func_permissions(func, Permission.DELETE) return func def UpdatePermission(func): # noqa: N802 + """ + :raises PermissionNotValid: If the permission is missing. + """ _set_func_permissions(func, Permission.UPDATE) return func diff --git a/src/pybind/mgr/dashboard/controllers/user.py b/src/pybind/mgr/dashboard/controllers/user.py index 645f5cbc2be..b70a9837bbf 100644 --- a/src/pybind/mgr/dashboard/controllers/user.py +++ b/src/pybind/mgr/dashboard/controllers/user.py @@ -10,17 +10,26 @@ import cherrypy from . import BaseController, ApiController, RESTController, Endpoint from .. import mgr from ..exceptions import DashboardException, UserAlreadyExists, \ - UserDoesNotExist, PasswordCheckException, PwdExpirationDateNotValid + UserDoesNotExist, PasswordPolicyException, PwdExpirationDateNotValid from ..security import Scope -from ..services.access_control import SYSTEM_ROLES, PasswordCheck +from ..services.access_control import SYSTEM_ROLES, PasswordPolicy from ..services.auth import JwtManager -def validate_password_policy(password, username, old_password=None): - pw_check = PasswordCheck(password, username, old_password) +def validate_password_policy(password, username=None, old_password=None): + """ + :param password: The password to validate. + :param username: The name of the user (optional). + :param old_password: The old password (optional). + :return: Returns the password complexity credits. + :rtype: int + :raises DashboardException: If a password policy fails. + """ + pw_policy = PasswordPolicy(password, username, old_password) try: - pw_check.check_all() - except PasswordCheckException as ex: + pw_policy.check_all() + return pw_policy.complexity_credits + except PasswordPolicyException as ex: raise DashboardException(msg=str(ex), code='password_policy_validation_failed', component='user') @@ -130,6 +139,36 @@ class User(RESTController): return User._user_to_dict(user) +@ApiController('/user') +class UserPasswordPolicy(RESTController): + @Endpoint('POST') + def validate_password(self, password, username=None, old_password=None): + """ + Check if the password meets the password policy. + :param password: The password to validate. + :param username: The name of the user (optional). + :param old_password: The old password (optional). + :return: An object with the properties valid, credits and valuation. + 'credits' contains the password complexity credits and + 'valuation' the textual summary of the validation. + """ + result = {'valid': False, 'credits': 0, 'valuation': None} + try: + result['credits'] = validate_password_policy(password, username, old_password) + if result['credits'] < 15: + result['valuation'] = 'Weak' + elif result['credits'] < 20: + result['valuation'] = 'OK' + elif result['credits'] < 25: + result['valuation'] = 'Strong' + else: + result['valuation'] = 'Very strong' + result['valid'] = True + except DashboardException as ex: + result['valuation'] = str(ex) + return result + + @ApiController('/user/{username}') class UserChangePassword(BaseController): @Endpoint('POST') diff --git a/src/pybind/mgr/dashboard/exceptions.py b/src/pybind/mgr/dashboard/exceptions.py index 672caeff180..d537efb6e11 100644 --- a/src/pybind/mgr/dashboard/exceptions.py +++ b/src/pybind/mgr/dashboard/exceptions.py @@ -113,5 +113,5 @@ class GrafanaError(Exception): pass -class PasswordCheckException(Exception): +class PasswordPolicyException(Exception): pass diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.html index 419c0059122..fd942c386c1 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.html @@ -40,7 +40,7 @@ for="password"> Password + html="{{ passwordPolicyHelpText }}">
@@ -59,18 +59,19 @@
-
-
+
+ title="{{ passwordValuation }}">
This field is required. Too weak + *ngIf="userForm.showError('password', formDir, 'passwordPolicy')"> + {{ passwordValuation }} +
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.spec.ts index 3a0c785b476..b106527aa8b 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.spec.ts @@ -110,41 +110,6 @@ describe('UserFormComponent', () => { formHelper.expectValidChange('confirmpassword', 'aaa'); }); - it('should validate password strength very strong', () => { - formHelper.setValue('password', 'testpassword#!$!@$'); - component.checkPassword('testpassword#!$!@$'); - expect(component.passwordStrengthDescription).toBe('Very strong'); - expect(component.passwordStrengthLevel).toBe('passwordStrengthLevel4'); - }); - - it('should validate password strength strong', () => { - formHelper.setValue('password', 'testpassword0047!@'); - component.checkPassword('testpassword0047!@'); - expect(component.passwordStrengthDescription).toBe('Strong'); - expect(component.passwordStrengthLevel).toBe('passwordStrengthLevel3'); - }); - - it('should validate password strength ok ', () => { - formHelper.setValue('password', 'mypassword1!@'); - component.checkPassword('mypassword1!@'); - expect(component.passwordStrengthDescription).toBe('OK'); - expect(component.passwordStrengthLevel).toBe('passwordStrengthLevel2'); - }); - - it('should validate password strength weak', () => { - formHelper.setValue('password', 'mypassword1'); - component.checkPassword('mypassword1'); - expect(component.passwordStrengthDescription).toBe('Weak'); - expect(component.passwordStrengthLevel).toBe('passwordStrengthLevel1'); - }); - - it('should validate password strength too weak', () => { - formHelper.setValue('password', 'bar0'); - component.checkPassword('bar0'); - expect(component.passwordStrengthDescription).toBe('Too weak'); - expect(component.passwordStrengthLevel).toBe('passwordStrengthLevel0'); - }); - it('should validate email', () => { formHelper.expectErrorChange('email', 'aaa', 'email'); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.ts index c7bdfe722fa..e954591c4a6 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.ts @@ -1,5 +1,5 @@ import { Component, OnInit, TemplateRef, ViewChild } from '@angular/core'; -import { FormControl, Validators } from '@angular/forms'; +import { Validators } from '@angular/forms'; import { ActivatedRoute, Router } from '@angular/router'; import { I18n } from '@ngx-translate/i18n-polyfill'; @@ -16,12 +16,13 @@ import { SelectMessages } from '../../../shared/components/select/select-message import { ActionLabelsI18n } from '../../../shared/constants/app.constants'; import { Icons } from '../../../shared/enum/icons.enum'; import { NotificationType } from '../../../shared/enum/notification-type.enum'; +import { CdFormBuilder } from '../../../shared/forms/cd-form-builder'; import { CdFormGroup } from '../../../shared/forms/cd-form-group'; import { CdValidators } from '../../../shared/forms/cd-validators'; import { CdPwdExpirationSettings } from '../../../shared/models/cd-pwd-expiration-settings'; import { AuthStorageService } from '../../../shared/services/auth-storage.service'; import { NotificationService } from '../../../shared/services/notification.service'; -import { UserChangePasswordService } from '../../../shared/services/user-change-password.service'; +import { PasswordPolicyService } from '../../../shared/services/password-policy.service'; import { UserFormMode } from './user-form-mode.enum'; import { UserFormRoleModel } from './user-form-role.model'; import { UserFormModel } from './user-form.model'; @@ -46,9 +47,9 @@ export class UserFormComponent implements OnInit { messages = new SelectMessages({ empty: this.i18n('There are no roles.') }, this.i18n); action: string; resource: string; - requiredPasswordRulesMessage: string; - passwordStrengthLevel: string; - passwordStrengthDescription: string; + passwordPolicyHelpText: string; + passwordStrengthLevelClass: string; + passwordValuation: string; icons = Icons; minDate: Date; bsConfig = { @@ -68,7 +69,8 @@ export class UserFormComponent implements OnInit { private notificationService: NotificationService, private i18n: I18n, public actionLabels: ActionLabelsI18n, - private userChangePasswordService: UserChangePasswordService, + private passwordPolicyService: PasswordPolicyService, + private formBuilder: CdFormBuilder, private settingsService: SettingsService ) { this.resource = this.i18n('user'); @@ -77,32 +79,32 @@ export class UserFormComponent implements OnInit { } createForm() { - this.requiredPasswordRulesMessage = this.userChangePasswordService.getPasswordRulesMessage(); - this.userForm = new CdFormGroup( + this.passwordPolicyHelpText = this.passwordPolicyService.getHelpText(); + this.userForm = this.formBuilder.group( { - username: new FormControl('', { - validators: [Validators.required] - }), - name: new FormControl(''), - password: new FormControl('', { - validators: [ - CdValidators.custom('checkPassword', () => { - return this.userForm && this.checkPassword(this.userForm.getValue('password')); - }) + username: ['', [Validators.required]], + name: [''], + password: [ + '', + [], + [ + CdValidators.passwordPolicy( + this.userService, + () => this.userForm.getValue('username'), + (_valid: boolean, credits: number, valuation: string) => { + this.passwordStrengthLevelClass = this.passwordPolicyService.mapCreditsToCssClass( + credits + ); + this.passwordValuation = _.defaultTo(valuation, ''); + } + ) ] - }), - confirmpassword: new FormControl('', { - updateOn: 'blur', - validators: [] - }), - pwdExpirationDate: new FormControl(''), - email: new FormControl('', { - validators: [Validators.email] - }), - roles: new FormControl([]), - enabled: new FormControl(true, { - validators: [Validators.required] - }) + ], + confirmpassword: [''], + pwdExpirationDate: [''], + email: ['', [CdValidators.email]], + roles: [[]], + enabled: [true, [Validators.required]] }, { validators: [CdValidators.match('password', 'confirmpassword')] @@ -225,14 +227,6 @@ export class UserFormComponent implements OnInit { } } - checkPassword(password: string) { - [ - this.passwordStrengthLevel, - this.passwordStrengthDescription - ] = this.userChangePasswordService.checkPasswordComplexity(password); - return password && this.passwordStrengthLevel === 'passwordStrengthLevel0'; - } - showExpirationDateField() { return ( this.userForm.getValue('pwdExpirationDate') > 0 || diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-password-form/user-password-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-password-form/user-password-form.component.html index c11565c1b1f..a318f0e1bde 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-password-form/user-password-form.component.html +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-password-form/user-password-form.component.html @@ -44,7 +44,7 @@ for="newpassword"> New password + html="{{ passwordPolicyHelpText }}"> @@ -63,10 +63,10 @@ -
-
+
+ title="{{ passwordValuation }}">
The old and new passwords must be different. Too weak + *ngIf="userForm.showError('newpassword', frm, 'passwordPolicy')"> + {{ passwordValuation }} +
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-password-form/user-password-form.component.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-password-form/user-password-form.component.spec.ts index 057f914be13..cfd8e0c7120 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-password-form/user-password-form.component.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-password-form/user-password-form.component.spec.ts @@ -65,41 +65,6 @@ describe('UserPasswordFormComponent', () => { formHelper.expectValidChange('confirmnewpassword', 'aaa'); }); - it('should validate password strength very strong', () => { - formHelper.setValue('newpassword', 'testpassword#!$!@$'); - component.checkPassword('testpassword#!$!@$'); - expect(component.passwordStrengthDescription).toBe('Very strong'); - expect(component.passwordStrengthLevel).toBe('passwordStrengthLevel4'); - }); - - it('should validate password strength strong', () => { - formHelper.setValue('newpassword', 'testpassword0047!@'); - component.checkPassword('testpassword0047!@'); - expect(component.passwordStrengthDescription).toBe('Strong'); - expect(component.passwordStrengthLevel).toBe('passwordStrengthLevel3'); - }); - - it('should validate password strength ok ', () => { - formHelper.setValue('newpassword', 'mypassword1!@'); - component.checkPassword('mypassword1!@'); - expect(component.passwordStrengthDescription).toBe('OK'); - expect(component.passwordStrengthLevel).toBe('passwordStrengthLevel2'); - }); - - it('should validate password strength weak', () => { - formHelper.setValue('newpassword', 'mypassword1'); - component.checkPassword('mypassword1'); - expect(component.passwordStrengthDescription).toBe('Weak'); - expect(component.passwordStrengthLevel).toBe('passwordStrengthLevel1'); - }); - - it('should validate password strength too weak', () => { - formHelper.setValue('newpassword', 'abc0'); - component.checkPassword('abc0'); - expect(component.passwordStrengthDescription).toBe('Too weak'); - expect(component.passwordStrengthLevel).toBe('passwordStrengthLevel0'); - }); - it('should submit', () => { spyOn(authStorageService, 'getUsername').and.returnValue('xyz'); formHelper.setMultipleValues({ diff --git a/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-password-form/user-password-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-password-form/user-password-form.component.ts index 53ed7f3e884..239072c7203 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-password-form/user-password-form.component.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-password-form/user-password-form.component.ts @@ -3,6 +3,7 @@ import { Validators } from '@angular/forms'; import { Router } from '@angular/router'; import { I18n } from '@ngx-translate/i18n-polyfill'; +import * as _ from 'lodash'; import { UserService } from '../../../shared/api/user.service'; import { ActionLabelsI18n } from '../../../shared/constants/app.constants'; @@ -13,7 +14,7 @@ import { CdFormGroup } from '../../../shared/forms/cd-form-group'; import { CdValidators } from '../../../shared/forms/cd-validators'; import { AuthStorageService } from '../../../shared/services/auth-storage.service'; import { NotificationService } from '../../../shared/services/notification.service'; -import { UserChangePasswordService } from '../../../shared/services/user-change-password.service'; +import { PasswordPolicyService } from '../../../shared/services/password-policy.service'; @Component({ selector: 'cd-user-password-form', @@ -24,9 +25,9 @@ export class UserPasswordFormComponent { userForm: CdFormGroup; action: string; resource: string; - requiredPasswordRulesMessage: string; - passwordStrengthLevel: string; - passwordStrengthDescription: string; + passwordPolicyHelpText: string; + passwordStrengthLevelClass: string; + passwordValuation: string; icons = Icons; constructor( @@ -37,7 +38,7 @@ export class UserPasswordFormComponent { private authStorageService: AuthStorageService, private formBuilder: CdFormBuilder, private router: Router, - private userChangePasswordService: UserChangePasswordService + private passwordPolicyService: PasswordPolicyService ) { this.action = this.actionLabels.CHANGE; this.resource = this.i18n('password'); @@ -45,7 +46,7 @@ export class UserPasswordFormComponent { } createForm() { - this.requiredPasswordRulesMessage = this.userChangePasswordService.getPasswordRulesMessage(); + this.passwordPolicyHelpText = this.passwordPolicyService.getHelpText(); this.userForm = this.formBuilder.group( { oldpassword: [ @@ -69,10 +70,19 @@ export class UserPasswordFormComponent { this.userForm && this.userForm.getValue('oldpassword') === this.userForm.getValue('newpassword') ); - }), - CdValidators.custom('checkPassword', () => { - return this.userForm && this.checkPassword(this.userForm.getValue('newpassword')); }) + ], + [ + CdValidators.passwordPolicy( + this.userService, + () => this.authStorageService.getUsername(), + (_valid: boolean, credits: number, valuation: string) => { + this.passwordStrengthLevelClass = this.passwordPolicyService.mapCreditsToCssClass( + credits + ); + this.passwordValuation = _.defaultTo(valuation, ''); + } + ) ] ], confirmnewpassword: [null, [Validators.required]] @@ -83,14 +93,6 @@ export class UserPasswordFormComponent { ); } - checkPassword(password: string) { - [ - this.passwordStrengthLevel, - this.passwordStrengthDescription - ] = this.userChangePasswordService.checkPasswordComplexity(password); - return password && this.passwordStrengthLevel === 'passwordStrengthLevel0'; - } - onSubmit() { if (this.userForm.pristine) { return; diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/user.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/user.service.spec.ts index 90a54cf4e1b..2dc813372a0 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/user.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/user.service.spec.ts @@ -80,4 +80,22 @@ describe('UserService', () => { }); expect(req.request.method).toBe('POST'); }); + + it('should call validatePassword', () => { + service.validatePassword('foo').subscribe(); + const req = httpTesting.expectOne('api/user/validate_password?password=foo'); + expect(req.request.method).toBe('POST'); + }); + + it('should call validatePassword (incl. name)', () => { + service.validatePassword('foo_bar', 'bar').subscribe(); + const req = httpTesting.expectOne('api/user/validate_password?password=foo_bar&username=bar'); + expect(req.request.method).toBe('POST'); + }); + + it('should call validatePassword (incl. old password)', () => { + service.validatePassword('foo', null, 'foo').subscribe(); + const req = httpTesting.expectOne('api/user/validate_password?password=foo&old_password=foo'); + expect(req.request.method).toBe('POST'); + }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/user.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/user.service.ts index 8e593875b45..9e23bd0cbcd 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/user.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/user.service.ts @@ -1,4 +1,4 @@ -import { HttpClient } from '@angular/common/http'; +import { HttpClient, HttpParams } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { UserFormModel } from '../../core/auth/user-form/user-form.model'; @@ -39,4 +39,16 @@ export class UserService { new_password: newPassword }); } + + validatePassword(password: string, username: string = null, oldPassword: string = null) { + let params = new HttpParams(); + params = params.append('password', password); + if (username) { + params = params.append('username', username); + } + if (oldPassword) { + params = params.append('old_password', oldPassword); + } + return this.http.post('api/user/validate_password', null, { params }); + } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/forms/cd-validators.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/forms/cd-validators.spec.ts index 06a3a230b6d..3e68f44357a 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/forms/cd-validators.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/forms/cd-validators.spec.ts @@ -555,4 +555,58 @@ describe('CdValidators', () => { ); }); }); + + describe('passwordPolicy', () => { + let valid: boolean; + let callbackCalled: boolean; + + const fakeUserService = { + validatePassword: () => { + return observableOf({ valid: valid, credits: 17, valuation: 'foo' }); + } + }; + + beforeEach(() => { + callbackCalled = false; + form = new CdFormGroup({ + x: new FormControl( + '', + null, + CdValidators.passwordPolicy( + fakeUserService, + () => 'admin', + () => { + callbackCalled = true; + } + ) + ) + }); + formHelper = new FormHelper(form); + }); + + it('should not error because of empty input', () => { + expectValid(''); + expect(callbackCalled).toBeTruthy(); + }); + + it('should not error because password matches the policy', fakeAsync(() => { + valid = true; + formHelper.setValue('x', 'abc', true); + tick(500); + formHelper.expectValid('x'); + })); + + it('should error because password does not match the policy', fakeAsync(() => { + valid = false; + formHelper.setValue('x', 'xyz', true); + tick(500); + formHelper.expectError('x', 'passwordPolicy'); + })); + + it('should call the callback function', fakeAsync(() => { + formHelper.setValue('x', 'xyz', true); + tick(500); + expect(callbackCalled).toBeTruthy(); + })); + }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/forms/cd-validators.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/forms/cd-validators.ts index 11533f7d60a..7ec202b0874 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/forms/cd-validators.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/forms/cd-validators.ts @@ -365,4 +365,50 @@ export class CdValidators { }; }; } + + /** + * Asynchronous validator that checks if the password meets the password + * policy. + * @param userServiceThis The object to be used as the 'this' object + * when calling the 'validatePassword' method of the 'UserService'. + * @param usernameFn Function to get the username that should be + * taken into account. + * @param callback Callback function that is called after the validation + * has been done. + * @return {AsyncValidatorFn} Returns an asynchronous validator function + * that returns an error map with the `passwordPolicy` property if the + * validation check fails, otherwise `null`. + */ + static passwordPolicy( + userServiceThis: any, + usernameFn?: Function, + callback?: (valid: boolean, credits?: number, valuation?: string) => void + ): AsyncValidatorFn { + return (control: AbstractControl): Observable => { + if (control.pristine || control.value === '') { + if (_.isFunction(callback)) { + callback(true, 0); + } + return observableOf(null); + } + let username; + if (_.isFunction(usernameFn)) { + username = usernameFn(); + } + return observableTimer(500).pipe( + switchMapTo(_.invoke(userServiceThis, 'validatePassword', control.value, username)), + map((resp: { valid: boolean; credits: number; valuation: string }) => { + if (_.isFunction(callback)) { + callback(resp.valid, resp.credits, resp.valuation); + } + if (resp.valid) { + return null; + } else { + return { passwordPolicy: true }; + } + }), + take(1) + ); + }; + } } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/password-policy.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/password-policy.service.ts new file mode 100644 index 00000000000..8fbfb057871 --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/password-policy.service.ts @@ -0,0 +1,45 @@ +import { Injectable } from '@angular/core'; + +import { I18n } from '@ngx-translate/i18n-polyfill'; + +@Injectable({ + providedIn: 'root' +}) +export class PasswordPolicyService { + constructor(private i18n: I18n) {} + + getHelpText() { + return this.i18n( + 'Required rules for password complexity:\n\ + - must contain at least 8 characters\n\ + - cannot contain username\n\ + - cannot contain any keyword used in Ceph\n\ + - cannot contain any repetitive characters e.g. "aaa"\n\ + - cannot contain any sequential characters e.g. "abc"\n\ + - must consist of characters from the following groups:\n\ + * alphabetic a-z, A-Z\n\ + * numbers 0-9\n\ + * special chars: !"#$%& \'()*+,-./:;<=>?@[\\]^_`{{|}}~\n\ + * any other characters (signs)' + ); + } + + /** + * Helper function to map password policy credits to a CSS class. + * @param credits The password policy credits. + * @return The name of the CSS class. + */ + mapCreditsToCssClass(credits: number): string { + let result = 'very-strong'; + if (credits < 10) { + result = 'too-weak'; + } else if (credits < 15) { + result = 'weak'; + } else if (credits < 20) { + result = 'ok'; + } else if (credits < 25) { + result = 'strong'; + } + return result; + } +} diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/user-change-password.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/user-change-password.service.ts deleted file mode 100644 index 439757693f7..00000000000 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/user-change-password.service.ts +++ /dev/null @@ -1,82 +0,0 @@ -import { Injectable } from '@angular/core'; - -import { I18n } from '@ngx-translate/i18n-polyfill'; -import * as _ from 'lodash'; - -@Injectable({ - providedIn: 'root' -}) -export class UserChangePasswordService { - requiredPasswordRulesMessage: string; - passwordStrengthLevel: string; - passwordStrengthDescription: string; - - constructor(private i18n: I18n) {} - getPasswordRulesMessage() { - return this.i18n( - 'Required rules for password complexity:\n\ - - must contain at least 8 characters\n\ - - cannot contain username\n\ - - cannot contain any keyword used in Ceph\n\ - - cannot contain any repetitive characters e.g. "aaa"\n\ - - cannot contain any sequencial characters e.g. "abc"\n\ - - must consist of characters from the following groups:\n\ - * alphabetic a-z, A-Z\n\ - * numbers 0-9\n\ - * special chars: !"#$%& \'()*+,-./:;<=>?@[\\]^_`{{|}}~\n\ - * any other characters (signs)' - ); - } - - checkPasswordComplexity(password): [string, string] { - this.passwordStrengthLevel = 'passwordStrengthLevel0'; - this.passwordStrengthDescription = ''; - const credits = this.checkPasswordComplexityLetters(password); - if (credits) { - if (password.length < 8 || credits < 10) { - this.passwordStrengthLevel = 'passwordStrengthLevel0'; - this.passwordStrengthDescription = this.i18n('Too weak'); - } else { - if (credits < 15) { - this.passwordStrengthLevel = 'passwordStrengthLevel1'; - this.passwordStrengthDescription = this.i18n('Weak'); - } else { - if (credits < 20) { - this.passwordStrengthLevel = 'passwordStrengthLevel2'; - this.passwordStrengthDescription = this.i18n('OK'); - } else { - if (credits < 25) { - this.passwordStrengthLevel = 'passwordStrengthLevel3'; - this.passwordStrengthDescription = this.i18n('Strong'); - } else { - this.passwordStrengthLevel = 'passwordStrengthLevel4'; - this.passwordStrengthDescription = this.i18n('Very strong'); - } - } - } - } - } - return [this.passwordStrengthLevel, this.passwordStrengthDescription]; - } - - private checkPasswordComplexityLetters(password): number { - if (_.isString(password)) { - const digitsNumber = password.replace(/[^0-9]/g, '').length; - const smallLettersNumber = password.replace(/[^a-z]/g, '').length; - const bigLettersNumber = password.replace(/[^A-Z]/g, '').length; - const punctuationNumber = password.replace(/[^!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~]/g, '').length; - const othersCharactersNumber = - password.length - - (digitsNumber + smallLettersNumber + bigLettersNumber + punctuationNumber); - return ( - digitsNumber + - smallLettersNumber + - bigLettersNumber * 2 + - punctuationNumber * 3 + - othersCharactersNumber * 5 - ); - } else { - return 0; - } - } -} diff --git a/src/pybind/mgr/dashboard/frontend/src/styles.scss b/src/pybind/mgr/dashboard/frontend/src/styles.scss index 8bcdac0147d..7e19d62af81 100644 --- a/src/pybind/mgr/dashboard/frontend/src/styles.scss +++ b/src/pybind/mgr/dashboard/frontend/src/styles.scss @@ -404,30 +404,30 @@ bfv-messages { } //Displaying the password strength -.passwordStrengthLevel { +.password-strength-level { flex: 100%; margin-top: 2px; - .passwordStrengthLevel1, - .passwordStrengthLevel2, - .passwordStrengthLevel3, - .passwordStrengthLevel4 { + .weak, + .ok, + .strong, + .very-strong { border-radius: 0.25rem; height: 13px; } - .passwordStrengthLevel1 { + .weak { width: 25%; background: $color-solid-red; } - .passwordStrengthLevel2 { + .ok { width: 50%; background: $color-bright-yellow; } - .passwordStrengthLevel3 { + .strong { width: 75%; background: $color-bright-green; } - .passwordStrengthLevel4 { + .very-strong { width: 100%; background: $color-green; } diff --git a/src/pybind/mgr/dashboard/services/access_control.py b/src/pybind/mgr/dashboard/services/access_control.py index b8f7c6d29ec..d136ca6c07f 100644 --- a/src/pybind/mgr/dashboard/services/access_control.py +++ b/src/pybind/mgr/dashboard/services/access_control.py @@ -24,7 +24,7 @@ from ..settings import Settings from ..exceptions import RoleAlreadyExists, RoleDoesNotExist, ScopeNotValid, \ PermissionNotValid, RoleIsAssociatedWithUser, \ UserAlreadyExists, UserDoesNotExist, ScopeNotInRole, \ - RoleNotInUser, PasswordCheckException, PwdExpirationDateNotValid + RoleNotInUser, PasswordPolicyException, PwdExpirationDateNotValid logger = logging.getLogger('access_control') @@ -44,13 +44,13 @@ def password_hash(password, salt_password=None): _P = Permission # short alias -class PasswordCheck(object): - def __init__(self, password, username, old_password=None): +class PasswordPolicy(object): + def __init__(self, password, username=None, old_password=None): """ :param password: The new plain password. :type password: str :param username: The name of the user. - :type username: str + :type username: str | None :param old_password: The old plain password. :type old_password: str | None """ @@ -73,6 +73,7 @@ class PasswordCheck(object): big_letter_credit = 2 special_character_credit = 3 other_character_credit = 5 + self.complexity_credits = 0 for ch in self.password: if ch in ascii_uppercase: self.complexity_credits += big_letter_credit @@ -90,6 +91,8 @@ class PasswordCheck(object): return self.old_password and self.password == self.old_password def check_if_contains_username(self): + if not self.username: + return False return self._check_if_contains_word(self.password, self.username) def check_if_contains_forbidden_words(self): @@ -109,23 +112,26 @@ class PasswordCheck(object): return True return False + def check_password_length(self, min_length=8): + return len(self.password) >= min_length + def check_all(self): """ Perform all password policy checks. - :raise PasswordCheckException: If a password policy check fails. + :raise PasswordPolicyException: If a password policy check fails. """ + if self.check_password_characters() < 10 or not self.check_password_length(): + raise PasswordPolicyException('Password is too weak.') if self.check_is_old_password(): - raise PasswordCheckException('Password cannot be the same as the previous one.') + raise PasswordPolicyException('Password cannot be the same as the previous one.') if self.check_if_contains_username(): - raise PasswordCheckException('Password cannot contain username.') + raise PasswordPolicyException('Password cannot contain username.') if self.check_if_contains_forbidden_words(): - raise PasswordCheckException('Password cannot contain keywords.') + raise PasswordPolicyException('Password cannot contain keywords.') if self.check_if_repetetive_characters(): - raise PasswordCheckException('Password cannot contain repetitive characters.') + raise PasswordPolicyException('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.') + raise PasswordPolicyException('Password cannot contain sequential characters.') class Role(object): @@ -686,11 +692,11 @@ def ac_user_create_cmd(_, username, password=None, rolename=None, name=None, try: if not force_password: - pw_check = PasswordCheck(password, username) + pw_check = PasswordPolicy(password, username) pw_check.check_all() user = mgr.ACCESS_CTRL_DB.create_user(username, password, name, email, enabled, pwd_expiration_date) - except PasswordCheckException as ex: + except PasswordPolicyException as ex: return -errno.EINVAL, '', str(ex) except UserAlreadyExists as ex: return -errno.EEXIST, '', str(ex) @@ -821,12 +827,12 @@ 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 = PasswordPolicy(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: + except PasswordPolicyException 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 79caa07c4e9..3837e25d97a 100644 --- a/src/pybind/mgr/dashboard/tests/test_access_control.py +++ b/src/pybind/mgr/dashboard/tests/test_access_control.py @@ -14,7 +14,7 @@ from .. import mgr from ..security import Scope, Permission from ..services.access_control import load_access_control_db, \ password_hash, AccessControlDB, \ - SYSTEM_ROLES + SYSTEM_ROLES, PasswordPolicy class AccessControlTest(unittest.TestCase, CLICommandTestMixin): @@ -791,3 +791,56 @@ class AccessControlTest(unittest.TestCase, CLICommandTestMixin): 'roles': ['administrator'], 'enabled': True }) + + def test_password_policy_pw_length(self): + pw_policy = PasswordPolicy('foo') + self.assertTrue(pw_policy.check_password_length(3)) + + def test_password_policy_pw_length_fail(self): + pw_policy = PasswordPolicy('bar') + self.assertFalse(pw_policy.check_password_length()) + + def test_password_policy_credits_too_weak(self): + pw_policy = PasswordPolicy('foo') + pw_credits = pw_policy.check_password_characters() + self.assertEqual(pw_credits, 3) + + def test_password_policy_credits_weak(self): + pw_policy = PasswordPolicy('mypassword1') + pw_credits = pw_policy.check_password_characters() + self.assertEqual(pw_credits, 11) + + def test_password_policy_credits_ok(self): + pw_policy = PasswordPolicy('mypassword1!@') + pw_credits = pw_policy.check_password_characters() + self.assertEqual(pw_credits, 17) + + def test_password_policy_credits_strong(self): + pw_policy = PasswordPolicy('testpassword0047!@') + pw_credits = pw_policy.check_password_characters() + self.assertEqual(pw_credits, 22) + + def test_password_policy_credits_very_strong(self): + pw_policy = PasswordPolicy('testpassword#!$!@$') + pw_credits = pw_policy.check_password_characters() + self.assertEqual(pw_credits, 30) + + def test_password_policy_forbidden_words(self): + pw_policy = PasswordPolicy('!@$testdashboard#!$') + self.assertTrue(pw_policy.check_if_contains_forbidden_words()) + + def test_password_policy_sequential_chars(self): + pw_policy = PasswordPolicy('!@$test123#!$') + self.assertTrue(pw_policy.check_if_sequential_characters()) + + def test_password_policy_repetetive_chars(self): + pw_policy = PasswordPolicy('!@$testfooo#!$') + self.assertTrue(pw_policy.check_if_repetetive_characters()) + + def test_password_policy_contain_username(self): + pw_policy = PasswordPolicy('%admin135)', 'admin') + self.assertTrue(pw_policy.check_if_contains_username()) + + def test_password_policy_is_old_pwd(self): + pw_policy = PasswordPolicy('foo', old_password='foo') + self.assertTrue(pw_policy.check_is_old_password())