From 8448087d03861c8f6fabeae22f30bb7153e2e3c3 Mon Sep 17 00:00:00 2001 From: Anthony Guimard Date: Tue, 4 Aug 2026 19:13:19 +0200 Subject: [PATCH 1/2] feat(agent-bff): fork the permission evaluator with a drift gate --- .../src/permissions/action-identifiers.ts | 25 ++ .../src/permissions/action-permissions.ts | 230 ++++++++++++++++ .../permissions/action-permissions.test.ts | 258 ++++++++++++++++++ .../test/permissions/evaluator-drift.test.ts | 144 ++++++++++ 4 files changed, 657 insertions(+) create mode 100644 packages/agent-bff/src/permissions/action-identifiers.ts create mode 100644 packages/agent-bff/src/permissions/action-permissions.ts create mode 100644 packages/agent-bff/test/permissions/action-permissions.test.ts create mode 100644 packages/agent-bff/test/permissions/evaluator-drift.test.ts diff --git a/packages/agent-bff/src/permissions/action-identifiers.ts b/packages/agent-bff/src/permissions/action-identifiers.ts new file mode 100644 index 0000000000..ab894d4956 --- /dev/null +++ b/packages/agent-bff/src/permissions/action-identifiers.ts @@ -0,0 +1,25 @@ +import { CollectionActionEvent } from '@forestadmin/forestadmin-client'; + +export { CollectionActionEvent }; + +export enum CustomActionEvent { + Trigger = 'trigger', + Approve = 'approve', + SelfApprove = 'self-approve', + RequireApproval = 'require-approval', +} + +export function generateCustomActionIdentifier( + actionEventName: CustomActionEvent, + customActionName: string, + collectionName: string, +): string { + return `custom:${collectionName}:${customActionName}:${actionEventName}`; +} + +export function generateCollectionActionIdentifier( + action: CollectionActionEvent, + collectionName: string, +): string { + return `collection:${collectionName}:${action}`; +} diff --git a/packages/agent-bff/src/permissions/action-permissions.ts b/packages/agent-bff/src/permissions/action-permissions.ts new file mode 100644 index 0000000000..0ca13647ef --- /dev/null +++ b/packages/agent-bff/src/permissions/action-permissions.ts @@ -0,0 +1,230 @@ +import { + CollectionActionEvent, + CustomActionEvent, + generateCollectionActionIdentifier, + generateCustomActionIdentifier, +} from './action-identifiers'; + +export type RightDescriptionWithRolesV4 = { roles: number[] }; +export type RightDescriptionV4 = boolean | RightDescriptionWithRolesV4; + +export type RightConditionByRolesV4 = { + roleId: number; + filter: unknown; +}; + +export interface EnvironmentCollectionAccessPermissionsV4 { + browseEnabled: RightDescriptionV4; + readEnabled: RightDescriptionV4; + editEnabled: RightDescriptionV4; + addEnabled: RightDescriptionV4; + deleteEnabled: RightDescriptionV4; + exportEnabled: RightDescriptionV4; +} + +export interface EnvironmentSmartActionPermissionsV4 { + triggerEnabled: RightDescriptionV4; + triggerConditions: RightConditionByRolesV4[]; + approvalRequired: RightDescriptionV4; + approvalRequiredConditions: RightConditionByRolesV4[]; + userApprovalEnabled: RightDescriptionV4; + userApprovalConditions: RightConditionByRolesV4[]; + selfApprovalEnabled: RightDescriptionV4; +} + +export interface EnvironmentCollectionActionPermissionsV4 { + [actionName: string]: EnvironmentSmartActionPermissionsV4; +} + +export interface EnvironmentCollectionPermissionsV4 { + collection: EnvironmentCollectionAccessPermissionsV4; + actions: EnvironmentCollectionActionPermissionsV4; +} + +export interface EnvironmentCollectionsPermissionsV4 { + [collectionName: string]: EnvironmentCollectionPermissionsV4; +} + +export type EnvironmentPermissionsV4Remote = { + collections: EnvironmentCollectionsPermissionsV4; +}; + +export type EnvironmentPermissionsV4 = EnvironmentPermissionsV4Remote | true; + +export type ActionPermission = { + allowedRoles: Set; + conditionsByRole?: Map; +}; + +export type ActionPermissions = { + isDevelopment: boolean; + actionsGloballyAllowed: Set; + actionsByRole: Map; +}; + +type IntermediateRightsList = { + [key: string]: { + description: RightDescriptionV4; + conditions?: RightConditionByRolesV4[]; + }; +}; + +function buildCollectionRights( + permissions: EnvironmentCollectionsPermissionsV4, +): IntermediateRightsList { + return Object.entries(permissions).reduce((acc, [collectionId, collectionPermissions]) => { + const { collection } = collectionPermissions; + + return { + ...acc, + [generateCollectionActionIdentifier(CollectionActionEvent.Browse, collectionId)]: { + description: collection.browseEnabled, + }, + [generateCollectionActionIdentifier(CollectionActionEvent.Read, collectionId)]: { + description: collection.readEnabled, + }, + [generateCollectionActionIdentifier(CollectionActionEvent.Edit, collectionId)]: { + description: collection.editEnabled, + }, + [generateCollectionActionIdentifier(CollectionActionEvent.Add, collectionId)]: { + description: collection.addEnabled, + }, + [generateCollectionActionIdentifier(CollectionActionEvent.Delete, collectionId)]: { + description: collection.deleteEnabled, + }, + [generateCollectionActionIdentifier(CollectionActionEvent.Export, collectionId)]: { + description: collection.exportEnabled, + }, + }; + }, {}); +} + +function buildCustomActionRights( + collectionId: string, + actions: EnvironmentCollectionActionPermissionsV4, +): IntermediateRightsList { + return Object.entries(actions).reduce( + (acc, [actionName, actionPermissions]) => ({ + ...acc, + [generateCustomActionIdentifier(CustomActionEvent.Approve, actionName, collectionId)]: { + description: actionPermissions.userApprovalEnabled, + conditions: actionPermissions.userApprovalConditions, + }, + [generateCustomActionIdentifier(CustomActionEvent.SelfApprove, actionName, collectionId)]: { + description: actionPermissions.selfApprovalEnabled, + }, + [generateCustomActionIdentifier(CustomActionEvent.Trigger, actionName, collectionId)]: { + description: actionPermissions.triggerEnabled, + conditions: actionPermissions.triggerConditions, + }, + [generateCustomActionIdentifier(CustomActionEvent.RequireApproval, actionName, collectionId)]: + { + description: actionPermissions.approvalRequired, + conditions: actionPermissions.approvalRequiredConditions, + }, + }), + {}, + ); +} + +function buildActionRights( + permissions: EnvironmentCollectionsPermissionsV4, +): IntermediateRightsList { + return Object.entries(permissions).reduce( + (acc, [collectionId, collectionPermissions]) => ({ + ...acc, + ...buildCustomActionRights(collectionId, collectionPermissions.actions), + }), + {}, + ); +} + +function collectGloballyAllowed(rights: IntermediateRightsList): Set { + return new Set( + Object.entries(rights) + .filter(([, right]) => right.description === true) + .map(([action]) => action), + ); +} + +function collectByRole(rights: IntermediateRightsList): Map { + return new Map( + Object.entries(rights) + .filter(([, right]) => typeof right.description !== 'boolean') + .map(([name, right]) => [ + name, + { + allowedRoles: new Set((right.description as RightDescriptionWithRolesV4).roles), + ...(right.conditions + ? { + conditionsByRole: new Map( + right.conditions.map(({ roleId, filter }) => [roleId, filter]), + ), + } + : {}), + }, + ]), + ); +} + +export function buildActionPermissions( + environmentPermissions: EnvironmentPermissionsV4, +): ActionPermissions { + if (environmentPermissions === true) { + return { + isDevelopment: true, + actionsGloballyAllowed: new Set(), + actionsByRole: new Map(), + }; + } + + const rights = { + ...buildCollectionRights(environmentPermissions.collections), + ...buildActionRights(environmentPermissions.collections), + }; + + return { + isDevelopment: false, + actionsGloballyAllowed: collectGloballyAllowed(rights), + actionsByRole: collectByRole(rights), + }; +} + +export function canRoleTriggerAction( + permissions: ActionPermissions, + roleId: number, + actionName: string, +): boolean { + return Boolean( + permissions.isDevelopment || + permissions.actionsGloballyAllowed.has(actionName) || + permissions.actionsByRole.get(actionName)?.allowedRoles.has(roleId), + ); +} + +export function canRolePerformCollectionAction( + permissions: ActionPermissions, + roleId: number, + action: CollectionActionEvent, + collectionName: string, +): boolean { + return canRoleTriggerAction( + permissions, + roleId, + generateCollectionActionIdentifier(action, collectionName), + ); +} + +export function canRolePerformCustomAction( + permissions: ActionPermissions, + roleId: number, + event: CustomActionEvent, + actionName: string, + collectionName: string, +): boolean { + return canRoleTriggerAction( + permissions, + roleId, + generateCustomActionIdentifier(event, actionName, collectionName), + ); +} diff --git a/packages/agent-bff/test/permissions/action-permissions.test.ts b/packages/agent-bff/test/permissions/action-permissions.test.ts new file mode 100644 index 0000000000..2d0406bf74 --- /dev/null +++ b/packages/agent-bff/test/permissions/action-permissions.test.ts @@ -0,0 +1,258 @@ +import type { EnvironmentPermissionsV4 } from '../../src/permissions/action-permissions'; + +import { + CollectionActionEvent, + CustomActionEvent, + generateCollectionActionIdentifier, + generateCustomActionIdentifier, +} from '../../src/permissions/action-identifiers'; +import { + buildActionPermissions, + canRolePerformCollectionAction, + canRolePerformCustomAction, + canRoleTriggerAction, +} from '../../src/permissions/action-permissions'; + +const ADMIN_ROLE = 1; +const VIEWER_ROLE = 2; + +function crud(overrides: Record = {}) { + return { + browseEnabled: { roles: [ADMIN_ROLE] }, + readEnabled: { roles: [ADMIN_ROLE] }, + editEnabled: { roles: [ADMIN_ROLE] }, + addEnabled: { roles: [ADMIN_ROLE] }, + deleteEnabled: { roles: [ADMIN_ROLE] }, + exportEnabled: { roles: [ADMIN_ROLE] }, + ...overrides, + }; +} + +function smartAction(overrides: Record = {}) { + return { + triggerEnabled: { roles: [ADMIN_ROLE] }, + triggerConditions: [], + approvalRequired: { roles: [ADMIN_ROLE] }, + approvalRequiredConditions: [], + userApprovalEnabled: { roles: [ADMIN_ROLE] }, + userApprovalConditions: [], + selfApprovalEnabled: { roles: [ADMIN_ROLE] }, + ...overrides, + }; +} + +const NORMAL_MODE = { + collections: { + users: { collection: crud(), actions: { 'Block user': smartAction() } }, + }, +} as unknown as EnvironmentPermissionsV4; + +describe('buildActionPermissions', () => { + describe('when the environment permissions are literally true', () => { + it('should flag development and return empty collections', () => { + expect(buildActionPermissions(true)).toEqual({ + isDevelopment: true, + actionsGloballyAllowed: new Set(), + actionsByRole: new Map(), + }); + }); + }); + + describe('when a right is granted to every role', () => { + it('should place the identifier in actionsGloballyAllowed rather than keying it by role', () => { + const permissions = buildActionPermissions({ + collections: { + users: { collection: crud({ browseEnabled: true }), actions: {} }, + }, + } as unknown as EnvironmentPermissionsV4); + const browse = generateCollectionActionIdentifier(CollectionActionEvent.Browse, 'users'); + + expect(permissions.actionsGloballyAllowed.has(browse)).toBe(true); + expect(permissions.actionsByRole.has(browse)).toBe(false); + }); + }); + + describe('when a right is granted to specific roles', () => { + it('should key the identifier by those roles', () => { + const permissions = buildActionPermissions(NORMAL_MODE); + const read = generateCollectionActionIdentifier(CollectionActionEvent.Read, 'users'); + + expect(permissions.actionsByRole.get(read)?.allowedRoles).toEqual(new Set([ADMIN_ROLE])); + }); + }); + + describe('when a right carries conditions', () => { + it('should map them by role id', () => { + const filter = { field: 'id', operator: 'equal', value: 1 }; + const permissions = buildActionPermissions({ + collections: { + users: { + collection: crud(), + actions: { + 'Block user': smartAction({ + triggerConditions: [{ roleId: VIEWER_ROLE, filter }], + }), + }, + }, + }, + } as unknown as EnvironmentPermissionsV4); + const trigger = generateCustomActionIdentifier( + CustomActionEvent.Trigger, + 'Block user', + 'users', + ); + + expect(permissions.actionsByRole.get(trigger)?.conditionsByRole).toEqual( + new Map([[VIEWER_ROLE, filter]]), + ); + }); + }); + + describe('when a right is denied to everyone', () => { + it('should expose it in neither collection', () => { + const permissions = buildActionPermissions({ + collections: { + users: { collection: crud({ deleteEnabled: false }), actions: {} }, + }, + } as unknown as EnvironmentPermissionsV4); + const remove = generateCollectionActionIdentifier(CollectionActionEvent.Delete, 'users'); + + expect(permissions.actionsGloballyAllowed.has(remove)).toBe(false); + expect(permissions.actionsByRole.has(remove)).toBe(false); + }); + }); +}); + +describe('canRoleTriggerAction', () => { + describe('when the environment is in development', () => { + it('should allow an action no descriptor mentions', () => { + expect(canRoleTriggerAction(buildActionPermissions(true), VIEWER_ROLE, 'anything')).toBe( + true, + ); + }); + }); + + describe('when the action is globally allowed', () => { + it('should allow a role the descriptor never named', () => { + const permissions = buildActionPermissions({ + collections: { + users: { collection: crud({ browseEnabled: true }), actions: {} }, + }, + } as unknown as EnvironmentPermissionsV4); + + expect( + canRolePerformCollectionAction( + permissions, + VIEWER_ROLE, + CollectionActionEvent.Browse, + 'users', + ), + ).toBe(true); + }); + }); + + describe('when the action is restricted to roles', () => { + it.each([ + ['the named role', ADMIN_ROLE, true], + ['a role mismatch', VIEWER_ROLE, false], + ])('should return %s -> %s', (_label, roleId, expected) => { + expect( + canRolePerformCollectionAction( + buildActionPermissions(NORMAL_MODE), + roleId as number, + CollectionActionEvent.Read, + 'users', + ), + ).toBe(expected); + }); + }); + + describe('when the identifier is absent from the permissions', () => { + it('should deny rather than throw', () => { + expect( + canRoleTriggerAction( + buildActionPermissions(NORMAL_MODE), + ADMIN_ROLE, + 'collection:ghost:browse', + ), + ).toBe(false); + }); + }); + + it.each([ + [CollectionActionEvent.Browse], + [CollectionActionEvent.Read], + [CollectionActionEvent.Edit], + [CollectionActionEvent.Add], + [CollectionActionEvent.Delete], + [CollectionActionEvent.Export], + ])('should resolve the %s collection right for the named role', action => { + expect( + canRolePerformCollectionAction( + buildActionPermissions(NORMAL_MODE), + ADMIN_ROLE, + action, + 'users', + ), + ).toBe(true); + }); + + it.each([ + [CustomActionEvent.Trigger], + [CustomActionEvent.Approve], + [CustomActionEvent.SelfApprove], + [CustomActionEvent.RequireApproval], + ])('should resolve the %s custom-action event for the named role', event => { + expect( + canRolePerformCustomAction( + buildActionPermissions(NORMAL_MODE), + ADMIN_ROLE, + event, + 'Block user', + 'users', + ), + ).toBe(true); + }); + + describe('when an action-event flag is missing from the payload', () => { + it('should throw, matching the source evaluator rather than inventing a denial', () => { + const { selfApprovalEnabled, ...withoutSelfApproval } = smartAction(); + + expect(selfApprovalEnabled).toBeDefined(); + expect(() => + buildActionPermissions({ + collections: { + users: { collection: crud(), actions: { 'Block user': withoutSelfApproval } }, + }, + } as unknown as EnvironmentPermissionsV4), + ).toThrow(TypeError); + }); + }); + + describe('when a CRUD descriptor is missing from the payload', () => { + it('should throw, matching the source evaluator', () => { + const { deleteEnabled, ...withoutDelete } = crud(); + + expect(deleteEnabled).toBeDefined(); + expect(() => + buildActionPermissions({ + collections: { users: { collection: withoutDelete, actions: {} } }, + } as unknown as EnvironmentPermissionsV4), + ).toThrow(TypeError); + }); + }); +}); + +describe('identifier builders', () => { + it('should build a collection-action identifier', () => { + expect(generateCollectionActionIdentifier(CollectionActionEvent.Browse, 'users')).toBe( + 'collection:users:browse', + ); + }); + + it('should build a custom-action identifier', () => { + expect( + generateCustomActionIdentifier(CustomActionEvent.RequireApproval, 'Block user', 'users'), + ).toBe('custom:users:Block user:require-approval'); + }); +}); diff --git a/packages/agent-bff/test/permissions/evaluator-drift.test.ts b/packages/agent-bff/test/permissions/evaluator-drift.test.ts new file mode 100644 index 0000000000..7fecc8c814 --- /dev/null +++ b/packages/agent-bff/test/permissions/evaluator-drift.test.ts @@ -0,0 +1,144 @@ +import type { EnvironmentPermissionsV4 } from '../../src/permissions/action-permissions'; + +import sourceActionPermissionService from '@forestadmin/forestadmin-client/dist/permissions/action-permission'; +import sourceGenerateActionsFromPermissions from '@forestadmin/forestadmin-client/dist/permissions/generate-actions-from-permissions'; + +import { + CollectionActionEvent, + CustomActionEvent, + generateCollectionActionIdentifier, + generateCustomActionIdentifier, +} from '../../src/permissions/action-identifiers'; +import { + buildActionPermissions, + canRoleTriggerAction, +} from '../../src/permissions/action-permissions'; + +const ADMIN_ROLE = 1; +const VIEWER_ROLE = 2; +const UNKNOWN_ROLE = 99; + +const COLLECTION = 'users'; +const ACTION = 'Block user'; + +const CONDITION = { field: 'id', operator: 'equal', value: 1 }; + +function crud(overrides: Record = {}) { + return { + browseEnabled: { roles: [ADMIN_ROLE] }, + readEnabled: true, + editEnabled: { roles: [ADMIN_ROLE, VIEWER_ROLE] }, + addEnabled: false, + deleteEnabled: { roles: [] }, + exportEnabled: { roles: [VIEWER_ROLE] }, + ...overrides, + }; +} + +function smartAction(overrides: Record = {}) { + return { + triggerEnabled: { roles: [ADMIN_ROLE] }, + triggerConditions: [{ roleId: VIEWER_ROLE, filter: CONDITION }], + approvalRequired: true, + approvalRequiredConditions: [], + userApprovalEnabled: { roles: [VIEWER_ROLE] }, + userApprovalConditions: [], + selfApprovalEnabled: false, + ...overrides, + }; +} + +const FIXTURES: [string, EnvironmentPermissionsV4][] = [ + ['a development environment', true as EnvironmentPermissionsV4], + [ + 'a normal environment mixing global, per-role, denied and empty-role rights', + { + collections: { + [COLLECTION]: { collection: crud(), actions: { [ACTION]: smartAction() } }, + posts: { + collection: crud({ browseEnabled: true, exportEnabled: false }), + actions: { Publish: smartAction({ triggerEnabled: true }) }, + }, + }, + } as unknown as EnvironmentPermissionsV4, + ], + [ + 'a normal environment with no collection at all', + { collections: {} } as unknown as EnvironmentPermissionsV4, + ], +]; + +const IDENTIFIERS = [ + ...[ + CollectionActionEvent.Browse, + CollectionActionEvent.Read, + CollectionActionEvent.Edit, + CollectionActionEvent.Add, + CollectionActionEvent.Delete, + CollectionActionEvent.Export, + ].flatMap(action => [ + generateCollectionActionIdentifier(action, COLLECTION), + generateCollectionActionIdentifier(action, 'posts'), + ]), + ...[ + CustomActionEvent.Trigger, + CustomActionEvent.Approve, + CustomActionEvent.SelfApprove, + CustomActionEvent.RequireApproval, + ].flatMap(event => [ + generateCustomActionIdentifier(event, ACTION, COLLECTION), + generateCustomActionIdentifier(event, 'Publish', 'posts'), + ]), + 'collection:ghost:browse', +]; + +const ROLES = [ADMIN_ROLE, VIEWER_ROLE, UNKNOWN_ROLE]; + +function sourceServiceFor(permissions: EnvironmentPermissionsV4) { + const options = { + permissionsCacheDurationInSeconds: 60, + instantCacheRefresh: true, + logger: () => {}, + }; + const serverInterface = { getEnvironmentPermissions: async () => permissions }; + + const SourceService = sourceActionPermissionService as unknown as new ( + serviceOptions: unknown, + server: unknown, + ) => { can(roleId: number, actionName: string): Promise }; + + return new SourceService(options, serverInterface); +} + +describe('the forked evaluator does not drift from forestadmin-client', () => { + describe.each(FIXTURES)('given %s', (_label, permissions) => { + it('should produce the same transformation as the source', () => { + const source = ( + sourceGenerateActionsFromPermissions as unknown as ( + input: EnvironmentPermissionsV4, + ) => unknown + )(permissions); + + expect(buildActionPermissions(permissions)).toEqual(source); + }); + + it('should return the same verdict as the source for every role and identifier', async () => { + const forked = buildActionPermissions(permissions); + const service = sourceServiceFor(permissions); + + const comparisons = await Promise.all( + ROLES.flatMap(roleId => + IDENTIFIERS.map(async identifier => ({ + identifier, + roleId, + source: await service.can(roleId, identifier), + forked: canRoleTriggerAction(forked, roleId, identifier), + })), + ), + ); + + expect(comparisons.filter(row => row.source !== row.forked)).toEqual([]); + expect(comparisons).toHaveLength(ROLES.length * IDENTIFIERS.length); + }); + }); +}); From acd8894025347cf965935f61ece773d0dc9d0ba0 Mon Sep 17 00:00:00 2001 From: Anthony Guimard Date: Tue, 4 Aug 2026 19:21:23 +0200 Subject: [PATCH 2/2] fix: add review feedbacks --- .../src/permissions/action-permissions.ts | 79 +++++++------------ .../permissions/action-permissions.test.ts | 32 ++++---- .../test/permissions/evaluator-drift.test.ts | 75 ++++++++++++++++-- 3 files changed, 114 insertions(+), 72 deletions(-) diff --git a/packages/agent-bff/src/permissions/action-permissions.ts b/packages/agent-bff/src/permissions/action-permissions.ts index 0ca13647ef..2c556e3e28 100644 --- a/packages/agent-bff/src/permissions/action-permissions.ts +++ b/packages/agent-bff/src/permissions/action-permissions.ts @@ -1,3 +1,5 @@ +import type { EnvironmentPermissionsV4, RawTreeWithSources } from '@forestadmin/forestadmin-client'; + import { CollectionActionEvent, CustomActionEvent, @@ -5,55 +7,22 @@ import { generateCustomActionIdentifier, } from './action-identifiers'; -export type RightDescriptionWithRolesV4 = { roles: number[] }; -export type RightDescriptionV4 = boolean | RightDescriptionWithRolesV4; +type RightDescriptionWithRolesV4 = { roles: number[] }; +type RightDescriptionV4 = boolean | RightDescriptionWithRolesV4; -export type RightConditionByRolesV4 = { +type RightConditionByRolesV4 = { roleId: number; - filter: unknown; + filter: RawTreeWithSources; }; -export interface EnvironmentCollectionAccessPermissionsV4 { - browseEnabled: RightDescriptionV4; - readEnabled: RightDescriptionV4; - editEnabled: RightDescriptionV4; - addEnabled: RightDescriptionV4; - deleteEnabled: RightDescriptionV4; - exportEnabled: RightDescriptionV4; -} - -export interface EnvironmentSmartActionPermissionsV4 { - triggerEnabled: RightDescriptionV4; - triggerConditions: RightConditionByRolesV4[]; - approvalRequired: RightDescriptionV4; - approvalRequiredConditions: RightConditionByRolesV4[]; - userApprovalEnabled: RightDescriptionV4; - userApprovalConditions: RightConditionByRolesV4[]; - selfApprovalEnabled: RightDescriptionV4; -} - -export interface EnvironmentCollectionActionPermissionsV4 { - [actionName: string]: EnvironmentSmartActionPermissionsV4; -} - -export interface EnvironmentCollectionPermissionsV4 { - collection: EnvironmentCollectionAccessPermissionsV4; - actions: EnvironmentCollectionActionPermissionsV4; -} - -export interface EnvironmentCollectionsPermissionsV4 { - [collectionName: string]: EnvironmentCollectionPermissionsV4; -} - -export type EnvironmentPermissionsV4Remote = { - collections: EnvironmentCollectionsPermissionsV4; -}; +type EnvironmentCollectionsPermissionsV4 = Exclude['collections']; -export type EnvironmentPermissionsV4 = EnvironmentPermissionsV4Remote | true; +type EnvironmentCollectionActionPermissionsV4 = + EnvironmentCollectionsPermissionsV4[string]['actions']; export type ActionPermission = { allowedRoles: Set; - conditionsByRole?: Map; + conditionsByRole?: Map; }; export type ActionPermissions = { @@ -190,7 +159,7 @@ export function buildActionPermissions( }; } -export function canRoleTriggerAction( +export function isActionIdentifierAllowedForRole( permissions: ActionPermissions, roleId: number, actionName: string, @@ -208,21 +177,29 @@ export function canRolePerformCollectionAction( action: CollectionActionEvent, collectionName: string, ): boolean { - return canRoleTriggerAction( + return isActionIdentifierAllowedForRole( permissions, roleId, generateCollectionActionIdentifier(action, collectionName), ); } -export function canRolePerformCustomAction( - permissions: ActionPermissions, - roleId: number, - event: CustomActionEvent, - actionName: string, - collectionName: string, -): boolean { - return canRoleTriggerAction( +export interface CanRolePerformCustomActionParams { + permissions: ActionPermissions; + roleId: number; + event: CustomActionEvent; + actionName: string; + collectionName: string; +} + +export function canRolePerformCustomAction({ + permissions, + roleId, + event, + actionName, + collectionName, +}: CanRolePerformCustomActionParams): boolean { + return isActionIdentifierAllowedForRole( permissions, roleId, generateCustomActionIdentifier(event, actionName, collectionName), diff --git a/packages/agent-bff/test/permissions/action-permissions.test.ts b/packages/agent-bff/test/permissions/action-permissions.test.ts index 2d0406bf74..cc03da1adf 100644 --- a/packages/agent-bff/test/permissions/action-permissions.test.ts +++ b/packages/agent-bff/test/permissions/action-permissions.test.ts @@ -1,4 +1,4 @@ -import type { EnvironmentPermissionsV4 } from '../../src/permissions/action-permissions'; +import type { EnvironmentPermissionsV4 } from '@forestadmin/forestadmin-client'; import { CollectionActionEvent, @@ -10,7 +10,7 @@ import { buildActionPermissions, canRolePerformCollectionAction, canRolePerformCustomAction, - canRoleTriggerAction, + isActionIdentifierAllowedForRole, } from '../../src/permissions/action-permissions'; const ADMIN_ROLE = 1; @@ -123,12 +123,12 @@ describe('buildActionPermissions', () => { }); }); -describe('canRoleTriggerAction', () => { +describe('isActionIdentifierAllowedForRole', () => { describe('when the environment is in development', () => { it('should allow an action no descriptor mentions', () => { - expect(canRoleTriggerAction(buildActionPermissions(true), VIEWER_ROLE, 'anything')).toBe( - true, - ); + expect( + isActionIdentifierAllowedForRole(buildActionPermissions(true), VIEWER_ROLE, 'anything'), + ).toBe(true); }); }); @@ -155,7 +155,7 @@ describe('canRoleTriggerAction', () => { it.each([ ['the named role', ADMIN_ROLE, true], ['a role mismatch', VIEWER_ROLE, false], - ])('should return %s -> %s', (_label, roleId, expected) => { + ])('should resolve %s to %s', (_label, roleId, expected) => { expect( canRolePerformCollectionAction( buildActionPermissions(NORMAL_MODE), @@ -170,7 +170,7 @@ describe('canRoleTriggerAction', () => { describe('when the identifier is absent from the permissions', () => { it('should deny rather than throw', () => { expect( - canRoleTriggerAction( + isActionIdentifierAllowedForRole( buildActionPermissions(NORMAL_MODE), ADMIN_ROLE, 'collection:ghost:browse', @@ -204,18 +204,18 @@ describe('canRoleTriggerAction', () => { [CustomActionEvent.RequireApproval], ])('should resolve the %s custom-action event for the named role', event => { expect( - canRolePerformCustomAction( - buildActionPermissions(NORMAL_MODE), - ADMIN_ROLE, + canRolePerformCustomAction({ + permissions: buildActionPermissions(NORMAL_MODE), + roleId: ADMIN_ROLE, event, - 'Block user', - 'users', - ), + actionName: 'Block user', + collectionName: 'users', + }), ).toBe(true); }); describe('when an action-event flag is missing from the payload', () => { - it('should throw, matching the source evaluator rather than inventing a denial', () => { + it('should throw rather than invent a denial', () => { const { selfApprovalEnabled, ...withoutSelfApproval } = smartAction(); expect(selfApprovalEnabled).toBeDefined(); @@ -230,7 +230,7 @@ describe('canRoleTriggerAction', () => { }); describe('when a CRUD descriptor is missing from the payload', () => { - it('should throw, matching the source evaluator', () => { + it('should throw rather than invent a denial', () => { const { deleteEnabled, ...withoutDelete } = crud(); expect(deleteEnabled).toBeDefined(); diff --git a/packages/agent-bff/test/permissions/evaluator-drift.test.ts b/packages/agent-bff/test/permissions/evaluator-drift.test.ts index 7fecc8c814..678e4dacd2 100644 --- a/packages/agent-bff/test/permissions/evaluator-drift.test.ts +++ b/packages/agent-bff/test/permissions/evaluator-drift.test.ts @@ -1,7 +1,10 @@ -import type { EnvironmentPermissionsV4 } from '../../src/permissions/action-permissions'; +import type { EnvironmentPermissionsV4 } from '@forestadmin/forestadmin-client'; import sourceActionPermissionService from '@forestadmin/forestadmin-client/dist/permissions/action-permission'; import sourceGenerateActionsFromPermissions from '@forestadmin/forestadmin-client/dist/permissions/generate-actions-from-permissions'; +import { createHash } from 'crypto'; +import { readFileSync } from 'fs'; +import { join } from 'path'; import { CollectionActionEvent, @@ -11,7 +14,7 @@ import { } from '../../src/permissions/action-identifiers'; import { buildActionPermissions, - canRoleTriggerAction, + isActionIdentifierAllowedForRole, } from '../../src/permissions/action-permissions'; const ADMIN_ROLE = 1; @@ -35,15 +38,20 @@ function crud(overrides: Record = {}) { }; } +const LAST_CONDITION_FOR_THE_SAME_ROLE = { field: 'id', operator: 'equal', value: 2 }; + function smartAction(overrides: Record = {}) { return { triggerEnabled: { roles: [ADMIN_ROLE] }, - triggerConditions: [{ roleId: VIEWER_ROLE, filter: CONDITION }], + triggerConditions: [ + { roleId: VIEWER_ROLE, filter: CONDITION }, + { roleId: VIEWER_ROLE, filter: LAST_CONDITION_FOR_THE_SAME_ROLE }, + ], approvalRequired: true, approvalRequiredConditions: [], userApprovalEnabled: { roles: [VIEWER_ROLE] }, userApprovalConditions: [], - selfApprovalEnabled: false, + selfApprovalEnabled: { roles: [VIEWER_ROLE] }, ...overrides, }; } @@ -111,6 +119,27 @@ function sourceServiceFor(permissions: EnvironmentPermissionsV4) { } describe('the forked evaluator does not drift from forestadmin-client', () => { + describe.each([ + ['action-permission.ts', '18b9f6a2c97008104f0bb78ee7a10b0f23df4a28c2825cc78da87b6bec867018'], + [ + 'generate-actions-from-permissions.ts', + '1716b8236d69cc16737772d408bf4b88fc8ccca70241e067cc172a2e47cacf2e', + ], + [ + 'generate-action-identifier.ts', + 'e1996d60241fafe389404980d1fe93d915a395002a985f2d78d507ac9bc418d8', + ], + ])('given the upstream source %s', (fileName, expectedSha256) => { + it('should still hash to the reviewed revision, or the fork must be re-reviewed', () => { + const source = readFileSync( + join(__dirname, '../../../forestadmin-client/src/permissions', fileName), + 'utf8', + ); + + expect(createHash('sha256').update(source).digest('hex')).toBe(expectedSha256); + }); + }); + describe.each(FIXTURES)('given %s', (_label, permissions) => { it('should produce the same transformation as the source', () => { const source = ( @@ -132,7 +161,7 @@ describe('the forked evaluator does not drift from forestadmin-client', () => { identifier, roleId, source: await service.can(roleId, identifier), - forked: canRoleTriggerAction(forked, roleId, identifier), + forked: isActionIdentifierAllowedForRole(forked, roleId, identifier), })), ), ); @@ -141,4 +170,40 @@ describe('the forked evaluator does not drift from forestadmin-client', () => { expect(comparisons).toHaveLength(ROLES.length * IDENTIFIERS.length); }); }); + + describe.each([ + [ + 'a CRUD descriptor', + () => { + const { deleteEnabled, ...withoutDelete } = crud(); + + return { collections: { [COLLECTION]: { collection: withoutDelete, actions: {} } } }; + }, + ], + [ + 'an action-event flag', + () => { + const { selfApprovalEnabled, ...withoutSelfApproval } = smartAction(); + + return { + collections: { + [COLLECTION]: { collection: crud(), actions: { [ACTION]: withoutSelfApproval } }, + }, + }; + }, + ], + ])('given a payload missing %s', (_label, build) => { + it('should fail the same way as the source rather than inventing a verdict', () => { + const payload = build() as unknown as EnvironmentPermissionsV4; + const runSource = () => + ( + sourceGenerateActionsFromPermissions as unknown as ( + input: EnvironmentPermissionsV4, + ) => unknown + )(payload); + + expect(runSource).toThrow(TypeError); + expect(() => buildActionPermissions(payload)).toThrow(TypeError); + }); + }); });