mgr/dashboard: fix-user-creation-validation

Fixes: https://tracker.ceph.com/issues/77024
Signed-off-by: Sagar Gopale <sagar.gopale@ibm.com>

diff --git a/src/pybind/mgr/dashboard/frontend/cypress/e2e/ui/user-mgmt.po.ts b/src/pybind/mgr/dashboard/frontend/cypress/e2e/ui/user-mgmt.po.ts
index 76679806a8835c2b602d462286c8507a09c0cc26..eb79748be1d3cec39c820d3003211975e286b100 100644 (file)
--- a/src/pybind/mgr/dashboard/frontend/cypress/e2e/ui/user-mgmt.po.ts
+++ b/src/pybind/mgr/dashboard/frontend/cypress/e2e/ui/user-mgmt.po.ts
@@ -16,6 +16,15 @@ export class UserMgmtPageHelper extends PageHelper {
     cy.get('#name').type(name);
     cy.get('#email').type(email);
 
+    // Select role 'administrator'
+    cy.get('cds-combo-box[type="multi"] input.cds--text-input').first().click({ force: true });
+    cy.get('.cds--list-box__menu.cds--multi-select').should('be.visible');
+    cy.get('.cds--list-box__menu.cds--multi-select .cds--checkbox-label')
+      .contains('.cds--checkbox-label-text', 'administrator', { matchCase: false })
+      .parent()
+      .click({ force: true });
+    cy.get('body').type('{esc}');
+
     // Click the create button and wait for user to be made
     cy.get('[data-testid=submitBtn]').click();
     this.getFirstTableCell(username).should('exist');

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 ea8f8aed07709fec24bc55e09698ffe3f9d4ed23..843c08e5b5cb50615b8e048d5904f69f3e49692e 100644 (file)
--- 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
@@ -1,73 +1,82 @@
-<div cdsCol
-     [columnNumbers]="{md: 4}">
+<div cdsCol [columnNumbers]="{ md: 4 }">
   <ng-container *cdFormLoading="loading">
-    <form #frm="ngForm"
-          #formDir="ngForm"
-          [formGroup]="userForm"
-          novalidate>
-      <div i18n="form title"
-           class="form-header">{{ action | titlecase }} {{ resource | upperFirst }}
+    <form #frm="ngForm" #formDir="ngForm" [formGroup]="userForm" novalidate>
+      <div i18n="form title" class="form-header">
+        {{ action | titlecase }} {{ resource | upperFirst }}
       </div>
       <!-- UserName -->
       <div class="form-item">
-        <cds-text-label labelInputID="username"
-                        cdRequiredField="Username"
-                        [invalid]="!userForm.controls.username.valid && userForm.controls.username.dirty"
-                        [invalidText]="usernameError"
-                        i18n>Username
-          <input cdsText
-                 placeholder="Username..."
-                 i18n-placeholder
-                 id="username"
-                 formControlName="username"
-                 [invalid]="!userForm.controls.username.valid && userForm.controls.username.dirty"
-                 autofocus
-                 ngbTooltip="White spaces at the beginning and end will be trimmed"
-                 i18n-ngbTooltip
-                 cdTrim>
+        <cds-text-label
+          labelInputID="username"
+          cdRequiredField="Username"
+          [invalid]="!userForm.controls.username.valid && userForm.controls.username.dirty"
+          [invalidText]="usernameError"
+          i18n
+          >Username
+          <input
+            cdsText
+            placeholder="Username..."
+            i18n-placeholder
+            id="username"
+            formControlName="username"
+            [invalid]="!userForm.controls.username.valid && userForm.controls.username.dirty"
+            autofocus
+            ngbTooltip="White spaces at the beginning and end will be trimmed"
+            i18n-ngbTooltip
+            cdTrim
+          />
         </cds-text-label>
         <ng-template #usernameError>
           <span *ngIf="userForm.showError('username', formDir, 'required')">
-            <ng-container i18n>
-             This field is required.
-            </ng-container>
+            <ng-container i18n> This field is required. </ng-container>
           </span>
           <span *ngIf="userForm.showError('username', formDir, 'notUnique')">
-            <ng-container i18n>
-              The username already exists.
-            </ng-container>
+            <ng-container i18n> The username already exists. </ng-container>
           </span>
         </ng-template>
       </div>
       <!-- Password -->
       <div class="form-item">
-        <cds-password-label labelInputID="password"
-                            label="Password..."
-                            [invalid]="!userForm.controls.password.valid && userForm.controls.password.dirty"
-                            [invalidText]="passwordError"
-                            i18n>Password
-        <cd-helper *ngIf="passwordPolicyHelpText.length > 0"
-                    class="text-pre-wrap"
-                    html="{{ passwordPolicyHelpText }}">
-        </cd-helper>
-        <input cdsPassword
-               type="password"
-               placeholder="Password..."
-               id="password"
-               autocomplete="new-password"
-               formControlName="password"
-               >
+        <cds-password-label
+          labelInputID="password"
+          label="Password..."
+          [cdRequiredField]="mode !== userFormMode.editing && !isSSO ? 'Password' : ''"
+          [invalid]="!userForm.controls.password.valid && userForm.controls.password.dirty"
+          [invalidText]="passwordError"
+          i18n
+          >Password
+          <cd-helper
+            *ngIf="passwordPolicyHelpText.length > 0"
+            class="text-pre-wrap"
+            html="{{ passwordPolicyHelpText }}"
+          >
+          </cd-helper>
+          <input
+            cdsPassword
+            type="password"
+            placeholder="Password..."
+            id="password"
+            autocomplete="new-password"
+            formControlName="password"
+          />
         </cds-password-label>
         <ng-template #passwordError>
-          <span class="invalid-feedback"
-                *ngIf="userForm.showError('password', formDir, 'match')"
-                i18n>Password confirmation doesn't match the password.
+          <span
+            class="invalid-feedback"
+            *ngIf="userForm.showError('password', formDir, 'match')"
+            i18n
+            >Password confirmation doesn't match the password.
           </span>
-          <span class="invalid-feedback"
-                *ngIf="userForm.showError('password', formDir, 'required')"
-                i18n>This field is required.</span>
-          <span class="invalid-feedback"
-                *ngIf="userForm.showError('password', formDir, 'passwordPolicy')">
+          <span
+            class="invalid-feedback"
+            *ngIf="userForm.showError('password', formDir, 'required')"
+            i18n
+            >This field is required.</span
+          >
+          <span
+            class="invalid-feedback"
+            *ngIf="userForm.showError('password', formDir, 'passwordPolicy')"
+          >
             {{ passwordValuation }}
           </span>
         </ng-template>
@@ -75,142 +84,171 @@
 
       <!-- Confirm password -->
       <div class="form-item">
-        <cds-password-label labelInputID="confirmpassword"
-                            label="Confirm password..."
-                            [invalid]="!userForm.controls.confirmpassword.valid && userForm.controls.confirmpassword.dirty"
-                            [invalidText]="confirmpasswordError"
-                            i18n> Confirm password
-          <input cdsPassword
-                 type="password"
-                 placeholder="Confirm password..."
-                 id="confirmpassword"
-                 formControlName="confirmpassword">
+        <cds-password-label
+          labelInputID="confirmpassword"
+          label="Confirm password..."
+          [cdRequiredField]="mode !== userFormMode.editing && !isSSO ? 'Confirm password' : ''"
+          [invalid]="
+            !userForm.controls.confirmpassword.valid && userForm.controls.confirmpassword.dirty
+          "
+          [invalidText]="confirmpasswordError"
+          i18n
+        >
+          Confirm password
+          <input
+            cdsPassword
+            type="password"
+            placeholder="Confirm password..."
+            id="confirmpassword"
+            formControlName="confirmpassword"
+          />
         </cds-password-label>
         <ng-template #confirmpasswordError>
-          <span class="invalid-feedback"
-                *ngIf="userForm.showError('confirmpassword', formDir, 'match')"
-                i18n>Password confirmation doesn't match the password.</span>
-          <span class="invalid-feedback"
-                *ngIf="userForm.showError('confirmpassword', formDir, 'required')"
-                i18n>This field is required.</span>
+          <span
+            class="invalid-feedback"
+            *ngIf="userForm.showError('confirmpassword', formDir, 'match')"
+            i18n
+            >Password confirmation doesn't match the password.</span
+          >
+          <span
+            class="invalid-feedback"
+            *ngIf="userForm.showError('confirmpassword', formDir, 'required')"
+            i18n
+            >This field is required.</span
+          >
         </ng-template>
       </div>
       <!-- Password expiration date -->
-      <div class="form-item"
-           *ngIf="!authStorageService.isSSO()">
-        <cds-text-label  [ngClass]="{'required': pwdExpirationSettings.pwdExpirationSpan > 0}">{{'Password Expiration Date'}}
-          <cd-helper class="text-pre-wrap"
-                     *ngIf="pwdExpirationSettings.pwdExpirationSpan == 0">
+      <div class="form-item" *ngIf="!isSSO">
+        <cds-text-label [ngClass]="{ required: pwdExpirationSettings.pwdExpirationSpan > 0 }"
+          >{{ 'Password Expiration Date' }}
+          <cd-helper class="text-pre-wrap" *ngIf="pwdExpirationSettings.pwdExpirationSpan == 0">
             <span>
-                The Dashboard setting defining the expiration interval of
-                passwords is currently set to <strong>0</strong>. This means
-                if a date is set, the user password will only expire once.
+              The Dashboard setting defining the expiration interval of passwords is currently set
+              to <strong>0</strong>. This means if a date is set, the user password will only expire
+              once.
             </span>
-            <span>Consider configuring the Dashboard setting
-              <a routerLink="/mgr-modules/edit/dashboard"
-                 class="alert-link">USER_PWD_EXPIRATION_SPAN</a>
-                  in order to let passwords expire periodically.
+            <span
+              >Consider configuring the Dashboard setting
+              <a routerLink="/mgr-modules/edit/dashboard" class="alert-link"
+                >USER_PWD_EXPIRATION_SPAN</a
+              >
+              in order to let passwords expire periodically.
             </span>
           </cd-helper>
-        <cd-date-time-picker [control]="userForm.get('pwdExpirationDate')"
-                             placeHolder="Password expiration date"
-                             [hasTime]="false"
-                             [defaultDate]="passwordexp"
-                             i18n-name
-                             >
-        </cd-date-time-picker>
-      </cds-text-label>
-        <span class="invalid-feedback"
-              *ngIf="userForm.showError('pwdExpirationDate', formDir, 'required')"
-              i18n>This field is required.
+          <cd-date-time-picker
+            [control]="userForm.get('pwdExpirationDate')"
+            placeHolder="Password expiration date"
+            [hasTime]="false"
+            [defaultDate]="passwordexp"
+            i18n-name
+          >
+          </cd-date-time-picker>
+        </cds-text-label>
+        <span
+          class="invalid-feedback"
+          *ngIf="userForm.showError('pwdExpirationDate', formDir, 'required')"
+          i18n
+          >This field is required.
         </span>
       </div>
       <!--Full Name-->
       <div class="form-item">
-        <cds-text-label for="name"
-                        i18n> Full Name
-          <input cdsText
-                 type="text"
-                 placeholder="Full Name..."
-                 id="name"
-                 formControlName="name">
+        <cds-text-label for="name" i18n>
+          Full Name
+          <input cdsText type="text" placeholder="Full Name..." id="name" formControlName="name" />
         </cds-text-label>
       </div>
       <!-- Email -->
       <div class="form-item">
-        <cds-text-label for="email"
-                        [invalid]="!userForm.controls.email.valid && userForm.controls.email.dirty"
-                        [invalidText]="emailError"
-                        i18n>
+        <cds-text-label
+          for="email"
+          [invalid]="!userForm.controls.email.valid && userForm.controls.email.dirty"
+          [invalidText]="emailError"
+          i18n
+        >
           Email
-          <input cdsText
-                 type="email"
-                 placeholder="Email..."
-                 id="email"
-                 formControlName="email">
+          <input cdsText type="email" placeholder="Email..." id="email" formControlName="email" />
         </cds-text-label>
         <ng-template #emailError>
-          <span class="invalid-feedback"
-                *ngIf="userForm.showError('email', formDir, 'email')"
-                i18n>Invalid email.
+          <span class="invalid-feedback" *ngIf="userForm.showError('email', formDir, 'email')" i18n
+            >Invalid email.
           </span>
         </ng-template>
       </div>
       <!-- Roles -->
-      <div class="form-item"
-           *ngIf="allRoles">
-        <cds-combo-box label="Roles"
-                       type="multi"
-                       selectionFeedback="top-after-reopen"
-                       for="roles"
-                       formControlName="roles"
-                       id="roles"
-                       placeholder="Select Roles..."
-                       i18n-placeholder
-                       [appendInline]="true"
-                       [items]="allRoles"
-                       itemValueKey="name"
-                       i18n>
+      <div class="form-item" *ngIf="allRoles">
+        <cds-combo-box
+          label="Roles"
+          type="multi"
+          selectionFeedback="top-after-reopen"
+          for="roles"
+          formControlName="roles"
+          id="roles"
+          placeholder="Select Roles..."
+          i18n-placeholder
+          [appendInline]="true"
+          [items]="allRoles"
+          itemValueKey="name"
+          [cdRequiredField]="!isSSO ? 'Roles' : ''"
+          [invalid]="!userForm.controls.roles.valid && userForm.controls.roles.dirty"
+          [invalidText]="rolesError"
+          i18n
+        >
           <cds-dropdown-list></cds-dropdown-list>
         </cds-combo-box>
+        <ng-template #rolesError>
+          <span
+            class="invalid-feedback"
+            *ngIf="userForm.showError('roles', formDir, 'required')"
+            i18n
+            >This field is required.</span
+          >
+        </ng-template>
       </div>
       <!-- Enabled -->
-      <div class="form-item"
-           *ngIf="!isCurrentUser()">
-        <cds-checkbox id="enabled"
-                      formControlName="enabled"
-                      name="enabled"
-                      i18n>Enabled
+      <div class="form-item" *ngIf="!isCurrentUser()">
+        <cds-checkbox id="enabled" formControlName="enabled" name="enabled" i18n
+          >Enabled
         </cds-checkbox>
       </div>
       <!-- Force change password -->
-      <div class="form-item"
-           *ngIf="!isCurrentUser() && !authStorageService.isSSO()">
-        <cds-checkbox id="pwdUpdateRequired"
-                      formControlName="pwdUpdateRequired"
-                      name="pwdUpdateRequired"
-                      i18n>User must change password at next login
+      <div class="form-item" *ngIf="!isCurrentUser() && !isSSO">
+        <cds-checkbox
+          id="pwdUpdateRequired"
+          formControlName="pwdUpdateRequired"
+          name="pwdUpdateRequired"
+          i18n
+          >User must change password at next login
         </cds-checkbox>
       </div>
       <!--Submit Button-->
-      <cd-form-button-panel (submitActionEvent)="submit()"
-                            [form]="userForm"
-                            [submitText]="(action | titlecase) + ' ' + (resource | upperFirst)"
-                            wrappingClass="text-right">
+      <cd-form-button-panel
+        (submitActionEvent)="submit()"
+        [form]="userForm"
+        [submitText]="(action | titlecase) + ' ' + (resource | upperFirst)"
+        wrappingClass="text-right"
+      >
       </cd-form-button-panel>
     </form>
   </ng-container>
 </div>
 
 <ng-template #removeSelfUserReadUpdatePermissionTpl>
-  <p><strong i18n>You are about to remove "user read / update" permissions from your own user.</strong></p>
-  <br>
+  <p>
+    <strong i18n
+      >You are about to remove "user read / update" permissions from your own user.</strong
+    >
+  </p>
+  <br />
   <p i18n>If you continue, you will no longer be able to add or remove roles from any user.</p>
 
   <ng-container i18n>Are you sure you want to continue?</ng-container>
 </ng-template>
 
 <ng-template #popContent>
-  <cd-date-time-picker [control]="userForm.get('pwdExpirationDate')"
-                       [hasTime]="false"></cd-date-time-picker>
+  <cd-date-time-picker
+    [control]="userForm.get('pwdExpirationDate')"
+    [hasTime]="false"
+  ></cd-date-time-picker>
 </ng-template>

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 130349344866cdb3d0b7521c2c444f59aac9b884..850dff1c5a7d7a8b88c3cc6065719f44587cf28d 100644 (file)
--- 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
@@ -62,12 +62,16 @@ describe('UserFormComponent', () => {
     form = component.userForm;
     httpTesting = TestBed.inject(HttpTestingController);
     userService = TestBed.inject(UserService);
+    spyOn(userService, 'validatePassword').and.returnValue(
+      of({ valid: true, credits: 10, valuation: 'strong' })
+    );
     modalService = TestBed.inject(ModalCdsService);
     router = TestBed.inject(Router);
     spyOn(router, 'navigate');
     fixture.detectChanges();
     const notify = TestBed.inject(NotificationService);
     spyOn(notify, 'show');
+    spyOn(TestBed.inject(AuthStorageService), 'isSSO').and.returnValue(false);
     formHelper = new FormHelper(form);
   });
 
@@ -109,6 +113,34 @@ describe('UserFormComponent', () => {
       formHelper.expectErrorChange('email', 'aaa', 'email');
     });
 
+    it('should validate password required in create mode', () => {
+      formHelper.expectErrorChange('password', '', 'required');
+      formHelper.expectValidChange('password', 'pass123');
+    });
+
+    it('should validate confirmpassword required in create mode', () => {
+      formHelper.setValue('password', 'pass123');
+      formHelper.expectErrorChange('confirmpassword', '', 'required');
+      formHelper.expectValidChange('confirmpassword', 'pass123');
+    });
+
+    it('should validate roles required in create mode', () => {
+      formHelper.expectErrorChange('roles', [], 'required');
+      formHelper.expectValidChange('roles', ['administrator']);
+    });
+
+    it('should not validate password and roles if SSO is enabled', () => {
+      component.isSSO = true;
+      component.createForm();
+      form = component.userForm;
+      form.get('password').updateValueAndValidity();
+      expect(form.get('password').valid).toBeTruthy();
+      form.get('confirmpassword').updateValueAndValidity();
+      expect(form.get('confirmpassword').valid).toBeTruthy();
+      form.get('roles').updateValueAndValidity();
+      expect(form.get('roles').valid).toBeTruthy();
+    });
+
     it('should set mode', () => {
       expect(component.mode).toBeUndefined();
     });
@@ -210,6 +242,13 @@ describe('UserFormComponent', () => {
       expect(component.mode).toBe('editing');
     });
 
+    it('should not validate password required in edit mode', () => {
+      form.get('password').setValue('');
+      expect(form.get('password').valid).toBeTruthy();
+      form.get('confirmpassword').setValue('');
+      expect(form.get('confirmpassword').valid).toBeTruthy();
+    });
+
     it('should alert if user is removing needed role permission', () => {
       spyOn(TestBed.inject(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 c67dbad95d37b0ce9e827b979ef68d5f7ea3fa08..6ee9b986525a03289e163a2a8701caf9d5117dc1 100644 (file)
--- 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
@@ -58,6 +58,7 @@ export class UserFormComponent extends CdForm implements OnInit {
   pwdExpirationFormat = 'YYYY-MM-DD';
   selectedRole: string[];
   passwordexp: boolean = false;
+  isSSO = false;
   constructor(
     private authService: AuthService,
     private authStorageService: AuthStorageService,
@@ -74,6 +75,7 @@ export class UserFormComponent extends CdForm implements OnInit {
   ) {
     super();
     this.resource = $localize`user`;
+    this.isSSO = this.authStorageService.isSSO();
     this.createForm();
     this.messages = new SelectMessages({ empty: $localize`There are no roles.` });
   }
@@ -92,8 +94,12 @@ export class UserFormComponent extends CdForm implements OnInit {
         name: [''],
         password: [
           '',
-          [],
-
+          [
+            (control) =>
+              this.mode === this.userFormMode.editing || this.isSSO
+                ? null
+                : Validators.required(control)
+          ],
           [
             CdValidators.passwordPolicy(
               this.userService,
@@ -107,10 +113,18 @@ export class UserFormComponent extends CdForm implements OnInit {
             )
           ]
         ],
-        confirmpassword: [''],
+        confirmpassword: [
+          '',
+          [
+            (control) =>
+              this.mode === this.userFormMode.editing || this.isSSO
+                ? null
+                : Validators.required(control)
+          ]
+        ],
         pwdExpirationDate: [''],
         email: ['', [CdValidators.email]],
-        roles: [[]],
+        roles: [[], [(control) => (this.isSSO ? null : Validators.required(control))]],
         enabled: [true, [Validators.required]],
         pwdUpdateRequired: [true]
       },
@@ -129,6 +143,8 @@ export class UserFormComponent extends CdForm implements OnInit {
       this.action = this.actionLabels.CREATE;
       this.passwordexp = true;
     }
+    this.userForm.get('password').updateValueAndValidity();
+    this.userForm.get('confirmpassword').updateValueAndValidity();
 
     const observables = [this.roleService.list(), this.settingsService.getStandardSettings()];
     observableForkJoin(observables).subscribe(
@@ -149,6 +165,7 @@ export class UserFormComponent extends CdForm implements OnInit {
             expirationDate.add(this.pwdExpirationSettings.pwdExpirationSpan, 'day');
             pwdExpirationDateField.setValue(expirationDate.format(this.pwdExpirationFormat));
             pwdExpirationDateField.setValidators([Validators.required]);
+            pwdExpirationDateField.updateValueAndValidity();
           }
 
           this.loadingReady();

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 997564a983c3c15f2cd923ae20e4c00265e213ad..288151c24fa7933b829a9aedede0964fbfaa3f5c 100644 (file)
--- 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
@@ -349,7 +349,8 @@ export class CdValidators {
         return null;
       }
       if (ctrl1.value !== ctrl2.value) {
-        ctrl2.setErrors({ match: true });
+        const errors = _.merge({}, ctrl2.errors, { match: true });
+        ctrl2.setErrors(errors);
       } else {
         const hasError = ctrl2.hasError('match');
         if (hasError) {