NAS-130474 / 24.10-RC.1 / UI should expose option to skip ACL traverse validation i…

src/app/helptext/storage/volumes/datasets/dataset-acl.ts

@@ -61,6 +61,11 @@ are submitted only when this box is set.'),
   dataset_acl_traverse_placeholder: T('Apply permissions to child datasets'),
   dataset_acl_traverse_tooltip: T('Apply permissions recursively to all child datasets of the current dataset.'),
 
+  dataset_acl_validate_placeholder: T('Validate effective ACL'),
+  dataset_acl_validate_tooltip: T('Ensure that ACL permissions are validated for all users and groups.\
+ Disabling this may allow configurations that do not provide the intended access. \
+ It is recommended to keep this option enabled.'),
+
   dataset_acl_dialog_warning: T('Warning'),
   dataset_acl_dialog_warning_message: T('Changing dataset permission mode\
  can severely affect existing permissions.'),

src/app/interfaces/acl.interface.ts

@@ -113,6 +113,7 @@ export interface SetAclOptions {
   recursive?: boolean;
   traverse?: boolean;
   canonicalize?: boolean;
+  validate_effective_acl?: boolean;
 }
 
 export interface SetAcl {

src/app/pages/datasets/modules/permissions/containers/dataset-acl-editor/acl-editor-save-controls/acl-editor-save-controls.component.html

@@ -0,0 +1,36 @@
+<div class="checkboxes" [formGroup]="saveParameters">
+  <ix-checkbox
+    formControlName="recursive"
+    class="recursive-checkbox"
+    [label]="helptext.dataset_acl_recursive_placeholder | translate"
+    [tooltip]="helptext.dataset_acl_recursive_tooltip | translate"
+  ></ix-checkbox>
+
+  @if (saveParameters.get('recursive').value) {
+    <ix-checkbox
+      formControlName="traverse"
+      [label]="helptext.dataset_acl_traverse_placeholder | translate"
+      [tooltip]="helptext.dataset_acl_traverse_tooltip | translate"
+    ></ix-checkbox>
+  }
+
+  @if (hasValidateAclCheckbox()) {
+    <ix-checkbox
+      formControlName="validate_effective_acl"
+      [label]="helptext.dataset_acl_validate_placeholder | translate"
+      [tooltip]="helptext.dataset_acl_validate_tooltip | translate"
+    ></ix-checkbox>
+  }
+</div>
+
+<button
+  *ixRequiresRoles="[Role.FilesystemAttrsWrite, Role.FilesystemFullControl]"
+  mat-button
+  color="primary"
+  class="save-button"
+  ixTest="save-acl"
+  [disabled]="!canBeSaved()"
+  (click)="onSavePressed()"
+>
+  {{ 'Save Access Control List' | translate }}
+</button>

src/app/pages/datasets/modules/permissions/containers/dataset-acl-editor/acl-editor-save-controls/acl-editor-save-controls.component.scss

@@ -0,0 +1,8 @@
+.checkboxes {
+  padding: 0 5px;
+}
+
+.save-button {
+  margin-bottom: 10px;
+  width: 100%;
+}

src/app/pages/datasets/modules/permissions/containers/dataset-acl-editor/acl-editor-save-controls/acl-editor-save-controls.component.spec.ts

@@ -0,0 +1,115 @@
+import { HarnessLoader } from '@angular/cdk/testing';
+import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed';
+import { ReactiveFormsModule } from '@angular/forms';
+import { MatButtonHarness } from '@angular/material/button/testing';
+import { createComponentFactory, mockProvider, Spectator } from '@ngneat/spectator/jest';
+import { BehaviorSubject, of } from 'rxjs';
+import { mockAuth } from 'app/core/testing/utils/mock-auth.utils';
+import { DirectoryServiceState } from 'app/enums/directory-service-state.enum';
+import { helptextAcl } from 'app/helptext/storage/volumes/datasets/dataset-acl';
+import { DialogService } from 'app/modules/dialog/dialog.service';
+import { IxCheckboxHarness } from 'app/modules/forms/ix-forms/components/ix-checkbox/ix-checkbox.harness';
+import { IxFormsModule } from 'app/modules/forms/ix-forms/ix-forms.module';
+import {
+  AclEditorSaveControlsComponent,
+} from 'app/pages/datasets/modules/permissions/containers/dataset-acl-editor/acl-editor-save-controls/acl-editor-save-controls.component';
+import { DatasetAclEditorStore } from 'app/pages/datasets/modules/permissions/stores/dataset-acl-editor.store';
+import { WebSocketService } from 'app/services/ws.service';
+
+describe('AclEditorSaveControlsComponent', () => {
+  let spectator: Spectator<AclEditorSaveControlsComponent>;
+  let loader: HarnessLoader;
+  const call$ = new BehaviorSubject({
+    activedirectory: DirectoryServiceState.Disabled,
+    ldap: DirectoryServiceState.Disabled,
+  });
+  const createComponent = createComponentFactory({
+    component: AclEditorSaveControlsComponent,
+    imports: [
+      IxFormsModule,
+      ReactiveFormsModule,
+    ],
+    providers: [
+      mockProvider(DatasetAclEditorStore, {
+        saveAcl: jest.fn(),
+      }),
+      mockProvider(DialogService, {
+        confirm: jest.fn(() => of(true)),
+      }),
+      mockProvider(WebSocketService, {
+        call: jest.fn(() => call$),
+      }),
+      mockAuth(),
+    ],
+  });
+
+  beforeEach(() => {
+    spectator = createComponent({
+      props: {
+        canBeSaved: true,
+        ownerValues: {
+          owner: 'root',
+          ownerGroup: 'wheel',
+          applyOwner: true,
+          applyGroup: false,
+        },
+      },
+    });
+
+    loader = TestbedHarnessEnvironment.loader(spectator.fixture);
+  });
+
+  it('marks save button as disabled when [canBeSaved] is false', async () => {
+    spectator.setInput('canBeSaved', false);
+
+    const saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save Access Control List' }));
+    expect(await saveButton.isDisabled()).toBe(true);
+  });
+
+  it('shows a warning when Recursive is selected', async () => {
+    const recursiveCheckbox = await loader.getHarness(IxCheckboxHarness.with({ label: 'Apply permissions recursively' }));
+    await recursiveCheckbox.setValue(true);
+
+    expect(spectator.inject(DialogService).confirm).toHaveBeenCalledWith({
+      title: helptextAcl.dataset_acl_recursive_dialog_warning,
+      message: helptextAcl.dataset_acl_recursive_dialog_warning_message,
+    });
+  });
+
+  it('shows Traverse checkbox when Recursive is selected', async () => {
+    const recursiveCheckbox = await loader.getHarness(IxCheckboxHarness.with({ label: 'Apply permissions recursively' }));
+    await recursiveCheckbox.setValue(true);
+
+    const traverseCheckbox = await loader.getHarness(IxCheckboxHarness.with({ label: 'Apply permissions to child datasets' }));
+    expect(traverseCheckbox).toBeTruthy();
+  });
+
+  it('shows Validate Effective ACL checkbox that defaults to true when directory services are enabled', async () => {
+    call$.next({
+      activedirectory: DirectoryServiceState.Healthy,
+      ldap: DirectoryServiceState.Disabled,
+    });
+
+    const validateAclCheckbox = await loader.getHarness(IxCheckboxHarness.with({ label: 'Validate effective ACL' }));
+    expect(validateAclCheckbox).toBeTruthy();
+    expect(await validateAclCheckbox.getValue()).toBe(true);
+  });
+
+  it('saves current ACL settings when save button is pressed', async () => {
+    const recursiveCheckbox = await loader.getHarness(IxCheckboxHarness.with({ label: 'Apply permissions recursively' }));
+    await recursiveCheckbox.setValue(true);
+
+    const saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save Access Control List' }));
+    await saveButton.click();
+
+    expect(spectator.inject(DatasetAclEditorStore).saveAcl).toHaveBeenCalledWith({
+      recursive: true,
+      traverse: false,
+      validateEffectiveAcl: true,
+      owner: 'root',
+      ownerGroup: 'wheel',
+      applyOwner: true,
+      applyGroup: false,
+    });
+  });
+});

src/app/pages/datasets/modules/permissions/containers/dataset-acl-editor/acl-editor-save-controls/acl-editor-save-controls.component.ts

@@ -0,0 +1,91 @@
+import {
+  ChangeDetectionStrategy, Component, input, OnInit,
+} from '@angular/core';
+import { toSignal } from '@angular/core/rxjs-interop';
+import { FormBuilder } from '@angular/forms';
+import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
+import { filter, map, switchMap } from 'rxjs/operators';
+import { DirectoryServiceState } from 'app/enums/directory-service-state.enum';
+import { Role } from 'app/enums/role.enum';
+import { helptextAcl } from 'app/helptext/storage/volumes/datasets/dataset-acl';
+import { DialogService } from 'app/modules/dialog/dialog.service';
+import { DatasetAclEditorStore } from 'app/pages/datasets/modules/permissions/stores/dataset-acl-editor.store';
+import { WebSocketService } from 'app/services/ws.service';
+
+@UntilDestroy()
+@Component({
+  selector: 'ix-acl-editor-save-controls',
+  templateUrl: './acl-editor-save-controls.component.html',
+  styleUrls: ['./acl-editor-save-controls.component.scss'],
+  changeDetection: ChangeDetectionStrategy.OnPush,
+})
+export class AclEditorSaveControlsComponent implements OnInit {
+  readonly canBeSaved = input(false);
+  readonly ownerValues = input.required<{
+    owner: string;
+    ownerGroup: string;
+    applyOwner: boolean;
+    applyGroup: boolean;
+  }>();
+
+  protected saveParameters = this.formBuilder.group({
+    recursive: [false],
+    traverse: [false],
+    validate_effective_acl: [true],
+  });
+
+  protected readonly helptext = helptextAcl;
+  protected readonly Role = Role;
+
+  constructor(
+    private formBuilder: FormBuilder,
+    private store: DatasetAclEditorStore,
+    private dialogService: DialogService,
+    private ws: WebSocketService,
+  ) {}
+
+  ngOnInit(): void {
+    this.setRecursiveCheckboxWarning();
+  }
+
+  // TODO: Move here and in other places to global store.
+  protected hasValidateAclCheckbox = toSignal(this.ws.call('directoryservices.get_state').pipe(
+    map((state) => {
+      return state.activedirectory !== DirectoryServiceState.Disabled
+      || state.ldap !== DirectoryServiceState.Disabled;
+    }),
+  ));
+
+  protected onSavePressed(): void {
+    const saveParameters = this.saveParameters.value;
+
+    this.store.saveAcl({
+      recursive: saveParameters.recursive,
+      traverse: saveParameters.recursive && saveParameters.traverse,
+      validateEffectiveAcl: saveParameters.validate_effective_acl,
+      owner: this.ownerValues().owner,
+      ownerGroup: this.ownerValues().ownerGroup,
+      applyOwner: this.ownerValues().applyOwner,
+      applyGroup: this.ownerValues().applyGroup,
+    });
+  }
+
+  private setRecursiveCheckboxWarning(): void {
+    this.saveParameters.get('recursive').valueChanges.pipe(
+      filter(Boolean),
+      switchMap(() => {
+        return this.dialogService.confirm({
+          title: helptextAcl.dataset_acl_recursive_dialog_warning,
+          message: helptextAcl.dataset_acl_recursive_dialog_warning_message,
+        });
+      }),
+      untilDestroyed(this),
+    ).subscribe((confirmed) => {
+      if (confirmed) {
+        return;
+      }
+
+      this.saveParameters.patchValue({ recursive: false });
+    });
+  }
+}

src/app/pages/datasets/modules/permissions/containers/dataset-acl-editor/dataset-acl-editor.component.html

@@ -70,35 +70,10 @@
 
           <div class="controls-container">
             <div class="controls">
-              <div class="checkboxes" [formGroup]="saveParameters">
-                <ix-checkbox
-                  formControlName="recursive"
-                  class="recursive-checkbox"
-                  [label]="helptext.dataset_acl_recursive_placeholder | translate"
-                  [tooltip]="helptext.dataset_acl_recursive_tooltip | translate"
-                ></ix-checkbox>
-
-                @if (saveParameters.get('recursive').value) {
-                  <ix-checkbox
-                    formControlName="traverse"
-                    [label]="helptext.dataset_acl_traverse_placeholder | translate"
-                    [tooltip]="helptext.dataset_acl_traverse_tooltip | translate"
-                  ></ix-checkbox>
-                }
-              </div>
-              <div class="control-buttons">
-                <button
-                  *ixRequiresRoles="[Role.FilesystemAttrsWrite, Role.FilesystemFullControl]"
-                  mat-button
-                  color="primary"
-                  class="action"
-                  ixTest="save-acl"
-                  [disabled]="acesWithError.length > 0 || !acl.acl.length || ownerFormGroup.invalid"
-                  (click)="onSavePressed()"
-                >
-                  {{ 'Save Access Control List' | translate }}
-                </button>
-              </div>
+              <ix-acl-editor-save-controls
+                [canBeSaved]="acesWithError.length === 0 && acl.acl.length && ownerFormGroup.valid"
+                [ownerValues]="ownerFormGroup.getRawValue()"
+              ></ix-acl-editor-save-controls>
               <div class="control-buttons">
                 @if (acl.trivial) {
                   <a
@@ -123,7 +98,7 @@
                 }
               </div>
 
-              <h4 class="presets-label">Presets</h4>
+              <h4 class="presets-label">{{ 'Presets' | translate }}</h4>
               <div class="control-buttons">
                 <button
                   mat-button

src/app/pages/datasets/modules/permissions/containers/dataset-acl-editor/dataset-acl-editor.component.scss

@@ -98,10 +98,6 @@
   justify-content: space-between;
   margin-bottom: 10px;
 
-  .checkboxes {
-    padding: 0 5px;
-  }
-
   ::ng-deep > * {
     flex-grow: 1;
     margin-right: 6px;

src/app/pages/datasets/modules/permissions/containers/dataset-acl-editor/dataset-acl-editor.component.spec.ts

@@ -38,6 +38,9 @@ import {
 import {
   StripAclModalComponent,
 } from 'app/pages/datasets/modules/permissions/components/strip-acl-modal/strip-acl-modal.component';
+import {
+  AclEditorSaveControlsComponent,
+} from 'app/pages/datasets/modules/permissions/containers/dataset-acl-editor/acl-editor-save-controls/acl-editor-save-controls.component';
 import {
   DatasetAclEditorComponent,
 } from 'app/pages/datasets/modules/permissions/containers/dataset-acl-editor/dataset-acl-editor.component';
@@ -89,6 +92,7 @@ describe('DatasetAclEditorComponent', () => {
     declarations: [
       MockComponent(EditPosixAceComponent),
       MockComponent(EditNfsAceComponent),
+      MockComponent(AclEditorSaveControlsComponent),
       AclEditorListComponent,
       PermissionsItemComponent,
     ],
@@ -197,29 +201,8 @@ describe('DatasetAclEditorComponent', () => {
   });
 
   describe('saving', () => {
-    let store: DatasetAclEditorStore;
-
-    beforeEach(() => {
-      store = spectator.inject(DatasetAclEditorStore);
-      jest.spyOn(store, 'saveAcl').mockImplementation();
-    });
-
-    afterEach(() => {
-      jest.restoreAllMocks();
-    });
-
-    it('saves acl items when Save Access Control List is pressed', async () => {
-      const saveButton = await loader.getHarness(MatButtonHarness.with({ text: 'Save Access Control List' }));
-      await saveButton.click();
-
-      expect(store.saveAcl).toHaveBeenCalledWith({
-        recursive: false,
-        traverse: false,
-        applyGroup: false,
-        applyOwner: false,
-        owner: 'john',
-        ownerGroup: 'johns',
-      });
+    it('renders save controls', () => {
+      expect(spectator.query(AclEditorSaveControlsComponent)).toExist();
     });
   });
 });