From: Ricardo Marques Date: Mon, 17 Sep 2018 11:03:37 +0000 (+0100) Subject: mgr/dashboard: User password should be optional X-Git-Tag: v14.0.1~246^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F24128%2Fhead;p=ceph.git mgr/dashboard: User password should be optional Fixes: https://tracker.ceph.com/issues/36031 Signed-off-by: Ricardo Marques --- diff --git a/doc/mgr/dashboard.rst b/doc/mgr/dashboard.rst index 66f6c41bb87..8b9ae7628c8 100644 --- a/doc/mgr/dashboard.rst +++ b/doc/mgr/dashboard.rst @@ -363,7 +363,7 @@ We provide a set of CLI commands to manage user accounts: - *Create User*:: - $ ceph dashboard ac-user-create [] [] [] + $ ceph dashboard ac-user-create [] [] [] [] - *Delete User*:: diff --git a/qa/tasks/mgr/dashboard/test_auth.py b/qa/tasks/mgr/dashboard/test_auth.py index 04344050207..0921b7d9b4f 100644 --- a/qa/tasks/mgr/dashboard/test_auth.py +++ b/qa/tasks/mgr/dashboard/test_auth.py @@ -76,6 +76,17 @@ class AuthTest(DashboardTestCase): "detail": "Invalid credentials" }) + def test_login_without_password(self): + self.create_user('admin2', '', ['administrator']) + self._post("/api/auth", {'username': 'admin2', 'password': ''}) + self.assertStatus(400) + self.assertJsonBody({ + "component": "auth", + "code": "invalid_credentials", + "detail": "Invalid credentials" + }) + self.delete_user('admin2') + def test_logout(self): self._post("/api/auth", {'username': 'admin', 'password': 'admin'}) self._delete("/api/auth") diff --git a/qa/tasks/mgr/dashboard/test_user.py b/qa/tasks/mgr/dashboard/test_user.py index 187d1a25d60..57521da7de4 100644 --- a/qa/tasks/mgr/dashboard/test_user.py +++ b/qa/tasks/mgr/dashboard/test_user.py @@ -75,15 +75,6 @@ class UserTest(DashboardTestCase): self.assertError(code='username_already_exists', component='user') - def test_create_user_no_password(self): - self._create_user(username='user1', - name='My Name', - email='admin@email.com', - roles=['administrator']) - self.assertStatus(400) - self.assertError(code='password_required', - component='user') - def test_create_user_invalid_role(self): self._create_user(username='user1', password='mypassword', diff --git a/src/pybind/mgr/dashboard/controllers/user.py b/src/pybind/mgr/dashboard/controllers/user.py index b49872f0778..a0671c5fe1c 100644 --- a/src/pybind/mgr/dashboard/controllers/user.py +++ b/src/pybind/mgr/dashboard/controllers/user.py @@ -47,10 +47,6 @@ class User(RESTController): raise DashboardException(msg='Username is required', code='username_required', component='user') - if not password: - raise DashboardException(msg='Password is required', - code='password_required', - component='user') user_roles = None if roles: user_roles = User._get_user_roles(roles) 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 a21232eb284..f42d8e7c6e0 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 @@ -43,8 +43,6 @@
@@ -75,8 +73,6 @@
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 b7a7f8a5f88..9d21da358eb 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 @@ -88,12 +88,8 @@ describe('UserFormComponent', () => { it('should validate username required', () => { form.get('username').setValue(''); expect(form.get('username').hasError('required')).toBeTruthy(); - }); - - it('should validate password required', () => { - ['password', 'confirmpassword'].forEach((key) => - expect(form.get(key).hasError('required')).toBeTruthy() - ); + form.get('username').setValue('user1'); + expect(form.get('username').hasError('required')).toBeFalsy(); }); it('should validate password match', () => { @@ -109,6 +105,13 @@ describe('UserFormComponent', () => { expect(form.get('email').hasError('email')).toBeTruthy(); }); + it('should validate all required fields', () => { + form.get('username').setValue(''); + expect(form.valid).toBeFalsy(); + form.get('username').setValue('user1'); + expect(form.valid).toBeTruthy(); + }); + it('should set mode', () => { expect(component.mode).toBeUndefined(); }); @@ -196,13 +199,6 @@ describe('UserFormComponent', () => { expect(component.mode).toBe('editing'); }); - it('should validate password not required', () => { - ['password', 'confirmpassword'].forEach((key) => { - form.get(key).setValue(''); - expect(form.get(key).hasError('required')).toBeFalsy(); - }); - }); - it('should alert if user is removing needed role permission', () => { spyOn(TestBed.get(AuthStorageService), 'getUsername').and.callFake(() => user.username); let modalBodyTpl = null; 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 c1a7be7e976..1b323f68e8c 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 @@ -83,21 +83,10 @@ export class UserFormComponent implements OnInit { }); if (this.mode === this.userFormMode.editing) { this.initEdit(); - } else { - this.initAdd(); } } - initAdd() { - ['password', 'confirmpassword'].forEach((controlName) => - this.userForm.get(controlName).setValidators([Validators.required]) - ); - } - initEdit() { - ['password', 'confirmpassword'].forEach((controlName) => - this.userForm.get(controlName).setValidators([]) - ); this.disableForEdit(); this.route.params.subscribe((params: { username: string }) => { const username = params.username; diff --git a/src/pybind/mgr/dashboard/services/access_control.py b/src/pybind/mgr/dashboard/services/access_control.py index 10eb705706b..61a4d380e1a 100644 --- a/src/pybind/mgr/dashboard/services/access_control.py +++ b/src/pybind/mgr/dashboard/services/access_control.py @@ -19,6 +19,8 @@ from ..exceptions import RoleAlreadyExists, RoleDoesNotExist, ScopeNotValid, \ # password hashing algorithm def password_hash(password, salt_password=None): + if not password: + return None if not salt_password: salt_password = bcrypt.gensalt() else: @@ -381,7 +383,7 @@ ACCESS_CONTROL_COMMANDS = [ { 'cmd': 'dashboard ac-user-create ' 'name=username,type=CephString ' - 'name=password,type=CephString ' + 'name=password,type=CephString,req=false ' 'name=rolename,type=CephString,req=false ' 'name=name,type=CephString,req=false ' 'name=email,type=CephString,req=false', @@ -545,7 +547,7 @@ Username and password updated''', '' elif cmd['prefix'] == 'dashboard ac-user-create': username = cmd['username'] - password = cmd['password'] + password = cmd['password'] if 'password' in cmd else None rolename = cmd['rolename'] if 'rolename' in cmd else None name = cmd['name'] if 'name' in cmd else None email = cmd['email'] if 'email' in cmd else None @@ -669,9 +671,10 @@ class LocalAuthenticator(object): def authenticate(self, username, password): try: user = ACCESS_CTRL_DB.get_user(username) - pass_hash = password_hash(password, user.password) - if pass_hash == user.password: - return user.permissions_dict() + if user.password: + pass_hash = password_hash(password, user.password) + if pass_hash == user.password: + return user.permissions_dict() except UserDoesNotExist: logger.debug("User '%s' does not exist", username) return None