Merge pull request #24128 from ricardoasmarques/wip-optional-user-password

mgr/dashboard: User password should be optional

Reviewed-by: Ricardo Dias <rdias@suse.com>
Reviewed-by: Stephan Müller <smueller@suse.com>

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 <username> <password> [<rolename>] [<name>] [<email>]
+  $ ceph dashboard ac-user-create <username> [<password>] [<rolename>] [<name>] [<email>]
 
 - *Delete User*::
 

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")

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',

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)

src/pybind/mgr/dashboard/frontend/src/app/core/auth/user-form/user-form.component.html

@@ -43,8 +43,6 @@
           <label i18n
                  class="control-label col-sm-3"
                  for="name">Password
-            <span class="required"
-                  *ngIf="mode !== userFormMode.editing"></span>
           </label>
           <div class="col-sm-9">
             <div class="input-group">
@@ -75,8 +73,6 @@
           <label i18n
                  class="control-label col-sm-3"
                  for="name">Confirm password
-            <span class="required"
-                  *ngIf="mode !== userFormMode.editing"></span>
           </label>
           <div class="col-sm-9">
             <div class="input-group">

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;

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;

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