Skip to content

Commit

Permalink
NAS-129686 / 24.10 / Fix SMB ACL form (#10271)
Browse files Browse the repository at this point in the history
  • Loading branch information
bvasilenko authored Jul 5, 2024
1 parent b6c082a commit 324fa0f
Show file tree
Hide file tree
Showing 6 changed files with 184 additions and 31 deletions.
2 changes: 2 additions & 0 deletions src/app/enums/nfs-acl.enum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export enum NfsAclTag {
Everyone = 'everyone@',
User = 'USER',
UserGroup = 'GROUP',
Both = 'BOTH', // middleware returns `ID_TYPE_BOTH` when it is not possible to determine whether an AD entity is a user or a group
}

export const nfsAclTagLabels = new Map<NfsAclTag, string>([
Expand All @@ -19,6 +20,7 @@ export const nfsAclTagLabels = new Map<NfsAclTag, string>([
export const smbAclTagLabels = new Map<NfsAclTag, string>([
[NfsAclTag.User, T('User')],
[NfsAclTag.UserGroup, T('Group')],
[NfsAclTag.Both, T('Unknown')],
[NfsAclTag.Everyone, T('everyone@')],
]);

Expand Down
2 changes: 1 addition & 1 deletion src/app/interfaces/smb-share.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export interface SmbSharesecAce {
ae_perm: SmbSharesecPermission;
ae_type: SmbSharesecType;
ae_who_id: {
id_type: NfsAclTag.Everyone | NfsAclTag.UserGroup | NfsAclTag.User | null;
id_type: NfsAclTag.Everyone | NfsAclTag.UserGroup | NfsAclTag.User | NfsAclTag.Both | null;
id: number;
};
ae_who_sid?: string;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { Observable, forkJoin } from 'rxjs';
import { map } from 'rxjs/operators';
import { Group } from 'app/interfaces/group.interface';
import { Option } from 'app/interfaces/option.interface';
import { User } from 'app/interfaces/user.interface';
import { IxComboboxProvider } from 'app/modules/forms/ix-forms/components/ix-combobox/ix-combobox-provider';
import { UserService } from 'app/services/user.service';

export class SmbBothComboboxProvider implements IxComboboxProvider {
protected page = 1;
readonly pageSize = 50;

excludeInitialOptions(options: Option[]): Option[] {
return options.filter((option) => {
return !this.initialOptions.find((initialOption) => initialOption.value === option.value);
});
}

queryResToOptions(users: User[], groups: Group[]): Option[] {
const userOptions = users
.filter((user) => user.id_type_both)
.map((user) => ({ label: user.username, value: user[this.userOptionsValueField] }));
const groupOptions = groups
.filter((user) => user.id_type_both)
.map((group) => ({ label: group.group, value: group[this.groupOptionsValueField] }));

return [...userOptions, ...groupOptions];
}

fetch(filterValue: string): Observable<Option[]> {
this.page = 0;
const offset = this.page * this.pageSize;

return forkJoin([
this.userService.smbUserQueryDsCache(filterValue, offset),
this.userService.smbGroupQueryDsCache(filterValue, false, offset),
]).pipe(
map(([users, groups]) => this.queryResToOptions(users, groups)),
map((options) => [...this.initialOptions, ...this.excludeInitialOptions(options)]),
);
}

nextPage(filterValue: string): Observable<Option[]> {
this.page++;
const offset = this.page * this.pageSize;
return forkJoin([
this.userService.smbUserQueryDsCache(filterValue, offset),
this.userService.smbGroupQueryDsCache(filterValue, false, offset),
]).pipe(
map(([users, groups]) => this.queryResToOptions(users, groups)),
map((options) => this.excludeInitialOptions(options)),
);
}

constructor(
protected userService: UserService,
private userOptionsValueField: 'username' | 'uid' | 'id' = 'username',
private groupOptionsValueField: 'group' | 'gid' | 'id' = 'group',
protected initialOptions: Option[] = [],
) {}
}
12 changes: 12 additions & 0 deletions src/app/pages/sharing/smb/smb-acl/smb-acl.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
formControlName="user"
[label]="'User' | translate"
[provider]="userProvider"
[allowCustomValue]="true"
[required]="true"
></ix-combobox>

Expand All @@ -48,6 +49,17 @@
formControlName="group"
[label]="'Group' | translate"
[provider]="groupProvider"
[allowCustomValue]="true"
[required]="true"
></ix-combobox>

<!-- TODO: Delete after the backend has support for distinguishing user from group -->
<ix-combobox
*ngIf="bothProvider && entry.controls.ae_who.value === nfsAclTag.Both"
formControlName="both"
[label]="'Unknown' | translate"
[provider]="bothProvider"
[allowCustomValue]="true"
[required]="true"
></ix-combobox>
</div>
Expand Down
82 changes: 68 additions & 14 deletions src/app/pages/sharing/smb/smb-acl/smb-acl.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,26 +109,80 @@ describe('SmbAclComponent', () => {
expect(title).toHaveText('Share ACL for myshare');
});

it('shows user combobox when Who is user', async () => {
await entriesList.pressAddButton();
const newListItem = await entriesList.getLastListItem();
await newListItem.fillForm({
Who: 'User',
describe('user ace', () => {
it('shows user combobox when Who is user', async () => {
await entriesList.pressAddButton();
const newListItem = await entriesList.getLastListItem();
await newListItem.fillForm({
Who: 'User',
});

const userSelect = await loader.getHarness(IxComboboxHarness.with({ label: 'User' }));
expect(userSelect).toExist();

const entries = spectator.component.form.value.entries;
expect(entries[entries.length - 1]).toEqual(
expect.not.objectContaining({ user: 0 }),
);
});

const userSelect = await loader.getHarness(IxComboboxHarness.with({ label: 'User' }));
expect(userSelect).toExist();
it('allows custom values in User combobox', async () => {
const newListItem = await entriesList.getLastListItem();
await newListItem.fillForm({
Who: 'User',
});

const fields = await newListItem.getControlHarnessesDict();

const userCombobox = fields['User'] as IxComboboxHarness;
await userCombobox.writeCustomValue('root');

const userSelect = await loader.getHarness(IxComboboxHarness.with({ label: 'User' }));
expect(userSelect).toExist();

const entries = spectator.component.form.value.entries;
expect(entries[entries.length - 1]).toEqual(
expect.objectContaining({ user: 0 }),
);
});
});

it('shows group combobox when Who is group', async () => {
await entriesList.pressAddButton();
const newListItem = await entriesList.getLastListItem();
await newListItem.fillForm({
Who: 'Group',
describe('group ace', () => {
it('shows group combobox when Who is group', async () => {
await entriesList.pressAddButton();
const newListItem = await entriesList.getLastListItem();
await newListItem.fillForm({
Who: 'Group',
});

const groupSelect = await loader.getHarness(IxComboboxHarness.with({ label: 'Group' }));
expect(groupSelect).toExist();

const entries = spectator.component.form.value.entries;
expect(entries[entries.length - 1]).toEqual(
expect.not.objectContaining({ group: 1 }),
);
});

const groupSelect = await loader.getHarness(IxComboboxHarness.with({ label: 'Group' }));
expect(groupSelect).toExist();
it('allows custom values in Group combobox', async () => {
const newListItem = await entriesList.getLastListItem();
await newListItem.fillForm({
Who: 'Group',
});

const fields = await newListItem.getControlHarnessesDict();

const groupCombobox = fields['Group'] as IxComboboxHarness;
await groupCombobox.writeCustomValue('wheel');

const groupSelect = await loader.getHarness(IxComboboxHarness.with({ label: 'Group' }));
expect(groupSelect).toExist();

const entries = spectator.component.form.value.entries;
expect(entries[entries.length - 1]).toEqual(
expect.objectContaining({ group: 1 }),
);
});
});

it('loads and shows current acl for a share', async () => {
Expand Down
56 changes: 40 additions & 16 deletions src/app/pages/sharing/smb/smb-acl/smb-acl.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ import {
import { FormBuilder } from '@ngneat/reactive-forms';
import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
import { TranslateService } from '@ngx-translate/core';
import _ from 'lodash';
import {
concatMap, forkJoin, from, Observable, of,
concatMap, firstValueFrom, forkJoin, mergeMap, Observable, of, from,
} from 'rxjs';
import { NfsAclTag, smbAclTagLabels } from 'app/enums/nfs-acl.enum';
import { Role } from 'app/enums/role.enum';
Expand All @@ -17,6 +18,7 @@ import { Option } from 'app/interfaces/option.interface';
import { QueryFilter } from 'app/interfaces/query-api.interface';
import { SmbSharesecAce } from 'app/interfaces/smb-share.interface';
import { User } from 'app/interfaces/user.interface';
import { SmbBothComboboxProvider } from 'app/modules/forms/ix-forms/classes/smb-both-combobox-provider';
import { SmbGroupComboboxProvider } from 'app/modules/forms/ix-forms/classes/smb-group-combobox-provider';
import { SmbUserComboboxProvider } from 'app/modules/forms/ix-forms/classes/smb-user-combobox-provider';
import { IxSlideInRef } from 'app/modules/forms/ix-forms/components/ix-slide-in/ix-slide-in-ref';
Expand All @@ -27,11 +29,12 @@ import { WebSocketService } from 'app/services/ws.service';

interface FormAclEntry {
ae_who_sid: string;
ae_who: NfsAclTag.Everyone | NfsAclTag.UserGroup | NfsAclTag.User | null;
ae_who: NfsAclTag.Everyone | NfsAclTag.UserGroup | NfsAclTag.User | NfsAclTag.Both | null;
ae_perm: SmbSharesecPermission;
ae_type: SmbSharesecType;
user: number | null;
group: number | null;
user: string | number | null;
group: string | number | null;
both: string | number | null;
}

@UntilDestroy()
Expand Down Expand Up @@ -80,6 +83,7 @@ export class SmbAclComponent implements OnInit {

readonly helptext = helptextSharingSmb;
readonly nfsAclTag = NfsAclTag;
readonly bothProvider = new SmbBothComboboxProvider(this.userService, 'uid', 'gid');
readonly userProvider = new SmbUserComboboxProvider(this.userService, 'uid');
protected groupProvider: SmbGroupComboboxProvider;

Expand Down Expand Up @@ -109,6 +113,7 @@ export class SmbAclComponent implements OnInit {
this.formBuilder.group({
ae_who_sid: [''],
ae_who: [null as never],
both: [null as never],
user: [null as never],
group: [null as never],
ae_perm: [null as SmbSharesecPermission],
Expand All @@ -123,9 +128,10 @@ export class SmbAclComponent implements OnInit {

onSubmit(): void {
this.isLoading = true;
const acl = this.getAclEntriesFromForm();

this.ws.call('sharing.smb.setacl', [{ share_name: this.shareAclName, share_acl: acl }])
of(undefined)
.pipe(mergeMap(() => this.getAclEntriesFromForm()))
.pipe(mergeMap((acl) => this.ws.call('sharing.smb.setacl', [{ share_name: this.shareAclName, share_acl: acl }])))
.pipe(untilDestroyed(this))
.subscribe({
next: () => {
Expand All @@ -149,11 +155,13 @@ export class SmbAclComponent implements OnInit {
this.shareAclName = shareAcl.share_name;
shareAcl.share_acl.forEach((ace, i) => {
this.addAce();

this.form.controls.entries.at(i).patchValue({
ae_who_sid: ace.ae_who_sid,
ae_who: ace.ae_who_id?.id_type || ace.ae_who_str as NfsAclTag.Everyone,
ae_perm: ace.ae_perm,
ae_type: ace.ae_type,
both: ace.ae_who_id?.id_type !== NfsAclTag.Everyone ? ace.ae_who_id?.id : null,
group: ace.ae_who_id?.id_type !== NfsAclTag.Everyone ? ace.ae_who_id?.id : null,
user: ace.ae_who_id?.id_type !== NfsAclTag.Everyone ? ace.ae_who_id?.id : null,
});
Expand All @@ -167,22 +175,38 @@ export class SmbAclComponent implements OnInit {
});
}

private getAclEntriesFromForm(): SmbSharesecAce[] {
return this.form.value.entries.map((ace) => {
const whoId = ace.ae_who === NfsAclTag.UserGroup ? ace.group : ace.user;
private async getAclEntriesFromForm(): Promise<SmbSharesecAce[]> {
const results = [] as SmbSharesecAce[];
for (const ace of this.form.value.entries) {
let whoIdOrName = ace.both;
if (ace.ae_who === NfsAclTag.User) {
whoIdOrName = ace.user;
} else if (ace.ae_who === NfsAclTag.UserGroup) {
whoIdOrName = ace.group;
}

const result = { ae_perm: ace.ae_perm, ae_type: ace.ae_type } as SmbSharesecAce;

if (ace.ae_who !== this.nfsAclTag.Everyone) {
result.ae_who_id = { id_type: ace.ae_who, id: whoId };
}

if (ace.ae_who === NfsAclTag.Everyone) {
result.ae_who_sid = 'S-1-1-0';
}
} else {
let id: number;
if (_.isNumber(whoIdOrName)) {
id = Number(whoIdOrName);
} else if (ace.ae_who === NfsAclTag.UserGroup) {
id = (await firstValueFrom(this.userService.getGroupByName(whoIdOrName.toString())))
.gr_gid;
} else {
id = (await firstValueFrom(this.userService.getUserByName(whoIdOrName.toString())))
.pw_uid;
}

return result;
});
// TODO: Backend does not yet support BOTH value
result.ae_who_id = { id_type: ace.ae_who === NfsAclTag.Both ? NfsAclTag.UserGroup : ace.ae_who, id };
}
results.push(result);
}
return results;
}

private initialValueDataFromAce(
Expand Down

0 comments on commit 324fa0f

Please sign in to comment.