From 3684d24a430ff9dc9193f354715c3339d950a52d Mon Sep 17 00:00:00 2001 From: Volker Theile Date: Mon, 27 Jan 2020 10:25:08 +0100 Subject: [PATCH] mgr/dashboard: Make password policy check configurable Fixes: https://tracker.ceph.com/issues/43089 Signed-off-by: Volker Theile --- doc/mgr/dashboard.rst | 50 +++++ qa/tasks/mgr/dashboard/test_user.py | 23 +- .../mgr/dashboard/controllers/settings.py | 25 ++- .../auth/user-form/user-form.component.html | 3 +- .../user-form/user-form.component.spec.ts | 2 + .../auth/user-form/user-form.component.ts | 6 +- .../user-password-form.component.html | 3 +- .../user-password-form.component.ts | 6 +- .../app/shared/api/settings.service.spec.ts | 22 ++ .../src/app/shared/api/settings.service.ts | 24 ++ .../services/password-policy.service.spec.ts | 205 ++++++++++++++++++ .../services/password-policy.service.ts | 75 +++++-- .../mgr/dashboard/services/access_control.py | 40 +++- src/pybind/mgr/dashboard/settings.py | 21 ++ .../dashboard/tests/test_access_control.py | 36 ++- .../mgr/dashboard/tests/test_settings.py | 9 + 16 files changed, 506 insertions(+), 44 deletions(-) create mode 100644 src/pybind/mgr/dashboard/frontend/src/app/shared/services/password-policy.service.spec.ts diff --git a/doc/mgr/dashboard.rst b/doc/mgr/dashboard.rst index 88c0466a8cf..8ee2c548bd3 100644 --- a/doc/mgr/dashboard.rst +++ b/doc/mgr/dashboard.rst @@ -619,6 +619,56 @@ dashboard in the future. User and Role Management ------------------------ +Password Policy +^^^^^^^^^^^^^^^ + +By default the password policy feature is enabled including the following +checks: + +- Is the password longer than N characters? +- Are the old and new password the same? + +The password policy feature can be switched on or off completely:: + + $ ceph dashboard set-pwd-policy-enabled + +The following individual checks can be switched on or off:: + + $ ceph dashboard set-pwd-policy-check-length-enabled + $ ceph dashboard set-pwd-policy-check-oldpwd-enabled + $ ceph dashboard set-pwd-policy-check-username-enabled + $ ceph dashboard set-pwd-policy-check-exclusion-list-enabled + $ ceph dashboard set-pwd-policy-check-complexity-enabled + $ ceph dashboard set-pwd-policy-check-sequential-chars-enabled + $ ceph dashboard set-pwd-policy-check-repetitive-chars-enabled + +Additionally the following options are available to configure the password +policy behaviour. + +- The minimum password length (defaults to 8):: + + $ ceph dashboard set-pwd-policy-min-length + +- The minimum password complexity (defaults to 10):: + + $ ceph dashboard set-pwd-policy-min-complexity + + The password complexity is calculated by classifying each character in + the password. The complexity count starts by 0. A character is rated by + the following rules in the given order. + + - Increase by 1 if the character is a digit. + - Increase by 1 if the character is a lower case ASCII character. + - Increase by 2 if the character is an upper case ASCII character. + - Increase by 3 if the character is a special character like ``!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~``. + - Increase by 5 if the character has not been classified by one of the previous rules. + +- A list of comma separated words that are not allowed to be used in a + password:: + + $ ceph dashboard set-pwd-policy-exclusion-list [,...] + + User Accounts ^^^^^^^^^^^^^ diff --git a/qa/tasks/mgr/dashboard/test_user.py b/qa/tasks/mgr/dashboard/test_user.py index e0fb6ddf76f..17808482d12 100644 --- a/qa/tasks/mgr/dashboard/test_user.py +++ b/qa/tasks/mgr/dashboard/test_user.py @@ -10,6 +10,27 @@ from .helper import DashboardTestCase, JObj, JLeaf class UserTest(DashboardTestCase): + @classmethod + def setUpClass(cls): + super(UserTest, cls).setUpClass() + cls._ceph_cmd(['dashboard', 'set-pwd-policy-enabled', 'true']) + cls._ceph_cmd(['dashboard', 'set-pwd-policy-check-length-enabled', 'true']) + cls._ceph_cmd(['dashboard', 'set-pwd-policy-check-oldpwd-enabled', 'true']) + cls._ceph_cmd(['dashboard', 'set-pwd-policy-check-username-enabled', 'true']) + cls._ceph_cmd(['dashboard', 'set-pwd-policy-check-exclusion-list-enabled', 'true']) + cls._ceph_cmd(['dashboard', 'set-pwd-policy-check-complexity-enabled', 'true']) + cls._ceph_cmd(['dashboard', 'set-pwd-policy-check-sequential-chars-enabled', 'true']) + cls._ceph_cmd(['dashboard', 'set-pwd-policy-check-repetitive-chars-enabled', 'true']) + + @classmethod + def tearDownClass(cls): + cls._ceph_cmd(['dashboard', 'set-pwd-policy-check-username-enabled', 'false']) + cls._ceph_cmd(['dashboard', 'set-pwd-policy-check-exclusion-list-enabled', 'false']) + cls._ceph_cmd(['dashboard', 'set-pwd-policy-check-complexity-enabled', 'false']) + cls._ceph_cmd(['dashboard', 'set-pwd-policy-check-sequential-chars-enabled', 'false']) + cls._ceph_cmd(['dashboard', 'set-pwd-policy-check-repetitive-chars-enabled', 'false']) + super(UserTest, cls).tearDownClass() + @classmethod def _create_user(cls, username=None, password=None, name=None, email=None, roles=None, enabled=True, pwd_expiration_date=None): @@ -220,7 +241,7 @@ class UserTest(DashboardTestCase): }) self.assertStatus(400) self.assertError('password_policy_validation_failed', 'user', - 'Password must not contain keywords.') + 'Password must not contain the keyword "OSD".') self._reset_login_to_admin('test1') def test_change_password_contains_sequential_characters(self): diff --git a/src/pybind/mgr/dashboard/controllers/settings.py b/src/pybind/mgr/dashboard/controllers/settings.py index 19e177c61c0..4fb63a379a8 100644 --- a/src/pybind/mgr/dashboard/controllers/settings.py +++ b/src/pybind/mgr/dashboard/controllers/settings.py @@ -35,11 +35,23 @@ class Settings(RESTController): def _to_native(setting): return setting.upper().replace('-', '_') - def list(self): - return [ - self._get(name) for name in Options.__dict__ + def list(self, names=None): + """ + Get the list of available options. + :param names: A comma separated list of option names that should + be processed. Defaults to ``None``. + :type names: None|str + :return: A list of available options. + :rtype: list[dict] + """ + option_names = [ + name for name in Options.__dict__ if name.isupper() and not name.startswith('_') ] + if names: + names = names.split(',') + option_names = list(set(option_names) & set(names)) + return [self._get(name) for name in option_names] def _get(self, name): with self._attribute_handler(name) as sname: @@ -52,6 +64,13 @@ class Settings(RESTController): } def get(self, name): + """ + Get the given option. + :param name: The name of the option. + :return: Returns a dict containing the name, type, + default value and current value of the given option. + :rtype: dict + """ return self._get(name) def set(self, name, value): 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 aac7cf4d09b..bdc0a32df77 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 @@ -37,7 +37,8 @@ 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 b106527aa8b..f466cdec96e 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 @@ -20,6 +20,7 @@ import { CdFormGroup } from '../../../shared/forms/cd-form-group'; import { AuthStorageService } from '../../../shared/services/auth-storage.service'; import { NotificationService } from '../../../shared/services/notification.service'; import { SharedModule } from '../../../shared/shared.module'; +import { PasswordPolicyService } from './../../../shared/services/password-policy.service'; import { UserFormComponent } from './user-form.component'; import { UserFormModel } from './user-form.model'; @@ -62,6 +63,7 @@ describe('UserFormComponent', () => { ); beforeEach(() => { + spyOn(TestBed.get(PasswordPolicyService), 'getHelpText').and.callFake(() => of('')); fixture = TestBed.createComponent(UserFormComponent); component = fixture.componentInstance; form = component.userForm; 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 ba3593e48b3..3299a088d04 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 @@ -47,7 +47,7 @@ export class UserFormComponent implements OnInit { messages = new SelectMessages({ empty: this.i18n('There are no roles.') }, this.i18n); action: string; resource: string; - passwordPolicyHelpText: string; + passwordPolicyHelpText = ''; passwordStrengthLevelClass: string; passwordValuation: string; icons = Icons; @@ -79,7 +79,9 @@ export class UserFormComponent implements OnInit { } createForm() { - this.passwordPolicyHelpText = this.passwordPolicyService.getHelpText(); + this.passwordPolicyService.getHelpText().subscribe((helpText: string) => { + this.passwordPolicyHelpText = helpText; + }); this.userForm = this.formBuilder.group( { username: ['', [Validators.required]], 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 00fbe1e391f..e513447b580 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 @@ -42,7 +42,8 @@ for="newpassword"> New password - 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 239072c7203..ae24f9e99ec 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 @@ -25,7 +25,7 @@ export class UserPasswordFormComponent { userForm: CdFormGroup; action: string; resource: string; - passwordPolicyHelpText: string; + passwordPolicyHelpText = ''; passwordStrengthLevelClass: string; passwordValuation: string; icons = Icons; @@ -46,7 +46,9 @@ export class UserPasswordFormComponent { } createForm() { - this.passwordPolicyHelpText = this.passwordPolicyService.getHelpText(); + this.passwordPolicyService.getHelpText().subscribe((helpText: string) => { + this.passwordPolicyHelpText = helpText; + }); this.userForm = this.formBuilder.group( { oldpassword: [ diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.spec.ts index 122f2691cfc..2b7075e3eac 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.spec.ts @@ -125,4 +125,26 @@ describe('SettingsService', () => { service.disableSetting(exampleUrl); expect(service['settings']).toEqual({ [exampleUrl]: '' }); }); + + it('should return the specified settings (1)', () => { + let result; + service.getValues('foo,bar').subscribe((resp) => { + result = resp; + }); + const req = httpTesting.expectOne('api/settings?names=foo,bar'); + expect(req.request.method).toBe('GET'); + req.flush([ + { name: 'foo', default: '', type: 'str', value: 'test' }, + { name: 'bar', default: 0, type: 'int', value: 2 } + ]); + expect(result).toEqual({ + foo: 'test', + bar: 2 + }); + }); + + it('should return the specified settings (2)', () => { + service.getValues(['abc', 'xyz']).subscribe(); + httpTesting.expectOne('api/settings?names=abc,xyz'); + }); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.ts index 75b88c61d26..0a6642f373e 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/settings.service.ts @@ -1,11 +1,20 @@ import { HttpClient } from '@angular/common/http'; import { Injectable } from '@angular/core'; +import * as _ from 'lodash'; import { Observable } from 'rxjs'; +import { map } from 'rxjs/operators'; import { CdPwdExpirationSettings } from '../models/cd-pwd-expiration-settings'; import { ApiModule } from './api.module'; +class SettingResponse { + name: string; + default: any; + type: string; + value: any; +} + @Injectable({ providedIn: ApiModule }) @@ -14,6 +23,21 @@ export class SettingsService { private settings: { [url: string]: string } = {}; + getValues(names: string | string[]): Observable<{ [key: string]: any }> { + if (_.isArray(names)) { + names = names.join(','); + } + return this.http.get(`api/settings?names=${names}`).pipe( + map((resp: SettingResponse[]) => { + const result = {}; + _.forEach(resp, (option: SettingResponse) => { + _.set(result, option.name, option.value); + }); + return result; + }) + ); + } + ifSettingConfigured(url: string, fn: (value?: string) => void, elseFn?: () => void): void { const setting = this.settings[url]; if (setting === undefined) { diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/password-policy.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/password-policy.service.spec.ts new file mode 100644 index 00000000000..2275c0bef5e --- /dev/null +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/password-policy.service.spec.ts @@ -0,0 +1,205 @@ +import { HttpClientTestingModule } from '@angular/common/http/testing'; +import { TestBed } from '@angular/core/testing'; + +import { of as observableOf } from 'rxjs'; + +import { configureTestBed, i18nProviders } from '../../../testing/unit-test-helper'; +import { SettingsService } from '../api/settings.service'; +import { SharedModule } from '../shared.module'; +import { PasswordPolicyService } from './password-policy.service'; + +describe('PasswordPolicyService', () => { + let service: PasswordPolicyService; + let settingsService: SettingsService; + + const helpTextHelper = { + get: (chk: string) => { + const chkTexts: { [key: string]: string } = { + chk_length: 'Must contain at least 10 characters', + chk_oldpwd: 'Must not be the same as the previous one', + chk_username: 'Cannot contain the username', + chk_exclusion_list: 'Cannot contain any configured keyword', + chk_repetitive: 'Cannot contain any repetitive characters e.g. "aaa"', + chk_sequential: 'Cannot contain any sequential characters e.g. "abc"', + chk_complexity: + '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)' + }; + return ['Required rules for passwords:', '- ' + chkTexts[chk]].join('\n'); + } + }; + + configureTestBed({ + imports: [HttpClientTestingModule, SharedModule], + providers: [i18nProviders] + }); + + beforeEach(() => { + service = TestBed.get(PasswordPolicyService); + settingsService = TestBed.get(SettingsService); + settingsService['settings'] = {}; + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); + + it('should not get help text', () => { + let helpText = ''; + spyOn(settingsService, 'getValues').and.returnValue( + observableOf({ + PWD_POLICY_ENABLED: false + }) + ); + service.getHelpText().subscribe((text) => (helpText = text)); + expect(helpText).toBe(''); + }); + + it('should get help text chk_length', () => { + let helpText = ''; + const expectedHelpText = helpTextHelper.get('chk_length'); + spyOn(settingsService, 'getValues').and.returnValue( + observableOf({ + PWD_POLICY_ENABLED: true, + PWD_POLICY_MIN_LENGTH: 10, + PWD_POLICY_CHECK_LENGTH_ENABLED: true, + PWD_POLICY_CHECK_OLDPWD_ENABLED: false, + PWD_POLICY_CHECK_SEQUENTIAL_CHARS_ENABLED: false, + PWD_POLICY_CHECK_COMPLEXITY_ENABLED: false + }) + ); + service.getHelpText().subscribe((text) => (helpText = text)); + expect(helpText).toBe(expectedHelpText); + }); + + it('should get help text chk_oldpwd', () => { + let helpText = ''; + const expectedHelpText = helpTextHelper.get('chk_oldpwd'); + spyOn(settingsService, 'getValues').and.returnValue( + observableOf({ + PWD_POLICY_ENABLED: true, + PWD_POLICY_CHECK_OLDPWD_ENABLED: true, + PWD_POLICY_CHECK_USERNAME_ENABLED: false, + PWD_POLICY_CHECK_EXCLUSION_LIST_ENABLED: false, + PWD_POLICY_CHECK_COMPLEXITY_ENABLED: false + }) + ); + service.getHelpText().subscribe((text) => (helpText = text)); + expect(helpText).toBe(expectedHelpText); + }); + + it('should get help text chk_username', () => { + let helpText = ''; + const expectedHelpText = helpTextHelper.get('chk_username'); + spyOn(settingsService, 'getValues').and.returnValue( + observableOf({ + PWD_POLICY_ENABLED: true, + PWD_POLICY_CHECK_OLDPWD_ENABLED: false, + PWD_POLICY_CHECK_USERNAME_ENABLED: true, + PWD_POLICY_CHECK_EXCLUSION_LIST_ENABLED: false + }) + ); + service.getHelpText().subscribe((text) => (helpText = text)); + expect(helpText).toBe(expectedHelpText); + }); + + it('should get help text chk_exclusion_list', () => { + let helpText = ''; + const expectedHelpText = helpTextHelper.get('chk_exclusion_list'); + spyOn(settingsService, 'getValues').and.returnValue( + observableOf({ + PWD_POLICY_ENABLED: true, + PWD_POLICY_CHECK_USERNAME_ENABLED: false, + PWD_POLICY_CHECK_EXCLUSION_LIST_ENABLED: true, + PWD_POLICY_CHECK_REPETITIVE_CHARS_ENABLED: false + }) + ); + service.getHelpText().subscribe((text) => (helpText = text)); + expect(helpText).toBe(expectedHelpText); + }); + + it('should get help text chk_repetitive', () => { + let helpText = ''; + const expectedHelpText = helpTextHelper.get('chk_repetitive'); + spyOn(settingsService, 'getValues').and.returnValue( + observableOf({ + PWD_POLICY_ENABLED: true, + PWD_POLICY_CHECK_OLDPWD_ENABLED: false, + PWD_POLICY_CHECK_EXCLUSION_LIST_ENABLED: false, + PWD_POLICY_CHECK_REPETITIVE_CHARS_ENABLED: true, + PWD_POLICY_CHECK_SEQUENTIAL_CHARS_ENABLED: false, + PWD_POLICY_CHECK_COMPLEXITY_ENABLED: false + }) + ); + service.getHelpText().subscribe((text) => (helpText = text)); + expect(helpText).toBe(expectedHelpText); + }); + + it('should get help text chk_sequential', () => { + let helpText = ''; + const expectedHelpText = helpTextHelper.get('chk_sequential'); + spyOn(settingsService, 'getValues').and.returnValue( + observableOf({ + PWD_POLICY_ENABLED: true, + PWD_POLICY_MIN_LENGTH: 8, + PWD_POLICY_CHECK_LENGTH_ENABLED: false, + PWD_POLICY_CHECK_OLDPWD_ENABLED: false, + PWD_POLICY_CHECK_USERNAME_ENABLED: false, + PWD_POLICY_CHECK_EXCLUSION_LIST_ENABLED: false, + PWD_POLICY_CHECK_REPETITIVE_CHARS_ENABLED: false, + PWD_POLICY_CHECK_SEQUENTIAL_CHARS_ENABLED: true, + PWD_POLICY_CHECK_COMPLEXITY_ENABLED: false + }) + ); + service.getHelpText().subscribe((text) => (helpText = text)); + expect(helpText).toBe(expectedHelpText); + }); + + it('should get help text chk_complexity', () => { + let helpText = ''; + const expectedHelpText = helpTextHelper.get('chk_complexity'); + spyOn(settingsService, 'getValues').and.returnValue( + observableOf({ + PWD_POLICY_ENABLED: true, + PWD_POLICY_MIN_LENGTH: 8, + PWD_POLICY_CHECK_LENGTH_ENABLED: false, + PWD_POLICY_CHECK_OLDPWD_ENABLED: false, + PWD_POLICY_CHECK_USERNAME_ENABLED: false, + PWD_POLICY_CHECK_EXCLUSION_LIST_ENABLED: false, + PWD_POLICY_CHECK_REPETITIVE_CHARS_ENABLED: false, + PWD_POLICY_CHECK_SEQUENTIAL_CHARS_ENABLED: false, + PWD_POLICY_CHECK_COMPLEXITY_ENABLED: true + }) + ); + service.getHelpText().subscribe((text) => (helpText = text)); + expect(helpText).toBe(expectedHelpText); + }); + + it('should get too-weak class', () => { + expect(service.mapCreditsToCssClass(0)).toBe('too-weak'); + expect(service.mapCreditsToCssClass(9)).toBe('too-weak'); + }); + + it('should get weak class', () => { + expect(service.mapCreditsToCssClass(10)).toBe('weak'); + expect(service.mapCreditsToCssClass(14)).toBe('weak'); + }); + + it('should get ok class', () => { + expect(service.mapCreditsToCssClass(15)).toBe('ok'); + expect(service.mapCreditsToCssClass(19)).toBe('ok'); + }); + + it('should get strong class', () => { + expect(service.mapCreditsToCssClass(20)).toBe('strong'); + expect(service.mapCreditsToCssClass(24)).toBe('strong'); + }); + + it('should get very-strong class', () => { + expect(service.mapCreditsToCssClass(25)).toBe('very-strong'); + expect(service.mapCreditsToCssClass(30)).toBe('very-strong'); + }); +}); 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 index 8fbfb057871..160b3eb6368 100644 --- 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 @@ -1,27 +1,72 @@ import { Injectable } from '@angular/core'; import { I18n } from '@ngx-translate/i18n-polyfill'; +import { Observable } from 'rxjs'; +import { map } from 'rxjs/operators'; + +import { SettingsService } from '../api/settings.service'; @Injectable({ providedIn: 'root' }) export class PasswordPolicyService { - constructor(private i18n: I18n) {} + constructor(private i18n: I18n, private settingsService: SettingsService) {} - 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)' - ); + getHelpText(): Observable { + return this.settingsService + .getValues([ + 'PWD_POLICY_ENABLED', + 'PWD_POLICY_MIN_LENGTH', + 'PWD_POLICY_CHECK_LENGTH_ENABLED', + 'PWD_POLICY_CHECK_OLDPWD_ENABLED', + 'PWD_POLICY_CHECK_USERNAME_ENABLED', + 'PWD_POLICY_CHECK_EXCLUSION_LIST_ENABLED', + 'PWD_POLICY_CHECK_REPETITIVE_CHARS_ENABLED', + 'PWD_POLICY_CHECK_SEQUENTIAL_CHARS_ENABLED', + 'PWD_POLICY_CHECK_COMPLEXITY_ENABLED' + ]) + .pipe( + map((resp: Object[]) => { + let helpText: string[] = []; + if (resp['PWD_POLICY_ENABLED']) { + helpText.push(this.i18n('Required rules for passwords:')); + const i18nHelp: { [key: string]: string } = { + PWD_POLICY_CHECK_LENGTH_ENABLED: this.i18n( + 'Must contain at least {{length}} characters', + { + length: resp['PWD_POLICY_MIN_LENGTH'] + } + ), + PWD_POLICY_CHECK_OLDPWD_ENABLED: this.i18n( + 'Must not be the same as the previous one' + ), + PWD_POLICY_CHECK_USERNAME_ENABLED: this.i18n('Cannot contain the username'), + PWD_POLICY_CHECK_EXCLUSION_LIST_ENABLED: this.i18n( + 'Cannot contain any configured keyword' + ), + PWD_POLICY_CHECK_REPETITIVE_CHARS_ENABLED: this.i18n( + 'Cannot contain any repetitive characters e.g. "aaa"' + ), + PWD_POLICY_CHECK_SEQUENTIAL_CHARS_ENABLED: this.i18n( + 'Cannot contain any sequential characters e.g. "abc"' + ), + PWD_POLICY_CHECK_COMPLEXITY_ENABLED: this.i18n( + '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)' + ) + }; + helpText = helpText.concat( + Object.keys(i18nHelp) + .filter((key) => resp[key]) + .map((key) => '- ' + i18nHelp[key]) + ); + } + return helpText.join('\n'); + }) + ); } /** diff --git a/src/pybind/mgr/dashboard/services/access_control.py b/src/pybind/mgr/dashboard/services/access_control.py index 339e4eb2b41..99222a97cf6 100644 --- a/src/pybind/mgr/dashboard/services/access_control.py +++ b/src/pybind/mgr/dashboard/services/access_control.py @@ -57,9 +57,7 @@ class PasswordPolicy(object): self.password = password self.username = username self.old_password = old_password - self.forbidden_words = ['osd', 'host', 'dashboard', 'pool', - 'block', 'nfs', 'ceph', 'monitors', - 'gateway', 'logs', 'crush', 'maps'] + self.forbidden_words = Settings.PWD_POLICY_EXCLUSION_LIST.split(',') self.complexity_credits = 0 @staticmethod @@ -67,7 +65,9 @@ class PasswordPolicy(object): return re.compile('(?:{0})'.format(word), flags=re.IGNORECASE).search(password) - def check_password_characters(self): + def check_password_complexity(self): + if not Settings.PWD_POLICY_CHECK_COMPLEXITY_ENABLED: + return Settings.PWD_POLICY_MIN_COMPLEXITY digit_credit = 1 small_letter_credit = 1 big_letter_credit = 2 @@ -88,47 +88,65 @@ class PasswordPolicy(object): return self.complexity_credits def check_is_old_password(self): + if not Settings.PWD_POLICY_CHECK_OLDPWD_ENABLED: + return False return self.old_password and self.password == self.old_password def check_if_contains_username(self): + if not Settings.PWD_POLICY_CHECK_USERNAME_ENABLED: + return False if not self.username: return False return self._check_if_contains_word(self.password, self.username) def check_if_contains_forbidden_words(self): + if not Settings.PWD_POLICY_CHECK_EXCLUSION_LIST_ENABLED: + return False return self._check_if_contains_word(self.password, '|'.join(self.forbidden_words)) def check_if_sequential_characters(self): + if not Settings.PWD_POLICY_CHECK_SEQUENTIAL_CHARS_ENABLED: + return False 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): + def check_if_repetitive_characters(self): + if not Settings.PWD_POLICY_CHECK_REPETITIVE_CHARS_ENABLED: + return False 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_password_length(self, min_length=8): - return len(self.password) >= min_length + def check_password_length(self): + if not Settings.PWD_POLICY_CHECK_LENGTH_ENABLED: + return True + return len(self.password) >= Settings.PWD_POLICY_MIN_LENGTH def check_all(self): """ Perform all password policy checks. :raise PasswordPolicyException: If a password policy check fails. """ - if self.check_password_characters() < 10 or not self.check_password_length(): + if not Settings.PWD_POLICY_ENABLED: + return + if self.check_password_complexity() < Settings.PWD_POLICY_MIN_COMPLEXITY: + raise PasswordPolicyException('Password is too weak.') + if not self.check_password_length(): raise PasswordPolicyException('Password is too weak.') if self.check_is_old_password(): raise PasswordPolicyException('Password must not be the same as the previous one.') if self.check_if_contains_username(): raise PasswordPolicyException('Password must not contain username.') - if self.check_if_contains_forbidden_words(): - raise PasswordPolicyException('Password must not contain keywords.') - if self.check_if_repetetive_characters(): + result = self.check_if_contains_forbidden_words() + if result: + raise PasswordPolicyException('Password must not contain the keyword "{}".'.format( + result.group(0))) + if self.check_if_repetitive_characters(): raise PasswordPolicyException('Password must not contain repetitive characters.') if self.check_if_sequential_characters(): raise PasswordPolicyException('Password must not contain sequential characters.') diff --git a/src/pybind/mgr/dashboard/settings.py b/src/pybind/mgr/dashboard/settings.py index c73a8c988c5..229f0c3d03d 100644 --- a/src/pybind/mgr/dashboard/settings.py +++ b/src/pybind/mgr/dashboard/settings.py @@ -62,6 +62,27 @@ class Options(object): USER_PWD_EXPIRATION_WARNING_1 = (10, int) USER_PWD_EXPIRATION_WARNING_2 = (5, int) + # Password policy + PWD_POLICY_ENABLED = (True, bool) + # Individual checks + PWD_POLICY_CHECK_LENGTH_ENABLED = (True, bool) + PWD_POLICY_CHECK_OLDPWD_ENABLED = (True, bool) + PWD_POLICY_CHECK_USERNAME_ENABLED = (False, bool) + PWD_POLICY_CHECK_EXCLUSION_LIST_ENABLED = (False, bool) + PWD_POLICY_CHECK_COMPLEXITY_ENABLED = (False, bool) + PWD_POLICY_CHECK_SEQUENTIAL_CHARS_ENABLED = (False, bool) + PWD_POLICY_CHECK_REPETITIVE_CHARS_ENABLED = (False, bool) + # Settings + PWD_POLICY_MIN_LENGTH = (8, int) + PWD_POLICY_MIN_COMPLEXITY = (10, int) + PWD_POLICY_EXCLUSION_LIST = (','.join(['osd', 'host', + 'dashboard', 'pool', + 'block', 'nfs', + 'ceph', 'monitors', + 'gateway', 'logs', + 'crush', 'maps']), + str) + @staticmethod def has_default_value(name): return getattr(Settings, name, None) is None or \ diff --git a/src/pybind/mgr/dashboard/tests/test_access_control.py b/src/pybind/mgr/dashboard/tests/test_access_control.py index 246dcbf85d0..f69cdd57a42 100644 --- a/src/pybind/mgr/dashboard/tests/test_access_control.py +++ b/src/pybind/mgr/dashboard/tests/test_access_control.py @@ -15,6 +15,7 @@ from ..security import Scope, Permission from ..services.access_control import load_access_control_db, \ password_hash, AccessControlDB, \ SYSTEM_ROLES, PasswordPolicy +from ..settings import Settings class AccessControlTest(unittest.TestCase, CLICommandTestMixin): @@ -790,54 +791,73 @@ class AccessControlTest(unittest.TestCase, CLICommandTestMixin): }) def test_password_policy_pw_length(self): + Settings.PWD_POLICY_CHECK_LENGTH_ENABLED = True + Settings.PWD_POLICY_MIN_LENGTH = 3 pw_policy = PasswordPolicy('foo') - self.assertTrue(pw_policy.check_password_length(3)) + self.assertTrue(pw_policy.check_password_length()) def test_password_policy_pw_length_fail(self): + Settings.PWD_POLICY_CHECK_LENGTH_ENABLED = True pw_policy = PasswordPolicy('bar') self.assertFalse(pw_policy.check_password_length()) def test_password_policy_credits_too_weak(self): + Settings.PWD_POLICY_CHECK_COMPLEXITY_ENABLED = True pw_policy = PasswordPolicy('foo') - pw_credits = pw_policy.check_password_characters() + pw_credits = pw_policy.check_password_complexity() self.assertEqual(pw_credits, 3) def test_password_policy_credits_weak(self): + Settings.PWD_POLICY_CHECK_COMPLEXITY_ENABLED = True pw_policy = PasswordPolicy('mypassword1') - pw_credits = pw_policy.check_password_characters() + pw_credits = pw_policy.check_password_complexity() self.assertEqual(pw_credits, 11) def test_password_policy_credits_ok(self): + Settings.PWD_POLICY_CHECK_COMPLEXITY_ENABLED = True pw_policy = PasswordPolicy('mypassword1!@') - pw_credits = pw_policy.check_password_characters() + pw_credits = pw_policy.check_password_complexity() self.assertEqual(pw_credits, 17) def test_password_policy_credits_strong(self): + Settings.PWD_POLICY_CHECK_COMPLEXITY_ENABLED = True pw_policy = PasswordPolicy('testpassword0047!@') - pw_credits = pw_policy.check_password_characters() + pw_credits = pw_policy.check_password_complexity() self.assertEqual(pw_credits, 22) def test_password_policy_credits_very_strong(self): + Settings.PWD_POLICY_CHECK_COMPLEXITY_ENABLED = True pw_policy = PasswordPolicy('testpassword#!$!@$') - pw_credits = pw_policy.check_password_characters() + pw_credits = pw_policy.check_password_complexity() self.assertEqual(pw_credits, 30) def test_password_policy_forbidden_words(self): + Settings.PWD_POLICY_CHECK_EXCLUSION_LIST_ENABLED = True pw_policy = PasswordPolicy('!@$testdashboard#!$') self.assertTrue(pw_policy.check_if_contains_forbidden_words()) + def test_password_policy_forbidden_words_custom(self): + Settings.PWD_POLICY_CHECK_EXCLUSION_LIST_ENABLED = True + Settings.PWD_POLICY_EXCLUSION_LIST = 'foo,bar' + pw_policy = PasswordPolicy('foo123bar') + self.assertTrue(pw_policy.check_if_contains_forbidden_words()) + def test_password_policy_sequential_chars(self): + Settings.PWD_POLICY_CHECK_SEQUENTIAL_CHARS_ENABLED = True pw_policy = PasswordPolicy('!@$test123#!$') self.assertTrue(pw_policy.check_if_sequential_characters()) - def test_password_policy_repetetive_chars(self): + def test_password_policy_repetitive_chars(self): + Settings.PWD_POLICY_CHECK_REPETITIVE_CHARS_ENABLED = True pw_policy = PasswordPolicy('!@$testfooo#!$') - self.assertTrue(pw_policy.check_if_repetetive_characters()) + self.assertTrue(pw_policy.check_if_repetitive_characters()) def test_password_policy_contain_username(self): + Settings.PWD_POLICY_CHECK_USERNAME_ENABLED = True pw_policy = PasswordPolicy('%admin135)', 'admin') self.assertTrue(pw_policy.check_if_contains_username()) def test_password_policy_is_old_pwd(self): + Settings.PWD_POLICY_CHECK_OLDPWD_ENABLED = True pw_policy = PasswordPolicy('foo', old_password='foo') self.assertTrue(pw_policy.check_is_old_password()) diff --git a/src/pybind/mgr/dashboard/tests/test_settings.py b/src/pybind/mgr/dashboard/tests/test_settings.py index 154af2c12d5..da54a20655d 100644 --- a/src/pybind/mgr/dashboard/tests/test_settings.py +++ b/src/pybind/mgr/dashboard/tests/test_settings.py @@ -116,6 +116,15 @@ class SettingsControllerTest(ControllerTestCase, KVStoreMockMixin): self.assertIn('name', data[0].keys()) self.assertIn('value', data[0].keys()) + def test_settings_list_filtered(self): + self._get('/api/settings?names=GRAFANA_ENABLED,PWD_POLICY_ENABLED') + self.assertStatus(200) + data = self.json_body() + self.assertTrue(len(data) == 2) + names = [option['name'] for option in data] + self.assertIn('GRAFANA_ENABLED', names) + self.assertIn('PWD_POLICY_ENABLED', names) + def test_rgw_daemon_get(self): self._get('/api/settings/grafana-api-username') self.assertStatus(200) -- 2.39.5