Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions e2e/components/list.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,15 @@ export class ListTableComponent {
}

async getTableData(): Promise<TableData[]> {
// Read headers from app-table-cell inside each header cell to avoid sort icon text
// Read headers from app-table-cell inside each header cell to avoid sort icon text.
// Signal-list headers put the sort icon (a material-icons ligature, e.g. "sort")
// inside the th text, so strip icon elements before reading.
const headerCells = await this.table.locator('.app-table__header-cell app-table-cell, th').all();
const headers = await Promise.all(headerCells.map(h => h.textContent().then(t => (t || '').trim())));
const headers = await Promise.all(headerCells.map(h => h.evaluate(el => {
const clone = el.cloneNode(true) as HTMLElement;
clone.querySelectorAll('.material-icons, mat-icon').forEach(icon => icon.remove());
return (clone.textContent || '').trim();
})));

const rows = await this.getRows().all();

Expand Down Expand Up @@ -123,12 +129,15 @@ export class ListTableComponent {

async openRowActionMenuByRow(row: Locator): Promise<MenuComponent> {
// Target the overflow menu button (more_vert btn-icon), not inline action buttons
const actionButton = row.locator('app-table-cell-actions button.btn-icon, button[aria-label="Actions"]').first();
const actionButton = row.locator(
'app-table-cell-actions button.btn-icon, button[aria-label="Actions"], button[data-test="row-actions"]',
).first();
await actionButton.click();

// Wait for either Angular Material menu or the custom table-cell-actions dropdown
// Wait for the Angular Material menu, the custom table-cell-actions
// dropdown, or the signal-list row-actions menu
const matMenu = this.page.locator('.mat-menu-content, .mat-mdc-menu-content');
const customMenu = row.locator('.table-cell-actions-menu--open');
const customMenu = row.locator('.table-cell-actions-menu--open, [data-test="row-actions-menu"]');
await Promise.any([
matMenu.waitFor({ state: 'visible', timeout: 5000 }),
customMenu.waitFor({ state: 'visible', timeout: 5000 }),
Expand Down Expand Up @@ -422,7 +431,9 @@ export class ListComponent {
public locator: Locator;

constructor(private page: Page, locator?: Locator) {
this.locator = locator || page.locator('app-list').first();
// Pages are migrating from the legacy app-list to app-signal-list; a page
// renders exactly one of the two, so match either.
this.locator = locator || page.locator('app-list, app-signal-list').first();
this.table = new ListTableComponent(page, this.locator);
this.cards = new ListCardComponent(page, this.locator);
this.header = new ListHeaderComponent(page, this.locator);
Expand Down
3 changes: 2 additions & 1 deletion e2e/tests/core/api-keys.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ test.describe('API Keys', () => {

await apiKeysPage.list.table.openRowActionMenuByIndex(rowIndex);
// Click Delete within the opened menu row only (not across all rows)
await sharedPage.locator('.table-cell-actions-menu--open button').filter({ hasText: 'Delete' }).click();
await sharedPage.locator('.table-cell-actions-menu--open button, [data-test="row-actions-menu"] button')
.filter({ hasText: 'Delete' }).click();

await ConfirmDialogComponent.expectDialogAndConfirm(sharedPage, 'Delete', 'Delete Key');

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import { HttpClient } from '@angular/common/http';
import { provideZonelessChangeDetection } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import {
CUSTOM_USER_PERMISSION_CHECKERS,
CurrentUserPermissionsService,
CurrentUserRolesSignalService,
PermissionConfig,
PermissionTypes,
StratosCurrentUserPermissions,
} from '@stratosui/core';
import { EndpointsDataService, SessionData } from '@stratosui/store';
import { firstValueFrom, of } from 'rxjs';
import { describe, expect, it, beforeEach } from 'vitest';

import { CfCurrentUserRolesSignalService } from './cf-current-user-roles-signal.service';
import { CfUserPermissionsChecker } from './cf-user-permissions-checkers';

/**
* Checker contract (#5570 / #5571): a checker signals "this config is not
* mine" by returning undefined (simple) / null (complex / fallback). Returning
* a denial observable instead makes CurrentUserPermissionsService.findChecker
* see two claimants for every Stratos-level check ("Found too many ...
* Permission Denied"), which denied every internal / internal-scope / api-key
* permission app-wide (Register Endpoint button, /api-keys route guard, ...).
*
* Stratos permission types are spelled as literals here because the enum is
* core-internal; the values are pinned by stratosPermissionConfigs.
*/
const STRATOS_INTERNAL = 'internal' as PermissionTypes;
const STRATOS_INTERNAL_SCOPE = 'internal-scope' as PermissionTypes;
const STRATOS_API_KEY = 'api-key' as PermissionTypes;

describe('CfUserPermissionsChecker contract', () => {
let checker: CfUserPermissionsChecker;

beforeEach(() => {
TestBed.configureTestingModule({
providers: [
provideZonelessChangeDetection(),
{ provide: CfCurrentUserRolesSignalService, useValue: {} },
{ provide: HttpClient, useValue: {} },
CfUserPermissionsChecker,
],
});
checker = TestBed.inject(CfUserPermissionsChecker);
});

it('returns undefined from getSimpleCheck for non-CF permission types', () => {
expect(checker.getSimpleCheck(new PermissionConfig(STRATOS_INTERNAL, 'isAdmin'))).toBeUndefined();
expect(checker.getSimpleCheck(new PermissionConfig(STRATOS_INTERNAL_SCOPE, 'password.write'))).toBeUndefined();
expect(checker.getSimpleCheck(new PermissionConfig(STRATOS_API_KEY, ''))).toBeUndefined();
});

it('returns null from getComplexCheck when it handles none of the config groups', () => {
const stratosOnlyGroup = [
new PermissionConfig(STRATOS_INTERNAL_SCOPE, 'password.write'),
new PermissionConfig(STRATOS_INTERNAL_SCOPE, 'scim.write'),
];
expect(checker.getComplexCheck(stratosOnlyGroup)).toBeNull();
});

it('returns null from getPermissionConfig for stratos-owned keys', () => {
expect(checker.getPermissionConfig(StratosCurrentUserPermissions.API_KEYS)).toBeFalsy();
});
});

describe('CurrentUserPermissionsService with both checkers registered', () => {
function setup(sessionData: Partial<SessionData> | null, isAdmin: boolean) {
TestBed.configureTestingModule({
providers: [
provideZonelessChangeDetection(),
{ provide: CfCurrentUserRolesSignalService, useValue: {} },
{ provide: HttpClient, useValue: {} },
{ provide: EndpointsDataService, useValue: {} },
{
provide: CurrentUserRolesSignalService,
useValue: {
stratosRole$: (role: string) => of(role === 'isAdmin' && isAdmin),
stratosHasScope$: () => of(false),
sessionData$: () => of(sessionData as SessionData),
},
},
CfUserPermissionsChecker,
{
provide: CUSTOM_USER_PERMISSION_CHECKERS,
useFactory: (cfChecker: CfUserPermissionsChecker) => [cfChecker],
deps: [CfUserPermissionsChecker],
},
CurrentUserPermissionsService,
],
});
return TestBed.inject(CurrentUserPermissionsService);
}

it('resolves stratos-admin permission for an admin (#5570 Register Endpoint)', async () => {
const service = setup(null, true);
await expect(
firstValueFrom(service.can(StratosCurrentUserPermissions.EDIT_ADMIN_ENDPOINT)),
).resolves.toBe(true);
});

it('resolves api-keys permission for an admin when APIKeysEnabled=admin_only (#5571 route guard)', async () => {
const service = setup({ config: { APIKeysEnabled: 'admin_only' } } as Partial<SessionData>, true);
await expect(
firstValueFrom(service.can(StratosCurrentUserPermissions.API_KEYS)),
).resolves.toBe(true);
});

it('still denies stratos-admin permission for a non-admin', async () => {
const service = setup(null, false);
await expect(
firstValueFrom(service.can(StratosCurrentUserPermissions.EDIT_ADMIN_ENDPOINT)),
).resolves.toBe(false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,28 @@ export class CfUserPermissionsChecker extends BaseCurrentUserPermissionsChecker
/**
* @param permissionConfig Single permission to be checked
*/
public getSimpleCheck(permissionConfig: PermissionConfig, endpointGuid?: string, orgOrSpaceGuid?: string, spaceGuid?: string) {
public getSimpleCheck(
permissionConfig: PermissionConfig,
endpointGuid?: string,
orgOrSpaceGuid?: string,
spaceGuid?: string
): Observable<boolean> | undefined {
const check$ = this.getBaseSimpleCheck(permissionConfig, endpointGuid, orgOrSpaceGuid, spaceGuid);
if (!check$) {
return undefined;
}
if (permissionConfig.type === CfPermissionTypes.ORGANIZATION || permissionConfig.type === CfPermissionTypes.SPACE) {
return this.applyAdminCheck(check$, endpointGuid);
}
return check$;
}

private getBaseSimpleCheck(permissionConfig: PermissionConfig, endpointGuid?: string, orgOrSpaceGuid?: string, spaceGuid?: string) {
private getBaseSimpleCheck(
permissionConfig: PermissionConfig,
endpointGuid?: string,
orgOrSpaceGuid?: string,
spaceGuid?: string
): Observable<boolean> | undefined {
switch (permissionConfig.type) {
case (CfPermissionTypes.FEATURE_FLAG):
return this.getFeatureFlagCheck(permissionConfig, endpointGuid);
Expand All @@ -181,8 +194,12 @@ export class CfUserPermissionsChecker extends BaseCurrentUserPermissionsChecker
case (CfPermissionTypes.ENDPOINT_SCOPE):
return this.getEndpointScopesCheck(permissionConfig.permission as CfScopeStrings, endpointGuid);
default:
// Unknown permission type — deny rather than emit an undefined stream.
return of(false);
// Not a CF permission type: returning undefined tells
// CurrentUserPermissionsService this checker does not own the config.
// Returning a denial stream here instead makes findChecker() count two
// claimants for every stratos-level check and deny it app-wide (#5570,
// #5571).
return undefined;
}
}

Expand Down Expand Up @@ -382,9 +399,9 @@ export class CfUserPermissionsChecker extends BaseCurrentUserPermissionsChecker
endpointGuid?: string,
orgOrSpaceGuid?: string,
spaceGuid?: string
): IPermissionCheckCombiner[] {
): IPermissionCheckCombiner[] | null {
const groupedChecks = this.groupConfigs(permissionConfigs);
return Object.keys(groupedChecks)
const combiners = Object.keys(groupedChecks)
.map((permission: PermissionTypes) => {
const configGroup = groupedChecks[permission];
const checkCombiner = this.getBaseCheckFromConfig(configGroup, permission, endpointGuid, orgOrSpaceGuid, spaceGuid);
Expand All @@ -394,6 +411,10 @@ export class CfUserPermissionsChecker extends BaseCurrentUserPermissionsChecker
return checkCombiner;
})
.filter((checkCombiner): checkCombiner is IPermissionCheckCombiner => !!checkCombiner);
// No group handled → this checker does not own the config; an empty array
// is truthy and would double-claim the check in findChecker() (see
// getBaseSimpleCheck's default case).
return combiners.length ? combiners : null;
}


Expand Down
Loading