Skip to content

Commit

Permalink
NAS-130474: UI should expose option to skip ACL traverse validation i…
Browse files Browse the repository at this point in the history
…n the ACL form (#10535)

(cherry picked from commit 46b5bc5)

Co-authored-by: Evgeny Stepanovych <[email protected]>
  • Loading branch information
bugclerk and undsoft authored Aug 23, 2024
1 parent a9faab2 commit 4f171d4
Show file tree
Hide file tree
Showing 101 changed files with 449 additions and 91 deletions.
5 changes: 5 additions & 0 deletions src/app/helptext/storage/volumes/datasets/dataset-acl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.'),
Expand Down
1 change: 1 addition & 0 deletions src/app/interfaces/acl.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export interface SetAclOptions {
recursive?: boolean;
traverse?: boolean;
canonicalize?: boolean;
validate_effective_acl?: boolean;
}

export interface SetAcl {
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.checkboxes {
padding: 0 5px;
}

.save-button {
margin-bottom: 10px;
width: 100%;
}
Original file line number Diff line number Diff line change
@@ -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,
});
});
});
Original file line number Diff line number Diff line change
@@ -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 });
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@
justify-content: space-between;
margin-bottom: 10px;

.checkboxes {
padding: 0 5px;
}

::ng-deep > * {
flex-grow: 1;
margin-right: 6px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -89,6 +92,7 @@ describe('DatasetAclEditorComponent', () => {
declarations: [
MockComponent(EditPosixAceComponent),
MockComponent(EditNfsAceComponent),
MockComponent(AclEditorSaveControlsComponent),
AclEditorListComponent,
PermissionsItemComponent,
],
Expand Down Expand Up @@ -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();
});
});
});
Loading

0 comments on commit 4f171d4

Please sign in to comment.