Merge pull request #9291 from truenas/NAS-125497

NAS-125497 / 24.04 / Fix what happens when trying to use user with reduced privileges

src/app/core/testing/classes/mock-websocket.service.ts

@@ -1,5 +1,6 @@
 import { Injectable } from '@angular/core';
 import { Router } from '@angular/router';
+import { TranslateService } from '@ngx-translate/core';
 import { when } from 'jest-when';
 import { Observable, of, Subject } from 'rxjs';
 import { ValuesType } from 'utility-types';
@@ -40,8 +41,9 @@ export class MockWebsocketService extends WebSocketService {
   constructor(
     protected router: Router,
     protected wsManager: WebsocketConnectionService,
+    protected translate: TranslateService,
   ) {
-    super(router, wsManager);
+    super(router, wsManager, translate);
 
     this.call = jest.fn();
     this.job = jest.fn();

src/app/core/testing/utils/mock-websocket.utils.ts

@@ -2,6 +2,7 @@ import {
   ExistingProvider, FactoryProvider, forwardRef, ValueProvider,
 } from '@angular/core';
 import { Router } from '@angular/router';
+import { TranslateService } from '@ngx-translate/core';
 import { MockWebsocketService } from 'app/core/testing/classes/mock-websocket.service';
 import {
   CallResponseOrFactory, JobResponseOrFactory,
@@ -47,8 +48,8 @@ export function mockWebsocket(
   return [
     {
       provide: WebSocketService,
-      useFactory: (router: Router, wsManager: WebsocketConnectionService) => {
-        const mockWebsocketService = new MockWebsocketService(router, wsManager);
+      useFactory: (router: Router, wsManager: WebsocketConnectionService, translate: TranslateService) => {
+        const mockWebsocketService = new MockWebsocketService(router, wsManager, translate);
         (mockResponses || []).forEach((mockResponse) => {
           if (mockResponse.type === MockWebsocketResponseType.Call) {
             mockWebsocketService.mockCall(mockResponse.method, mockResponse.response);
@@ -61,7 +62,7 @@ export function mockWebsocket(
         });
         return mockWebsocketService;
       },
-      deps: [Router, WebsocketConnectionService],
+      deps: [Router, WebsocketConnectionService, TranslateService],
     },
     {
       provide: MockWebsocketService,

src/app/directives/common/has-roles/has-roles.directive.spec.ts

@@ -1,88 +1,53 @@
 import { createHostFactory, mockProvider, Spectator } from '@ngneat/spectator/jest';
-import { BehaviorSubject } from 'rxjs';
 import { Role } from 'app/enums/role.enum';
-import { LoggedInUser } from 'app/interfaces/ds-cache.interface';
 import { AuthService } from 'app/services/auth/auth.service';
 import { HasRolesDirective } from './has-roles.directive';
 
 describe('HasRolesDirective', () => {
   let spectator: Spectator<HasRolesDirective>;
-  const currentUser$ = new BehaviorSubject<LoggedInUser>(null);
 
   const createHost = createHostFactory({
     component: HasRolesDirective,
     providers: [
-      mockProvider(AuthService, {
-        user$: currentUser$,
-      }),
+      mockProvider(AuthService),
     ],
   });
 
-  it('does not show an element when there is no logged in user', () => {
+  it('does not show an element when user doe not have correct roles', () => {
     spectator = createHost(
       '<div *ixHasRoles="[Role.Readonly]">Content</div>',
       {
         hostProps: { Role },
+        providers: [
+          mockProvider(AuthService, {
+            hasRole: jest.fn(() => false),
+          }),
+        ],
       },
     );
 
-    expect(spectator.query('div')).not.toExist();
-  });
-
-  it('shows an element when user has a FullAdmin role regardless of roles on the element', () => {
-    currentUser$.next({
-      privilege: {
-        roles: {
-          $set: [Role.FullAdmin],
-        },
-      },
-    } as LoggedInUser);
-
-    spectator = createHost(
-      '<div *ixHasRoles="[Role.DatasetWrite]">Content</div>',
-      {
-        hostProps: { Role },
-      },
-    );
+    const authService = spectator.inject(AuthService);
 
-    expect(spectator.query('div')).toHaveText('Content');
+    expect(authService.hasRole).toHaveBeenCalledWith([Role.Readonly]);
+    expect(spectator.query('div')).not.toExist();
   });
 
-  it('shows an element when one of the user`s roles matches one of the roles required on the element', () => {
-    currentUser$.next({
-      privilege: {
-        roles: {
-          $set: [Role.DatasetRead, Role.DatasetWrite],
-        },
-      },
-    } as LoggedInUser);
-
+  it('shows an element when user has correct roles', () => {
     spectator = createHost(
-      '<div *ixHasRoles="[Role.DatasetRead, Role.Readonly]">Content</div>',
+      '<div *ixHasRoles="[Role.NetworkInterfaceWrite]">Content</div>',
       {
         hostProps: { Role },
+        providers: [
+          mockProvider(AuthService, {
+            hasRole: jest.fn(() => true),
+          }),
+        ],
       },
     );
 
-    expect(spectator.query('div')).toHaveText('Content');
-  });
-
-  it('does not show an element when none of the user`s roles matches any of the roles required on the element', () => {
-    currentUser$.next({
-      privilege: {
-        roles: {
-          $set: [Role.DatasetRead, Role.DatasetWrite],
-        },
-      },
-    } as LoggedInUser);
-
-    spectator = createHost(
-      '<div *ixHasRoles="[Role.Readonly]">Content</div>',
-      {
-        hostProps: { Role },
-      },
-    );
+    const authService = spectator.inject(AuthService);
 
-    expect(spectator.query('div')).not.toExist();
+    expect(authService.hasRole).toHaveBeenCalledWith([Role.NetworkInterfaceWrite]);
+    expect(spectator.query('div')).toExist();
   });
 });

src/app/directives/common/has-roles/has-roles.directive.ts

@@ -1,45 +1,24 @@
 import {
-  ChangeDetectorRef,
   Directive, Input, TemplateRef, ViewContainerRef,
 } from '@angular/core';
-import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
+import { UntilDestroy } from '@ngneat/until-destroy';
 import { Role } from 'app/enums/role.enum';
 import { AuthService } from 'app/services/auth/auth.service';
 
 @UntilDestroy()
 @Directive({ selector: '[ixHasRoles]' })
 export class HasRolesDirective {
-  private currentUserRoles: Role[] = [];
-
   @Input() set ixHasRoles(roles: Role[]) {
     this.viewContainer.clear();
 
-    if (this.checkRoles(roles)) {
+    if (this.authService.hasRole(roles)) {
       this.viewContainer.createEmbeddedView(this.templateRef);
     }
   }
 
   constructor(
     private templateRef: TemplateRef<unknown>,
     private viewContainer: ViewContainerRef,
-    private cdr: ChangeDetectorRef,
     private authService: AuthService,
-  ) {
-    this.authService.user$.pipe(untilDestroyed(this)).subscribe((user) => {
-      this.currentUserRoles = user?.privilege?.roles?.$set || [];
-      this.cdr.markForCheck();
-    });
-  }
-
-  private checkRoles(roles: Role[]): boolean {
-    if (!roles?.length || !this.currentUserRoles?.length) {
-      return false;
-    }
-
-    if (this.currentUserRoles.includes(Role.FullAdmin)) {
-      return true;
-    }
-
-    return roles.some((role) => this.currentUserRoles.includes(role));
-  }
+  ) {}
 }

src/app/helptext/account/priviledge.ts

@@ -0,0 +1,5 @@
+import { marker as T } from '@biesbjerg/ngx-translate-extract-marker';
+
+export default {
+  minimalRolesTooltip: T('Only Readonly, Sharing Manager or Full Admin roles are supported in WebUI.'),
+};

src/app/interfaces/ds-cache.interface.ts

@@ -1,5 +1,6 @@
 import { Role } from 'app/enums/role.enum';
 import { Preferences } from 'app/interfaces/preferences.interface';
+import { UserTwoFactorConfig } from 'app/interfaces/two-factor-config.interface';
 import { DashConfigItem } from 'app/pages/dashboard/components/widget-controller/widget-controller.component';
 import { User } from './user.interface';
 
@@ -19,6 +20,7 @@ export interface DsUncachedUser {
 
 export interface AuthMeUser extends DsUncachedUser {
   privilege: AuthMePrivilege;
+  two_factor_config: UserTwoFactorConfig;
 }
 
 export interface AuthMePrivilege {

src/app/interfaces/privilege.interface.ts

@@ -23,4 +23,5 @@ export interface PrivilegeRole {
   name: Role;
   title: string;
   includes: Role[];
+  builtin: boolean;
 }

src/app/pages/account/groups/group-list/group-list.component.html

@@ -4,18 +4,14 @@
       [disabled]="isLoading$ | async"
       (search)="onSearch($event)"
     ></ix-search-input>
-    <button mat-button color="primary" ixTest="add" (click)="doAdd()">
-      {{ 'Add' | translate }}
-    </button>
-    <button
-      mat-button
-      color="accent"
+    <mat-slide-toggle
+      color="primary"
       ixTest="show-built-in-groups"
-      [disabled]="isLoading$ | async"
-      (click)="toggleBuiltins()"
+      [checked]="!hideBuiltinGroups"
+      (change)="toggleBuiltins()"
     >
-      {{ hideBuiltinGroups ? ('Show Built-In Groups' | translate) : ('Hide Built-In Groups' | translate) }}
-    </button>
+      <label>{{ 'Show Built-in Groups' | translate }}</label>
+    </mat-slide-toggle>
     <a
       mat-button
       color="accent"
@@ -24,6 +20,9 @@
     >
       {{ 'Privileges' | translate }}
     </a>
+    <button mat-button color="primary" ixTest="add" (click)="doAdd()">
+      {{ 'Add' | translate }}
+    </button>
   </ix-page-title-header>
 </ng-template>
 

src/app/pages/account/groups/privilege/privilege-form/privilege-form.component.html

@@ -19,7 +19,7 @@
 
         <ix-chips
           formControlName="ds_groups"
-          [label]="'DS Groups' | translate"
+          [label]="'Directory Services Groups' | translate"
           [autocompleteProvider]="dsGroupsProvider"
           [allowNewEntries]="false"
         ></ix-chips>
@@ -30,6 +30,7 @@
           [options]="rolesOptions$"
           [multiple]="true"
           [showSelectAll]="true"
+          [hint]="helptext.minimalRolesTooltip | translate"
         ></ix-select>
 
         <ix-checkbox

src/app/pages/account/groups/privilege/privilege-form/privilege-form.component.spec.ts

@@ -7,6 +7,7 @@ import { mockCall, mockWebsocket } from 'app/core/testing/utils/mock-websocket.u
 import { Role } from 'app/enums/role.enum';
 import { Group } from 'app/interfaces/group.interface';
 import { Privilege, PrivilegeRole } from 'app/interfaces/privilege.interface';
+import { IxSelectHarness } from 'app/modules/ix-forms/components/ix-select/ix-select.harness';
 import { IxSlideInRef } from 'app/modules/ix-forms/components/ix-slide-in/ix-slide-in-ref';
 import { SLIDE_IN_DATA } from 'app/modules/ix-forms/components/ix-slide-in/ix-slide-in.token';
 import { IxFormsModule } from 'app/modules/ix-forms/ix-forms.module';
@@ -43,9 +44,11 @@ describe('PrivilegeFormComponent', () => {
         mockCall('privilege.create'),
         mockCall('privilege.update'),
         mockCall('privilege.roles', [
-          { name: Role.FullAdmin, title: Role.FullAdmin },
-          { name: Role.SharingManager, title: Role.SharingManager },
-          { name: Role.Readonly, title: Role.Readonly },
+          { name: Role.FullAdmin, title: Role.FullAdmin, builtin: false },
+          { name: Role.SharingManager, title: Role.SharingManager, builtin: false },
+          { name: Role.Readonly, title: Role.Readonly, builtin: false },
+          { name: Role.SharingSmbRead, title: Role.SharingSmbRead, builtin: false },
+          { name: Role.SharingSmbWrite, title: Role.SharingSmbWrite, builtin: false },
         ] as PrivilegeRole[]),
       ]),
       mockProvider(IxSlideInRef),
@@ -60,6 +63,18 @@ describe('PrivilegeFormComponent', () => {
       ws = spectator.inject(WebSocketService);
     });
 
+    it('shows roles sorted alphabetically with compound (non-builtin) roles on top', async () => {
+      const roles = await loader.getHarness(IxSelectHarness.with({ label: 'Roles' }));
+      const options = await roles.getOptionLabels();
+      expect(options).toEqual([
+        'Full Admin',
+        'Readonly',
+        'Sharing Manager',
+        'Sharing SMB Read',
+        'Sharing SMB Write',
+      ]);
+    });
+
     it('sends a create payload to websocket and closes modal when save is pressed', async () => {
       const form = await loader.getHarness(IxFormHarness);
       await form.fillForm({
@@ -100,7 +115,7 @@ describe('PrivilegeFormComponent', () => {
         Name: 'privilege',
         'Web Shell Access': true,
         'Local Groups': ['Group A', 'Group B'],
-        'DS Groups': [],
+        'Directory Services Groups': [],
         Roles: ['Readonly'],
       });
     });

src/app/pages/account/groups/privilege/privilege-form/privilege-form.component.ts

@@ -7,6 +7,7 @@ import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
 import { TranslateService } from '@ngx-translate/core';
 import { Observable, map } from 'rxjs';
 import { Role, roleNames } from 'app/enums/role.enum';
+import helptext from 'app/helptext/account/priviledge';
 import { Group } from 'app/interfaces/group.interface';
 import { Privilege, PrivilegeUpdate } from 'app/interfaces/privilege.interface';
 import { ChipsProvider } from 'app/modules/ix-forms/components/ix-chips/chips-provider';
@@ -21,18 +22,20 @@ import { WebSocketService } from 'app/services/ws.service';
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
 export class PrivilegeFormComponent implements OnInit {
-  isLoading = false;
-  localGroups: Group[] = [];
-  dsGroups: Group[] = [];
+  protected isLoading = false;
+  protected localGroups: Group[] = [];
+  protected dsGroups: Group[] = [];
 
-  form = this.formBuilder.group({
+  protected form = this.formBuilder.group({
     name: ['', [Validators.required]],
     local_groups: [[] as string[]],
     ds_groups: [[] as string[]],
     web_shell: [false],
     roles: [[] as Role[]],
   });
 
+  protected readonly helptext = helptext;
+
   get isNew(): boolean {
     return !this.existingPrivilege;
   }
@@ -45,12 +48,19 @@ export class PrivilegeFormComponent implements OnInit {
 
   readonly rolesOptions$ = this.ws.call('privilege.roles').pipe(
     map((roles) => {
-      const options = roles.map((role) => ({
+      const sortedRoles = roles.sort((a, b) => {
+        // Show compound roles first, then sort by name.
+        if (a.builtin === b.builtin) {
+          return a.name.localeCompare(b.name);
+        }
+
+        return a.builtin ? 1 : -1;
+      });
+
+      return sortedRoles.map((role) => ({
         label: roleNames.has(role.name) ? this.translate.instant(roleNames.get(role.name)) : role.name,
         value: role.name,
       }));
-
-      return options.sort((a, b) => a.label.localeCompare(b.label));
     }),
   );