From 4befd1c60a3e7bdf62ed12d79d55fb37d54d27ee Mon Sep 17 00:00:00 2001 From: Artem Niehrieiev Date: Thu, 19 Mar 2026 13:57:52 +0000 Subject: [PATCH 1/3] refactor: enhance user permission checks and streamline access evaluation logic --- .../cedar-permissions.service.ts | 409 ++++++++++-------- 1 file changed, 230 insertions(+), 179 deletions(-) diff --git a/backend/src/entities/cedar-authorization/cedar-permissions.service.ts b/backend/src/entities/cedar-authorization/cedar-permissions.service.ts index 885ff1cac..f5738c623 100644 --- a/backend/src/entities/cedar-authorization/cedar-permissions.service.ts +++ b/backend/src/entities/cedar-authorization/cedar-permissions.service.ts @@ -6,20 +6,21 @@ import { IGlobalDatabaseContext } from '../../common/application/global-database import { BaseType } from '../../common/data-injection.tokens.js'; import { GroupEntity } from '../group/group.entity.js'; import { ITablePermissionData } from '../permission/permission.interface.js'; -import { - CedarAction, - CedarResourceType, - CEDAR_ACTION_TYPE, - CEDAR_USER_TYPE, -} from './cedar-action-map.js'; +import { CedarAction, CedarResourceType, CEDAR_ACTION_TYPE, CEDAR_USER_TYPE } from './cedar-action-map.js'; import { buildCedarEntities } from './cedar-entity-builder.js'; import { CEDAR_SCHEMA } from './cedar-schema.js'; import * as cedarWasm from '@cedar-policy/cedar-wasm/nodejs'; import { IUserAccessRepository } from '../user-access/repository/user-access.repository.interface.js'; +interface EvalContext { + userGroups: Array; + policies: string[]; +} + @Injectable() export class CedarPermissionsService implements IUserAccessRepository { private readonly schema: Record = CEDAR_SCHEMA as Record; + private suspendedCheckCache = new Map(); constructor( @Inject(BaseType.GLOBAL_DB_CONTEXT) @@ -28,30 +29,111 @@ export class CedarPermissionsService implements IUserAccessRepository { async getUserConnectionAccessLevel(cognitoUserName: string, connectionId: string): Promise { await this.assertUserNotSuspended(cognitoUserName); - const editAllowed = await this.evaluateAction(cognitoUserName, connectionId, CedarAction.ConnectionEdit); - if (editAllowed) return AccessLevelEnum.edit; - const readAllowed = await this.evaluateAction(cognitoUserName, connectionId, CedarAction.ConnectionRead); - if (readAllowed) return AccessLevelEnum.readonly; + const ctx = await this.loadContext(connectionId, cognitoUserName); + if (!ctx) return AccessLevelEnum.none; + + const entities = buildCedarEntities(cognitoUserName, ctx.userGroups, connectionId); + if ( + this.evaluatePolicies( + cognitoUserName, + CedarAction.ConnectionEdit, + CedarResourceType.Connection, + connectionId, + ctx.policies, + entities, + ) + ) { + return AccessLevelEnum.edit; + } + if ( + this.evaluatePolicies( + cognitoUserName, + CedarAction.ConnectionRead, + CedarResourceType.Connection, + connectionId, + ctx.policies, + entities, + ) + ) { + return AccessLevelEnum.readonly; + } return AccessLevelEnum.none; } async checkUserConnectionRead(cognitoUserName: string, connectionId: string): Promise { - const level = await this.getUserConnectionAccessLevel(cognitoUserName, connectionId); - return level === AccessLevelEnum.edit || level === AccessLevelEnum.readonly; + await this.assertUserNotSuspended(cognitoUserName); + const ctx = await this.loadContext(connectionId, cognitoUserName); + if (!ctx) return false; + + const entities = buildCedarEntities(cognitoUserName, ctx.userGroups, connectionId); + return ( + this.evaluatePolicies( + cognitoUserName, + CedarAction.ConnectionRead, + CedarResourceType.Connection, + connectionId, + ctx.policies, + entities, + ) || + this.evaluatePolicies( + cognitoUserName, + CedarAction.ConnectionEdit, + CedarResourceType.Connection, + connectionId, + ctx.policies, + entities, + ) + ); } async checkUserConnectionEdit(cognitoUserName: string, connectionId: string): Promise { - const level = await this.getUserConnectionAccessLevel(cognitoUserName, connectionId); - return level === AccessLevelEnum.edit; + await this.assertUserNotSuspended(cognitoUserName); + const ctx = await this.loadContext(connectionId, cognitoUserName); + if (!ctx) return false; + + const entities = buildCedarEntities(cognitoUserName, ctx.userGroups, connectionId); + return this.evaluatePolicies( + cognitoUserName, + CedarAction.ConnectionEdit, + CedarResourceType.Connection, + connectionId, + ctx.policies, + entities, + ); } async getGroupAccessLevel(cognitoUserName: string, groupId: string): Promise { await this.assertUserNotSuspended(cognitoUserName); const connectionId = await this.getConnectionId(groupId); - const editAllowed = await this.evaluateAction(cognitoUserName, connectionId, CedarAction.GroupEdit, undefined, groupId); - if (editAllowed) return AccessLevelEnum.edit; - const readAllowed = await this.evaluateAction(cognitoUserName, connectionId, CedarAction.GroupRead, undefined, groupId); - if (readAllowed) return AccessLevelEnum.readonly; + const ctx = await this.loadContext(connectionId, cognitoUserName); + if (!ctx) return AccessLevelEnum.none; + + const entities = buildCedarEntities(cognitoUserName, ctx.userGroups, connectionId); + const resourceId = groupId; + if ( + this.evaluatePolicies( + cognitoUserName, + CedarAction.GroupEdit, + CedarResourceType.Group, + resourceId, + ctx.policies, + entities, + ) + ) { + return AccessLevelEnum.edit; + } + if ( + this.evaluatePolicies( + cognitoUserName, + CedarAction.GroupRead, + CedarResourceType.Group, + resourceId, + ctx.policies, + entities, + ) + ) { + return AccessLevelEnum.readonly; + } return AccessLevelEnum.none; } @@ -72,28 +154,12 @@ export class CedarPermissionsService implements IUserAccessRepository { _masterPwd: string, ): Promise { await this.assertUserNotSuspended(cognitoUserName); - const results = await this.evaluateBatch(cognitoUserName, connectionId, [ - CedarAction.TableRead, - CedarAction.TableAdd, - CedarAction.TableEdit, - CedarAction.TableDelete, - ], tableName); - - const canRead = results.get(CedarAction.TableRead); - const canAdd = results.get(CedarAction.TableAdd); - const canEdit = results.get(CedarAction.TableEdit); - const canDelete = results.get(CedarAction.TableDelete); + const ctx = await this.loadContext(connectionId, cognitoUserName); + if (!ctx) { + return { tableName, accessLevel: { visibility: false, readonly: false, add: false, delete: false, edit: false } }; + } - return { - tableName, - accessLevel: { - visibility: canRead || canAdd || canEdit || canDelete, - readonly: canRead && !canAdd && !canEdit && !canDelete, - add: canAdd, - delete: canDelete, - edit: canEdit, - }, - }; + return this.evaluateTablePermissions(cognitoUserName, connectionId, tableName, ctx); } async getUserPermissionsForAvailableTables( @@ -102,51 +168,16 @@ export class CedarPermissionsService implements IUserAccessRepository { tableNames: Array, ): Promise> { await this.assertUserNotSuspended(cognitoUserName); + const ctx = await this.loadContext(connectionId, cognitoUserName); + if (!ctx) return []; - const userGroups = await this.globalDbContext.groupRepository.findAllUserGroupsInConnection(connectionId, cognitoUserName); - if (userGroups.length === 0) { - return []; - } - const groupPolicies = await this.loadPoliciesPerGroup(connectionId, userGroups); - if (groupPolicies.length === 0) { - return []; - } - - const actions = [CedarAction.TableRead, CedarAction.TableAdd, CedarAction.TableEdit, CedarAction.TableDelete]; const result: Array = []; - for (const tableName of tableNames) { - const entities = buildCedarEntities(cognitoUserName, userGroups, connectionId, tableName); - const actionResults = new Map(); - - for (const action of actions) { - const resourceId = `${connectionId}/${tableName}`; - const allowed = this.evaluatePolicies( - cognitoUserName, action, CedarResourceType.Table, resourceId, groupPolicies, entities, - ); - actionResults.set(action, allowed); - } - - const canRead = actionResults.get(CedarAction.TableRead); - const canAdd = actionResults.get(CedarAction.TableAdd); - const canEdit = actionResults.get(CedarAction.TableEdit); - const canDelete = actionResults.get(CedarAction.TableDelete); - const visibility = canRead || canAdd || canEdit || canDelete; - - if (visibility) { - result.push({ - tableName, - accessLevel: { - visibility: true, - readonly: canRead && !canAdd && !canEdit && !canDelete, - add: canAdd, - delete: canDelete, - edit: canEdit, - }, - }); + const perm = this.evaluateTablePermissions(cognitoUserName, connectionId, tableName, ctx); + if (perm.accessLevel.visibility) { + result.push(perm); } } - return result; } @@ -157,7 +188,18 @@ export class CedarPermissionsService implements IUserAccessRepository { _masterPwd: string, ): Promise { await this.assertUserNotSuspended(cognitoUserName); - return this.evaluateAction(cognitoUserName, connectionId, CedarAction.TableRead, tableName); + 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, + ); } async checkTableAdd( @@ -167,7 +209,18 @@ export class CedarPermissionsService implements IUserAccessRepository { _masterPwd: string, ): Promise { await this.assertUserNotSuspended(cognitoUserName); - return this.evaluateAction(cognitoUserName, connectionId, CedarAction.TableAdd, tableName); + const ctx = await this.loadContext(connectionId, cognitoUserName); + if (!ctx) return false; + + const entities = buildCedarEntities(cognitoUserName, ctx.userGroups, connectionId, tableName); + return this.evaluatePolicies( + cognitoUserName, + CedarAction.TableAdd, + CedarResourceType.Table, + `${connectionId}/${tableName}`, + ctx.policies, + entities, + ); } async checkTableDelete( @@ -177,7 +230,18 @@ export class CedarPermissionsService implements IUserAccessRepository { _masterPwd: string, ): Promise { await this.assertUserNotSuspended(cognitoUserName); - return this.evaluateAction(cognitoUserName, connectionId, CedarAction.TableDelete, tableName); + const ctx = await this.loadContext(connectionId, cognitoUserName); + if (!ctx) return false; + + const entities = buildCedarEntities(cognitoUserName, ctx.userGroups, connectionId, tableName); + return this.evaluatePolicies( + cognitoUserName, + CedarAction.TableDelete, + CedarResourceType.Table, + `${connectionId}/${tableName}`, + ctx.policies, + entities, + ); } async checkTableEdit( @@ -187,7 +251,18 @@ export class CedarPermissionsService implements IUserAccessRepository { _masterPwd: string, ): Promise { await this.assertUserNotSuspended(cognitoUserName); - return this.evaluateAction(cognitoUserName, connectionId, CedarAction.TableEdit, tableName); + const ctx = await this.loadContext(connectionId, cognitoUserName); + if (!ctx) return false; + + const entities = buildCedarEntities(cognitoUserName, ctx.userGroups, connectionId, tableName); + return this.evaluatePolicies( + cognitoUserName, + CedarAction.TableEdit, + CedarResourceType.Table, + `${connectionId}/${tableName}`, + ctx.policies, + entities, + ); } async getConnectionId(groupId: string): Promise { @@ -198,7 +273,12 @@ export class CedarPermissionsService implements IUserAccessRepository { return group.connection.id; } - async improvedCheckTableRead(userId: string, connectionId: string, tableName: string, _masterPwd?: string): Promise { + async improvedCheckTableRead( + userId: string, + connectionId: string, + tableName: string, + _masterPwd?: string, + ): Promise { const cachedReadPermission: boolean | null = Cacher.getUserTableReadPermissionCache( userId, connectionId, @@ -208,59 +288,63 @@ export class CedarPermissionsService implements IUserAccessRepository { return cachedReadPermission; } - const canRead = await this.evaluateAction(userId, connectionId, CedarAction.TableRead, tableName); + const canRead = await this.checkTableRead(userId, connectionId, tableName, undefined); Cacher.setUserTableReadPermissionCache(userId, connectionId, tableName, canRead); return canRead; } - private async evaluateBatch( + private evaluateTablePermissions( userId: string, connectionId: string, - actions: CedarAction[], - tableName?: string, - groupId?: string, - ): Promise> { - const userGroups = await this.globalDbContext.groupRepository.findAllUserGroupsInConnection(connectionId, userId); - if (userGroups.length === 0) { - return new Map(actions.map(a => [a, false])); - } - - const groupPolicies = await this.loadPoliciesPerGroup(connectionId, userGroups); - if (groupPolicies.length === 0) { - return new Map(actions.map(a => [a, false])); - } - - const dashboardId = undefined; - const entities = buildCedarEntities(userId, userGroups, connectionId, tableName, dashboardId); - - const results = new Map(); - for (const action of actions) { - const actionPrefix = action.split(':')[0]; - let resourceType: CedarResourceType; - let resourceId: string; - - switch (actionPrefix) { - case 'connection': - resourceType = CedarResourceType.Connection; - resourceId = connectionId; - break; - case 'group': - resourceType = CedarResourceType.Group; - resourceId = groupId; - break; - case 'table': - resourceType = CedarResourceType.Table; - resourceId = `${connectionId}/${tableName}`; - break; - default: - results.set(action, false); - continue; - } + tableName: string, + ctx: EvalContext, + ): ITablePermissionData { + const entities = buildCedarEntities(userId, ctx.userGroups, connectionId, tableName); + const resourceId = `${connectionId}/${tableName}`; - results.set(action, this.evaluatePolicies(userId, action, resourceType, resourceId, groupPolicies, entities)); - } + const canRead = this.evaluatePolicies( + userId, + CedarAction.TableRead, + CedarResourceType.Table, + resourceId, + ctx.policies, + entities, + ); + const canAdd = this.evaluatePolicies( + userId, + CedarAction.TableAdd, + CedarResourceType.Table, + resourceId, + ctx.policies, + entities, + ); + const canEdit = this.evaluatePolicies( + userId, + CedarAction.TableEdit, + CedarResourceType.Table, + resourceId, + ctx.policies, + entities, + ); + const canDelete = this.evaluatePolicies( + userId, + CedarAction.TableDelete, + CedarResourceType.Table, + resourceId, + ctx.policies, + entities, + ); - return results; + return { + tableName, + accessLevel: { + visibility: canRead || canAdd || canEdit || canDelete, + readonly: canRead && !canAdd && !canEdit && !canDelete, + add: canAdd, + delete: canDelete, + edit: canEdit, + }, + }; } private evaluatePolicies( @@ -290,66 +374,33 @@ export class CedarPermissionsService implements IUserAccessRepository { return false; } - private async evaluateAction( - userId: string, - connectionId: string, - action: CedarAction, - tableName?: string, - groupId?: string, - ): Promise { + private async loadContext(connectionId: string, userId: string): Promise { const userGroups = await this.globalDbContext.groupRepository.findAllUserGroupsInConnection(connectionId, userId); - if (userGroups.length === 0) return false; - - const groupPolicies = await this.loadPoliciesPerGroup(connectionId, userGroups); - if (groupPolicies.length === 0) return false; - - const entities = buildCedarEntities(userId, userGroups, connectionId, tableName); - - const actionPrefix = action.split(':')[0]; - let resourceType: CedarResourceType; - let resourceId: string; - - switch (actionPrefix) { - case 'connection': - resourceType = CedarResourceType.Connection; - resourceId = connectionId; - break; - case 'group': - resourceType = CedarResourceType.Group; - resourceId = groupId; - break; - case 'table': - resourceType = CedarResourceType.Table; - resourceId = `${connectionId}/${tableName}`; - break; - default: - return false; - } + if (userGroups.length === 0) return null; - return this.evaluatePolicies(userId, action, resourceType, resourceId, groupPolicies, entities); - } + const policies = userGroups.map((g) => g.cedarPolicy).filter(Boolean); + if (policies.length === 0) return null; - private async loadPoliciesPerGroup(connectionId: string, userGroups: Array): Promise { - const groups = await this.globalDbContext.groupRepository.findAllGroupsInConnection(connectionId); - const userGroupIdSet = new Set(userGroups.map((g) => g.id)); - return groups - .filter((g) => userGroupIdSet.has(g.id)) - .map((g) => g.cedarPolicy) - .filter(Boolean); + return { userGroups, policies }; } private async assertUserNotSuspended(userId: string): Promise { + const cached = this.suspendedCheckCache.get(userId); + if (cached !== undefined) { + if (cached) { + throw new HttpException({ message: Messages.ACCOUNT_SUSPENDED }, HttpStatus.FORBIDDEN); + } + return; + } + const user = await this.globalDbContext.userRepository.findOne({ where: { id: userId }, select: ['id', 'suspended'], }); - if (user?.suspended) { - throw new HttpException( - { - message: Messages.ACCOUNT_SUSPENDED, - }, - HttpStatus.FORBIDDEN, - ); + const isSuspended = !!user?.suspended; + this.suspendedCheckCache.set(userId, isSuspended); + if (isSuspended) { + throw new HttpException({ message: Messages.ACCOUNT_SUSPENDED }, HttpStatus.FORBIDDEN); } } } From ac5270fe34072014483edd690c5c3cb432e2fcff Mon Sep 17 00:00:00 2001 From: Artem Niehrieiev Date: Thu, 19 Mar 2026 14:27:50 +0000 Subject: [PATCH 2/3] refactor: remove suspended user check cache for improved clarity and performance --- .../cedar-permissions.service.ts | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/backend/src/entities/cedar-authorization/cedar-permissions.service.ts b/backend/src/entities/cedar-authorization/cedar-permissions.service.ts index f5738c623..0eb1da1ed 100644 --- a/backend/src/entities/cedar-authorization/cedar-permissions.service.ts +++ b/backend/src/entities/cedar-authorization/cedar-permissions.service.ts @@ -20,7 +20,6 @@ interface EvalContext { @Injectable() export class CedarPermissionsService implements IUserAccessRepository { private readonly schema: Record = CEDAR_SCHEMA as Record; - private suspendedCheckCache = new Map(); constructor( @Inject(BaseType.GLOBAL_DB_CONTEXT) @@ -385,21 +384,11 @@ export class CedarPermissionsService implements IUserAccessRepository { } private async assertUserNotSuspended(userId: string): Promise { - const cached = this.suspendedCheckCache.get(userId); - if (cached !== undefined) { - if (cached) { - throw new HttpException({ message: Messages.ACCOUNT_SUSPENDED }, HttpStatus.FORBIDDEN); - } - return; - } - const user = await this.globalDbContext.userRepository.findOne({ where: { id: userId }, select: ['id', 'suspended'], }); - const isSuspended = !!user?.suspended; - this.suspendedCheckCache.set(userId, isSuspended); - if (isSuspended) { + if (user?.suspended) { throw new HttpException({ message: Messages.ACCOUNT_SUSPENDED }, HttpStatus.FORBIDDEN); } } From 5227e27e77916e3e2a8abe757558d2d115191638 Mon Sep 17 00:00:00 2001 From: Artem Niehrieiev Date: Thu, 19 Mar 2026 14:30:55 +0000 Subject: [PATCH 3/3] refactor: remove user suspension check (already checked in auth middlewares) --- .../cedar-permissions.service.ts | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/backend/src/entities/cedar-authorization/cedar-permissions.service.ts b/backend/src/entities/cedar-authorization/cedar-permissions.service.ts index 0eb1da1ed..7d85424d0 100644 --- a/backend/src/entities/cedar-authorization/cedar-permissions.service.ts +++ b/backend/src/entities/cedar-authorization/cedar-permissions.service.ts @@ -27,7 +27,6 @@ export class CedarPermissionsService implements IUserAccessRepository { ) {} async getUserConnectionAccessLevel(cognitoUserName: string, connectionId: string): Promise { - await this.assertUserNotSuspended(cognitoUserName); const ctx = await this.loadContext(connectionId, cognitoUserName); if (!ctx) return AccessLevelEnum.none; @@ -60,7 +59,6 @@ export class CedarPermissionsService implements IUserAccessRepository { } async checkUserConnectionRead(cognitoUserName: string, connectionId: string): Promise { - await this.assertUserNotSuspended(cognitoUserName); const ctx = await this.loadContext(connectionId, cognitoUserName); if (!ctx) return false; @@ -86,7 +84,6 @@ export class CedarPermissionsService implements IUserAccessRepository { } async checkUserConnectionEdit(cognitoUserName: string, connectionId: string): Promise { - await this.assertUserNotSuspended(cognitoUserName); const ctx = await this.loadContext(connectionId, cognitoUserName); if (!ctx) return false; @@ -102,7 +99,6 @@ export class CedarPermissionsService implements IUserAccessRepository { } async getGroupAccessLevel(cognitoUserName: string, groupId: string): Promise { - await this.assertUserNotSuspended(cognitoUserName); const connectionId = await this.getConnectionId(groupId); const ctx = await this.loadContext(connectionId, cognitoUserName); if (!ctx) return AccessLevelEnum.none; @@ -152,7 +148,6 @@ export class CedarPermissionsService implements IUserAccessRepository { tableName: string, _masterPwd: string, ): Promise { - await this.assertUserNotSuspended(cognitoUserName); const ctx = await this.loadContext(connectionId, cognitoUserName); if (!ctx) { return { tableName, accessLevel: { visibility: false, readonly: false, add: false, delete: false, edit: false } }; @@ -166,7 +161,6 @@ export class CedarPermissionsService implements IUserAccessRepository { connectionId: string, tableNames: Array, ): Promise> { - await this.assertUserNotSuspended(cognitoUserName); const ctx = await this.loadContext(connectionId, cognitoUserName); if (!ctx) return []; @@ -186,7 +180,6 @@ export class CedarPermissionsService implements IUserAccessRepository { tableName: string, _masterPwd: string, ): Promise { - await this.assertUserNotSuspended(cognitoUserName); const ctx = await this.loadContext(connectionId, cognitoUserName); if (!ctx) return false; @@ -207,7 +200,6 @@ export class CedarPermissionsService implements IUserAccessRepository { tableName: string, _masterPwd: string, ): Promise { - await this.assertUserNotSuspended(cognitoUserName); const ctx = await this.loadContext(connectionId, cognitoUserName); if (!ctx) return false; @@ -228,7 +220,6 @@ export class CedarPermissionsService implements IUserAccessRepository { tableName: string, _masterPwd: string, ): Promise { - await this.assertUserNotSuspended(cognitoUserName); const ctx = await this.loadContext(connectionId, cognitoUserName); if (!ctx) return false; @@ -249,7 +240,6 @@ export class CedarPermissionsService implements IUserAccessRepository { tableName: string, _masterPwd: string, ): Promise { - await this.assertUserNotSuspended(cognitoUserName); const ctx = await this.loadContext(connectionId, cognitoUserName); if (!ctx) return false; @@ -383,13 +373,4 @@ export class CedarPermissionsService implements IUserAccessRepository { return { userGroups, policies }; } - private async assertUserNotSuspended(userId: string): Promise { - const user = await this.globalDbContext.userRepository.findOne({ - where: { id: userId }, - select: ['id', 'suspended'], - }); - if (user?.suspended) { - throw new HttpException({ message: Messages.ACCOUNT_SUSPENDED }, HttpStatus.FORBIDDEN); - } - } }