From 010468675ca50ef66b8025e0e92cfb335191ece7 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 27 Oct 2026 15:14:33 +0100 Subject: [PATCH 01/10] fix(agent): check the segment query permission wherever a live query segment is used Each route that builds its filter from a live query segment checks that the user may run that segment, on the collection whose filter carries it, whether the segment comes from the query string or the request body: listing, counting and exporting records, deleting them, associating and dissociating related records, running an action, its form hooks and its approval, and charts. Users allowed to run a segment keep every feature on it. Refs: PRD-1759 Co-Authored-By: Claude Opus 5.5 --- packages/agent/src/routes/access/chart.ts | 5 + .../agent/src/routes/access/count-related.ts | 4 + packages/agent/src/routes/access/count.ts | 4 + .../agent/src/routes/access/csv-related.ts | 4 + packages/agent/src/routes/access/csv.ts | 4 + .../agent/src/routes/access/list-related.ts | 4 + packages/agent/src/routes/access/list.ts | 4 + .../src/routes/modification/action/action.ts | 5 + .../routes/modification/associate-related.ts | 5 + .../agent/src/routes/modification/delete.ts | 5 + .../modification/dissociate-delete-related.ts | 5 + .../services/authorization/authorization.ts | 47 +++-- .../authorization/authorization.ts | 1 + .../agent/test/routes/access/chart.test.ts | 169 +++++++++++++++ .../test/routes/access/count-related.test.ts | 21 ++ .../agent/test/routes/access/count.test.ts | 21 ++ .../test/routes/access/csv-related.test.ts | 24 +++ packages/agent/test/routes/access/csv.test.ts | 19 ++ .../test/routes/access/list-related.test.ts | 25 +++ .../agent/test/routes/access/list.test.ts | 89 ++++++++ .../routes/modification/action/action.test.ts | 163 +++++++++++++++ .../modification/associate-related.test.ts | 30 +++ .../test/routes/modification/delete.test.ts | 118 +++++++++++ .../dissociate-delete-related.test.ts | 31 +++ .../authorization/authorization.test.ts | 196 +++++++++++------- 25 files changed, 900 insertions(+), 103 deletions(-) diff --git a/packages/agent/src/routes/access/chart.ts b/packages/agent/src/routes/access/chart.ts index 5dc6746874..e326126bf1 100644 --- a/packages/agent/src/routes/access/chart.ts +++ b/packages/agent/src/routes/access/chart.ts @@ -319,6 +319,11 @@ export default class ChartRoute extends CollectionRoute { } private async getFilter(context: Context): Promise { + await this.services.authorization.assertCanExecuteLiveQuerySegment( + context, + this.collection.name, + ); + const scope = await this.services.authorization.getScope(this.collection, context); return ContextFilterFactory.build(this.collection, context, scope); diff --git a/packages/agent/src/routes/access/count-related.ts b/packages/agent/src/routes/access/count-related.ts index 3d0a45d80d..2887c4a6ee 100644 --- a/packages/agent/src/routes/access/count-related.ts +++ b/packages/agent/src/routes/access/count-related.ts @@ -18,6 +18,10 @@ export default class CountRelatedRoute extends RelationRoute { public async handleCountRelated(context: Context): Promise { await this.services.authorization.assertCanBrowse(context, this.foreignCollection.name); + await this.services.authorization.assertCanExecuteLiveQuerySegment( + context, + this.foreignCollection.name, + ); if (this.foreignCollection.schema.countable) { await this.services.authorization.assertCanReadQueryFields(context, this.foreignCollection, [ diff --git a/packages/agent/src/routes/access/count.ts b/packages/agent/src/routes/access/count.ts index a6802e043e..153f07121e 100644 --- a/packages/agent/src/routes/access/count.ts +++ b/packages/agent/src/routes/access/count.ts @@ -14,6 +14,10 @@ export default class CountRoute extends CollectionRoute { public async handleCount(context: Context): Promise { await this.services.authorization.assertCanBrowse(context, this.collection.name); + await this.services.authorization.assertCanExecuteLiveQuerySegment( + context, + this.collection.name, + ); if (this.collection.schema.countable) { await this.services.authorization.assertCanReadQueryFields(context, this.collection, [ diff --git a/packages/agent/src/routes/access/csv-related.ts b/packages/agent/src/routes/access/csv-related.ts index ef5325cfb4..56e7c25d00 100644 --- a/packages/agent/src/routes/access/csv-related.ts +++ b/packages/agent/src/routes/access/csv-related.ts @@ -22,6 +22,10 @@ export default class CsvRelatedRoute extends RelationRoute { async handleRelatedCsv(context: Context): Promise { await this.services.authorization.assertCanBrowse(context, this.foreignCollection.name); + await this.services.authorization.assertCanExecuteLiveQuerySegment( + context, + this.foreignCollection.name, + ); await this.services.authorization.assertCanExport(context, this.foreignCollection.name); await this.services.authorization.assertCanReadQueryFields(context, this.foreignCollection); diff --git a/packages/agent/src/routes/access/csv.ts b/packages/agent/src/routes/access/csv.ts index 02eb795465..7683faa922 100644 --- a/packages/agent/src/routes/access/csv.ts +++ b/packages/agent/src/routes/access/csv.ts @@ -16,6 +16,10 @@ export default class CsvRoute extends CollectionRoute { async handleCsv(context: Context): Promise { await this.services.authorization.assertCanBrowse(context, this.collection.name); + await this.services.authorization.assertCanExecuteLiveQuerySegment( + context, + this.collection.name, + ); await this.services.authorization.assertCanExport(context, this.collection.name); await this.services.authorization.assertCanReadQueryFields(context, this.collection); diff --git a/packages/agent/src/routes/access/list-related.ts b/packages/agent/src/routes/access/list-related.ts index f12a17533a..edf83a7d3f 100644 --- a/packages/agent/src/routes/access/list-related.ts +++ b/packages/agent/src/routes/access/list-related.ts @@ -18,6 +18,10 @@ export default class ListRelatedRoute extends RelationRoute { public async handleListRelated(context: Context): Promise { await this.services.authorization.assertCanBrowse(context, this.foreignCollection.name); + await this.services.authorization.assertCanExecuteLiveQuerySegment( + context, + this.foreignCollection.name, + ); await this.services.authorization.assertCanReadQueryFields(context, this.foreignCollection); const parentId = IdUtils.unpackId(this.collection.schema, context.params.parentId); diff --git a/packages/agent/src/routes/access/list.ts b/packages/agent/src/routes/access/list.ts index 081cd6bb19..566dcf5bf9 100644 --- a/packages/agent/src/routes/access/list.ts +++ b/packages/agent/src/routes/access/list.ts @@ -12,6 +12,10 @@ export default class ListRoute extends CollectionRoute { public async handleList(context: Context) { await this.services.authorization.assertCanBrowse(context, this.collection.name); + await this.services.authorization.assertCanExecuteLiveQuerySegment( + context, + this.collection.name, + ); await this.services.authorization.assertCanReadQueryFields(context, this.collection); const scope = await this.services.authorization.getScope(this.collection, context); diff --git a/packages/agent/src/routes/modification/action/action.ts b/packages/agent/src/routes/modification/action/action.ts index c8795ad58b..ed5be82a3a 100644 --- a/packages/agent/src/routes/modification/action/action.ts +++ b/packages/agent/src/routes/modification/action/action.ts @@ -385,6 +385,11 @@ export default class ActionRoute extends CollectionRoute { const attributes = body?.data?.attributes; // Match user filter + search + scope? + segment. + await this.services.authorization.assertCanExecuteLiveQuerySegment( + context, + this.collection.name, + ); + const scope = includeUserScope ? await this.services.authorization.getScope(this.collection, context) : null; diff --git a/packages/agent/src/routes/modification/associate-related.ts b/packages/agent/src/routes/modification/associate-related.ts index 96967434c8..2e007f4a9f 100644 --- a/packages/agent/src/routes/modification/associate-related.ts +++ b/packages/agent/src/routes/modification/associate-related.ts @@ -59,6 +59,11 @@ export default class AssociateRelatedRoute extends RelationRoute { targetedRelationId: CompositeId, context: Context, ) { + await this.services.authorization.assertCanExecuteLiveQuerySegment( + context, + this.foreignCollection.name, + ); + const [id] = SchemaUtils.getPrimaryKeys(this.foreignCollection.schema); let value = await CollectionUtils.getValue( this.foreignCollection, diff --git a/packages/agent/src/routes/modification/delete.ts b/packages/agent/src/routes/modification/delete.ts index d0293fcb4d..f7df1bf1b8 100644 --- a/packages/agent/src/routes/modification/delete.ts +++ b/packages/agent/src/routes/modification/delete.ts @@ -36,6 +36,11 @@ export default class DeleteRoute extends CollectionRoute { } private async deleteRecords(context: Context, selectionIds: SelectionIds): Promise { + await this.services.authorization.assertCanExecuteLiveQuerySegment( + context, + this.collection.name, + ); + let selectedIds = ConditionTreeFactory.matchIds(this.collection.schema, selectionIds.ids); if (selectionIds.areExcluded) selectedIds = selectedIds.inverse(); diff --git a/packages/agent/src/routes/modification/dissociate-delete-related.ts b/packages/agent/src/routes/modification/dissociate-delete-related.ts index 49c6930419..16125dcfd5 100644 --- a/packages/agent/src/routes/modification/dissociate-delete-related.ts +++ b/packages/agent/src/routes/modification/dissociate-delete-related.ts @@ -105,6 +105,11 @@ export default class DissociateDeleteRelatedRoute extends RelationRoute { * The filter that is generated by this condition it _not_ restricted by the parent record */ private async getBaseForeignFilter(context: Context): Promise { + await this.services.authorization.assertCanExecuteLiveQuerySegment( + context, + this.foreignCollection.name, + ); + const selectionIds = BodyParser.parseSelectionIds(this.foreignCollection.schema, context); let selectedIds = ConditionTreeFactory.matchIds( this.foreignCollection.schema, diff --git a/packages/agent/src/services/authorization/authorization.ts b/packages/agent/src/services/authorization/authorization.ts index f14bfad821..e420dd5f91 100644 --- a/packages/agent/src/services/authorization/authorization.ts +++ b/packages/agent/src/services/authorization/authorization.ts @@ -187,6 +187,29 @@ export default class AuthorizationService { } } + public async assertCanExecuteLiveQuerySegment( + context: Context, + collectionName: string, + ): Promise { + const liveQuerySegment = context.request + ? QueryStringParser.parseLiveQuerySegment(context) + : null; + + if (!liveQuerySegment) return; + + const { id: userId, renderingId } = context.state.user; + const canExecuteSegmentQuery = + await this.forestAdminClient.permissionService.canExecuteSegmentQuery({ + userId, + collectionName, + renderingId, + segmentQuery: liveQuerySegment.query, + connectionName: liveQuerySegment.connectionName, + }); + + if (!canExecuteSegmentQuery) context.throw(HttpCode.Forbidden, 'Forbidden'); + } + /** The root is skipped: `browse` gates a listing, `read` a get, and the signed hash a chart. */ private async getReadPermissions( context: Context, @@ -207,7 +230,7 @@ export default class AuthorizationService { context: Context, collectionName: string, ) { - const { id: userId, renderingId } = context.state.user; + const { id: userId } = context.state.user; const canOnCollection = await this.forestAdminClient.permissionService.canOnCollection({ userId, @@ -215,28 +238,6 @@ export default class AuthorizationService { collectionName, }); - if ( - context.request?.query && - CollectionActionEvent.Browse === event && - (context.request.query as { segmentQuery?: string }).segmentQuery - ) { - const { segmentQuery, connectionName } = context.request.query as { - segmentQuery?: string; - connectionName?: string; - }; - - const canExecuteSegmentQuery = - await this.forestAdminClient.permissionService.canExecuteSegmentQuery({ - userId, - collectionName, - renderingId, - segmentQuery, - connectionName, - }); - - if (!canExecuteSegmentQuery) context.throw(HttpCode.Forbidden, 'Forbidden'); - } - if (!canOnCollection) { context.throw(HttpCode.Forbidden, 'Forbidden'); } diff --git a/packages/agent/test/__factories__/authorization/authorization.ts b/packages/agent/test/__factories__/authorization/authorization.ts index 321ae86312..4740e30192 100644 --- a/packages/agent/test/__factories__/authorization/authorization.ts +++ b/packages/agent/test/__factories__/authorization/authorization.ts @@ -14,6 +14,7 @@ export class AuthorizationsFactory extends Factory { Authorizations.assertCanExport = jest.fn(); Authorizations.getScope = jest.fn(); Authorizations.assertCanExecuteChart = jest.fn(); + Authorizations.assertCanExecuteLiveQuerySegment = jest.fn(); Authorizations.invalidateScopeCache = jest.fn(); Authorizations.canRead = jest.fn().mockResolvedValue(true); Authorizations.assertCanReadQueryFields = jest.fn(); diff --git a/packages/agent/test/routes/access/chart.test.ts b/packages/agent/test/routes/access/chart.test.ts index 3715ceaad7..6f7c9c9a74 100644 --- a/packages/agent/test/routes/access/chart.test.ts +++ b/packages/agent/test/routes/access/chart.test.ts @@ -6,6 +6,7 @@ import { import { createMockContext } from '@shopify/jest-koa-mocks'; import Chart from '../../../src/routes/access/chart'; +import AuthorizationService from '../../../src/services/authorization/authorization'; import * as factories from '../../__factories__'; describe('ChartRoute', () => { @@ -1128,4 +1129,172 @@ describe('ChartRoute', () => { expect(router.post).toHaveBeenCalledWith('/stats/books\\+\\?\\*', expect.any(Function)); }); }); + + describe('with a live query segment and the real authorization service', () => { + const throwingThrow = () => + jest.fn((status: number, message: string) => { + throw Object.assign(new Error(message), { status }); + }); + + const setup = (canExecuteSegmentQuery: boolean) => { + const forestAdminClient = factories.forestAdminClient.build(); + (forestAdminClient.permissionService.canExecuteChart as jest.Mock).mockResolvedValue(true); + (forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock).mockResolvedValue( + canExecuteSegmentQuery, + ); + const services = factories.forestAdminHttpDriverServices.build({ + authorization: new AuthorizationService(forestAdminClient), + }); + (services.chartHandler.getChartWithContextInjected as jest.Mock).mockImplementation( + ({ chartRequest }) => chartRequest, + ); + const books = factories.collection.build({ + name: 'books', + schema: factories.collectionSchema.build({ + fields: { id: factories.columnSchema.uuidPrimaryKey().build() }, + }), + aggregate: jest.fn().mockResolvedValue([{ value: 1234, group: null }]), + }); + const dataSource = factories.dataSource.buildWithCollection(books); + const chart = new Chart( + services, + factories.forestAdminHttpDriverOptions.build(), + dataSource, + 'books', + ); + const context = createMockContext({ + throw: throwingThrow(), + requestBody: { type: 'Value', aggregator: 'Count', sourceCollectionName: 'books' }, + customProperties: { + query: { + timezone: 'Europe/Paris', + segmentQuery: 'SELECT id FROM books', + connectionName: 'main', + }, + }, + state: { user: { email: 'john.doe@domain.com', id: 100, renderingId: 7 } }, + }); + + return { forestAdminClient, books, chart, context }; + }; + + const expectedSegmentCheck = { + userId: 100, + collectionName: 'books', + renderingId: 7, + segmentQuery: 'SELECT id FROM books', + connectionName: 'main', + }; + + test('refuses a chart on a segment the user is not allowed to run', async () => { + const { forestAdminClient, books, chart, context } = setup(false); + + await expect(chart.handleChart(context)).rejects.toMatchObject({ status: 403 }); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith( + expectedSegmentCheck, + ); + expect(books.aggregate).not.toHaveBeenCalled(); + }); + + test('checks a count leaderboard segment against the charted collection only', async () => { + const forestAdminClient = factories.forestAdminClient.build(); + (forestAdminClient.permissionService.canExecuteChart as jest.Mock).mockResolvedValue(true); + (forestAdminClient.permissionService.canOnCollection as jest.Mock).mockResolvedValue(true); + (forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock).mockImplementation( + async ({ collectionName }) => collectionName === 'persons', + ); + const services = factories.forestAdminHttpDriverServices.build({ + authorization: new AuthorizationService(forestAdminClient), + }); + (services.chartHandler.getChartWithContextInjected as jest.Mock).mockImplementation( + ({ chartRequest }) => chartRequest, + ); + const persons = factories.collection.build({ + name: 'persons', + schema: factories.collectionSchema.build({ + fields: { + id: factories.columnSchema.uuidPrimaryKey().build(), + books: factories.oneToManySchema.build({ + foreignCollection: 'books', + originKey: 'authorId', + originKeyTarget: 'id', + }), + }, + }), + }); + const books = factories.collection.build({ + name: 'books', + schema: factories.collectionSchema.build({ + fields: { + id: factories.columnSchema.uuidPrimaryKey().build(), + authorId: factories.columnSchema.build({ columnType: 'Uuid' }), + author: factories.manyToOneSchema.build({ + foreignCollection: 'persons', + foreignKey: 'authorId', + foreignKeyTarget: 'id', + }), + }, + }), + aggregate: jest.fn().mockResolvedValue([{ value: 3, group: { 'author:id': 'a1' } }]), + }); + const dataSource = factories.dataSource.buildWithCollections([persons, books]); + const chart = new Chart( + services, + factories.forestAdminHttpDriverOptions.build(), + dataSource, + 'persons', + ); + const context = createMockContext({ + throw: throwingThrow(), + requestBody: { + type: 'Leaderboard', + aggregator: 'Count', + sourceCollectionName: 'persons', + labelFieldName: 'id', + relationshipFieldName: 'books', + limit: 5, + }, + customProperties: { + query: { + timezone: 'Europe/Paris', + segmentQuery: 'SELECT id FROM persons', + connectionName: 'main', + }, + }, + state: { user: { email: 'john.doe@domain.com', id: 100, renderingId: 7 } }, + }); + + await chart.handleChart(context); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledTimes(1); + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ + userId: 100, + collectionName: 'persons', + renderingId: 7, + segmentQuery: 'SELECT id FROM persons', + connectionName: 'main', + }); + expect(context.response.body).toMatchObject({ + data: { attributes: { value: [{ key: 'a1', value: 3 }] } }, + }); + }); + + test('computes a chart on a segment the user is allowed to run', async () => { + const { forestAdminClient, books, chart, context } = setup(true); + + await chart.handleChart(context); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith( + expectedSegmentCheck, + ); + expect(books.aggregate).toHaveBeenCalledWith( + expect.objectContaining({ id: 100 }), + expect.objectContaining({ + liveQuerySegment: { query: 'SELECT id FROM books', connectionName: 'main' }, + }), + { field: undefined, groups: undefined, operation: 'Count' }, + ); + }); + }); }); diff --git a/packages/agent/test/routes/access/count-related.test.ts b/packages/agent/test/routes/access/count-related.test.ts index 6ef8ad53f2..827ecf970a 100644 --- a/packages/agent/test/routes/access/count-related.test.ts +++ b/packages/agent/test/routes/access/count-related.test.ts @@ -76,6 +76,27 @@ describe('CountRelatedRoute', () => { ); }); + test('refuses the count of the related records when their live query segment is not allowed', async () => { + const { services, dataSource, options } = setupWithOneToManyRelation(); + (services.authorization.assertCanExecuteLiveQuerySegment as jest.Mock).mockRejectedValueOnce( + new Error('Forbidden'), + ); + const aggregateRelation = jest + .spyOn(CollectionUtils, 'aggregateRelation') + .mockResolvedValue([{ value: 1, group: {} }]); + const route = new CountRelatedRoute(services, options, dataSource, 'books', 'myBookPersons'); + const context = setupContext(); + + await expect(route.handleCountRelated(context)).rejects.toThrow('Forbidden'); + + expect(services.authorization.assertCanExecuteLiveQuerySegment).toHaveBeenCalledWith( + context, + 'bookPersons', + ); + expect(aggregateRelation).not.toHaveBeenCalled(); + aggregateRelation.mockRestore(); + }); + describe('handleCountRelated', () => { describe('when the request is correct', () => { test('should aggregate the relation and return the result', async () => { diff --git a/packages/agent/test/routes/access/count.test.ts b/packages/agent/test/routes/access/count.test.ts index c0f4e379ed..f52e9f1a59 100644 --- a/packages/agent/test/routes/access/count.test.ts +++ b/packages/agent/test/routes/access/count.test.ts @@ -51,6 +51,27 @@ describe('CountRoute', () => { expect(context.response.body).toEqual({ count: 2 }); }); + test('refuses the count before aggregating when the live query segment is not allowed', async () => { + const aggregateSpy = jest.fn().mockReturnValue([{ value: 2 }]); + dataSource.getCollection('books').aggregate = aggregateSpy; + ( + services.authorization.assertCanExecuteLiveQuerySegment as jest.Mock + ).mockRejectedValueOnce(new Error('Forbidden')); + const count = new Count(services, options, dataSource, 'books'); + const context = createMockContext({ + customProperties: { query: { timezone: 'Europe/Paris' } }, + state: { user: { email: 'john.doe@domain.com' } }, + }); + + await expect(count.handleCount(context)).rejects.toThrow('Forbidden'); + + expect(services.authorization.assertCanExecuteLiveQuerySegment).toHaveBeenCalledWith( + context, + 'books', + ); + expect(aggregateSpy).not.toHaveBeenCalled(); + }); + test('should ignore the Forest-Projection header', async () => { const aggregateSpy = jest.fn().mockReturnValue([{ value: 2 }]); dataSource.getCollection('books').aggregate = aggregateSpy; diff --git a/packages/agent/test/routes/access/csv-related.test.ts b/packages/agent/test/routes/access/csv-related.test.ts index c25df6d350..bc596719fc 100644 --- a/packages/agent/test/routes/access/csv-related.test.ts +++ b/packages/agent/test/routes/access/csv-related.test.ts @@ -60,6 +60,30 @@ describe('CsvRelatedRoute', () => { ); }); + test('refuses the export of the related records when their live query segment is not allowed', async () => { + const { options, services, dataSource } = setupWithOneToManyRelation(); + (services.authorization.assertCanExecuteLiveQuerySegment as jest.Mock).mockRejectedValueOnce( + new Error('Forbidden'), + ); + const listRelation = jest.spyOn(CollectionUtils, 'listRelation').mockResolvedValue([]); + const csvRoute = new CsvRoute(services, options, dataSource, 'books', 'myPersons'); + const context = createMockContext({ + customProperties: { + params: { parentId: '2d162303-78bf-599e-b197-93590ac3d315' }, + query: { 'fields[persons]': 'name', timezone: 'Europe/Paris' }, + }, + }); + + await expect(csvRoute.handleRelatedCsv(context)).rejects.toThrow('Forbidden'); + + expect(services.authorization.assertCanExecuteLiveQuerySegment).toHaveBeenCalledWith( + context, + 'persons', + ); + expect(listRelation).not.toHaveBeenCalled(); + listRelation.mockRestore(); + }); + describe('handleRelatedCsv', () => { it('should set the response headers correctly', async () => { const { options, services, dataSource } = setupWithOneToManyRelation(); diff --git a/packages/agent/test/routes/access/csv.test.ts b/packages/agent/test/routes/access/csv.test.ts index 0d6393b164..2c7c01e901 100644 --- a/packages/agent/test/routes/access/csv.test.ts +++ b/packages/agent/test/routes/access/csv.test.ts @@ -38,6 +38,25 @@ describe('CsvRoute', () => { describe('handleCsv', () => { const limitExportSize = 5000; + it('refuses the export before listing when the live query segment is not allowed', async () => { + const { options, services, dataSource } = setup(); + (services.authorization.assertCanExecuteLiveQuerySegment as jest.Mock).mockRejectedValueOnce( + new Error('Forbidden'), + ); + const csvRoute = new CsvRoute(services, options, dataSource, 'books'); + const context = createMockContext({ + customProperties: { query: { 'fields[books]': 'name', timezone: 'Europe/Paris' } }, + }); + + await expect(csvRoute.handleCsv(context)).rejects.toThrow('Forbidden'); + + expect(services.authorization.assertCanExecuteLiveQuerySegment).toHaveBeenCalledWith( + context, + 'books', + ); + expect(dataSource.getCollection('books').list).not.toHaveBeenCalled(); + }); + it('should set the response headers correctly', async () => { const { options, services, dataSource } = setup(); const csvRoute = new CsvRoute(services, options, dataSource, 'books'); diff --git a/packages/agent/test/routes/access/list-related.test.ts b/packages/agent/test/routes/access/list-related.test.ts index 3b95976421..acb14b5080 100644 --- a/packages/agent/test/routes/access/list-related.test.ts +++ b/packages/agent/test/routes/access/list-related.test.ts @@ -60,6 +60,31 @@ describe('ListRelatedRoute', () => { ); }); + test('refuses the listing of the related records when their live query segment is not allowed', async () => { + const { services, dataSource, options } = setupWithOneToManyRelation(); + (services.authorization.assertCanExecuteLiveQuerySegment as jest.Mock).mockRejectedValueOnce( + new Error('Forbidden'), + ); + const listRelation = jest.spyOn(CollectionUtils, 'listRelation').mockResolvedValue([]); + const route = new ListRelatedRoute(services, options, dataSource, 'books', 'myPersons'); + const context = createMockContext({ + state: { user: { email: 'john.doe@domain.com' } }, + customProperties: { + params: { parentId: '2d162303-78bf-599e-b197-93590ac3d315' }, + query: { timezone: 'Europe/Paris' }, + }, + }); + + await expect(route.handleListRelated(context)).rejects.toThrow('Forbidden'); + + expect(services.authorization.assertCanExecuteLiveQuerySegment).toHaveBeenCalledWith( + context, + 'persons', + ); + expect(listRelation).not.toHaveBeenCalled(); + listRelation.mockRestore(); + }); + describe('handleListRelated', () => { describe('when the request is correct', () => { test('should return the record result', async () => { diff --git a/packages/agent/test/routes/access/list.test.ts b/packages/agent/test/routes/access/list.test.ts index 047a41709b..acf4622aac 100644 --- a/packages/agent/test/routes/access/list.test.ts +++ b/packages/agent/test/routes/access/list.test.ts @@ -2,6 +2,8 @@ import { Projection } from '@forestadmin/datasource-toolkit'; import { createMockContext } from '@shopify/jest-koa-mocks'; import List from '../../../src/routes/access/list'; +import AuthorizationService from '../../../src/services/authorization/authorization'; +import SegmentQueryHandler from '../../../src/services/segment-query-handler'; import * as factories from '../../__factories__'; describe('ListRoute', () => { @@ -284,4 +286,91 @@ describe('ListRoute', () => { expect(router.get).toHaveBeenCalledWith('/books\\+\\?\\*', expect.any(Function)); }); }); + + describe('with a live query segment and the real authorization service', () => { + const throwingThrow = () => + jest.fn((status: number, message: string) => { + throw Object.assign(new Error(message), { status }); + }); + + const setupWithRealAuthorization = (canExecuteSegmentQuery: boolean) => { + const { dataSource, options, collection } = setup(); + const forestAdminClient = factories.forestAdminClient.build(); + (forestAdminClient.permissionService.canOnCollection as jest.Mock).mockResolvedValue(true); + (forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock).mockResolvedValue( + canExecuteSegmentQuery, + ); + ( + forestAdminClient.contextVariablesInstantiator.buildContextVariables as jest.Mock + ).mockResolvedValue({ getValue: jest.fn() }); + const services = factories.forestAdminHttpDriverServices.build({ + authorization: new AuthorizationService(forestAdminClient), + segmentQueryHandler: new SegmentQueryHandler( + forestAdminClient.contextVariablesInstantiator, + ), + }); + jest.spyOn(collection, 'list').mockResolvedValue([{ id: 1 }]); + + return { + forestAdminClient, + list: new List(services, options, dataSource, collection.name), + collection, + }; + }; + + const buildContext = () => + createMockContext({ + throw: throwingThrow(), + state: { user: { id: 35, renderingId: 42, email: 'john.doe@domain.com' } }, + customProperties: { + query: { + timezone: 'Europe/Paris', + segmentQuery: 'SELECT id FROM books', + connectionName: 'main', + }, + }, + }); + + const expectedSegmentCheck = { + userId: 35, + collectionName: 'books', + renderingId: 42, + segmentQuery: 'SELECT id FROM books', + connectionName: 'main', + }; + + test('refuses to list a segment the user is not allowed to run', async () => { + const { forestAdminClient, list, collection } = setupWithRealAuthorization(false); + const context = buildContext(); + + await expect(list.handleList(context)).rejects.toMatchObject({ status: 403 }); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith( + expectedSegmentCheck, + ); + expect(collection.list).not.toHaveBeenCalled(); + }); + + test('lists a segment the user is allowed to run', async () => { + const { forestAdminClient, list, collection } = setupWithRealAuthorization(true); + const context = buildContext(); + + await list.handleList(context); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith( + expectedSegmentCheck, + ); + expect(collection.list).toHaveBeenCalledWith( + expect.objectContaining({ timezone: 'Europe/Paris' }), + expect.objectContaining({ + liveQuerySegment: { + query: 'SELECT id FROM books', + connectionName: 'main', + contextVariables: {}, + }, + }), + expect.anything(), + ); + }); + }); }); diff --git a/packages/agent/test/routes/modification/action/action.test.ts b/packages/agent/test/routes/modification/action/action.test.ts index 6e3544e179..a16d11c7c0 100644 --- a/packages/agent/test/routes/modification/action/action.test.ts +++ b/packages/agent/test/routes/modification/action/action.test.ts @@ -6,6 +6,7 @@ import { Readable } from 'stream'; import ActionRoute from '../../../../src/routes/modification/action/action'; import CustomActionTriggerForbiddenError from '../../../../src/routes/modification/action/errors/custom-action-trigger-forbidden-error'; +import AuthorizationService from '../../../../src/services/authorization/authorization'; import * as factories from '../../../__factories__'; // This part of the context is the same in all tests. @@ -1927,4 +1928,166 @@ describe('ActionRoute', () => { }); }); }); + + describe('with a live query segment and the real authorization service', () => { + const throwingThrow = () => + jest.fn((status: number, message: string) => { + throw Object.assign(new Error(message), { status }); + }); + + let realServices: ReturnType; + + const segmentQuery = 'SELECT id FROM books'; + const expectedSegmentCheck = { + userId: '42', + collectionName: 'books', + renderingId: 7, + segmentQuery, + connectionName: 'main', + }; + + const buildContext = (attributes: Record = {}) => + createMockContext({ + ...baseContext, + throw: throwingThrow(), + state: { user: { ...baseContext.state.user, renderingId: 7 } }, + requestBody: { + data: { + attributes: { + ...baseContext.requestBody.data.attributes, + ids: [], + all_records: true, + values: { firstname: 'John' }, + all_records_subset_query: { segmentQuery, connectionName: 'main' }, + ...attributes, + }, + }, + }, + }); + + const allowSegmentQuery = (allowed: boolean) => + ( + options.forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock + ).mockResolvedValue(allowed); + + beforeEach(() => { + realServices = factories.forestAdminHttpDriverServices.build({ + authorization: new AuthorizationService(options.forestAdminClient), + }); + dataSource = factories.dataSource.buildWithCollections([ + factories.collection.build({ + name: 'books', + schema: { + actions: { MyBulkAction: { scope: 'Bulk' } }, + fields: { id: factories.columnSchema.uuidPrimaryKey().build() }, + }, + getForm: jest.fn().mockResolvedValue([{ type: 'String', label: 'firstname' }]), + execute: jest.fn().mockResolvedValue({ type: 'Success', message: 'ok', invalidated: [] }), + }), + ]); + + route = new ActionRoute(realServices, options, dataSource, 'books', 'MyBulkAction'); + }); + + test('refuses to execute an action on a segment the user is not allowed to run', async () => { + allowSegmentQuery(false); + const context = buildContext(); + + // @ts-expect-error: test private method + await expect(route.handleExecute(context)).rejects.toMatchObject({ status: 403 }); + + expect( + options.forestAdminClient.permissionService.canExecuteSegmentQuery, + ).toHaveBeenCalledWith(expectedSegmentCheck); + expect(dataSource.getCollection('books').execute).not.toHaveBeenCalled(); + }); + + test('executes an action on a segment the user is allowed to run', async () => { + allowSegmentQuery(true); + const context = buildContext(); + + // @ts-expect-error: test private method + await route.handleExecute(context); + + expect( + options.forestAdminClient.permissionService.canExecuteSegmentQuery, + ).toHaveBeenCalledWith(expectedSegmentCheck); + expect(dataSource.getCollection('books').execute).toHaveBeenCalledWith( + expect.objectContaining({ id: '42' }), + 'MyBulkAction', + { firstname: 'John' }, + expect.objectContaining({ + liveQuerySegment: { query: segmentQuery, connectionName: 'main' }, + }), + ); + expect(context.response.body).toEqual(expect.objectContaining({ success: 'ok' })); + }); + + test('refuses to load an action form on a segment the user is not allowed to run', async () => { + allowSegmentQuery(false); + const context = buildContext(); + + // @ts-expect-error: test private method + await expect(route.handleHook(context)).rejects.toMatchObject({ status: 403 }); + + expect( + options.forestAdminClient.permissionService.canExecuteSegmentQuery, + ).toHaveBeenCalledWith(expectedSegmentCheck); + expect(dataSource.getCollection('books').getForm).not.toHaveBeenCalled(); + }); + + test('loads an action form on a segment the user is allowed to run', async () => { + allowSegmentQuery(true); + const context = buildContext(); + + // @ts-expect-error: test private method + await route.handleHook(context); + + expect( + options.forestAdminClient.permissionService.canExecuteSegmentQuery, + ).toHaveBeenCalledWith(expectedSegmentCheck); + expect(dataSource.getCollection('books').getForm).toHaveBeenCalledWith( + expect.objectContaining({ id: '42' }), + 'MyBulkAction', + null, + expect.objectContaining({ + liveQuerySegment: { query: segmentQuery, connectionName: 'main' }, + }), + expect.objectContaining({ includeHiddenFields: false }), + ); + }); + + test('refuses an approval on a segment the approver is not allowed to run', async () => { + allowSegmentQuery(false); + const context = buildContext({ requester_id: 12 }); + + // @ts-expect-error: test private method + await expect(route.handleExecute(context)).rejects.toMatchObject({ status: 403 }); + + expect( + options.forestAdminClient.permissionService.canExecuteSegmentQuery, + ).toHaveBeenCalledWith(expectedSegmentCheck); + expect(dataSource.getCollection('books').execute).not.toHaveBeenCalled(); + }); + + test('executes an approval on a segment the approver is allowed to run', async () => { + allowSegmentQuery(true); + const context = buildContext({ requester_id: 12 }); + + // @ts-expect-error: test private method + await route.handleExecute(context); + + expect( + options.forestAdminClient.permissionService.canExecuteSegmentQuery, + ).toHaveBeenCalledWith(expectedSegmentCheck); + expect(dataSource.getCollection('books').execute).toHaveBeenCalledWith( + expect.objectContaining({ id: '42' }), + 'MyBulkAction', + { firstname: 'John' }, + expect.objectContaining({ + liveQuerySegment: { query: segmentQuery, connectionName: 'main' }, + }), + ); + }); + }); }); diff --git a/packages/agent/test/routes/modification/associate-related.test.ts b/packages/agent/test/routes/modification/associate-related.test.ts index 462cf01f9c..a36676a5c5 100644 --- a/packages/agent/test/routes/modification/associate-related.test.ts +++ b/packages/agent/test/routes/modification/associate-related.test.ts @@ -58,6 +58,36 @@ describe('AssociateRelatedRoute', () => { ); }); + test('refuses the association before updating when the live query segment is not allowed', async () => { + const { services, dataSource, options } = setupWithOneToManyRelation(); + (services.authorization.assertCanExecuteLiveQuerySegment as jest.Mock).mockRejectedValueOnce( + new Error('Forbidden'), + ); + const route = new AssociateRelatedRoute( + services, + options, + dataSource, + 'books', + 'myBookPersons', + ); + const context = createMockContext({ + state: { user: { email: 'john.doe@domain.com' } }, + requestBody: { data: [{ id: '123e4567-e89b-12d3-a456-222222222222' }] }, + customProperties: { + query: { timezone: 'Europe/Paris' }, + params: { parentId: '123e4567-e89b-12d3-a456-111111111111' }, + }, + }); + + await expect(route.handleAssociateRelatedRoute(context)).rejects.toThrow('Forbidden'); + + expect(services.authorization.assertCanExecuteLiveQuerySegment).toHaveBeenCalledWith( + context, + 'bookPersons', + ); + expect(dataSource.getCollection('bookPersons').update).not.toHaveBeenCalled(); + }); + test('should update the related records', async () => { // given const { services, dataSource, options } = setupWithOneToManyRelation(); diff --git a/packages/agent/test/routes/modification/delete.test.ts b/packages/agent/test/routes/modification/delete.test.ts index 761b39d7be..c0a32fd409 100644 --- a/packages/agent/test/routes/modification/delete.test.ts +++ b/packages/agent/test/routes/modification/delete.test.ts @@ -1,6 +1,7 @@ import { createMockContext } from '@shopify/jest-koa-mocks'; import DeleteRoute from '../../../src/routes/modification/delete'; +import AuthorizationService from '../../../src/services/authorization/authorization'; import { HttpCode } from '../../../src/types'; import * as factories from '../../__factories__'; @@ -465,6 +466,123 @@ describe('DeleteRoute', () => { }); }); + describe('with a live query segment and the real authorization service', () => { + const throwingThrow = () => + jest.fn((status: number, message: string) => { + throw Object.assign(new Error(message), { status }); + }); + + const setup = (canExecuteSegmentQuery: boolean) => { + const forestAdminClient = factories.forestAdminClient.build(); + (forestAdminClient.permissionService.canOnCollection as jest.Mock).mockResolvedValue(true); + (forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock).mockResolvedValue( + canExecuteSegmentQuery, + ); + const realServices = factories.forestAdminHttpDriverServices.build({ + authorization: new AuthorizationService(forestAdminClient), + }); + const bookCollection = factories.collection.build({ + name: 'books', + schema: factories.collectionSchema.build({ + fields: { id: factories.columnSchema.numericPrimaryKey().build() }, + }), + }); + const dataSource = factories.dataSource.buildWithCollection(bookCollection); + const deleteRoute = new DeleteRoute(realServices, options, dataSource, 'books'); + + return { forestAdminClient, bookCollection, deleteRoute }; + }; + + const listDeleteContext = () => + createMockContext({ + throw: throwingThrow(), + state: { user: { id: 35, renderingId: 42, email: 'john.doe@domain.com' } }, + customProperties: { query: { timezone: 'Europe/Paris' } }, + requestBody: { + data: { + attributes: { + ids: [], + all_records: true, + all_records_ids_excluded: [], + all_records_subset_query: { + segmentQuery: 'SELECT id FROM books', + connectionName: 'main', + }, + }, + }, + }, + }); + + test('refuses a bulk delete whose segment query the user is not allowed to run', async () => { + const { forestAdminClient, bookCollection, deleteRoute } = setup(false); + const context = listDeleteContext(); + + await expect(deleteRoute.handleListDelete(context)).rejects.toMatchObject({ + status: HttpCode.Forbidden, + }); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ + userId: 35, + collectionName: 'books', + renderingId: 42, + segmentQuery: 'SELECT id FROM books', + connectionName: 'main', + }); + expect(bookCollection.delete).not.toHaveBeenCalled(); + }); + + test('deletes the records of a segment the user is allowed to run', async () => { + const { forestAdminClient, bookCollection, deleteRoute } = setup(true); + const context = listDeleteContext(); + + await deleteRoute.handleListDelete(context); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ + userId: 35, + collectionName: 'books', + renderingId: 42, + segmentQuery: 'SELECT id FROM books', + connectionName: 'main', + }); + expect(bookCollection.delete).toHaveBeenCalledWith( + expect.objectContaining({ timezone: 'Europe/Paris' }), + expect.objectContaining({ + liveQuerySegment: { query: 'SELECT id FROM books', connectionName: 'main' }, + }), + ); + expect(context.response.status).toEqual(HttpCode.NoContent); + }); + + test('refuses a single delete whose query string carries a segment query the user is not allowed to run', async () => { + const { forestAdminClient, bookCollection, deleteRoute } = setup(false); + const context = createMockContext({ + throw: throwingThrow(), + state: { user: { id: 35, renderingId: 42, email: 'john.doe@domain.com' } }, + customProperties: { + params: { id: '1523' }, + query: { + timezone: 'Europe/Paris', + segmentQuery: 'SELECT id FROM books', + connectionName: 'main', + }, + }, + }); + + await expect(deleteRoute.handleDelete(context)).rejects.toMatchObject({ + status: HttpCode.Forbidden, + }); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ + userId: 35, + collectionName: 'books', + renderingId: 42, + segmentQuery: 'SELECT id FROM books', + connectionName: 'main', + }); + expect(bookCollection.delete).not.toHaveBeenCalled(); + }); + }); + describe('with special characters in names', () => { it('should register a route with escaped characters', () => { const bookCollection = factories.collection.build({ diff --git a/packages/agent/test/routes/modification/dissociate-delete-related.test.ts b/packages/agent/test/routes/modification/dissociate-delete-related.test.ts index c4e49c8bc5..2f762d5c9a 100644 --- a/packages/agent/test/routes/modification/dissociate-delete-related.test.ts +++ b/packages/agent/test/routes/modification/dissociate-delete-related.test.ts @@ -65,6 +65,37 @@ describe('DissociateDeleteRelatedRoute', () => { ); }); + test('refuses to dissociate or delete related records when their live query segment is not allowed', async () => { + const { services, dataSource, options } = setupWithOneToManyRelation(); + (services.authorization.assertCanExecuteLiveQuerySegment as jest.Mock).mockRejectedValueOnce( + new Error('Forbidden'), + ); + const route = new DissociateDeleteRoute( + services, + options, + dataSource, + 'books', + 'myBookPersons', + ); + const context = createMockContext({ + state: { user: { email: 'john.doe@domain.com' } }, + customProperties: { + query: { timezone: 'Europe/Paris', delete: 'true' }, + params: { parentId: '123e4567-e89b-12d3-a456-426614174088' }, + }, + requestBody: { data: [{ id: '123e4567-e89b-12d3-a456-426614174001' }] }, + }); + + await expect(route.handleDissociateDeleteRelatedRoute(context)).rejects.toThrow('Forbidden'); + + expect(services.authorization.assertCanExecuteLiveQuerySegment).toHaveBeenCalledWith( + context, + 'bookPersons', + ); + expect(dataSource.getCollection('bookPersons').delete).not.toHaveBeenCalled(); + expect(dataSource.getCollection('bookPersons').update).not.toHaveBeenCalled(); + }); + describe('DissociateDeleteRelatedRoute > dissociate', () => { describe('when it is a one to many relation', () => { test('should update the foreign key in the foreign collection', async () => { diff --git a/packages/agent/test/services/authorization/authorization.test.ts b/packages/agent/test/services/authorization/authorization.test.ts index 30ebb5a815..36cbeb8cde 100644 --- a/packages/agent/test/services/authorization/authorization.test.ts +++ b/packages/agent/test/services/authorization/authorization.test.ts @@ -83,94 +83,130 @@ describe('AuthorizationService', () => { }); }); - describe('assertCanBrowseOnCollection', () => { - describe('when a live query segment is present', () => { - it('should not do anything for authorized users', async () => { - const forestAdminClient = factories.forestAdminClient.build(); - const authorizationService = new AuthorizationService(forestAdminClient); - - const context = { - request: { - query: { - segmentQuery: `SELECT id FROM books where type = 'comics'`, - connectionName: 'library', - }, - }, - state: { - user: { - id: 35, - renderingId: 42, - }, - }, - throw: jest.fn(), - } as unknown as Context; - - (forestAdminClient.permissionService.canOnCollection as jest.Mock).mockResolvedValue(true); - (forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock).mockResolvedValue( - true, - ); - - await authorizationService.assertCanBrowse(context, 'books'); - - expect(context.throw).not.toHaveBeenCalled(); - - expect(forestAdminClient.permissionService.canOnCollection).toHaveBeenCalledWith({ - userId: 35, - event: CollectionActionEvent.Browse, - collectionName: 'books', - }); - - expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ - userId: 35, - collectionName: 'books', - renderingId: 42, - segmentQuery: `SELECT id FROM books where type = 'comics'`, - connectionName: 'library', - }); + describe('assertCanExecuteLiveQuerySegment', () => { + const buildContext = (request: { query?: object; body?: object }) => + ({ + request: { query: {}, ...request }, + state: { user: { id: 35, renderingId: 42 } }, + throw: jest.fn(), + } as unknown as Context); + + const subsetQueryBody = (segmentQuery: string, connectionName: string) => ({ + data: { attributes: { all_records_subset_query: { segmentQuery, connectionName } } }, + }); + + let forestAdminClient: ReturnType; + let authorizationService: AuthorizationService; + + beforeEach(() => { + forestAdminClient = factories.forestAdminClient.build(); + authorizationService = new AuthorizationService(forestAdminClient); + }); + + it('refuses the request when the segment query of the query string is not allowed', async () => { + (forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock).mockResolvedValue( + false, + ); + const context = buildContext({ + query: { segmentQuery: 'SELECT id FROM books', connectionName: 'library' }, }); - describe('when query is not authorized', () => { - it('should throw forbidden error', async () => { - const forestAdminClient = factories.forestAdminClient.build(); - const authorizationService = new AuthorizationService(forestAdminClient); - - const context = { - request: { - query: { - segmentQuery: `DROP TABLE books;`, - connectionName: 'library', - }, - }, - state: { - user: { - id: 35, - renderingId: 42, - }, - }, - throw: jest.fn(), - } as unknown as Context; + await authorizationService.assertCanExecuteLiveQuerySegment(context, 'books'); - (forestAdminClient.permissionService.canOnCollection as jest.Mock).mockResolvedValue( - true, - ); - ( - forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock - ).mockResolvedValue(false); + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ + userId: 35, + collectionName: 'books', + renderingId: 42, + segmentQuery: 'SELECT id FROM books', + connectionName: 'library', + }); + expect(context.throw).toHaveBeenCalledWith(HttpCode.Forbidden, 'Forbidden'); + }); - await authorizationService.assertCanBrowse(context, 'books'); + it('refuses the request when the segment query of the request body is not allowed', async () => { + (forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock).mockResolvedValue( + false, + ); + const context = buildContext({ body: subsetQueryBody('SELECT id FROM books', 'library') }); - expect(context.throw).toHaveBeenCalledWith(HttpCode.Forbidden, 'Forbidden'); + await authorizationService.assertCanExecuteLiveQuerySegment(context, 'books'); - expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ - userId: 35, - collectionName: 'books', - renderingId: 42, - segmentQuery: `DROP TABLE books;`, - connectionName: 'library', - }); - }); + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ + userId: 35, + collectionName: 'books', + renderingId: 42, + segmentQuery: 'SELECT id FROM books', + connectionName: 'library', + }); + expect(context.throw).toHaveBeenCalledWith(HttpCode.Forbidden, 'Forbidden'); + }); + + it('checks the segment query the filter will use when both the body and the query string carry one', async () => { + (forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock).mockResolvedValue( + true, + ); + const context = buildContext({ + query: { segmentQuery: 'SELECT id FROM books', connectionName: 'library' }, + body: subsetQueryBody('SELECT id FROM authors', 'archive'), + }); + + await authorizationService.assertCanExecuteLiveQuerySegment(context, 'books'); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledTimes(1); + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ + userId: 35, + collectionName: 'books', + renderingId: 42, + segmentQuery: 'SELECT id FROM authors', + connectionName: 'archive', }); }); + + it('allows the request when the segment query is allowed', async () => { + (forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock).mockResolvedValue( + true, + ); + const context = buildContext({ body: subsetQueryBody('SELECT id FROM books', 'library') }); + + await authorizationService.assertCanExecuteLiveQuerySegment(context, 'books'); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ + userId: 35, + collectionName: 'books', + renderingId: 42, + segmentQuery: 'SELECT id FROM books', + connectionName: 'library', + }); + expect(context.throw).not.toHaveBeenCalled(); + }); + + it('does not check any segment query when the request carries none', async () => { + const context = buildContext({ query: { timezone: 'Europe/Paris' } }); + + await authorizationService.assertCanExecuteLiveQuerySegment(context, 'books'); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).not.toHaveBeenCalled(); + expect(context.throw).not.toHaveBeenCalled(); + }); + + it.each([ + 'assertCanAdd', + 'assertCanBrowse', + 'assertCanDelete', + 'assertCanEdit', + 'assertCanExport', + 'assertCanRead', + ])('%s leaves the segment query to the route whose filter uses it', async assertion => { + (forestAdminClient.permissionService.canOnCollection as jest.Mock).mockResolvedValue(true); + const context = buildContext({ + query: { segmentQuery: 'SELECT id FROM books', connectionName: 'library' }, + }); + + await authorizationService[assertion](context, 'authors'); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).not.toHaveBeenCalled(); + expect(context.throw).not.toHaveBeenCalled(); + }); }); describe('getScope', () => { From 7425188d5bfc2eab625835c71617296714d4d36e Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 27 Oct 2026 16:13:41 +0100 Subject: [PATCH 02/10] test(agent): drive the action segment tests through the registered routes Refs: PRD-1759 Co-Authored-By: Claude Opus 5.5 --- .../routes/modification/action/action.test.ts | 54 +++++++++++++------ 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/packages/agent/test/routes/modification/action/action.test.ts b/packages/agent/test/routes/modification/action/action.test.ts index a16d11c7c0..d6bab134aa 100644 --- a/packages/agent/test/routes/modification/action/action.test.ts +++ b/packages/agent/test/routes/modification/action/action.test.ts @@ -1965,6 +1965,36 @@ describe('ActionRoute', () => { }, }); + const callRoute = async (path: string, context: ReturnType) => { + const actionRouter = factories.router.mockAllMethods().build(); + route.setupRoutes(actionRouter); + const [, ...handlers] = (actionRouter.post as jest.Mock).mock.calls.find( + ([registeredPath]) => registeredPath === path, + ); + const dispatch = (index: number): Promise => + index < handlers.length ? handlers[index](context, () => dispatch(index + 1)) : undefined; + + return dispatch(0); + }; + + const executePath = '/_actions/books/0/mybulkaction'; + const loadHookPath = '/_actions/books/0/mybulkaction/hooks/load'; + + const buildApprovalContext = (allowed: boolean) => { + const signedRequest = buildContext({ requester_id: 12 }).request.body; + (options.forestAdminClient.verifySignedActionParameters as jest.Mock).mockReturnValue( + signedRequest, + ); + allowSegmentQuery(allowed); + + return createMockContext({ + ...baseContext, + throw: throwingThrow(), + state: { user: { ...baseContext.state.user, renderingId: 7 } }, + requestBody: { data: { attributes: { signed_approval_request: 'signed-request' } } }, + }); + }; + const allowSegmentQuery = (allowed: boolean) => ( options.forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock @@ -1993,8 +2023,7 @@ describe('ActionRoute', () => { allowSegmentQuery(false); const context = buildContext(); - // @ts-expect-error: test private method - await expect(route.handleExecute(context)).rejects.toMatchObject({ status: 403 }); + await expect(callRoute(executePath, context)).rejects.toMatchObject({ status: 403 }); expect( options.forestAdminClient.permissionService.canExecuteSegmentQuery, @@ -2006,8 +2035,7 @@ describe('ActionRoute', () => { allowSegmentQuery(true); const context = buildContext(); - // @ts-expect-error: test private method - await route.handleExecute(context); + await callRoute(executePath, context); expect( options.forestAdminClient.permissionService.canExecuteSegmentQuery, @@ -2027,8 +2055,7 @@ describe('ActionRoute', () => { allowSegmentQuery(false); const context = buildContext(); - // @ts-expect-error: test private method - await expect(route.handleHook(context)).rejects.toMatchObject({ status: 403 }); + await expect(callRoute(loadHookPath, context)).rejects.toMatchObject({ status: 403 }); expect( options.forestAdminClient.permissionService.canExecuteSegmentQuery, @@ -2040,8 +2067,7 @@ describe('ActionRoute', () => { allowSegmentQuery(true); const context = buildContext(); - // @ts-expect-error: test private method - await route.handleHook(context); + await callRoute(loadHookPath, context); expect( options.forestAdminClient.permissionService.canExecuteSegmentQuery, @@ -2058,11 +2084,9 @@ describe('ActionRoute', () => { }); test('refuses an approval on a segment the approver is not allowed to run', async () => { - allowSegmentQuery(false); - const context = buildContext({ requester_id: 12 }); + const context = buildApprovalContext(false); - // @ts-expect-error: test private method - await expect(route.handleExecute(context)).rejects.toMatchObject({ status: 403 }); + await expect(callRoute(executePath, context)).rejects.toMatchObject({ status: 403 }); expect( options.forestAdminClient.permissionService.canExecuteSegmentQuery, @@ -2071,11 +2095,9 @@ describe('ActionRoute', () => { }); test('executes an approval on a segment the approver is allowed to run', async () => { - allowSegmentQuery(true); - const context = buildContext({ requester_id: 12 }); + const context = buildApprovalContext(true); - // @ts-expect-error: test private method - await route.handleExecute(context); + await callRoute(executePath, context); expect( options.forestAdminClient.permissionService.canExecuteSegmentQuery, From db68f22a9acae05b4622e8076b310c122c0675b2 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 27 Oct 2026 16:19:56 +0100 Subject: [PATCH 03/10] test(agent): declare the segment permission helper before the route helpers using it Refs: PRD-1759 Co-Authored-By: Claude Opus 5.5 --- .../test/routes/modification/action/action.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/agent/test/routes/modification/action/action.test.ts b/packages/agent/test/routes/modification/action/action.test.ts index d6bab134aa..e6309539c7 100644 --- a/packages/agent/test/routes/modification/action/action.test.ts +++ b/packages/agent/test/routes/modification/action/action.test.ts @@ -1965,6 +1965,11 @@ describe('ActionRoute', () => { }, }); + const allowSegmentQuery = (allowed: boolean) => + ( + options.forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock + ).mockResolvedValue(allowed); + const callRoute = async (path: string, context: ReturnType) => { const actionRouter = factories.router.mockAllMethods().build(); route.setupRoutes(actionRouter); @@ -1995,11 +2000,6 @@ describe('ActionRoute', () => { }); }; - const allowSegmentQuery = (allowed: boolean) => - ( - options.forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock - ).mockResolvedValue(allowed); - beforeEach(() => { realServices = factories.forestAdminHttpDriverServices.build({ authorization: new AuthorizationService(options.forestAdminClient), From 0ccb192564af3d48b7199f17482da47d2c21d87d Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Wed, 28 Oct 2026 15:25:31 +0100 Subject: [PATCH 04/10] fix(action): leave the segment out of an explicit id selection Explicit ids already pin the records, so the selection's live query segment is neither checked nor run, as forest-express and forest-rails do. Co-Authored-By: Claude Opus 5.5 --- .../src/routes/modification/action/action.ts | 31 +++++++++---- .../agent/test/routes/access/list.test.ts | 2 +- .../routes/modification/action/action.test.ts | 46 +++++++++++++++---- 3 files changed, 59 insertions(+), 20 deletions(-) diff --git a/packages/agent/src/routes/modification/action/action.ts b/packages/agent/src/routes/modification/action/action.ts index ed5be82a3a..d24bc4cebd 100644 --- a/packages/agent/src/routes/modification/action/action.ts +++ b/packages/agent/src/routes/modification/action/action.ts @@ -384,23 +384,34 @@ export default class ActionRoute extends CollectionRoute { const body = context.request.body as any; const attributes = body?.data?.attributes; + const actionScope = this.collection.schema.actions[this.actionName].scope; + const selectionIds = + actionScope === 'Global' + ? null + : BodyParser.parseSelectionIds(this.collection.schema, context); + // Explicit ids already pin the records: the selection's segment adds nothing, so it never runs. + const isExplicitIdSelection = selectionIds !== null && !selectionIds.areExcluded; + // Match user filter + search + scope? + segment. - await this.services.authorization.assertCanExecuteLiveQuerySegment( - context, - this.collection.name, - ); + if (!isExplicitIdSelection) { + await this.services.authorization.assertCanExecuteLiveQuerySegment( + context, + this.collection.name, + ); + } const scope = includeUserScope ? await this.services.authorization.getScope(this.collection, context) : null; - let filter = ContextFilterFactory.build(this.collection, context, scope); + let filter = ContextFilterFactory.build( + this.collection, + context, + scope, + isExplicitIdSelection ? { liveQuerySegment: undefined } : undefined, + ); // Restrict the filter to the selected records for single or bulk actions. - const actionScope = this.collection.schema.actions[this.actionName].scope; - - if (actionScope !== 'Global') { - const selectionIds = BodyParser.parseSelectionIds(this.collection.schema, context); - + if (selectionIds) { // A single action runs on its first record, or on nothing: refuse any selection but one id. if (actionScope === 'Single' && (selectionIds.areExcluded || selectionIds.ids.length !== 1)) { throw new UnprocessableError('This action can only be triggered on one record at a time.'); diff --git a/packages/agent/test/routes/access/list.test.ts b/packages/agent/test/routes/access/list.test.ts index acf4622aac..83c809e74c 100644 --- a/packages/agent/test/routes/access/list.test.ts +++ b/packages/agent/test/routes/access/list.test.ts @@ -369,7 +369,7 @@ describe('ListRoute', () => { contextVariables: {}, }, }), - expect.anything(), + ['id'], ); }); }); diff --git a/packages/agent/test/routes/modification/action/action.test.ts b/packages/agent/test/routes/modification/action/action.test.ts index e6309539c7..25847f6518 100644 --- a/packages/agent/test/routes/modification/action/action.test.ts +++ b/packages/agent/test/routes/modification/action/action.test.ts @@ -1803,7 +1803,7 @@ describe('ActionRoute', () => { ]); }); - test('resolves the live-query segment context variables before listing the audited selection', async () => { + test('lists an explicit audited selection by its ids, without its live-query segment', async () => { const context = createMockContext({ ...baseContext, customProperties: { @@ -1830,15 +1830,19 @@ describe('ActionRoute', () => { await route.handleExecute(context); expect(dataSource.getCollection('books').list).toHaveBeenCalledWith( - expect.anything(), + expect.objectContaining({ id: '42' }), expect.objectContaining({ - liveQuerySegment: expect.objectContaining({ - query: 'SELECT 1', - connectionName: 'main', - contextVariables: {}, + liveQuerySegment: expect.toBeNil(), + conditionTree: expect.objectContaining({ + field: 'id', + operator: 'In', + value: [ + '123e4567-e89b-12d3-a456-426614174000', + '123e4567-e89b-12d3-a456-426614174001', + ], }), }), - expect.anything(), + ['id'], ); }); @@ -1985,8 +1989,8 @@ describe('ActionRoute', () => { const executePath = '/_actions/books/0/mybulkaction'; const loadHookPath = '/_actions/books/0/mybulkaction/hooks/load'; - const buildApprovalContext = (allowed: boolean) => { - const signedRequest = buildContext({ requester_id: 12 }).request.body; + const buildApprovalContext = (allowed: boolean, attributes: Record = {}) => { + const signedRequest = buildContext({ requester_id: 12, ...attributes }).request.body; (options.forestAdminClient.verifySignedActionParameters as jest.Mock).mockReturnValue( signedRequest, ); @@ -2094,6 +2098,30 @@ describe('ActionRoute', () => { expect(dataSource.getCollection('books').execute).not.toHaveBeenCalled(); }); + test('executes an approval on selected records without checking or running their segment', async () => { + const selectedId = '123e4567-e89b-12d3-a456-426614174000'; + const context = buildApprovalContext(false, { all_records: false, ids: [selectedId] }); + + await callRoute(executePath, context); + + expect( + options.forestAdminClient.permissionService.canExecuteSegmentQuery, + ).not.toHaveBeenCalled(); + expect(dataSource.getCollection('books').execute).toHaveBeenCalledWith( + expect.objectContaining({ id: '42' }), + 'MyBulkAction', + { firstname: 'John' }, + expect.objectContaining({ + liveQuerySegment: expect.toBeNil(), + conditionTree: expect.objectContaining({ + field: 'id', + operator: 'Equal', + value: selectedId, + }), + }), + ); + }); + test('executes an approval on a segment the approver is allowed to run', async () => { const context = buildApprovalContext(true); From b63b71b080688ae6891bca3c141b8ae897988f82 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Fri, 30 Oct 2026 13:58:25 +0100 Subject: [PATCH 05/10] fix(datasource-sequelize): run live queries in a transaction that is always rolled back Live query segments and SQL charts only read. executeNativeQuery now runs them in a transaction it always rolls back, read-only on Postgres, so no statement they run is ever committed, whatever the dialect. Co-Authored-By: Claude Opus 5.5 --- .../datasource-sequelize/src/datasource.ts | 27 +++++-- .../test/datasource.unit.test.ts | 75 +++++++++++++++++-- .../native-query.integration.test.ts | 74 ++++++++++++++++++ 3 files changed, 165 insertions(+), 11 deletions(-) create mode 100644 packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts diff --git a/packages/datasource-sequelize/src/datasource.ts b/packages/datasource-sequelize/src/datasource.ts index 9e9a642875..76bcedc710 100644 --- a/packages/datasource-sequelize/src/datasource.ts +++ b/packages/datasource-sequelize/src/datasource.ts @@ -58,13 +58,30 @@ export default class SequelizeDataSource extends BaseDataSource( - query, - { + const { instance } = this.nativeQueryConnections[connectionName] as NativeQueryConnection; + // Live queries only read: whatever their text runs, it is rolled back, never committed. + const transaction = await instance.transaction(); + let rows: R[]; + + try { + if (instance.getDialect() === 'postgres') { + await instance.query('SET TRANSACTION READ ONLY', { transaction }); + } + + rows = await instance.query(query, { bind: contextVariables, type: QueryTypes.SELECT, raw: true, - }, - ); + transaction, + }); + } catch (error) { + // The failed query may have ended the transaction already; its error is the one to report. + await transaction.rollback().catch(() => undefined); + throw error; + } + + await transaction.rollback(); + + return rows; } } diff --git a/packages/datasource-sequelize/test/datasource.unit.test.ts b/packages/datasource-sequelize/test/datasource.unit.test.ts index 2eb0f1e519..2aff1176af 100644 --- a/packages/datasource-sequelize/test/datasource.unit.test.ts +++ b/packages/datasource-sequelize/test/datasource.unit.test.ts @@ -65,25 +65,88 @@ describe('SequelizeDataSource', () => { }); describe('executeNativeQuery', () => { - it('should execute the given query on the correct connection', async () => { - const sequelize = new Sequelize({ dialect: 'postgres' }); + const setup = (dialect: 'postgres' | 'mssql') => { + const sequelize = new Sequelize({ dialect }); sequelize.define('cars', {}); - const logger = jest.fn(); - const spyQuery = jest.spyOn(sequelize, 'query').mockImplementation(); - const dataSource = new SequelizeDataSource(sequelize, logger, { + const transaction = { rollback: jest.fn().mockResolvedValue(undefined) }; + jest.spyOn(sequelize, 'transaction').mockResolvedValue(transaction as never); + const spyQuery = jest.spyOn(sequelize, 'query').mockResolvedValue([{ id: 1 }] as never); + const dataSource = new SequelizeDataSource(sequelize, jest.fn(), { liveQueryConnections: 'main', }); + return { dataSource, spyQuery, transaction }; + }; + + it('should execute the given query on the correct connection, in a transaction', async () => { + const { dataSource, spyQuery, transaction } = setup('mssql'); + await dataSource.executeNativeQuery('main', 'query', { something: 'value' }); - expect(spyQuery).toHaveBeenCalled(); expect(spyQuery).toHaveBeenCalledWith('query', { bind: { something: 'value' }, type: QueryTypes.SELECT, raw: true, + transaction, }); }); + it('should return the rows and roll the transaction back', async () => { + const { dataSource, transaction } = setup('mssql'); + + const rows = await dataSource.executeNativeQuery('main', 'query'); + + expect(rows).toEqual([{ id: 1 }]); + expect(transaction.rollback).toHaveBeenCalledTimes(1); + }); + + it('should make the transaction read-only on postgres before running the query', async () => { + const { dataSource, spyQuery, transaction } = setup('postgres'); + + await dataSource.executeNativeQuery('main', 'query'); + + expect(spyQuery).toHaveBeenNthCalledWith(1, 'SET TRANSACTION READ ONLY', { transaction }); + expect(spyQuery).toHaveBeenNthCalledWith( + 2, + 'query', + expect.objectContaining({ transaction }), + ); + }); + + it('should not set a read-only transaction outside postgres', async () => { + const { dataSource, spyQuery } = setup('mssql'); + + await dataSource.executeNativeQuery('main', 'query'); + + expect(spyQuery).toHaveBeenCalledTimes(1); + expect(spyQuery).toHaveBeenCalledWith('query', expect.anything()); + }); + + it('should roll back and rethrow the query error when the query fails', async () => { + const { dataSource, spyQuery, transaction } = setup('mssql'); + spyQuery.mockRejectedValue(new Error('syntax error')); + + await expect(dataSource.executeNativeQuery('main', 'query')).rejects.toThrow('syntax error'); + expect(transaction.rollback).toHaveBeenCalledTimes(1); + }); + + it('should report the query error when the rollback fails after it', async () => { + const { dataSource, spyQuery, transaction } = setup('mssql'); + spyQuery.mockRejectedValue(new Error('syntax error')); + transaction.rollback.mockRejectedValue(new Error('no transaction')); + + await expect(dataSource.executeNativeQuery('main', 'query')).rejects.toThrow('syntax error'); + }); + + it('should fail when the rollback fails after a successful query', async () => { + const { dataSource, transaction } = setup('mssql'); + transaction.rollback.mockRejectedValue(new Error('no transaction')); + + await expect(dataSource.executeNativeQuery('main', 'query')).rejects.toThrow( + 'no transaction', + ); + }); + describe('when giving an unknown connection name', () => { it('should throw an error', async () => { const sequelize = new Sequelize({ dialect: 'postgres' }); diff --git a/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts b/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts new file mode 100644 index 0000000000..21519397ef --- /dev/null +++ b/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts @@ -0,0 +1,74 @@ +import type { Dialect } from 'sequelize'; + +import { DataTypes, Sequelize } from 'sequelize'; + +import { SequelizeDataSource } from '../../../src'; +import CONNECTIONS from '../../__tests/connections'; + +const ROWS_DELETING_QUERY: Record string> = { + mysql: table => `DELETE FROM ${table}`, + mssql: table => `DELETE FROM ${table} OUTPUT deleted.id`, +}; + +function deleteAllRows(dialect: Dialect, table: string): string { + return (ROWS_DELETING_QUERY[dialect] ?? (t => `DELETE FROM ${t} RETURNING id`))(table); +} + +describe('Native queries', () => { + const sqlite = { + name: 'SQLite', + open: async () => new Sequelize({ dialect: 'sqlite', storage: ':memory:', logging: false }), + }; + const servers = CONNECTIONS.map(connection => ({ + name: connection.name, + open: async () => { + const db = 'forest_test_native_query'; + const server = new Sequelize(connection.uri(), { logging: false }); + await server.getQueryInterface().dropDatabase(db); + await connection.query.createDatabase(server, db); + await server.close(); + + return new Sequelize(connection.uri(db), { logging: false }); + }, + })); + + describe.each([sqlite, ...servers])('On $name', ({ open }) => { + let sequelize: Sequelize; + let dataSource: SequelizeDataSource; + let table: string; + + beforeAll(async () => { + sequelize = await open(); + sequelize.define('Label', { title: DataTypes.STRING }); + await sequelize.sync({ force: true }); + table = sequelize.getQueryInterface().quoteIdentifier('Labels'); + dataSource = new SequelizeDataSource(sequelize, jest.fn(), { liveQueryConnections: 'main' }); + }, 60_000); + + beforeEach(async () => { + await sequelize.models.Label.destroy({ truncate: true }); + await sequelize.models.Label.bulkCreate([{ title: 'a' }, { title: 'b' }]); + }); + + afterAll(async () => { + await sequelize?.close(); + }); + + it('should return the rows a query selects', async () => { + const rows = await dataSource.executeNativeQuery<{ title: string }>( + 'main', + `SELECT title FROM ${table} ORDER BY title`, + ); + + expect(rows).toEqual([{ title: 'a' }, { title: 'b' }]); + }); + + it('should leave the data untouched when a query writes', async () => { + await dataSource + .executeNativeQuery('main', deleteAllRows(sequelize.getDialect() as Dialect, table)) + .catch(() => undefined); + + expect(await sequelize.models.Label.count()).toBe(2); + }); + }); +}); From 7b02139db76e972d544b2345fbd72517d2a6c375 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 2 Nov 2026 15:43:10 +0100 Subject: [PATCH 06/10] fix(datasource-sequelize): keep concurrent writes on an in-memory SQLite database Sequelize runs every query of an in-memory SQLite database on one connection, so the rolled back live query transaction also undid writes other requests made while it was open. Live queries there now run without a transaction; file SQLite and server dialects keep it. Co-Authored-By: Claude Opus 5.5 --- .../datasource-sequelize/src/datasource.ts | 20 +++++++---- .../src/type-overrides.ts | 5 +++ .../test/datasource.unit.test.ts | 27 ++++++++++++-- .../native-query.integration.test.ts | 35 ++++++++++++++++--- 4 files changed, 74 insertions(+), 13 deletions(-) diff --git a/packages/datasource-sequelize/src/datasource.ts b/packages/datasource-sequelize/src/datasource.ts index 76bcedc710..3b8d4e6f6a 100644 --- a/packages/datasource-sequelize/src/datasource.ts +++ b/packages/datasource-sequelize/src/datasource.ts @@ -1,3 +1,4 @@ +import type { SequelizeExt } from './type-overrides'; import type { SequelizeDatasourceOptions } from './types'; import type { Logger } from '@forestadmin/datasource-toolkit'; import type { Sequelize } from 'sequelize'; @@ -11,6 +12,12 @@ interface NativeQueryConnection { instance: Sequelize; } +function isInMemorySqlite(instance: Sequelize): boolean { + const { storage, host } = (instance as SequelizeExt).options; + + return instance.getDialect() === 'sqlite' && (storage ?? (host || ':memory:')) === ':memory:'; +} + export default class SequelizeDataSource extends BaseDataSource { /** * We can't directly use the Sequelize version we install in the package.json @@ -59,6 +66,12 @@ export default class SequelizeDataSource extends BaseDataSource(query, selectOptions); + // Live queries only read: whatever their text runs, it is rolled back, never committed. const transaction = await instance.transaction(); let rows: R[]; @@ -68,12 +81,7 @@ export default class SequelizeDataSource extends BaseDataSource(query, { - bind: contextVariables, - type: QueryTypes.SELECT, - raw: true, - transaction, - }); + rows = await instance.query(query, { ...selectOptions, transaction }); } catch (error) { // The failed query may have ended the transaction already; its error is the one to report. await transaction.rollback().catch(() => undefined); diff --git a/packages/datasource-sequelize/src/type-overrides.ts b/packages/datasource-sequelize/src/type-overrides.ts index 0adf58884c..b31cb77dd9 100644 --- a/packages/datasource-sequelize/src/type-overrides.ts +++ b/packages/datasource-sequelize/src/type-overrides.ts @@ -8,6 +8,7 @@ import type { BelongsToMany, Model, ModelAttributeColumnOptions, + Sequelize, } from 'sequelize'; export interface BelongsToManyExt extends BelongsToMany { @@ -22,3 +23,7 @@ export interface ArrayDataTypeExt { type: AbstractDataType; } + +export interface SequelizeExt extends Sequelize { + options: { storage?: string; host?: string }; +} diff --git a/packages/datasource-sequelize/test/datasource.unit.test.ts b/packages/datasource-sequelize/test/datasource.unit.test.ts index 2aff1176af..0e0f0f4a29 100644 --- a/packages/datasource-sequelize/test/datasource.unit.test.ts +++ b/packages/datasource-sequelize/test/datasource.unit.test.ts @@ -65,8 +65,8 @@ describe('SequelizeDataSource', () => { }); describe('executeNativeQuery', () => { - const setup = (dialect: 'postgres' | 'mssql') => { - const sequelize = new Sequelize({ dialect }); + const setup = (dialect: 'postgres' | 'mssql' | 'sqlite', storage?: string) => { + const sequelize = new Sequelize({ dialect, storage }); sequelize.define('cars', {}); const transaction = { rollback: jest.fn().mockResolvedValue(undefined) }; jest.spyOn(sequelize, 'transaction').mockResolvedValue(transaction as never); @@ -122,6 +122,29 @@ describe('SequelizeDataSource', () => { expect(spyQuery).toHaveBeenCalledWith('query', expect.anything()); }); + it('should run the query outside a transaction on an in-memory sqlite database', async () => { + const { dataSource, spyQuery, transaction } = setup('sqlite', ':memory:'); + + const rows = await dataSource.executeNativeQuery('main', 'query', { something: 'value' }); + + expect(rows).toEqual([{ id: 1 }]); + expect(spyQuery).toHaveBeenCalledWith('query', { + bind: { something: 'value' }, + type: QueryTypes.SELECT, + raw: true, + }); + expect(transaction.rollback).not.toHaveBeenCalled(); + }); + + it('should run the query in a rolled back transaction on a sqlite file', async () => { + const { dataSource, spyQuery, transaction } = setup('sqlite', '/data/app.sqlite'); + + await dataSource.executeNativeQuery('main', 'query'); + + expect(spyQuery).toHaveBeenCalledWith('query', expect.objectContaining({ transaction })); + expect(transaction.rollback).toHaveBeenCalledTimes(1); + }); + it('should roll back and rethrow the query error when the query fails', async () => { const { dataSource, spyQuery, transaction } = setup('mssql'); spyQuery.mockRejectedValue(new Error('syntax error')); diff --git a/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts b/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts index 21519397ef..e8cc66ef3b 100644 --- a/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts +++ b/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts @@ -1,5 +1,7 @@ import type { Dialect } from 'sequelize'; +import os from 'os'; +import path from 'path'; import { DataTypes, Sequelize } from 'sequelize'; import { SequelizeDataSource } from '../../../src'; @@ -15,12 +17,24 @@ function deleteAllRows(dialect: Dialect, table: string): string { } describe('Native queries', () => { - const sqlite = { - name: 'SQLite', + const sqliteInMemory = { + name: 'SQLite in memory', + rollsBackWrites: false, open: async () => new Sequelize({ dialect: 'sqlite', storage: ':memory:', logging: false }), }; + const sqliteFile = { + name: 'SQLite file', + rollsBackWrites: true, + open: async () => + new Sequelize({ + dialect: 'sqlite', + storage: path.join(os.tmpdir(), 'forest_test_native_query.sqlite'), + logging: false, + }), + }; const servers = CONNECTIONS.map(connection => ({ name: connection.name, + rollsBackWrites: true, open: async () => { const db = 'forest_test_native_query'; const server = new Sequelize(connection.uri(), { logging: false }); @@ -32,7 +46,9 @@ describe('Native queries', () => { }, })); - describe.each([sqlite, ...servers])('On $name', ({ open }) => { + const databases = [sqliteInMemory, sqliteFile, ...servers]; + + describe.each(databases)('On $name', ({ open, rollsBackWrites }) => { let sequelize: Sequelize; let dataSource: SequelizeDataSource; let table: string; @@ -63,12 +79,21 @@ describe('Native queries', () => { expect(rows).toEqual([{ title: 'a' }, { title: 'b' }]); }); - it('should leave the data untouched when a query writes', async () => { + it('should roll back what a query writes unless its connection is shared', async () => { await dataSource .executeNativeQuery('main', deleteAllRows(sequelize.getDialect() as Dialect, table)) .catch(() => undefined); - expect(await sequelize.models.Label.count()).toBe(2); + expect(await sequelize.models.Label.count()).toBe(rollsBackWrites ? 2 : 0); + }); + + it('should keep a write made while a query runs', async () => { + await Promise.all([ + dataSource.executeNativeQuery('main', `SELECT title FROM ${table}`), + sequelize.models.Label.create({ title: 'c' }), + ]); + + expect(await sequelize.models.Label.count()).toBe(3); }); }); }); From 21ea8b9fa95e88c1ff372659a8213eb2485cc262 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 2 Nov 2026 15:47:23 +0100 Subject: [PATCH 07/10] fix(datasource-sequelize): run live queries on a temporary SQLite database again With an empty storage, each Sequelize SQLite connection opens its own private database, so the transaction's connection saw no tables and every live query failed. Temporary SQLite now runs them outside a transaction, like in-memory SQLite. Co-Authored-By: Claude Opus 5.5 --- packages/datasource-sequelize/src/datasource.ts | 16 +++++++++++----- .../test/datasource.unit.test.ts | 13 +++++++++++++ .../native-query.integration.test.ts | 7 ++++++- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/packages/datasource-sequelize/src/datasource.ts b/packages/datasource-sequelize/src/datasource.ts index 3b8d4e6f6a..d5622db0ed 100644 --- a/packages/datasource-sequelize/src/datasource.ts +++ b/packages/datasource-sequelize/src/datasource.ts @@ -12,10 +12,15 @@ interface NativeQueryConnection { instance: Sequelize; } -function isInMemorySqlite(instance: Sequelize): boolean { +const EPHEMERAL_SQLITE_STORAGES = [':memory:', '']; + +function isEphemeralSqlite(instance: Sequelize): boolean { const { storage, host } = (instance as SequelizeExt).options; - return instance.getDialect() === 'sqlite' && (storage ?? (host || ':memory:')) === ':memory:'; + return ( + instance.getDialect() === 'sqlite' && + EPHEMERAL_SQLITE_STORAGES.includes(storage ?? (host || ':memory:')) + ); } export default class SequelizeDataSource extends BaseDataSource { @@ -68,9 +73,10 @@ export default class SequelizeDataSource extends BaseDataSource(query, selectOptions); + // A transaction cannot isolate an ephemeral SQLite database: in memory, Sequelize runs every + // query on one connection, so its rollback would undo other requests' writes; on an empty + // storage, each connection opens its own private database, where the tables do not exist. + if (isEphemeralSqlite(instance)) return instance.query(query, selectOptions); // Live queries only read: whatever their text runs, it is rolled back, never committed. const transaction = await instance.transaction(); diff --git a/packages/datasource-sequelize/test/datasource.unit.test.ts b/packages/datasource-sequelize/test/datasource.unit.test.ts index 0e0f0f4a29..b2346ba22b 100644 --- a/packages/datasource-sequelize/test/datasource.unit.test.ts +++ b/packages/datasource-sequelize/test/datasource.unit.test.ts @@ -136,6 +136,19 @@ describe('SequelizeDataSource', () => { expect(transaction.rollback).not.toHaveBeenCalled(); }); + it('should run the query outside a transaction on a temporary sqlite database', async () => { + const { dataSource, spyQuery, transaction } = setup('sqlite', ''); + + await dataSource.executeNativeQuery('main', 'query'); + + expect(spyQuery).toHaveBeenCalledWith('query', { + bind: {}, + type: QueryTypes.SELECT, + raw: true, + }); + expect(transaction.rollback).not.toHaveBeenCalled(); + }); + it('should run the query in a rolled back transaction on a sqlite file', async () => { const { dataSource, spyQuery, transaction } = setup('sqlite', '/data/app.sqlite'); diff --git a/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts b/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts index e8cc66ef3b..f1713f1b4d 100644 --- a/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts +++ b/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts @@ -22,6 +22,11 @@ describe('Native queries', () => { rollsBackWrites: false, open: async () => new Sequelize({ dialect: 'sqlite', storage: ':memory:', logging: false }), }; + const sqliteTemporary = { + name: 'SQLite temporary', + rollsBackWrites: false, + open: async () => new Sequelize({ dialect: 'sqlite', storage: '', logging: false }), + }; const sqliteFile = { name: 'SQLite file', rollsBackWrites: true, @@ -46,7 +51,7 @@ describe('Native queries', () => { }, })); - const databases = [sqliteInMemory, sqliteFile, ...servers]; + const databases = [sqliteInMemory, sqliteTemporary, sqliteFile, ...servers]; describe.each(databases)('On $name', ({ open, rollsBackWrites }) => { let sequelize: Sequelize; From 999f521133c131a51c5ed1c694ec231dd6a6e973 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 2 Nov 2026 15:51:07 +0100 Subject: [PATCH 08/10] fix(datasource-sequelize): open the live query transaction as read-only A transaction Sequelize opens without readOnly takes a write pool connection, so live queries and SQL charts left the read replica they used before. The transaction is now opened with readOnly: true. Co-Authored-By: Claude Opus 5.5 --- packages/datasource-sequelize/src/datasource.ts | 2 +- .../test/datasource.unit.test.ts | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/datasource-sequelize/src/datasource.ts b/packages/datasource-sequelize/src/datasource.ts index d5622db0ed..15360e2a61 100644 --- a/packages/datasource-sequelize/src/datasource.ts +++ b/packages/datasource-sequelize/src/datasource.ts @@ -79,7 +79,7 @@ export default class SequelizeDataSource extends BaseDataSource(query, selectOptions); // Live queries only read: whatever their text runs, it is rolled back, never committed. - const transaction = await instance.transaction(); + const transaction = await instance.transaction({ readOnly: true }); let rows: R[]; try { diff --git a/packages/datasource-sequelize/test/datasource.unit.test.ts b/packages/datasource-sequelize/test/datasource.unit.test.ts index b2346ba22b..02bc8ad27f 100644 --- a/packages/datasource-sequelize/test/datasource.unit.test.ts +++ b/packages/datasource-sequelize/test/datasource.unit.test.ts @@ -69,13 +69,15 @@ describe('SequelizeDataSource', () => { const sequelize = new Sequelize({ dialect, storage }); sequelize.define('cars', {}); const transaction = { rollback: jest.fn().mockResolvedValue(undefined) }; - jest.spyOn(sequelize, 'transaction').mockResolvedValue(transaction as never); + const spyTransaction = jest + .spyOn(sequelize, 'transaction') + .mockResolvedValue(transaction as never); const spyQuery = jest.spyOn(sequelize, 'query').mockResolvedValue([{ id: 1 }] as never); const dataSource = new SequelizeDataSource(sequelize, jest.fn(), { liveQueryConnections: 'main', }); - return { dataSource, spyQuery, transaction }; + return { dataSource, spyQuery, spyTransaction, transaction }; }; it('should execute the given query on the correct connection, in a transaction', async () => { @@ -91,6 +93,14 @@ describe('SequelizeDataSource', () => { }); }); + it('should open a read-only transaction, so it may run on a read replica', async () => { + const { dataSource, spyTransaction } = setup('mssql'); + + await dataSource.executeNativeQuery('main', 'query'); + + expect(spyTransaction).toHaveBeenCalledWith({ readOnly: true }); + }); + it('should return the rows and roll the transaction back', async () => { const { dataSource, transaction } = setup('mssql'); From c51b6784ae732448fd663f2feb46fcc4ca756efb Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 2 Nov 2026 20:41:04 +0100 Subject: [PATCH 09/10] test(agent): approve an action on the payload the server actually signs The approval fixture built a signed payload carrying `all_records: true` and an `all_records_subset_query`, a shape `generateCustomActionRequest` never produces: the server freezes the selection into explicit record ids and always sends `allRecordsSubsetQuery: null`. The two tests riding that fixture locked an approver refusal that cannot happen. The fixture now builds the server's own shape, with `canExecuteSegmentQuery` left unmocked so any check that fires answers 403 and fails the test, and covers the one segment an approver still controls: the query string, which the signed payload never replaces. Co-Authored-By: Claude Opus 5 (1M context) --- .../routes/modification/action/action.test.ts | 67 ++++++++++++------- 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/packages/agent/test/routes/modification/action/action.test.ts b/packages/agent/test/routes/modification/action/action.test.ts index 25847f6518..03a27f162a 100644 --- a/packages/agent/test/routes/modification/action/action.test.ts +++ b/packages/agent/test/routes/modification/action/action.test.ts @@ -1989,17 +1989,39 @@ describe('ActionRoute', () => { const executePath = '/_actions/books/0/mybulkaction'; const loadHookPath = '/_actions/books/0/mybulkaction/hooks/load'; - const buildApprovalContext = (allowed: boolean, attributes: Record = {}) => { - const signedRequest = buildContext({ requester_id: 12, ...attributes }).request.body; - (options.forestAdminClient.verifySignedActionParameters as jest.Mock).mockReturnValue( - signedRequest, - ); - allowSegmentQuery(allowed); + const approvedId = '123e4567-e89b-12d3-a456-426614174000'; + + // The shape forestadmin-server signs (generateCustomActionRequest): the selection was frozen + // into explicit record ids when the request was created, and allRecordsSubsetQuery is always + // null, so an approval carries no segment of its own. `canExecuteSegmentQuery` is left + // unmocked, hence falsy: any check that fires answers 403 and fails the test. + const buildApprovalContext = ( + attributes: Record = {}, + query: Record = {}, + ) => { + (options.forestAdminClient.verifySignedActionParameters as jest.Mock).mockReturnValue({ + data: { + attributes: { + requester_id: 12, + ids: [approvedId], + collection_name: 'books', + smart_action_id: 'smartActionId', + values: { firstname: 'John' }, + parent_collection_name: null, + parent_collection_id: null, + parent_association_name: null, + all_records: false, + all_records_subset_query: null, + ...attributes, + }, + }, + }); return createMockContext({ ...baseContext, throw: throwingThrow(), state: { user: { ...baseContext.state.user, renderingId: 7 } }, + customProperties: { query: { ...baseContext.customProperties.query, ...query } }, requestBody: { data: { attributes: { signed_approval_request: 'signed-request' } } }, }); }; @@ -2087,20 +2109,8 @@ describe('ActionRoute', () => { ); }); - test('refuses an approval on a segment the approver is not allowed to run', async () => { - const context = buildApprovalContext(false); - - await expect(callRoute(executePath, context)).rejects.toMatchObject({ status: 403 }); - - expect( - options.forestAdminClient.permissionService.canExecuteSegmentQuery, - ).toHaveBeenCalledWith(expectedSegmentCheck); - expect(dataSource.getCollection('books').execute).not.toHaveBeenCalled(); - }); - - test('executes an approval on selected records without checking or running their segment', async () => { - const selectedId = '123e4567-e89b-12d3-a456-426614174000'; - const context = buildApprovalContext(false, { all_records: false, ids: [selectedId] }); + test('executes an approval on the frozen selection, with no segment to check', async () => { + const context = buildApprovalContext(); await callRoute(executePath, context); @@ -2116,26 +2126,33 @@ describe('ActionRoute', () => { conditionTree: expect.objectContaining({ field: 'id', operator: 'Equal', - value: selectedId, + value: approvedId, }), }), ); }); - test('executes an approval on a segment the approver is allowed to run', async () => { - const context = buildApprovalContext(true); + // The signed payload replaces the body, never the query string, so this is the one segment an + // approver still controls. The frozen selection drops it before it can be run. + test('ignores a segment an approver appends to the query string of an approval', async () => { + const context = buildApprovalContext({}, { segmentQuery, connectionName: 'main' }); await callRoute(executePath, context); expect( options.forestAdminClient.permissionService.canExecuteSegmentQuery, - ).toHaveBeenCalledWith(expectedSegmentCheck); + ).not.toHaveBeenCalled(); expect(dataSource.getCollection('books').execute).toHaveBeenCalledWith( expect.objectContaining({ id: '42' }), 'MyBulkAction', { firstname: 'John' }, expect.objectContaining({ - liveQuerySegment: { query: segmentQuery, connectionName: 'main' }, + liveQuerySegment: expect.toBeNil(), + conditionTree: expect.objectContaining({ + field: 'id', + operator: 'Equal', + value: approvedId, + }), }), ); }); From c56edb70471fc32ca436d01702dfc0289b237a46 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 2 Nov 2026 22:08:43 +0100 Subject: [PATCH 10/10] fix(datasource-toolkit): never nest a filter carrying a live query segment A nested filter kept its live query segment, so a many to many related list, count or export ran it on the through collection and matched its ids against that collection's primary key. Such a filter is no longer nestable: the related routes resolve it on the target collection, and a leaderboard refuses it as it already refuses a named segment. Also: a refused live query segment names its collection and connection, the native query rollback test fails when its delete never ran, and an SQLite database with no storage is covered. Co-Authored-By: Claude Opus 5.5 --- .../services/authorization/authorization.ts | 8 +++- .../agent/test/routes/access/chart.test.ts | 8 ++-- .../authorization/authorization.test.ts | 12 +++++- .../test/datasource.unit.test.ts | 13 +++++++ .../native-query.integration.test.ts | 20 ++++++++-- .../interfaces/query/filter/unpaginated.ts | 2 +- .../test/interfaces/filter-instances.test.ts | 9 +++++ .../test/utils/collection.test.ts | 39 +++++++++++++++++++ 8 files changed, 98 insertions(+), 13 deletions(-) diff --git a/packages/agent/src/services/authorization/authorization.ts b/packages/agent/src/services/authorization/authorization.ts index e420dd5f91..c85ea43947 100644 --- a/packages/agent/src/services/authorization/authorization.ts +++ b/packages/agent/src/services/authorization/authorization.ts @@ -207,7 +207,13 @@ export default class AuthorizationService { connectionName: liveQuerySegment.connectionName, }); - if (!canExecuteSegmentQuery) context.throw(HttpCode.Forbidden, 'Forbidden'); + if (!canExecuteSegmentQuery) { + context.throw( + HttpCode.Forbidden, + `You are not allowed to run this live query segment on the '${collectionName}' ` + + `collection (connection '${liveQuerySegment.connectionName}').`, + ); + } } /** The root is skipped: `browse` gates a listing, `read` a get, and the signed hash a chart. */ diff --git a/packages/agent/test/routes/access/chart.test.ts b/packages/agent/test/routes/access/chart.test.ts index 6f7c9c9a74..abf00cc663 100644 --- a/packages/agent/test/routes/access/chart.test.ts +++ b/packages/agent/test/routes/access/chart.test.ts @@ -1197,7 +1197,7 @@ describe('ChartRoute', () => { expect(books.aggregate).not.toHaveBeenCalled(); }); - test('checks a count leaderboard segment against the charted collection only', async () => { + test('checks a count leaderboard segment against the charted collection, then refuses to nest it', async () => { const forestAdminClient = factories.forestAdminClient.build(); (forestAdminClient.permissionService.canExecuteChart as jest.Mock).mockResolvedValue(true); (forestAdminClient.permissionService.canOnCollection as jest.Mock).mockResolvedValue(true); @@ -1265,7 +1265,7 @@ describe('ChartRoute', () => { state: { user: { email: 'john.doe@domain.com', id: 100, renderingId: 7 } }, }); - await chart.handleChart(context); + await expect(chart.handleChart(context)).rejects.toThrow("Filter can't be nested"); expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledTimes(1); expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ @@ -1275,9 +1275,7 @@ describe('ChartRoute', () => { segmentQuery: 'SELECT id FROM persons', connectionName: 'main', }); - expect(context.response.body).toMatchObject({ - data: { attributes: { value: [{ key: 'a1', value: 3 }] } }, - }); + expect(books.aggregate).not.toHaveBeenCalled(); }); test('computes a chart on a segment the user is allowed to run', async () => { diff --git a/packages/agent/test/services/authorization/authorization.test.ts b/packages/agent/test/services/authorization/authorization.test.ts index 36cbeb8cde..999e689ed9 100644 --- a/packages/agent/test/services/authorization/authorization.test.ts +++ b/packages/agent/test/services/authorization/authorization.test.ts @@ -120,7 +120,11 @@ describe('AuthorizationService', () => { segmentQuery: 'SELECT id FROM books', connectionName: 'library', }); - expect(context.throw).toHaveBeenCalledWith(HttpCode.Forbidden, 'Forbidden'); + expect(context.throw).toHaveBeenCalledWith( + HttpCode.Forbidden, + "You are not allowed to run this live query segment on the 'books' collection " + + "(connection 'library').", + ); }); it('refuses the request when the segment query of the request body is not allowed', async () => { @@ -138,7 +142,11 @@ describe('AuthorizationService', () => { segmentQuery: 'SELECT id FROM books', connectionName: 'library', }); - expect(context.throw).toHaveBeenCalledWith(HttpCode.Forbidden, 'Forbidden'); + expect(context.throw).toHaveBeenCalledWith( + HttpCode.Forbidden, + "You are not allowed to run this live query segment on the 'books' collection " + + "(connection 'library').", + ); }); it('checks the segment query the filter will use when both the body and the query string carry one', async () => { diff --git a/packages/datasource-sequelize/test/datasource.unit.test.ts b/packages/datasource-sequelize/test/datasource.unit.test.ts index 02bc8ad27f..62a826735e 100644 --- a/packages/datasource-sequelize/test/datasource.unit.test.ts +++ b/packages/datasource-sequelize/test/datasource.unit.test.ts @@ -146,6 +146,19 @@ describe('SequelizeDataSource', () => { expect(transaction.rollback).not.toHaveBeenCalled(); }); + it('should run the query outside a transaction on a sqlite database with no storage', async () => { + const { dataSource, spyQuery, spyTransaction } = setup('sqlite'); + + await dataSource.executeNativeQuery('main', 'query'); + + expect(spyQuery).toHaveBeenCalledWith('query', { + bind: {}, + type: QueryTypes.SELECT, + raw: true, + }); + expect(spyTransaction).not.toHaveBeenCalled(); + }); + it('should run the query outside a transaction on a temporary sqlite database', async () => { const { dataSource, spyQuery, transaction } = setup('sqlite', ''); diff --git a/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts b/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts index f1713f1b4d..ddc81e3420 100644 --- a/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts +++ b/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts @@ -16,6 +16,13 @@ function deleteAllRows(dialect: Dialect, table: string): string { return (ROWS_DELETING_QUERY[dialect] ?? (t => `DELETE FROM ${t} RETURNING id`))(table); } +const DELETE_OUTCOME: Partial> = { + postgres: { error: expect.stringContaining('read-only transaction') }, + // MySQL has no RETURNING: Sequelize fails to map the DELETE's result header as rows, after the + // statement ran. + mysql: { error: expect.stringContaining('TypeError') }, +}; + describe('Native queries', () => { const sqliteInMemory = { name: 'SQLite in memory', @@ -85,10 +92,15 @@ describe('Native queries', () => { }); it('should roll back what a query writes unless its connection is shared', async () => { - await dataSource - .executeNativeQuery('main', deleteAllRows(sequelize.getDialect() as Dialect, table)) - .catch(() => undefined); - + const dialect = sequelize.getDialect() as Dialect; + const outcome = await dataSource + .executeNativeQuery('main', deleteAllRows(dialect, table)) + .then( + rows => ({ deletedRows: rows.length }), + (error: Error) => ({ error: `${error.name}: ${error.message}` }), + ); + + expect(outcome).toEqual(DELETE_OUTCOME[dialect] ?? { deletedRows: 2 }); expect(await sequelize.models.Label.count()).toBe(rollsBackWrites ? 2 : 0); }); diff --git a/packages/datasource-toolkit/src/interfaces/query/filter/unpaginated.ts b/packages/datasource-toolkit/src/interfaces/query/filter/unpaginated.ts index 91676269c3..c3d3b532b6 100644 --- a/packages/datasource-toolkit/src/interfaces/query/filter/unpaginated.ts +++ b/packages/datasource-toolkit/src/interfaces/query/filter/unpaginated.ts @@ -30,7 +30,7 @@ export default class Filter { liveQuerySegment?: LiveQuerySegment; get isNestable(): boolean { - return !this.search && !this.segment; + return !this.search && !this.segment && !this.liveQuerySegment; } constructor(parts: FilterComponents) { diff --git a/packages/datasource-toolkit/test/interfaces/filter-instances.test.ts b/packages/datasource-toolkit/test/interfaces/filter-instances.test.ts index 1059023072..35f62a6595 100644 --- a/packages/datasource-toolkit/test/interfaces/filter-instances.test.ts +++ b/packages/datasource-toolkit/test/interfaces/filter-instances.test.ts @@ -76,5 +76,14 @@ describe('Filter', () => { expect(segmentFilter.isNestable).toBeFalsy(); expect(() => segmentFilter.nest('prefix')).toThrow(); }); + + test('nest should crash with a live query segment', () => { + const segmentFilter = new Filter({ + liveQuerySegment: { query: 'SELECT id FROM books', connectionName: 'main' }, + }); + + expect(segmentFilter.isNestable).toBe(false); + expect(() => segmentFilter.nest('prefix')).toThrow("Filter can't be nested"); + }); }); }); diff --git a/packages/datasource-toolkit/test/utils/collection.test.ts b/packages/datasource-toolkit/test/utils/collection.test.ts index 6a1d165782..feab5c1a89 100644 --- a/packages/datasource-toolkit/test/utils/collection.test.ts +++ b/packages/datasource-toolkit/test/utils/collection.test.ts @@ -504,6 +504,45 @@ describe('CollectionUtils', () => { expect(listResults).toEqual([{ id: 1, aField: 'aValue' }]); }); + + test('should run a live query segment on the foreign collection, not the through one', async () => { + const { dataSource } = setupWithManyToManyRelation(); + const libraries = factories.collection.build({ name: 'libraries' }); + jest.spyOn(dataSource, 'getCollection').mockImplementation(name => { + if (name === 'libraries') return libraries; + + return dataSource.collections.find(collection => collection.name === name); + }); + const through = dataSource.getCollection('librariesBooks'); + jest.spyOn(through, 'list').mockResolvedValue([{ libraryId: 'library-1' }]); + const liveQuerySegment = { query: 'SELECT id FROM libraries', connectionName: 'main' }; + const filter = factories.filter.build({ liveQuerySegment }); + const caller = factories.caller.build(); + const projection = new Projection('id'); + + await CollectionUtils.listRelation( + dataSource.getCollection('books'), + [2], + 'manyToManyRelationField', + caller, + filter, + projection, + ); + + expect(through.list).toHaveBeenCalledWith( + caller, + expect.objectContaining({ liveQuerySegment: undefined }), + new Projection('libraryId'), + ); + expect(libraries.list).toHaveBeenCalledWith( + caller, + expect.objectContaining({ + liveQuerySegment, + conditionTree: new ConditionTreeLeaf('id', 'In', ['library-1']), + }), + projection, + ); + }); }); });