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
6 changes: 6 additions & 0 deletions backend/src/entities/cedar-authorization/cedar-action-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ export enum CedarAction {
GroupRead = 'group:read',
GroupEdit = 'group:edit',
TableRead = 'table:read',
TableQuery = 'table:query',
ColumnRead = 'column:read',
TableAdd = 'table:add',
TableEdit = 'table:edit',
TableDelete = 'table:delete',
Expand All @@ -24,6 +26,7 @@ export enum CedarResourceType {
Connection = 'RocketAdmin::Connection',
Group = 'RocketAdmin::Group',
Table = 'RocketAdmin::Table',
Column = 'RocketAdmin::Column',
ActionEvent = 'RocketAdmin::ActionEvent',
Dashboard = 'RocketAdmin::Dashboard',
Panel = 'RocketAdmin::Panel',
Expand All @@ -35,12 +38,15 @@ export const CEDAR_GROUP_TYPE = 'RocketAdmin::Group';

export const ACTION_EVENT_PROBE_ID = '__probe__';

export const COLUMN_PROBE_ID = '__probe__';

export interface CedarValidationRequest {
userId: string;
action: CedarAction;
connectionId?: string;
groupId?: string;
tableName?: string;
columnName?: string;
actionEventId?: string;
dashboardId?: string;
panelId?: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class CedarAuthorizationService implements ICedarAuthorizationService, On
}

async validate(request: CedarValidationRequest): Promise<boolean> {
const { userId, action, groupId, tableName, dashboardId, panelId, actionEventId } = request;
const { userId, action, groupId, tableName, columnName, dashboardId, panelId, actionEventId } = request;
let { connectionId } = request;

const actionPrefix = action.split(':')[0];
Expand All @@ -59,7 +59,46 @@ export class CedarAuthorizationService implements ICedarAuthorizationService, On
if (!connectionId) return false;
resourceType = CedarResourceType.Table;
resourceId = `${connectionId}/${tableName}`;
if (action === CedarAction.TableQuery) {
// table:read is an alias for table:query + column:read(*). Honor legacy or
// hand-written policies that grant table:read directly as a QueryTable grant.
if (await this.evaluate(userId, connectionId, CedarAction.TableQuery, resourceType, resourceId, tableName)) {
return true;
}
return this.evaluate(userId, connectionId, CedarAction.TableRead, resourceType, resourceId, tableName);
}
break;
case 'column': {
if (!connectionId) return false;
if (!tableName || !columnName) return false;
resourceType = CedarResourceType.Column;
resourceId = `${connectionId}/${tableName}/${columnName}`;
if (
await this.evaluate(
userId,
connectionId,
action,
resourceType,
resourceId,
tableName,
undefined,
undefined,
undefined,
columnName,
)
) {
return true;
}
// Legacy alias: a direct table:read grant covers every column of the table.
return this.evaluate(
userId,
connectionId,
CedarAction.TableRead,
CedarResourceType.Table,
`${connectionId}/${tableName}`,
tableName,
);
}
case 'actionEvent': {
if (!connectionId) return false;
if (!tableName || !actionEventId) return false;
Expand Down Expand Up @@ -220,6 +259,7 @@ export class CedarAuthorizationService implements ICedarAuthorizationService, On
dashboardId?: string,
panelId?: string,
actionEventId?: string,
columnName?: string,
): Promise<boolean> {
await this.assertUserNotSuspended(userId);

Expand All @@ -237,6 +277,7 @@ export class CedarAuthorizationService implements ICedarAuthorizationService, On
dashboardId,
panelId,
actionEventId,
columnName,
);

for (const policy of groupPolicies) {
Expand Down Expand Up @@ -350,6 +391,19 @@ export class CedarAuthorizationService implements ICedarAuthorizationService, On
}
}

const columnResourceIds = [...cedarPolicy.matchAll(/resource\s*(?:==|in)\s*RocketAdmin::Column::"([^"]+)"/g)].map(
(m) => m[1],
);

for (const columnRef of columnResourceIds) {
if (!columnRef.startsWith(`${connectionId}/`)) {
throw new HttpException(
{ message: Messages.CEDAR_POLICY_REFERENCES_FOREIGN_CONNECTION },
HttpStatus.BAD_REQUEST,
);
}
}

const dashboardResourceIds = [...cedarPolicy.matchAll(/resource\s*==\s*RocketAdmin::Dashboard::"([^"]+)"/g)].map(
(m) => m[1],
);
Expand Down
11 changes: 11 additions & 0 deletions backend/src/entities/cedar-authorization/cedar-entity-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export function buildCedarEntities(
dashboardId?: string,
panelId?: string,
actionEventId?: string,
columnName?: string,
): Array<CedarEntityRecord> {
const entities: Array<CedarEntityRecord> = [];

Expand Down Expand Up @@ -52,6 +53,16 @@ export function buildCedarEntities(
});
}

// Column entity, parented by its Table — required so `resource in Table::"..."`
// policies authorize reading any column without naming each one (the table:read alias).
if (columnName && tableName) {
entities.push({
uid: { type: 'RocketAdmin::Column', id: `${connectionId}/${tableName}/${columnName}` },
attrs: { connectionId: connectionId, tableName: tableName },
parents: [{ type: 'RocketAdmin::Table', id: `${connectionId}/${tableName}` }],
});
}

// ActionEvent entity, parented by its Table — required so `resource in Table::"..."`
// policies authorize triggering specific events without naming each event.
if (actionEventId && tableName) {
Expand Down
118 changes: 101 additions & 17 deletions backend/src/entities/cedar-authorization/cedar-permissions.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
CEDAR_USER_TYPE,
CedarAction,
CedarResourceType,
COLUMN_PROBE_ID,
} from './cedar-action-map.js';
import { buildCedarEntities } from './cedar-entity-builder.js';
import { CEDAR_SCHEMA } from './cedar-schema.js';
Expand Down Expand Up @@ -256,6 +257,8 @@ export class CedarPermissionsService implements IUserAccessRepository {
return result;
}

// "Table read" now means "may query this table" (the QueryTable half of the table:read
// alias). Column-level visibility is enforced separately via checkColumnRead/getReadableColumns.
async checkTableRead(
cognitoUserName: string,
connectionId: string,
Expand All @@ -265,15 +268,44 @@ export class CedarPermissionsService implements IUserAccessRepository {
const ctx = await this.loadContext(connectionId, cognitoUserName);
if (!ctx) return false;

const entities = buildCedarEntities(cognitoUserName, ctx.userGroups, connectionId, tableName);
return this.evaluatePolicies(
cognitoUserName,
CedarAction.TableRead,
CedarResourceType.Table,
`${connectionId}/${tableName}`,
ctx.policies,
entities,
);
return this.evaluateTableQuery(cognitoUserName, connectionId, tableName, ctx);
}

async checkColumnRead(
cognitoUserName: string,
connectionId: string,
tableName: string,
columnName: string,
): Promise<boolean> {
const ctx = await this.loadContext(connectionId, cognitoUserName);
if (!ctx) return false;

return this.evaluateColumnRead(cognitoUserName, connectionId, tableName, columnName, ctx);
}

// Returns the subset of `allColumnNames` the user may read. A single probe detects a
// table-wide grant (the table:read alias → ColumnRead(table, *)); only column-restricted
// tables pay a per-column evaluation.
async getReadableColumns(
cognitoUserName: string,
connectionId: string,
tableName: string,
allColumnNames: Array<string>,
): Promise<Set<string>> {
const ctx = await this.loadContext(connectionId, cognitoUserName);
if (!ctx) return new Set();

if (this.evaluateColumnRead(cognitoUserName, connectionId, tableName, COLUMN_PROBE_ID, ctx)) {
return new Set(allColumnNames);
}
Comment on lines +298 to +300

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 | 🟠 Major | ⚡ Quick win

Probe-based full-column shortcut can over-grant access.

Line 298 treats an allow on synthetic __probe__ as “all columns readable”. If a real column (or explicit policy resource) uses that same id, users can be escalated from one-column access to all columns.

Suggested fix
-		if (this.evaluateColumnRead(cognitoUserName, connectionId, tableName, COLUMN_PROBE_ID, ctx)) {
-			return new Set(allColumnNames);
-		}
-
 		const readable = new Set<string>();
 		for (const columnName of allColumnNames) {
 			if (this.evaluateColumnRead(cognitoUserName, connectionId, tableName, columnName, ctx)) {
 				readable.add(columnName);
 			}
 		}
 		return readable;
📝 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
if (this.evaluateColumnRead(cognitoUserName, connectionId, tableName, COLUMN_PROBE_ID, ctx)) {
return new Set(allColumnNames);
}
const readable = new Set<string>();
for (const columnName of allColumnNames) {
if (this.evaluateColumnRead(cognitoUserName, connectionId, tableName, columnName, ctx)) {
readable.add(columnName);
}
}
return readable;
🤖 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/src/entities/cedar-authorization/cedar-permissions.service.ts` around
lines 298 - 300, The evaluateColumnRead method call at line 298 uses
COLUMN_PROBE_ID as a synthetic probe to determine if a user can read all
columns, but if a real column or policy resource actually uses that same probe
ID value, it can over-grant access. Ensure that COLUMN_PROBE_ID is guaranteed to
be a unique, non-colliding identifier that cannot match any real column name, or
alternatively refactor the authorization logic to avoid this shortcut and
instead enumerate individual column permissions rather than treating a single
probe result as proxy for all columns.


const readable = new Set<string>();
for (const columnName of allColumnNames) {
if (this.evaluateColumnRead(cognitoUserName, connectionId, tableName, columnName, ctx)) {
readable.add(columnName);
}
}
return readable;
}

async checkTableAdd(
Expand Down Expand Up @@ -402,14 +434,7 @@ export class CedarPermissionsService implements IUserAccessRepository {
const entities = buildCedarEntities(userId, ctx.userGroups, connectionId, tableName);
const resourceId = `${connectionId}/${tableName}`;

const canRead = this.evaluatePolicies(
userId,
CedarAction.TableRead,
CedarResourceType.Table,
resourceId,
ctx.policies,
entities,
);
const canRead = this.evaluateTableQuery(userId, connectionId, tableName, ctx);
const canAdd = this.evaluatePolicies(
userId,
CedarAction.TableAdd,
Expand Down Expand Up @@ -478,6 +503,65 @@ export class CedarPermissionsService implements IUserAccessRepository {
};
}

// QueryTable check honoring the table:read alias: a direct table:read grant (legacy or
// hand-written policy) also permits querying the table.
private evaluateTableQuery(userId: string, connectionId: string, tableName: string, ctx: EvalContext): boolean {
const entities = buildCedarEntities(userId, ctx.userGroups, connectionId, tableName);
const resourceId = `${connectionId}/${tableName}`;
return (
this.evaluatePolicies(
userId,
CedarAction.TableQuery,
CedarResourceType.Table,
resourceId,
ctx.policies,
entities,
) ||
this.evaluatePolicies(userId, CedarAction.TableRead, CedarResourceType.Table, resourceId, ctx.policies, entities)
);
}

private evaluateColumnRead(
userId: string,
connectionId: string,
tableName: string,
columnName: string,
ctx: EvalContext,
): boolean {
const columnEntities = buildCedarEntities(
userId,
ctx.userGroups,
connectionId,
tableName,
undefined,
undefined,
undefined,
columnName,
);
if (
this.evaluatePolicies(
userId,
CedarAction.ColumnRead,
CedarResourceType.Column,
`${connectionId}/${tableName}/${columnName}`,
ctx.policies,
columnEntities,
)
) {
return true;
}
// Legacy alias: a direct table:read grant covers every column of the table.
const tableEntities = buildCedarEntities(userId, ctx.userGroups, connectionId, tableName);
return this.evaluatePolicies(
userId,
CedarAction.TableRead,
CedarResourceType.Table,
`${connectionId}/${tableName}`,
ctx.policies,
tableEntities,
);
}

private evaluatePolicies(
userId: string,
action: CedarAction,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,26 @@ export function generateCedarPolicyForGroup(
// action events is a side-effect-only capability and does not imply table visibility.
const hasAnyAccess = access.visibility || access.add || access.delete || access.edit;
if (hasAnyAccess) {
// QueryTable: may run a read query against the table at all (checked before the query).
policies.push(
`permit(\n principal,\n action == RocketAdmin::Action::"table:read",\n resource == ${tableRef}\n);`,
`permit(\n principal,\n action == RocketAdmin::Action::"table:query",\n resource == ${tableRef}\n);`,
);
// ColumnRead (checked after the query). `table:read` is an alias for
// QueryTable + ColumnRead(table, *): when no explicit column whitelist is given we
// grant every column via `resource in Table`; otherwise one grant per allowed column.
const readableColumns = table.readableColumns;
if (readableColumns && readableColumns.length > 0) {
for (const columnName of readableColumns) {
const columnRef = `RocketAdmin::Column::"${connectionId}/${table.tableName}/${columnName}"`;
policies.push(
`permit(\n principal,\n action == RocketAdmin::Action::"column:read",\n resource == ${columnRef}\n);`,
);
}
} else {
policies.push(
`permit(\n principal,\n action == RocketAdmin::Action::"column:read",\n resource in ${tableRef}\n);`,
);
}
}
if (access.add) {
policies.push(
Expand Down
35 changes: 35 additions & 0 deletions backend/src/entities/cedar-authorization/cedar-policy-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export function parseCedarPolicyToClassicalPermissions(
result.group.accessLevel = AccessLevelEnum.edit;
break;
case 'table:read':
case 'table:query':
case 'table:add':
case 'table:edit':
case 'table:delete':
Expand All @@ -78,6 +79,25 @@ export function parseCedarPolicyToClassicalPermissions(
applyTableAction(tableEntry, permit.action);
break;
}
case 'column:read': {
if (permit.resourceType === 'RocketAdmin::Table' && permit.isInRelation) {
// Wildcard: read every column on this table (the table:read alias). No explicit
// column whitelist — leave readableColumns undefined ⇒ "all columns".
const tableName = extractTableName(permit.resourceId, connectionId);
if (!tableName) break;
getOrCreateTableEntry(tableMap, tableName);
} else if (permit.resourceType === 'RocketAdmin::Column') {
// Per-column grant: add this column to the table's readable whitelist.
const parts = extractColumnResource(permit.resourceId, connectionId);
if (!parts) break;
const tableEntry = getOrCreateTableEntry(tableMap, parts.tableName);
if (!tableEntry.readableColumns) tableEntry.readableColumns = [];
if (!tableEntry.readableColumns.includes(parts.columnName)) {
tableEntry.readableColumns.push(parts.columnName);
}
}
break;
}
case 'actionEvent:trigger': {
if (permit.resourceType === 'RocketAdmin::Table' && permit.isInRelation) {
// Blanket: trigger any event on this table
Expand Down Expand Up @@ -273,6 +293,7 @@ function getOrCreateTableEntry(map: Map<string, ITablePermissionData>, tableName
function applyTableAction(entry: ITablePermissionData, action: string): void {
switch (action) {
case 'table:read':
case 'table:query':
entry.accessLevel.visibility = true;
break;
case 'table:add':
Expand All @@ -290,6 +311,20 @@ function applyTableAction(entry: ITablePermissionData, action: string): void {
}
}

function extractColumnResource(
resourceId: string | null,
connectionId: string,
): { tableName: string; columnName: string } | null {
if (!resourceId) return null;
const prefix = `${connectionId}/`;
const stripped = resourceId.startsWith(prefix) ? resourceId.slice(prefix.length) : resourceId;
const slash = stripped.indexOf('/');
if (slash <= 0 || slash === stripped.length - 1) return null;
const tableName = stripped.slice(0, slash);
const columnName = stripped.slice(slash + 1);
return { tableName, columnName };
}

function extractActionEventResource(
resourceId: string | null,
connectionId: string,
Expand Down
Loading
Loading