From cae369bd2d5ad30d7c963e7f40b03c97280252e0 Mon Sep 17 00:00:00 2001 From: Evgeny Stepanovych Date: Tue, 12 Nov 2024 15:15:41 +0100 Subject: [PATCH] NAS-132102 / 25.04 / Use usernames and groups in permissions (#11011) --- src/app/interfaces/acl.interface.ts | 2 + .../interfaces/filesystem-stat.interface.ts | 2 + ...dataset-trivial-permissions.component.html | 8 +- ...aset-trivial-permissions.component.spec.ts | 21 ++-- .../dataset-trivial-permissions.component.ts | 44 +++---- .../stores/dataset-acl-editor.store.ts | 119 +++--------------- 6 files changed, 61 insertions(+), 135 deletions(-) diff --git a/src/app/interfaces/acl.interface.ts b/src/app/interfaces/acl.interface.ts index 4c9d8cb87ae..e769a64f626 100644 --- a/src/app/interfaces/acl.interface.ts +++ b/src/app/interfaces/acl.interface.ts @@ -119,7 +119,9 @@ export interface SetAclOptions { export interface SetAcl { path: string; uid?: number; + user?: string; gid?: number; + group?: string; dacl: NfsAclItem[] | PosixAclItem[]; nfs41_flags?: Nfs41Flags; acltype?: AclType; diff --git a/src/app/interfaces/filesystem-stat.interface.ts b/src/app/interfaces/filesystem-stat.interface.ts index 5978fb3c64c..c74382cdb66 100644 --- a/src/app/interfaces/filesystem-stat.interface.ts +++ b/src/app/interfaces/filesystem-stat.interface.ts @@ -44,7 +44,9 @@ export interface FilesystemSetPermParams { path: string; mode: string; uid?: number; + user?: string; gid?: number; + group?: string; options: { stripacl?: boolean; recursive?: boolean; diff --git a/src/app/pages/datasets/modules/permissions/containers/dataset-trivial-permissions/dataset-trivial-permissions.component.html b/src/app/pages/datasets/modules/permissions/containers/dataset-trivial-permissions/dataset-trivial-permissions.component.html index cc22a0f7e10..c182e86d1d9 100644 --- a/src/app/pages/datasets/modules/permissions/containers/dataset-trivial-permissions/dataset-trivial-permissions.component.html +++ b/src/app/pages/datasets/modules/permissions/containers/dataset-trivial-permissions/dataset-trivial-permissions.component.html @@ -1,4 +1,6 @@ + + {{ 'Unix Permissions Editor' | translate }} @@ -15,7 +17,7 @@
{{ 'Save' | translate }} diff --git a/src/app/pages/datasets/modules/permissions/containers/dataset-trivial-permissions/dataset-trivial-permissions.component.spec.ts b/src/app/pages/datasets/modules/permissions/containers/dataset-trivial-permissions/dataset-trivial-permissions.component.spec.ts index 60fe398ac1b..6a6c30b467a 100644 --- a/src/app/pages/datasets/modules/permissions/containers/dataset-trivial-permissions/dataset-trivial-permissions.component.spec.ts +++ b/src/app/pages/datasets/modules/permissions/containers/dataset-trivial-permissions/dataset-trivial-permissions.component.spec.ts @@ -46,18 +46,18 @@ describe('DatasetTrivialPermissionsComponent', () => { mockProvider(StorageService, { filesystemStat: jest.fn(() => of({ mode: 16877, - uid: 0, - gid: 1001, + user: 'root', + group: 'kmem', })), }), mockProvider(UserService, { groupQueryDsCache: () => of([ - { group: 'kmem', gid: 1001 }, - { group: 'wheel', gid: 1002 }, + { group: 'kmem' }, + { group: 'wheel' }, ]), userQueryDsCache: () => of([ - { username: 'root', uid: 0 }, - { username: 'games', uid: 103 }, + { username: 'root' }, + { username: 'games' }, ]), }), mockProvider(DialogService, { @@ -100,7 +100,7 @@ describe('DatasetTrivialPermissionsComponent', () => { it('saves new user and group when form is saved', async () => { await form.fillForm({ User: 'games', - Group: 'kmem', + Group: 'wheel', 'Apply User': true, 'Apply Group': true, }); @@ -109,8 +109,9 @@ describe('DatasetTrivialPermissionsComponent', () => { expect(websocket.job).toHaveBeenCalledWith('filesystem.setperm', [{ path: '/mnt/pool/trivial', - uid: 103, - gid: 1001, + mode: '755', + user: 'games', + group: 'wheel', options: { recursive: false, stripacl: false, @@ -131,7 +132,7 @@ describe('DatasetTrivialPermissionsComponent', () => { mode: '777', options: { recursive: false, - stripacl: true, + stripacl: false, traverse: false, }, }]); diff --git a/src/app/pages/datasets/modules/permissions/containers/dataset-trivial-permissions/dataset-trivial-permissions.component.ts b/src/app/pages/datasets/modules/permissions/containers/dataset-trivial-permissions/dataset-trivial-permissions.component.ts index 0cebc009b45..046ff839438 100644 --- a/src/app/pages/datasets/modules/permissions/containers/dataset-trivial-permissions/dataset-trivial-permissions.component.ts +++ b/src/app/pages/datasets/modules/permissions/containers/dataset-trivial-permissions/dataset-trivial-permissions.component.ts @@ -1,5 +1,7 @@ import { AsyncPipe } from '@angular/common'; -import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core'; +import { + ChangeDetectionStrategy, Component, OnInit, signal, +} from '@angular/core'; import { Validators, ReactiveFormsModule } from '@angular/forms'; import { MatButton } from '@angular/material/button'; import { @@ -27,6 +29,7 @@ import { IxComboboxComponent } from 'app/modules/forms/ix-forms/components/ix-co import { IxFieldsetComponent } from 'app/modules/forms/ix-forms/components/ix-fieldset/ix-fieldset.component'; import { IxPermissionsComponent } from 'app/modules/forms/ix-forms/components/ix-permissions/ix-permissions.component'; import { IxValidatorsService } from 'app/modules/forms/ix-forms/services/ix-validators.service'; +import { FakeProgressBarComponent } from 'app/modules/loader/components/fake-progress-bar/fake-progress-bar.component'; import { SnackbarService } from 'app/modules/snackbar/services/snackbar.service'; import { TestDirective } from 'app/modules/test-id/test.directive'; import { ErrorHandlerService } from 'app/services/error-handler.service'; @@ -58,18 +61,19 @@ import { WebSocketService } from 'app/services/ws.service'; RouterLink, TranslateModule, AsyncPipe, + FakeProgressBarComponent, ], }) export class DatasetTrivialPermissionsComponent implements OnInit { protected readonly requiredRoles = [Role.DatasetWrite]; form = this.formBuilder.group({ - uid: [null as number, [this.validatorService.validateOnCondition( + owner: [null as string, [this.validatorService.validateOnCondition( () => this.isToApplyUser, Validators.required, )]], applyUser: [false], - gid: [null as number, [this.validatorService.validateOnCondition( + ownerGroup: [null as string, [this.validatorService.validateOnCondition( () => this.isToApplyGroup, Validators.required, )]], @@ -80,13 +84,14 @@ export class DatasetTrivialPermissionsComponent implements OnInit { traverse: [false], }); - isLoading = false; + protected readonly isLoading = signal(false); + aclType: AclType; datasetPath: string; datasetId: string; - readonly userProvider = new UserComboboxProvider(this.userService, 'uid'); - readonly groupProvider = new GroupComboboxProvider(this.userService, 'gid'); + readonly userProvider = new UserComboboxProvider(this.userService); + readonly groupProvider = new GroupComboboxProvider(this.userService); readonly tooltips = { user: helptextPermissions.dataset_permissions_user_tooltip, @@ -165,7 +170,7 @@ export class DatasetTrivialPermissionsComponent implements OnInit { } private loadPermissionsInformation(): void { - this.isLoading = true; + this.isLoading.set(true); forkJoin([ this.ws.call('pool.dataset.query', [[['id', '=', this.datasetId]]]), this.storageService.filesystemStat(this.datasetPath), @@ -173,18 +178,18 @@ export class DatasetTrivialPermissionsComponent implements OnInit { .pipe(untilDestroyed(this)) .subscribe({ next: ([datasets, stat]) => { - this.isLoading = false; + this.isLoading.set(false); // TODO: DatasetAclType and AclType may represent the same thing this.aclType = datasets[0].acltype.value as unknown as AclType; - this.oldDatasetMode = stat.mode.toString(8).substring(2, 5); + const mode = stat.mode.toString(8).substring(2, 5); this.form.patchValue({ - mode: this.oldDatasetMode, - uid: stat.uid, - gid: stat.gid, + mode, + owner: stat.user, + ownerGroup: stat.group, }); }, error: (error: unknown) => { - this.isLoading = false; + this.isLoading.set(false); this.dialog.error(this.errorHandler.parseError(error)); }, }); @@ -195,23 +200,20 @@ export class DatasetTrivialPermissionsComponent implements OnInit { const update = { path: this.datasetPath, + mode: values.mode, options: { - stripacl: false, + stripacl: values.recursive, recursive: values.recursive, traverse: values.traverse, }, } as FilesystemSetPermParams; + if (values.applyUser) { - update.uid = values.uid; + update.user = values.owner; } if (values.applyGroup) { - update.gid = values.gid; - } - - if (this.oldDatasetMode !== values.mode) { - update.mode = values.mode; - update.options.stripacl = true; + update.group = values.ownerGroup; } return update; diff --git a/src/app/pages/datasets/modules/permissions/stores/dataset-acl-editor.store.ts b/src/app/pages/datasets/modules/permissions/stores/dataset-acl-editor.store.ts index f7138729b7e..c55ae21f8de 100644 --- a/src/app/pages/datasets/modules/permissions/stores/dataset-acl-editor.store.ts +++ b/src/app/pages/datasets/modules/permissions/stores/dataset-acl-editor.store.ts @@ -11,8 +11,6 @@ import { } from 'rxjs/operators'; import { AclType, DefaultAclType } from 'app/enums/acl-type.enum'; import { mntPath } from 'app/enums/mnt-path.enum'; -import { NfsAclTag } from 'app/enums/nfs-acl.enum'; -import { PosixAclTag } from 'app/enums/posix-acl.enum'; import { helptextAcl } from 'app/helptext/storage/volumes/datasets/dataset-acl'; import { Acl, AclTemplateByPath, NfsAclItem, PosixAclItem, SetAcl, @@ -188,7 +186,7 @@ export class DatasetAclEditorStore extends ComponentStore // Prepare request withLatestFrom(saveParams$), - switchMap(([, saveParams]) => this.prepareSetAcl(this.get(), saveParams)), + map(([, saveParams]) => this.prepareSetAcl(this.get(), saveParams)), // Save switchMap((setAcl) => this.makeSaveRequest(setAcl)), @@ -264,108 +262,27 @@ export class DatasetAclEditorStore extends ComponentStore /** * Validates and converts user and group names to ids * and prepares an SetACl object. - * TODO: Validation does not belong here and should be handled by form control. - * TODO: Converting should not be necessary, id should be coming from form control. */ - private prepareSetAcl(editorState: DatasetAclEditorState, options: AclSaveFormParams): Observable { - const markAceAsHavingErrors = (aceIndex: number): void => { - this.patchState((state) => ({ - ...state, - acesWithError: union(state.acesWithError, [aceIndex]), - })); - }; - - // Load ids for all user and group who's - const userWhoToIds = new Map(); - const groupWhoToIds = new Map(); - const requests: Observable[] = []; - - (editorState.acl.acl as (NfsAclItem | PosixAclItem)[]).forEach((ace, index) => { - if ([NfsAclTag.User, PosixAclTag.User].includes(ace.tag)) { - requests.push( - this.userService.getUserByName(ace.who).pipe( - tap((user) => userWhoToIds.set(ace.who, user.pw_uid)), - catchError((error: unknown) => { - this.dialogService.error(this.errorHandler.parseError(error)); - markAceAsHavingErrors(index); - return EMPTY; - }), - ), - ); - - return; + private prepareSetAcl(editorState: DatasetAclEditorState, options: AclSaveFormParams): SetAcl { + const dacl = editorState.acl.acl.map((ace) => { + if (ace.who === null && ace.id) { + return ace; } - if ([NfsAclTag.UserGroup, PosixAclTag.Group].includes(ace.tag)) { - requests.push( - this.userService.getGroupByName(ace.who).pipe( - tap((group) => groupWhoToIds.set(ace.who, group.gr_gid)), - catchError((error: unknown) => { - this.dialogService.error(this.errorHandler.parseError(error)); - markAceAsHavingErrors(index); - return EMPTY; - }), - ), - ); - } + return omit(ace, 'id'); }); - requests.push( - this.userService.getUserByName(options.owner).pipe( - tap((user) => userWhoToIds.set(options.owner, user.pw_uid)), - catchError((error: unknown) => { - this.dialogService.error(this.errorHandler.parseError(error)); - return EMPTY; - }), - ), - ); - - requests.push( - this.userService.getGroupByName(options.ownerGroup).pipe( - tap((group) => groupWhoToIds.set(options.ownerGroup, group.gr_gid)), - catchError((error: unknown) => { - this.dialogService.error(this.errorHandler.parseError(error)); - return EMPTY; - }), - ), - ); - - return forkJoin(requests).pipe( - withLatestFrom(this.state$), - filter(([, currentState]) => currentState.acesWithError.length === 0), - map(([, currentState]) => { - const convertedAces = (currentState.acl.acl as (NfsAclItem | PosixAclItem)[]).map((ace) => { - const aceAttributes = omit(ace, ['who']); - if ([NfsAclTag.User, PosixAclTag.User].includes(ace.tag)) { - const id = userWhoToIds.has(ace.who) ? userWhoToIds.get(ace.who) : -1; - return { ...aceAttributes, id }; - } - if ([NfsAclTag.UserGroup, PosixAclTag.Group].includes(ace.tag)) { - const id = groupWhoToIds.has(ace.who) ? groupWhoToIds.get(ace.who) : -1; - return { ...aceAttributes, id }; - } - - return { - ...aceAttributes, - id: -1, // -1 is effectively null for middleware - }; - }); - - return { - options: { - recursive: options.recursive, - traverse: options.traverse, - validate_effective_acl: options.validateEffectiveAcl, - }, - uid: userWhoToIds.has(options.owner) && options.applyOwner ? userWhoToIds.get(options.owner) : null, - gid: groupWhoToIds.has(options.ownerGroup) && options.applyGroup - ? groupWhoToIds.get(options.ownerGroup) - : null, - acltype: editorState.acl.acltype, - path: editorState.mountpoint, - dacl: convertedAces as NfsAclItem[] | PosixAclItem[], - } as SetAcl; - }), - ); + return { + dacl, + options: { + recursive: options.recursive, + traverse: options.traverse, + validate_effective_acl: options.validateEffectiveAcl, + }, + user: options.owner, + group: options.ownerGroup, + acltype: editorState.acl.acltype, + path: editorState.mountpoint, + } as SetAcl; } }