From 8df5c42d38653ea93f1b6b2b24cd59fae3bb1302 Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Fri, 4 Sep 2026 12:08:21 +0200 Subject: [PATCH 1/2] feat: add option to skip rbac --- packages/agent/src/routes/capabilities.ts | 1 + .../services/authorization/authorization.ts | 12 +- .../agent/src/services/authorization/index.ts | 2 +- packages/agent/src/types.ts | 8 ++ packages/agent/src/utils/options-validator.ts | 1 + .../forest-admin-http-driver-options.ts | 1 + .../agent/test/routes/capabilities.test.ts | 28 +++++ .../security/related-read-permissions.test.ts | 113 +++++++++++++++++- 8 files changed, 162 insertions(+), 4 deletions(-) diff --git a/packages/agent/src/routes/capabilities.ts b/packages/agent/src/routes/capabilities.ts index fa342838ac..6c51f72e60 100644 --- a/packages/agent/src/routes/capabilities.ts +++ b/packages/agent/src/routes/capabilities.ts @@ -46,6 +46,7 @@ export default class Capabilities extends BaseRoute { // `x-forest-correlation-id` is emitted on every response regardless — the frontend must // gate the History tab on this flag rather than inferring the feature from that header. canUseAuditTrail: this.options.auditTrail !== null, + checksRelationReadPermissions: !this.options.skipRelationReadPermissions, }, collections: collections?.map(collection => { diff --git a/packages/agent/src/services/authorization/authorization.ts b/packages/agent/src/services/authorization/authorization.ts index 9fec566db8..f14bfad821 100644 --- a/packages/agent/src/services/authorization/authorization.ts +++ b/packages/agent/src/services/authorization/authorization.ts @@ -27,7 +27,10 @@ export type QueryComponent = 'filter' | 'sort' | 'search'; const ALL_QUERY_COMPONENTS: QueryComponent[] = ['filter', 'sort', 'search']; export default class AuthorizationService { - constructor(private readonly forestAdminClient: ForestAdminClient) {} + constructor( + private readonly forestAdminClient: ForestAdminClient, + private readonly skipRelationReadPermissions = false, + ) {} public async assertCanBrowse(context: Context, collectionName: string) { await this.assertCanOnCollection(CollectionActionEvent.Browse, context, collectionName); @@ -72,6 +75,9 @@ export default class AuthorizationService { requested: RequestedProjection, ): Promise { const { projection, namedByCaller } = requested; + + if (this.skipRelationReadPermissions) return new Projection(...projection); + const owners = projection.map(path => FieldPathUtils.getLeafCollection(collection, path).name); const permissions = await this.getReadPermissions(context, collection.name, owners); const isReadable = (index: number) => permissions.get(owners[index]); @@ -107,6 +113,8 @@ export default class AuthorizationService { collection: Collection, consumes: QueryComponent[] = ALL_QUERY_COMPONENTS, ): Promise { + if (this.skipRelationReadPermissions) return; + const usages: FieldUsage[] = []; const push = (action: string, path: string) => usages.push({ @@ -162,6 +170,8 @@ export default class AuthorizationService { rootCollectionName: string, usages: FieldUsage[], ): Promise { + if (this.skipRelationReadPermissions) return; + const permissions = await this.getReadPermissions( context, rootCollectionName, diff --git a/packages/agent/src/services/authorization/index.ts b/packages/agent/src/services/authorization/index.ts index 80d1943161..64c035a916 100644 --- a/packages/agent/src/services/authorization/index.ts +++ b/packages/agent/src/services/authorization/index.ts @@ -5,5 +5,5 @@ import AuthorizationService from './authorization'; export default function authorizationServiceFactory( options: AgentOptionsWithDefaults, ): AuthorizationService { - return new AuthorizationService(options.forestAdminClient); + return new AuthorizationService(options.forestAdminClient, options.skipRelationReadPermissions); } diff --git a/packages/agent/src/types.ts b/packages/agent/src/types.ts index b49b9be255..0af3f56e13 100644 --- a/packages/agent/src/types.ts +++ b/packages/agent/src/types.ts @@ -74,6 +74,14 @@ export type AgentOptions = { */ ignoreMissingSchemaElementErrors?: boolean; useUnsafeActionEndpoint?: boolean; + /** + * Serve columns of collections the caller has no `read` permission on when they are reached + * through a relation path (`holder:nationalId`) in a projection, a filter, a sort, a search or a + * chart. The route's own `browse`/`read`/`export` check on the collection being queried is + * unaffected. + * @default false + */ + skipRelationReadPermissions?: boolean; /** * Max number of records a "select all" approval-required action may target. * Must not exceed the Forest server's own cap (500). diff --git a/packages/agent/src/utils/options-validator.ts b/packages/agent/src/utils/options-validator.ts index 7176e6c653..3d390a043d 100644 --- a/packages/agent/src/utils/options-validator.ts +++ b/packages/agent/src/utils/options-validator.ts @@ -44,6 +44,7 @@ export default class OptionsValidator { copyOptions.instantCacheRefresh = copyOptions.instantCacheRefresh ?? true; copyOptions.workflowExecutorUrl = copyOptions.workflowExecutorUrl ?? null; copyOptions.auditTrail = copyOptions.auditTrail ?? null; + copyOptions.skipRelationReadPermissions = copyOptions.skipRelationReadPermissions ?? false; // Number.isFinite so NaN (e.g. Number() on an unset env var) also gets the default. copyOptions.maxRecordsForApproval = Number.isFinite(copyOptions.maxRecordsForApproval) ? copyOptions.maxRecordsForApproval diff --git a/packages/agent/test/__factories__/forest-admin-http-driver-options.ts b/packages/agent/test/__factories__/forest-admin-http-driver-options.ts index fc12942c57..d8daa409b0 100644 --- a/packages/agent/test/__factories__/forest-admin-http-driver-options.ts +++ b/packages/agent/test/__factories__/forest-admin-http-driver-options.ts @@ -29,6 +29,7 @@ export default Factory.define(() => ({ }, ignoreMissingSchemaElementErrors: false, useUnsafeActionEndpoint: false, + skipRelationReadPermissions: false, maxRecordsForApproval: 500, workflowExecutorUrl: null, auditTrail: null, diff --git a/packages/agent/test/routes/capabilities.test.ts b/packages/agent/test/routes/capabilities.test.ts index 44a2223e20..bf3fc8f643 100644 --- a/packages/agent/test/routes/capabilities.test.ts +++ b/packages/agent/test/routes/capabilities.test.ts @@ -81,6 +81,7 @@ describe('Capabilities', () => { canUseProjectionViaHeaderOnList: true, canUseMultipleFieldsProjectionOnRelation: true, canUseAuditTrail: false, + checksRelationReadPermissions: true, }, collections: [], }); @@ -106,6 +107,7 @@ describe('Capabilities', () => { canUseProjectionViaHeaderOnList: true, canUseMultipleFieldsProjectionOnRelation: true, canUseAuditTrail: false, + checksRelationReadPermissions: true, }, collections: [], }); @@ -129,12 +131,37 @@ describe('Capabilities', () => { canUseProjectionViaHeaderOnList: true, canUseMultipleFieldsProjectionOnRelation: true, canUseAuditTrail: false, + checksRelationReadPermissions: true, }, collections: [], }); }); }); + describe('when skipRelationReadPermissions is set', () => { + test('reports checksRelationReadPermissions: false so the front stops pruning related columns', async () => { + const unsafeOptions = factories.forestAdminHttpDriverOptions.build({ + skipRelationReadPermissions: true, + }); + const dataSource = factories.dataSource.buildWithCollection( + factories.collection.build({ name: 'books' }), + ); + const unsafeRoute = new Capabilities(services, unsafeOptions, dataSource); + const context = createMockContext({ + ...defaultContext, + requestBody: { collectionNames: [] }, + }); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + await unsafeRoute.fetchCapabilities(context); + + expect(context.response.body).toMatchObject({ + agentCapabilities: { checksRelationReadPermissions: false }, + }); + }); + }); + describe('when auditTrail is configured', () => { test('reports canUseAuditTrail: true so the front can gate the History tab on it', async () => { const auditTrailOptions = factories.forestAdminHttpDriverOptions.build({ @@ -178,6 +205,7 @@ describe('Capabilities', () => { canUseProjectionViaHeaderOnList: true, canUseMultipleFieldsProjectionOnRelation: true, canUseAuditTrail: false, + checksRelationReadPermissions: true, }, collections: [ { diff --git a/packages/agent/test/security/related-read-permissions.test.ts b/packages/agent/test/security/related-read-permissions.test.ts index 14da834bc7..85450f46e2 100644 --- a/packages/agent/test/security/related-read-permissions.test.ts +++ b/packages/agent/test/security/related-read-permissions.test.ts @@ -105,7 +105,10 @@ describe('read permissions on related collections', () => { }), ]); - const buildServices = (readableCollections: string[] = []) => { + const buildServices = ( + readableCollections: string[] = [], + skipRelationReadPermissions = false, + ) => { const forestAdminClient = factories.forestAdminClient.build(); (forestAdminClient.permissionService.canOnCollection as jest.Mock).mockImplementation( @@ -115,7 +118,10 @@ describe('read permissions on related collections', () => { ); const services = factories.forestAdminHttpDriverServices.build(); - services.authorization = new AuthorizationService(forestAdminClient); + services.authorization = new AuthorizationService( + forestAdminClient, + skipRelationReadPermissions, + ); services.serializer.serialize = jest.fn(); services.serializer.serializeWithSearchMetadata = jest.fn(); @@ -834,4 +840,107 @@ describe('read permissions on related collections', () => { }); }); }); + + describe('options.skipRelationReadPermissions', () => { + const unsafeOptions = factories.forestAdminHttpDriverOptions.build({ + skipRelationReadPermissions: true, + }); + const unsafeServices = () => buildServices([], true); + + it('should serve a named field of a collection the caller cannot read', async () => { + const dataSource = buildDataSource(); + const services = unsafeServices(); + const list = jest.spyOn(dataSource.getCollection('cards'), 'list').mockResolvedValue([]); + + await new List(services, unsafeOptions, dataSource, 'cards').handleList( + buildContext({}, { 'forest-projection': 'id,holder:nationalId' }), + ); + + expect([...list.mock.calls[0][2]].sort()).toEqual(['holder:id', 'holder:nationalId', 'id']); + }); + + it('should keep an unnamed projection whole instead of redacting it', async () => { + const dataSource = buildDataSource(); + const services = unsafeServices(); + const list = jest.spyOn(dataSource.getCollection('cards'), 'list').mockResolvedValue([]); + + await new List(services, unsafeOptions, dataSource, 'cards').handleList(buildContext({})); + + expect([...list.mock.calls[0][2]]).toContain('holder:nationalId'); + }); + + it('should serve a filter on a collection the caller cannot read', async () => { + const dataSource = buildDataSource(); + const services = unsafeServices(); + const list = jest.spyOn(dataSource.getCollection('cards'), 'list').mockResolvedValue([]); + + await new List(services, unsafeOptions, dataSource, 'cards').handleList( + buildContext({ + query: { + filters: JSON.stringify({ + field: 'holder:nationalId', + operator: 'starts_with', + value: '1850', + }), + }, + }), + ); + + expect(list.mock.calls[0][1].conditionTree).toMatchObject({ + field: 'holder:nationalId', + operator: 'StartsWith', + value: '1850', + }); + }); + + it('should serve a sort on a collection the caller cannot read', async () => { + const dataSource = buildDataSource(); + const services = unsafeServices(); + const list = jest.spyOn(dataSource.getCollection('cards'), 'list').mockResolvedValue([]); + + await new List(services, unsafeOptions, dataSource, 'cards').handleList( + buildContext({ query: { sort: '-account.balance' } }), + ); + + expect(list.mock.calls[0][1].sort).toEqual([{ field: 'account:balance', ascending: false }]); + }); + + it('should serve an extended search the stack cannot describe', async () => { + const dataSource = buildDataSource(); + const services = unsafeServices(); + const cards = dataSource.getCollection('cards') as CollectionDecorator; + const list = jest.spyOn(cards, 'list').mockResolvedValue([]); + + cards.getSearchedFields = () => null; + + await new List(services, unsafeOptions, dataSource, 'cards').handleList( + buildContext({ query: { search: 'martin', searchExtended: '1' } }), + ); + + expect(list.mock.calls[0][1]).toMatchObject({ search: 'martin', searchExtended: true }); + }); + + it('should serve a chart grouping by a collection the caller cannot read', async () => { + const dataSource = buildDataSource(); + const services = unsafeServices(); + const aggregate = jest + .spyOn(dataSource.getCollection('cards'), 'aggregate') + .mockResolvedValue([]); + const body = { + type: 'Pie', + aggregator: 'Count', + groupByFieldName: 'holder:fullName', + }; + + (services.chartHandler.getChartWithContextInjected as jest.Mock).mockResolvedValue(body); + + await new Chart(services, unsafeOptions, dataSource, 'cards').handleChart( + buildContext({}, {}, body), + ); + + expect(aggregate.mock.calls[0][2]).toMatchObject({ + groups: [{ field: 'holder:fullName' }], + }); + }); + }); }); From e3684fd061aceec2b6230a5283bb339c83b76088 Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Mon, 7 Sep 2026 10:38:30 +0200 Subject: [PATCH 2/2] chore: review --- packages/agent/src/routes/access/chart.ts | 4 +- packages/agent/src/types.ts | 5 +- packages/agent/src/utils/options-validator.ts | 8 ++ .../security/related-read-permissions.test.ts | 102 ++++++++++++++++++ .../test/utils/http-driver-options.test.ts | 37 +++++++ 5 files changed, 152 insertions(+), 4 deletions(-) diff --git a/packages/agent/src/routes/access/chart.ts b/packages/agent/src/routes/access/chart.ts index 90ca1da19b..5dc6746874 100644 --- a/packages/agent/src/routes/access/chart.ts +++ b/packages/agent/src/routes/access/chart.ts @@ -258,8 +258,8 @@ export default class ChartRoute extends CollectionRoute { ]); // A count exposes the cardinality of the relation, which `/relationships//count` puts - // behind `browse`. - if (!aggregation.field) { + // behind `browse`. Added with the related-read checks, so it goes away with them. + if (!aggregation.field && !this.options.skipRelationReadPermissions) { await this.services.authorization.assertCanBrowse(context, field.foreignCollection); } diff --git a/packages/agent/src/types.ts b/packages/agent/src/types.ts index 0af3f56e13..77bbd6e8bd 100644 --- a/packages/agent/src/types.ts +++ b/packages/agent/src/types.ts @@ -77,8 +77,9 @@ export type AgentOptions = { /** * Serve columns of collections the caller has no `read` permission on when they are reached * through a relation path (`holder:nationalId`) in a projection, a filter, a sort, a search or a - * chart. The route's own `browse`/`read`/`export` check on the collection being queried is - * unaffected. + * chart, and drop the `browse` a `Count` leaderboard requires on the collection it counts. No + * check on the collection being queried is affected: the route's own `browse`/`read`/`export` + * still runs. * @default false */ skipRelationReadPermissions?: boolean; diff --git a/packages/agent/src/utils/options-validator.ts b/packages/agent/src/utils/options-validator.ts index 3d390a043d..db3c5ccb1f 100644 --- a/packages/agent/src/utils/options-validator.ts +++ b/packages/agent/src/utils/options-validator.ts @@ -62,6 +62,14 @@ export default class OptionsValidator { ); } + if (copyOptions.skipRelationReadPermissions) { + copyOptions.logger( + 'Warn', + 'options.skipRelationReadPermissions=true: columns of collections the caller has no ' + + 'read permission on are served when a relation path reaches them', + ); + } + if (copyOptions.skipSchemaUpdate && copyOptions.experimental) { copyOptions.logger( 'Warn', diff --git a/packages/agent/test/security/related-read-permissions.test.ts b/packages/agent/test/security/related-read-permissions.test.ts index 85450f46e2..14adb86b4d 100644 --- a/packages/agent/test/security/related-read-permissions.test.ts +++ b/packages/agent/test/security/related-read-permissions.test.ts @@ -16,6 +16,7 @@ import ListRelated from '../../src/routes/access/list-related'; import Update from '../../src/routes/modification/update'; import AuthorizationService from '../../src/services/authorization/authorization'; import Serializer from '../../src/services/serializer'; +import { HttpCode } from '../../src/types'; import * as factories from '../__factories__'; describe('read permissions on related collections', () => { @@ -942,5 +943,106 @@ describe('read permissions on related collections', () => { groups: [{ field: 'holder:fullName' }], }); }); + + // The `browse` a `Count` leaderboard asserts on the collection it counts came in with the + // related-read checks, so the option drops it too — a leaderboard that worked before them + // works again. + it('should serve a leaderboard counting a collection the caller cannot browse', async () => { + const dataSource = buildDataSource(); + const forestAdminClient = factories.forestAdminClient.build(); + const services = factories.forestAdminHttpDriverServices.build(); + services.authorization = new AuthorizationService(forestAdminClient, true); + const body = { + type: 'Leaderboard', + aggregator: 'Count', + relationshipFieldName: 'cards', + labelFieldName: 'fullName', + limit: 5, + }; + + const aggregate = jest + .spyOn(dataSource.getCollection('cards'), 'aggregate') + .mockResolvedValue([]); + + (forestAdminClient.permissionService.canOnCollection as jest.Mock).mockImplementation( + ({ collectionName }) => collectionName === 'holders', + ); + (forestAdminClient.permissionService.canExecuteChart as jest.Mock).mockResolvedValue(true); + (services.chartHandler.getChartWithContextInjected as jest.Mock).mockResolvedValue(body); + + const context = buildContext({}, {}, body); + + await new Chart(services, unsafeOptions, dataSource, 'holders').handleChart(context); + + expect(forestAdminClient.permissionService.canOnCollection).not.toHaveBeenCalledWith( + expect.objectContaining({ + event: CollectionActionEvent.Browse, + collectionName: 'cards', + }), + ); + expect(context.throw).not.toHaveBeenCalled(); + expect(aggregate.mock.calls[0][2]).toMatchObject({ operation: 'Count' }); + }); + + // The option widens what a permitted request may reach; it must not make an unpermitted one + // permitted. The route's own check on the collection being queried is the boundary, and + // nothing above exercises it: `buildServices` allows every event on `cards`. + describe('the check on the collection being queried', () => { + const denyEverything = () => { + const forestAdminClient = factories.forestAdminClient.build(); + + (forestAdminClient.permissionService.canOnCollection as jest.Mock).mockResolvedValue(false); + + const services = factories.forestAdminHttpDriverServices.build(); + services.authorization = new AuthorizationService(forestAdminClient, true); + services.serializer.serialize = jest.fn(); + services.serializer.serializeWithSearchMetadata = jest.fn(); + + return services; + }; + + it('should still refuse a listing when browse is denied on the root collection', async () => { + const dataSource = buildDataSource(); + const services = denyEverything(); + jest.spyOn(dataSource.getCollection('cards'), 'list').mockResolvedValue([]); + const context = buildContext({}); + + await new List(services, unsafeOptions, dataSource, 'cards').handleList(context); + + expect(context.throw).toHaveBeenCalledWith(HttpCode.Forbidden, 'Forbidden'); + }); + + it('should still refuse a get-one when read is denied on the root collection', async () => { + const dataSource = buildDataSource(); + const services = denyEverything(); + jest.spyOn(dataSource.getCollection('cards'), 'list').mockResolvedValue([{ id: 'card-1' }]); + const context = buildContext({ + params: { id: '2d162303-78bf-599e-b197-93590ac3d315' }, + }); + + await new Get(services, unsafeOptions, dataSource, 'cards').handleGet(context); + + expect(context.throw).toHaveBeenCalledWith(HttpCode.Forbidden, 'Forbidden'); + }); + + it('should still refuse a related export when export is denied on the foreign collection', async () => { + const dataSource = buildDataSource(); + const services = denyEverything(); + const context = buildContext({ + params: { parentId: '2d162303-78bf-599e-b197-93590ac3d315' }, + query: { 'fields[cards]': 'id,panLast4', header: 'Id,Pan' }, + }); + + await new CsvRelated( + services, + unsafeOptions, + dataSource, + 'holders', + 'cards', + ).handleRelatedCsv(context); + + expect(context.throw).toHaveBeenCalledWith(HttpCode.Forbidden, 'Forbidden'); + }); + }); }); }); diff --git a/packages/agent/test/utils/http-driver-options.test.ts b/packages/agent/test/utils/http-driver-options.test.ts index 4ea7eac3b6..5d3888b463 100644 --- a/packages/agent/test/utils/http-driver-options.test.ts +++ b/packages/agent/test/utils/http-driver-options.test.ts @@ -19,6 +19,43 @@ describe('OptionsValidator', () => { expect(options).toHaveProperty('instantCacheRefresh', true); expect(options).toHaveProperty('permissionsCacheDurationInSeconds', 31560000); expect(options).toHaveProperty('skipSchemaUpdate', false); + expect(options).toHaveProperty('skipRelationReadPermissions', false); + }); + + describe('skipRelationReadPermissions', () => { + test('keeps a configured value', () => { + const options = OptionsValidator.withDefaults({ + ...mandatoryOptions, + skipRelationReadPermissions: true, + logger: jest.fn(), + }); + + expect(options).toHaveProperty('skipRelationReadPermissions', true); + }); + + test('warns on boot so the weakened posture shows in the logs', () => { + const logger = jest.fn(); + + OptionsValidator.withDefaults({ + ...mandatoryOptions, + skipRelationReadPermissions: true, + logger, + }); + + expect(logger).toHaveBeenCalledWith( + 'Warn', + 'options.skipRelationReadPermissions=true: columns of collections the caller has no ' + + 'read permission on are served when a relation path reaches them', + ); + }); + + test('stays quiet when the option is left off', () => { + const logger = jest.fn(); + + OptionsValidator.withDefaults({ ...mandatoryOptions, logger }); + + expect(logger).not.toHaveBeenCalledWith('Warn', expect.stringContaining('skipRelation')); + }); }); describe('maxRecordsForApproval', () => {