Skip to content

Commit 7a41f62

Browse files
ArtuomkaArtuomka
committed
Refactor Cedar policy tests to remove group principal references
- Updated Cedar policy definitions in non-SaaS and SaaS end-to-end tests to use 'principal' instead of 'principal in RocketAdmin::Group::"{groupId}"'. - Removed redundant tests that checked for foreign group references in Cedar policies. - Ensured consistency across all Cedar policy tests by standardizing the policy format.
1 parent 772a995 commit 7a41f62

11 files changed

Lines changed: 145 additions & 233 deletions

‎backend/src/entities/cedar-authorization/cedar-authorization.service.ts‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,8 @@ export class CedarAuthorizationService implements ICedarAuthorizationService, On
186186
const userGroups = await this.globalDbContext.groupRepository.findAllUserGroupsInConnection(connectionId, userId);
187187
if (userGroups.length === 0) return false;
188188

189-
const policies = await this.loadPoliciesForConnection(connectionId);
189+
const userGroupIds = userGroups.map((g) => g.id);
190+
const policies = await this.loadPoliciesForUser(connectionId, userId, userGroupIds);
190191
if (!policies) return false;
191192

192193
const entities = buildCedarEntities(userId, userGroups, connectionId, tableName, dashboardId);
@@ -210,17 +211,21 @@ export class CedarAuthorizationService implements ICedarAuthorizationService, On
210211
return false;
211212
}
212213

213-
private async loadPoliciesForConnection(connectionId: string): Promise<string | null> {
214-
const cached = Cacher.getCedarPolicyCache(connectionId);
214+
private async loadPoliciesForUser(connectionId: string, userId: string, userGroupIds: string[]): Promise<string | null> {
215+
const cached = Cacher.getCedarPolicyCache(connectionId, userId);
215216
if (cached !== null) return cached;
216217

217218
const groups = await this.globalDbContext.groupRepository.findAllGroupsInConnection(connectionId);
218-
const policyTexts = groups.map((g) => g.cedarPolicy).filter(Boolean);
219+
const userGroupIdSet = new Set(userGroupIds);
220+
const policyTexts = groups
221+
.filter((g) => userGroupIdSet.has(g.id))
222+
.map((g) => g.cedarPolicy)
223+
.filter(Boolean);
219224

220225
if (policyTexts.length === 0) return null;
221226

222227
const combined = policyTexts.join('\n\n');
223-
Cacher.setCedarPolicyCache(connectionId, combined);
228+
Cacher.setCedarPolicyCache(connectionId, userId, combined);
224229
return combined;
225230
}
226231

@@ -271,19 +276,6 @@ export class CedarAuthorizationService implements ICedarAuthorizationService, On
271276
groupId: string,
272277
): Promise<void> {
273278

274-
const principalGroupIds = [
275-
...cedarPolicy.matchAll(/principal\s+in\s+RocketAdmin::Group::"([^"]+)"/g),
276-
].map((m) => m[1]);
277-
278-
for (const principalGroupId of principalGroupIds) {
279-
if (principalGroupId !== groupId) {
280-
throw new HttpException(
281-
{ message: Messages.CEDAR_POLICY_REFERENCES_FOREIGN_PRINCIPAL },
282-
HttpStatus.BAD_REQUEST,
283-
);
284-
}
285-
}
286-
287279
const connectionIds = [
288280
...cedarPolicy.matchAll(/resource\s*==\s*RocketAdmin::Connection::"([^"]+)"/g),
289281
].map((m) => m[1]);

‎backend/src/entities/cedar-authorization/cedar-entity-builder.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export function buildCedarEntities(
1919
entities.push({
2020
uid: { type: 'RocketAdmin::User', id: userId },
2121
attrs: { suspended: false },
22-
parents: userGroups.map((g) => ({ type: 'RocketAdmin::Group', id: g.id })),
22+
parents: [],
2323
});
2424

2525
// Group entities

‎backend/src/entities/cedar-authorization/cedar-policy-generator.ts‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,11 @@ export function generateCedarPolicyForGroup(
88
permissions: IComplexPermission,
99
): string {
1010
const policies: Array<string> = [];
11-
const groupRef = `RocketAdmin::Group::"${groupId}"`;
1211
const connectionRef = `RocketAdmin::Connection::"${connectionId}"`;
1312

1413
if (isMain) {
1514
policies.push(
16-
`permit(\n principal in ${groupRef},\n action,\n resource\n);`,
15+
`permit(\n principal,\n action,\n resource\n);`,
1716
);
1817
return policies.join('\n\n');
1918
}
@@ -22,14 +21,14 @@ export function generateCedarPolicyForGroup(
2221
const connAccess = permissions.connection.accessLevel;
2322
if (connAccess === AccessLevelEnum.edit) {
2423
policies.push(
25-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"connection:read",\n resource == ${connectionRef}\n);`,
24+
`permit(\n principal,\n action == RocketAdmin::Action::"connection:read",\n resource == ${connectionRef}\n);`,
2625
);
2726
policies.push(
28-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"connection:edit",\n resource == ${connectionRef}\n);`,
27+
`permit(\n principal,\n action == RocketAdmin::Action::"connection:edit",\n resource == ${connectionRef}\n);`,
2928
);
3029
} else if (connAccess === AccessLevelEnum.readonly) {
3130
policies.push(
32-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"connection:read",\n resource == ${connectionRef}\n);`,
31+
`permit(\n principal,\n action == RocketAdmin::Action::"connection:read",\n resource == ${connectionRef}\n);`,
3332
);
3433
}
3534

@@ -38,14 +37,14 @@ export function generateCedarPolicyForGroup(
3837
const groupResourceRef = `RocketAdmin::Group::"${permissions.group.groupId}"`;
3938
if (groupAccess === AccessLevelEnum.edit) {
4039
policies.push(
41-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"group:read",\n resource == ${groupResourceRef}\n);`,
40+
`permit(\n principal,\n action == RocketAdmin::Action::"group:read",\n resource == ${groupResourceRef}\n);`,
4241
);
4342
policies.push(
44-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"group:edit",\n resource == ${groupResourceRef}\n);`,
43+
`permit(\n principal,\n action == RocketAdmin::Action::"group:edit",\n resource == ${groupResourceRef}\n);`,
4544
);
4645
} else if (groupAccess === AccessLevelEnum.readonly) {
4746
policies.push(
48-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"group:read",\n resource == ${groupResourceRef}\n);`,
47+
`permit(\n principal,\n action == RocketAdmin::Action::"group:read",\n resource == ${groupResourceRef}\n);`,
4948
);
5049
}
5150

@@ -59,32 +58,32 @@ export function generateCedarPolicyForGroup(
5958
if (access.read) {
6059
hasReadPermission = true;
6160
policies.push(
62-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"dashboard:read",\n resource == ${dashboardRef}\n);`,
61+
`permit(\n principal,\n action == RocketAdmin::Action::"dashboard:read",\n resource == ${dashboardRef}\n);`,
6362
);
6463
}
6564
if (access.create) {
6665
hasCreatePermission = true;
6766
}
6867
if (access.edit) {
6968
policies.push(
70-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"dashboard:edit",\n resource == ${dashboardRef}\n);`,
69+
`permit(\n principal,\n action == RocketAdmin::Action::"dashboard:edit",\n resource == ${dashboardRef}\n);`,
7170
);
7271
}
7372
if (access.delete) {
7473
policies.push(
75-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"dashboard:delete",\n resource == ${dashboardRef}\n);`,
74+
`permit(\n principal,\n action == RocketAdmin::Action::"dashboard:delete",\n resource == ${dashboardRef}\n);`,
7675
);
7776
}
7877
}
7978
const newDashboardRef = `RocketAdmin::Dashboard::"${connectionId}/__new__"`;
8079
if (hasReadPermission) {
8180
policies.push(
82-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"dashboard:read",\n resource == ${newDashboardRef}\n);`,
81+
`permit(\n principal,\n action == RocketAdmin::Action::"dashboard:read",\n resource == ${newDashboardRef}\n);`,
8382
);
8483
}
8584
if (hasCreatePermission) {
8685
policies.push(
87-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"dashboard:create",\n resource == ${newDashboardRef}\n);`,
86+
`permit(\n principal,\n action == RocketAdmin::Action::"dashboard:create",\n resource == ${newDashboardRef}\n);`,
8887
);
8988
}
9089
}
@@ -96,22 +95,22 @@ export function generateCedarPolicyForGroup(
9695
const hasAnyAccess = access.visibility || access.add || access.delete || access.edit;
9796
if (hasAnyAccess) {
9897
policies.push(
99-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"table:read",\n resource == ${tableRef}\n);`,
98+
`permit(\n principal,\n action == RocketAdmin::Action::"table:read",\n resource == ${tableRef}\n);`,
10099
);
101100
}
102101
if (access.add) {
103102
policies.push(
104-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"table:add",\n resource == ${tableRef}\n);`,
103+
`permit(\n principal,\n action == RocketAdmin::Action::"table:add",\n resource == ${tableRef}\n);`,
105104
);
106105
}
107106
if (access.edit) {
108107
policies.push(
109-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"table:edit",\n resource == ${tableRef}\n);`,
108+
`permit(\n principal,\n action == RocketAdmin::Action::"table:edit",\n resource == ${tableRef}\n);`,
110109
);
111110
}
112111
if (access.delete) {
113112
policies.push(
114-
`permit(\n principal in ${groupRef},\n action == RocketAdmin::Action::"table:delete",\n resource == ${tableRef}\n);`,
113+
`permit(\n principal,\n action == RocketAdmin::Action::"table:delete",\n resource == ${tableRef}\n);`,
115114
);
116115
}
117116
}

‎backend/src/entities/cedar-authorization/cedar-policy-parser.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,9 @@ function parsePermitBody(body: string): ParsedPermitStatement {
147147
isWildcard: false,
148148
};
149149

150-
const principalMatch = body.match(/principal\s+in\s+RocketAdmin::Group::"([^"]+)"/);
151-
if (principalMatch) {
152-
result.groupId = principalMatch[1];
150+
const principalInMatch = body.match(/principal\s+in\s+RocketAdmin::Group::"([^"]+)"/);
151+
if (principalInMatch) {
152+
result.groupId = principalInMatch[1];
153153
}
154154

155155
const actionMatch = body.match(/action\s*==\s*RocketAdmin::Action::"([^"]+)"/);

‎backend/src/entities/cedar-authorization/cedar-schema.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ export const CEDAR_SCHEMA = {
22
RocketAdmin: {
33
entityTypes: {
44
User: {
5-
memberOfTypes: ['Group'],
5+
memberOfTypes: [],
66
shape: {
77
type: 'Record',
88
attributes: {

‎backend/src/helpers/cache/cacher.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,17 +66,23 @@ export class Cacher {
6666
return userInvitations <= 10 && groupInvitations <= 10;
6767
}
6868

69-
public static getCedarPolicyCache(connectionId: string): string | null {
70-
const cached = cedarPolicyCache.get(connectionId);
69+
public static getCedarPolicyCache(connectionId: string, userId: string): string | null {
70+
const cacheKey = `${connectionId}:${userId}`;
71+
const cached = cedarPolicyCache.get(cacheKey);
7172
return cached !== undefined ? cached : null;
7273
}
7374

74-
public static setCedarPolicyCache(connectionId: string, policies: string): void {
75-
cedarPolicyCache.set(connectionId, policies);
75+
public static setCedarPolicyCache(connectionId: string, userId: string, policies: string): void {
76+
const cacheKey = `${connectionId}:${userId}`;
77+
cedarPolicyCache.set(cacheKey, policies);
7678
}
7779

7880
public static invalidateCedarPolicyCache(connectionId: string): void {
79-
cedarPolicyCache.delete(connectionId);
81+
for (const key of cedarPolicyCache.keys()) {
82+
if (key.startsWith(`${connectionId}:`)) {
83+
cedarPolicyCache.delete(key);
84+
}
85+
}
8086
}
8187

8288
public static async clearAllCache(): Promise<void> {

‎backend/test/ava-tests/non-saas-tests/non-saas-cedar-entity-builder.test.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,14 @@ function makeGroup(id: string, isMain: boolean): GroupEntity {
99
return { id, isMain } as unknown as GroupEntity;
1010
}
1111

12-
test('user entity has correct type, id, suspended=false, and group parents', (t) => {
12+
test('user entity has correct type, id, suspended=false, and empty parents', (t) => {
1313
const groups = [makeGroup('g1', false), makeGroup('g2', true)];
1414
const entities = buildCedarEntities(userId, groups, connectionId);
1515
const userEntity = entities.find((e) => e.uid.type === 'RocketAdmin::User');
1616
t.truthy(userEntity);
1717
t.is(userEntity.uid.id, userId);
1818
t.is(userEntity.attrs.suspended, false);
19-
t.is(userEntity.parents.length, 2);
20-
t.deepEqual(userEntity.parents[0], { type: 'RocketAdmin::Group', id: 'g1' });
21-
t.deepEqual(userEntity.parents[1], { type: 'RocketAdmin::Group', id: 'g2' });
19+
t.deepEqual(userEntity.parents, []);
2220
});
2321

2422
test('group entities have correct type, isMain attribute, connectionId attribute, empty parents', (t) => {

‎backend/test/ava-tests/non-saas-tests/non-saas-cedar-policy-generator.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ function makePermissions(overrides: Partial<IComplexPermission> = {}): IComplexP
1717

1818
test('isMain=true generates a single wildcard permit', (t) => {
1919
const result = generateCedarPolicyForGroup(groupId, connectionId, true, makePermissions());
20-
t.true(result.includes('principal in RocketAdmin::Group::"test-group-id"'));
20+
t.true(result.includes('principal,'));
2121
t.true(result.includes('action,'));
2222
t.true(result.includes('resource'));
2323
// Should be a single policy
@@ -307,7 +307,7 @@ test('resource ref format validation', (t) => {
307307
],
308308
}),
309309
);
310-
t.true(result.includes(`RocketAdmin::Group::"${groupId}"`));
310+
t.true(result.includes('principal,'));
311311
t.true(result.includes(`RocketAdmin::Connection::"${connectionId}"`));
312312
t.true(result.includes(`RocketAdmin::Table::"${connectionId}/users"`));
313313
});

0 commit comments

Comments
 (0)