fix(rbac): withhold cross-domain links a reader cannot follow

e2e/rbac-dead-end-controls.spec.ts

@@ -43,7 +43,14 @@
 import { test, expect } from './fixtures';
 import { login, loginAsSeededUser } from './utils/fineract-login';
 import { selectTab } from './utils/ionic-locators';
-import { createApiContext, seedActiveLoan, seedRestrictedUser, statusAs } from './utils/seed-api';
+import {
+  createApiContext,
+  seedActiveLoan,
+  seedClient,
+  seedGroup,
+  seedRestrictedUser,
+  statusAs,
+} from './utils/seed-api';
 
 // Seeds a loan and two users, then signs in as each.
 test.describe.configure({ mode: 'serial', timeout: 180_000 });
@@ -141,3 +148,47 @@ test.describe('the office transactions list', () => {
     await expect(page.locator('table[cdk-table]')).toHaveCount(0);
   });
 });
+
+test.describe("a group's member list", () => {
+  test('names the member without linking when the reader cannot open clients', async ({ page }) => {
+    const api = await createApiContext();
+    const member = await seedClient(api, 'E2EGroupDeadEnd');
+    const group = await seedGroup(api, 'E2EGroupDeadEnd', [member.clientId]);
+    const reader = await seedRestrictedUser(api, ['READ_GROUP']);
+    await api.dispose();
+
+    expect(await statusAs(reader, 'GET', `/clients/${member.clientId}`)).toBe(403);
+
+    await loginAsSeededUser(page, reader);
+    await page.goto(`/groups/view/${group.groupId}`);
+    await expect(page.getByText(group.groupName).first()).toBeVisible({ timeout: 30_000 });
+    await page.getByTestId('group-tab-members').click();
+
+    // The membership is what this screen is for, so the member is still named...
+    await expect(page.getByText(member.displayName).first()).toBeVisible({ timeout: 20_000 });
+    // ...but not offered as a door that only opens onto Access Denied.
+    await expect(page.locator(`[href*="/clients/view/${member.clientId}"]`)).toHaveCount(0);
+  });
+
+  test('keeps the link for a reader who holds READ_CLIENT', async ({ page }) => {
+    const api = await createApiContext();
+    const member = await seedClient(api, 'E2EGroupDeadEndOk');
+    const group = await seedGroup(api, 'E2EGroupDeadEndOk', [member.clientId]);
+    const reader = await seedRestrictedUser(api, ['READ_GROUP', 'READ_CLIENT']);
+    await api.dispose();
+
+    expect(await statusAs(reader, 'GET', `/clients/${member.clientId}`)).toBe(200);
+
+    await loginAsSeededUser(page, reader);
+    await page.goto(`/groups/view/${group.groupId}`);
+    await expect(page.getByText(group.groupName).first()).toBeVisible({ timeout: 30_000 });
+    await page.getByTestId('group-tab-members').click();
+
+    const link = page.locator(`[href*="/clients/view/${member.clientId}"]`).first();
+    await expect(link).toBeVisible({ timeout: 20_000 });
+    await link.click();
+    await expect(page).toHaveURL(new RegExp(`/clients/view/${member.clientId}$`), {
+      timeout: 30_000,
+    });
+  });
+});

e2e/utils/seed-api.ts

@@ -862,6 +862,7 @@ export interface SeededGroup {
 export async function seedGroup(
   api: APIRequestContext,
   namePrefix = 'E2EGroup',
+  clientIds: number[] = [],
 ): Promise<SeededGroup> {
   const groupName = `${namePrefix} ${seedSuffix()}`;
   const { resourceId } = await post<{ resourceId: number }>(api, '/groups', {
@@ -870,6 +871,10 @@ export async function seedGroup(
     active: false,
     locale: LOCALE,
     dateFormat: DATE_FORMAT,
+    // `clientMembers` at creation rather than a follow-up association command: the group screen
+    // reads `clientMembers` from the `associations=all` fetch, and this is the shorter path to a
+    // group that has one.
+    ...(clientIds.length ? { clientMembers: clientIds } : {}),
   });
   return { groupId: resourceId, groupName };
 }

src/app/features/centers/center-view.component.ts

@@ -19,6 +19,8 @@
 
 import { HttpClient } from '@angular/common/http';
 import { Component, OnInit, computed, inject, signal } from '@angular/core';
+
+import { createPermissionCheck } from '../../shared/utils/permission-check';
 import { ActivatedRoute, Router, RouterModule } from '@angular/router';
 import { Observable } from 'rxjs';
 import {
@@ -320,9 +322,13 @@ export type CenterTab = (typeof CENTER_TAB)[keyof typeof CENTER_TAB];
                   [localLogic]="true"
                 >
                   <ng-template appCellTemplate="name" let-row>
-                    <a [routerLink]="['/groups/view', row.id]" data-testid="center-group-link">
+                    @if (canViewGroup()) {
+                      <a [routerLink]="['/groups/view', row.id]" data-testid="center-group-link">
+                        {{ row.name }}
+                      </a>
+                    } @else {
                       {{ row.name }}
-                    </a>
+                    }
                   </ng-template>
                   <ng-template appCellTemplate="status" let-row>
                     <app-status-badge [status]="row.status?.value ?? ''"></app-status-badge>
@@ -443,6 +449,12 @@ export class CenterViewComponent implements OnInit {
     { key: 'status', label: 'COMMON.STATUS', sortable: false },
   ];
 
+  /**
+   * A center's groups come back with READ_CENTER alone, but the group screen is gated on
+   * READ_GROUP. Same shape as the client link on the group screen.
+   */
+  protected readonly canViewGroup = createPermissionCheck('READ_GROUP');
+
   readonly groupMembers = computed<CenterGroupMember[]>(() => this.center()?.groupMembers ?? []);
   readonly activationDate = computed(() =>
     formatArrayDate(this.center()?.timeline?.activatedOnDate),

src/app/features/clients/client-view.component.ts

@@ -33,8 +33,7 @@ import {
 } from '../../api';
 import { StatusBadgeComponent, LoadErrorComponent } from '../../shared';
 import { RequiresPermissionDirective } from '../../shared/directives/requires-permission.directive';
-import { AuthService } from '../../core/services/auth.service';
-import { ConfigService } from '../../core/services/config.service';
+import { createPermissionCheck } from '../../shared/utils/permission-check';
 import { skipErrorToast } from '../../core/http/http-context';
 import { resolveAccountActionType } from '../../core/utils/account-type-resolver';
 import { ClientActionDialogComponent } from './client-action-dialog.component';
@@ -1139,34 +1138,19 @@ export class ClientViewComponent implements OnInit {
   private readonly dialogService = inject(DialogService);
   private readonly shareAccountService = inject(ShareAccountService);
   private readonly i18n = inject(I18N);
-  private readonly auth = inject(AuthService);
-  private readonly config = inject(ConfigService);
 
   /**
    * Whether the account screens this client's tables link to can actually be opened.
    *
    * A client's accounts come back with READ_CLIENT alone, but each account screen is gated on its
-   * own code. Without this, a user holding only READ_CLIENT was shown the account numbers as
-   * links whose single destination was `/forbidden` — confirmed against a real Fineract instance,
-   * where the same click succeeds for a user who also holds READ_LOAN. The number is still
-   * rendered; only the link is withheld, because the account's existence is not the secret.
-   *
-   * Mirrors {@link HasPermissionDirective}, short-circuit included, rather than using it: the
-   * directive has no else-branch, and the plain-text fallback is the point. `currentUser()` is
-   * read so the computed re-evaluates on a session change — `hasPermission` reads it internally,
-   * but not through a signal this would otherwise track.
+   * own code, so a reader was shown account numbers as links whose only destination was
+   * `/forbidden`. The number still renders; only the link is withheld.
    */
-  private canRead(code: string): boolean {
-    if (!this.config.rbacEnabled()) return true;
-    this.auth.currentUser();
-    return this.auth.hasPermission(code);
-  }
-
-  protected readonly canViewLoan = computed(() => this.canRead('READ_LOAN'));
-  protected readonly canViewSavings = computed(() => this.canRead('READ_SAVINGSACCOUNT'));
-  protected readonly canViewFixedDeposit = computed(() => this.canRead('READ_FIXEDDEPOSITACCOUNT'));
-  protected readonly canViewRecurringDeposit = computed(() =>
-    this.canRead('READ_RECURRINGDEPOSITACCOUNT'),
+  protected readonly canViewLoan = createPermissionCheck('READ_LOAN');
+  protected readonly canViewSavings = createPermissionCheck('READ_SAVINGSACCOUNT');
+  protected readonly canViewFixedDeposit = createPermissionCheck('READ_FIXEDDEPOSITACCOUNT');
+  protected readonly canViewRecurringDeposit = createPermissionCheck(
+    'READ_RECURRINGDEPOSITACCOUNT',
   );
 
   readonly clientId = signal(0);

src/app/features/fintech/asset-owner-view/asset-owner-view.component.test.ts

@@ -25,10 +25,14 @@ import { provideRouter } from '@angular/router';
 import { provideTranslateTesting } from '../../../testing/i18n-testing';
 import { expectLookedUp } from '../../../testing/translated-text';
 import { AssetOwnerViewComponent } from './asset-owner-view.component';
+import { AuthService } from '../../../core/services/auth.service';
+import { provideTestConfig } from '../../../testing/config';
+import { createSpyObj, SpyObj } from '../../../testing/mocks';
 
 describe('AssetOwnerViewComponent', () => {
   let fixture: ComponentFixture<AssetOwnerViewComponent>;
   let http: HttpTestingController;
+  let authServiceSpy: SpyObj<AuthService>;
 
   /** Answers every outstanding request whose URL ends with `suffix`, however many views asked. */
   function flushAll(suffix: string, body: object): void {
@@ -37,7 +41,13 @@ describe('AssetOwnerViewComponent', () => {
     }
   }
 
-  beforeEach(async () => {
+  /** Renders the screen for a user who does, or does not, hold READ_LOAN. */
+  async function render(canReadLoan: boolean): Promise<void> {
+    authServiceSpy = Object.assign(createSpyObj<AuthService>(['hasPermission']), {
+      currentUser: () => ({ permissions: canReadLoan ? ['READ_LOAN'] : [] }),
+    });
+    authServiceSpy.hasPermission.mockReturnValue(canReadLoan);
+
     await TestBed.configureTestingModule({
       imports: [AssetOwnerViewComponent],
       providers: [
@@ -46,6 +56,8 @@ describe('AssetOwnerViewComponent', () => {
         provideHttpClient(),
         provideHttpClientTesting(),
         provideRouter([]),
+        { provide: AuthService, useValue: authServiceSpy },
+        provideTestConfig({ rbacEnabled: true }),
       ],
     }).compileComponents();
 
@@ -61,9 +73,23 @@ describe('AssetOwnerViewComponent', () => {
     fixture.detectChanges();
     flushAll('/transfers/7/journal-entries', { journalEntryData: { content: [] } });
     fixture.detectChanges();
-  });
+  }
+
+  it('renders the loan link and the journal entries tab through the translation adapter', async () => {
+    await render(true);
 
-  it('renders the loan link and the journal entries tab through the translation adapter', () => {
     expectLookedUp(fixture.nativeElement, ['ASSET_OWNERS.VIEW_LOAN_ACCOUNT', 'nav.journalEntries']);
+    expect(
+      fixture.nativeElement.querySelector('[data-testid="asset-owner-view-loan"]'),
+    ).not.toBeNull();
+  });
+
+  it('withholds the loan link from a user without READ_LOAN', async () => {
+    await render(false);
+
+    // The loan screen is gated on READ_LOAN and this one is not, so the button could only have
+    // led to Access Denied. The rest of the screen is unaffected.
+    expect(fixture.nativeElement.querySelector('[data-testid="asset-owner-view-loan"]')).toBeNull();
+    expectLookedUp(fixture.nativeElement, ['nav.journalEntries']);
   });
 });

src/app/features/fintech/asset-owner-view/asset-owner-view.component.ts

@@ -18,6 +18,8 @@
  */
 
 import { Component, OnInit, inject, signal } from '@angular/core';
+
+import { HasPermissionDirective } from '../../../shared/directives/has-permission.directive';
 import { CommonModule } from '@angular/common';
 import { ActivatedRoute, RouterModule } from '@angular/router';
 import { TranslatePipe } from '@ngx-translate/core';
@@ -65,6 +67,7 @@ export type AssetOwnerTab = (typeof ASSET_OWNER_TAB)[keyof typeof ASSET_OWNER_TA
   imports: [
     CommonModule,
     RouterModule,
+    HasPermissionDirective,
     TranslatePipe,
     CdkTableModule,
     DataTableComponent,
@@ -89,9 +92,16 @@ export type AssetOwnerTab = (typeof ASSET_OWNER_TAB)[keyof typeof ASSET_OWNER_TA
               <app-status-badge [status]="transfer.status"></app-status-badge>
             </ion-card-title>
             <div class="header-actions">
+              <!--
+                Removed rather than disabled: it navigates elsewhere, and the loan screen is
+                gated on READ_LOAN while this one is not, so a reader without it was offered a
+                button whose only destination was Access Denied.
+              -->
               <ion-button
+                *appHasPermission="'READ_LOAN'"
                 fill="outline"
                 color="primary"
+                data-testid="asset-owner-view-loan"
                 [routerLink]="['/loans/view', transfer.loan?.loanId]"
               >
                 <ion-icon name="business-outline"></ion-icon>

src/app/features/groups/group-view.component.ts

@@ -18,6 +18,8 @@
  */
 
 import { Component, OnInit, computed, inject, signal } from '@angular/core';
+
+import { createPermissionCheck } from '../../shared/utils/permission-check';
 import { HttpClient } from '@angular/common/http';
 import { ActivatedRoute, Router, RouterModule } from '@angular/router';
 import { Observable } from 'rxjs';
@@ -352,7 +354,11 @@ export type GroupTab = (typeof GROUP_TAB)[keyof typeof GROUP_TAB];
                 [localLogic]="true"
               >
                 <ng-template appCellTemplate="displayName" let-row>
-                  <a [routerLink]="['/clients/view', row.id]">{{ row.displayName }}</a>
+                  @if (canViewClient()) {
+                    <a [routerLink]="['/clients/view', row.id]">{{ row.displayName }}</a>
+                  } @else {
+                    {{ row.displayName }}
+                  }
                 </ng-template>
                 <ng-template appCellTemplate="status" let-row>
                   <app-status-badge [status]="row.status"></app-status-badge>
@@ -533,6 +539,13 @@ export class GroupViewComponent implements OnInit {
    * officer would use to chase them, and makes the member count disagree with what the platform
    * will let you disassociate.
    */
+  /**
+   * A group's members come back with READ_GROUP alone, but the client screen is gated on
+   * READ_CLIENT, so a reader without it was offered a name whose only destination was
+   * `/forbidden`. The member is still named — the membership is what this tab is for.
+   */
+  protected readonly canViewClient = createPermissionCheck('READ_CLIENT');
+
   readonly members = computed<GroupClientMember[]>(() => this.group()?.clientMembers ?? []);
   readonly roles = computed<GroupRoleAssignment[]>(() => this.group()?.groupRoles ?? []);