Skip to content
Draft
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { ApiProperty } from '@nestjs/swagger';

export class AvailablePermissionDs {
@ApiProperty()
value: string;

@ApiProperty({ required: false })
resource?: string;
}

export class AvailablePermissionsResponseDs {
@ApiProperty({ isArray: true, type: AvailablePermissionDs })
actions: Array<AvailablePermissionDs>;
}
30 changes: 30 additions & 0 deletions backend/src/entities/permission/permission-catalog.builder.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { CEDAR_SCHEMA } from '../cedar-authorization/cedar-schema.js';
import { AvailablePermissionDs } from './application/data-structures/available-permissions.ds.js';

export function buildPermissionCatalog(): Array<AvailablePermissionDs> {
const schemaActions = (CEDAR_SCHEMA as SchemaShape).RocketAdmin.actions;
return Object.entries(schemaActions).map(([value, definition]) =>
buildAction(value, definition.appliesTo.resourceTypes),
);
}

function buildAction(value: string, resourceTypes: Array<string>): AvailablePermissionDs {
const action: AvailablePermissionDs = { value };
const resource = deriveResource(resourceTypes);
if (resource) {
action.resource = resource;
}
return action;
}

function deriveResource(resourceTypes: Array<string>): string | undefined {
const first = resourceTypes[0];
if (!first) return undefined;
return first.charAt(0).toLowerCase() + first.slice(1);
}

type SchemaShape = {
RocketAdmin: {
actions: Record<string, { appliesTo: { principalTypes: Array<string>; resourceTypes: Array<string> } }>;
};
};
14 changes: 14 additions & 0 deletions backend/src/entities/permission/permission.controller.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
Body,
Controller,
Get,
HttpException,
HttpStatus,
Inject,
Expand All @@ -19,7 +20,9 @@ import { InTransactionEnum } from '../../enums/in-transaction.enum.js';
import { Messages } from '../../exceptions/text/messages.js';
import { ConnectionEditGuard } from '../../guards/connection-edit.guard.js';
import { SentryInterceptor } from '../../interceptors/sentry.interceptor.js';
import { AvailablePermissionsResponseDs } from './application/data-structures/available-permissions.ds.js';
import { ComplexPermissionDs, CreatePermissionsDs } from './application/data-structures/create-permissions.ds.js';
import { buildPermissionCatalog } from './permission-catalog.builder.js';
import { ICreateOrUpdatePermissions } from './use-cases/permissions-use-cases.interface.js';

@UseInterceptors(SentryInterceptor)
Expand All @@ -34,6 +37,17 @@ export class PermissionController {
private readonly createOrUpdatePermissionsUseCase: ICreateOrUpdatePermissions,
) {}

@ApiOperation({ summary: 'List available permissions derived from the Cedar schema' })
@ApiResponse({
status: 200,
description: 'Flat list of permissions with their resource scope.',
type: AvailablePermissionsResponseDs,
})
@Get('permissions/available')
async getAvailablePermissions(): Promise<AvailablePermissionsResponseDs> {
return { actions: buildPermissionCatalog() };
}

@ApiOperation({ summary: 'Create or update permissions in group' })
@ApiBody({ type: ComplexPermissionDs })
@ApiResponse({
Expand Down
7 changes: 6 additions & 1 deletion backend/src/entities/permission/permission.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ import { CreateOrUpdatePermissionsUseCase } from './use-cases/create-or-update-p
})
export class PermissionModule implements NestModule {
public configure(consumer: MiddlewareConsumer): any {
consumer.apply(AuthMiddleware).forRoutes({ path: 'permissions/:slug', method: RequestMethod.PUT });
consumer
.apply(AuthMiddleware)
.forRoutes(
{ path: 'permissions/:slug', method: RequestMethod.PUT },
{ path: 'permissions/available', method: RequestMethod.GET },
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/* eslint-disable @typescript-eslint/no-unused-vars */

import { INestApplication, ValidationPipe } from '@nestjs/common';
import { Test } from '@nestjs/testing';
import test from 'ava';
import { ValidationError } from 'class-validator';
import cookieParser from 'cookie-parser';
import request from 'supertest';
import { ApplicationModule } from '../../../src/app.module.js';
import { CedarAction } from '../../../src/entities/cedar-authorization/cedar-action-map.js';
import { WinstonLogger } from '../../../src/entities/logging/winston-logger.js';
import { AllExceptionsFilter } from '../../../src/exceptions/all-exceptions.filter.js';
import { ValidationException } from '../../../src/exceptions/custom-exceptions/validation-exception.js';
import { Cacher } from '../../../src/helpers/cache/cacher.js';
import { DatabaseModule } from '../../../src/shared/database/database.module.js';
import { DatabaseService } from '../../../src/shared/database/database.service.js';
import { registerUserAndReturnUserInfo } from '../../utils/register-user-and-return-user-info.js';
import { setSaasEnvVariable } from '../../utils/set-saas-env-variable.js';
import { TestUtils } from '../../utils/test.utils.js';

let app: INestApplication;

test.before(async () => {
setSaasEnvVariable();
const moduleFixture = await Test.createTestingModule({
imports: [ApplicationModule, DatabaseModule],
providers: [DatabaseService, TestUtils],
}).compile();
app = moduleFixture.createNestApplication();

app.use(cookieParser());
app.useGlobalFilters(new AllExceptionsFilter(app.get(WinstonLogger)));
app.useGlobalPipes(
new ValidationPipe({
exceptionFactory(validationErrors: ValidationError[] = []) {
return new ValidationException(validationErrors);
},
}),
);
await app.init();
app.getHttpServer().listen(0);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove the explicit server listen in this E2E setup.

supertest uses app.getHttpServer() directly; listen(0) here is redundant and may create teardown flakiness because it isn’t awaited.

Suggested fix
- app.getHttpServer().listen(0);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
app.getHttpServer().listen(0);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@backend/test/ava-tests/non-saas-tests/non-saas-permission-catalog-e2e.test.ts`
at line 41, Remove the explicit call to app.getHttpServer().listen(0) in the
test setup; supertest directly uses app.getHttpServer(), so delete the listen
invocation (reference: the call to app.getHttpServer().listen(0)) to avoid
redundant server startup and teardown flakiness and let tests use the provided
server instance without awaiting or manually listening.

});

test.after(async () => {
await Cacher.clearAllCache();
await app.close();
});

test.serial('GET /permissions/available returns catalog covering every CedarAction', async (t) => {
const token = (await registerUserAndReturnUserInfo(app)).token;

const response = await request(app.getHttpServer())
.get('/permissions/available')
.set('Cookie', token)
.set('Accept', 'application/json');

t.is(response.status, 200);

const body = response.body as {
actions: Array<{ value: string; resource?: string }>;
};

t.true(Array.isArray(body.actions));
t.true(body.actions.length > 0);

const values = new Set(body.actions.map((a) => a.value));

for (const cedarValue of Object.values(CedarAction)) {
t.true(values.has(cedarValue), `catalog missing CedarAction ${cedarValue}`);
}

t.false(values.has('*'), 'catalog must NOT include synthesized wildcards');
t.false(values.has('table:*'), 'catalog must NOT include synthesized wildcards');
t.false(values.has('dashboard:*'), 'catalog must NOT include synthesized wildcards');

const byValue = new Map(body.actions.map((a) => [a.value, a]));

t.is(byValue.get('connection:read')!.resource, 'connection');
t.is(byValue.get('group:edit')!.resource, 'group');
t.is(byValue.get('table:read')!.resource, 'table');
t.is(byValue.get('actionEvent:trigger')!.resource, 'actionEvent');
t.is(byValue.get('dashboard:read')!.resource, 'dashboard');
t.is(byValue.get('dashboard:create')!.resource, 'dashboard');
t.is(byValue.get('panel:read')!.resource, 'panel');

for (const action of body.actions) {
t.is(Object.hasOwn(action, 'label'), false, `action ${action.value} should not have label`);
t.is(Object.hasOwn(action, 'shortLabel'), false, `action ${action.value} should not have shortLabel`);
t.is(Object.hasOwn(action, 'icon'), false, `action ${action.value} should not have icon`);
}
});

test.serial('GET /permissions/available requires authentication', async (t) => {
const response = await request(app.getHttpServer()).get('/permissions/available').set('Accept', 'application/json');

t.is(response.status, 401);
});
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
<mat-optgroup [label]="actionGroup.group">
@for (action of actionGroup.actions; track action.value) {
<mat-option [value]="action.value">
{{ action.label }}
{{ getActionLabel(action.value) }}
</mat-option>
}
</mat-optgroup>
Expand Down Expand Up @@ -125,7 +125,7 @@
<mat-optgroup [label]="group.group">
@for (action of group.actions; track action.value) {
<mat-option [value]="action.value">
{{ action.label }}
{{ getActionLabel(action.value) }}
</mat-option>
}
</mat-optgroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,45 @@
import { signal } from '@angular/core';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { FormsModule } from '@angular/forms';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
import { PolicyAction, PolicyActionGroup } from 'src/app/lib/cedar-policy-items';
import { UsersService } from 'src/app/services/users.service';
import { CedarPolicyListComponent } from './cedar-policy-list.component';

const fixtureGroups: PolicyActionGroup[] = [
{
group: 'Connection',
actions: [
{ value: 'connection:read', resource: 'connection' },
{ value: 'connection:edit', resource: 'connection' },
],
},
{
group: 'Group',
actions: [
{ value: 'group:read', resource: 'group' },
{ value: 'group:edit', resource: 'group' },
],
},
{
group: 'Table',
actions: [
{ value: 'table:read', resource: 'table' },
{ value: 'table:edit', resource: 'table' },
],
},
{
group: 'Dashboard',
actions: [
{ value: 'dashboard:read', resource: 'dashboard' },
{ value: 'dashboard:create', resource: 'dashboard' },
{ value: 'dashboard:edit', resource: 'dashboard' },
],
},
];

const flatActions: PolicyAction[] = fixtureGroups.flatMap((g) => g.actions);

describe('CedarPolicyListComponent', () => {
let component: CedarPolicyListComponent;
let fixture: ComponentFixture<CedarPolicyListComponent>;
Expand All @@ -18,8 +55,16 @@ describe('CedarPolicyListComponent', () => {
];

beforeEach(async () => {
const groupsSignal = signal(fixtureGroups);
const actionsSignal = signal(flatActions);
const mockUsersService: Partial<UsersService> = {
availablePermissionGroups: groupsSignal.asReadonly() as UsersService['availablePermissionGroups'],
availablePermissions: actionsSignal.asReadonly() as UsersService['availablePermissions'],
};

await TestBed.configureTestingModule({
imports: [CedarPolicyListComponent, FormsModule, BrowserAnimationsModule],
providers: [{ provide: UsersService, useValue: mockUsersService }],
}).compileComponents();

fixture = TestBed.createComponent(CedarPolicyListComponent);
Expand Down Expand Up @@ -210,9 +255,36 @@ describe('CedarPolicyListComponent', () => {
expect(component.needsDashboard).toBe(true);
});

it('should treat dashboard:create as scopeless', () => {
component.newAction = 'dashboard:create';
expect(component.needsDashboard).toBe(false);
});

it('should return correct dashboard display names', () => {
expect(component.getDashboardDisplayName('dash-1')).toBe('Sales Dashboard');
expect(component.getDashboardDisplayName('unknown')).toBe('unknown');
expect(component.getDashboardDisplayName('*')).toBe('All dashboards');
});

it('should synthesize General and prefix wildcards in addActionGroups', () => {
const testable = component as CedarPolicyListComponent & {
addActionGroups: () => PolicyActionGroup[];
};
const groups = testable.addActionGroups();
const general = groups.find((g) => g.group === 'General');
expect(general).toBeTruthy();
expect(general!.actions[0].value).toBe('*');

const table = groups.find((g) => g.group === 'Table');
expect(table).toBeTruthy();
expect(table!.actions[0].value).toBe('table:*');
expect(table!.actions[0].resource).toBe('table');

const dashboard = groups.find((g) => g.group === 'Dashboard');
expect(dashboard).toBeTruthy();
expect(dashboard!.actions[0].value).toBe('dashboard:*');

const connection = groups.find((g) => g.group === 'Connection');
expect(connection!.actions[0].value).toBe('connection:read');
});
});
Loading
Loading