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..d24bc4cebd 100644 --- a/packages/agent/src/routes/modification/action/action.ts +++ b/packages/agent/src/routes/modification/action/action.ts @@ -384,18 +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. + 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/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..c85ea43947 100644 --- a/packages/agent/src/services/authorization/authorization.ts +++ b/packages/agent/src/services/authorization/authorization.ts @@ -187,6 +187,35 @@ 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, + `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. */ private async getReadPermissions( context: Context, @@ -207,7 +236,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 +244,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..abf00cc663 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,170 @@ 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, 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); + (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 expect(chart.handleChart(context)).rejects.toThrow("Filter can't be nested"); + + 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(books.aggregate).not.toHaveBeenCalled(); + }); + + 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..83c809e74c 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: {}, + }, + }), + ['id'], + ); + }); + }); }); diff --git a/packages/agent/test/routes/modification/action/action.test.ts b/packages/agent/test/routes/modification/action/action.test.ts index 6e3544e179..03a27f162a 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. @@ -1802,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: { @@ -1829,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'], ); }); @@ -1927,4 +1932,229 @@ 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); + + 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 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' } } }, + }); + }; + + 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(); + + 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 action on a segment the user is allowed to run', async () => { + allowSegmentQuery(true); + const context = buildContext(); + + await callRoute(executePath, 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(); + + await expect(callRoute(loadHookPath, 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(); + + await callRoute(loadHookPath, 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('executes an approval on the frozen selection, with no segment to check', async () => { + const context = buildApprovalContext(); + + 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: approvedId, + }), + }), + ); + }); + + // 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, + ).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: approvedId, + }), + }), + ); + }); + }); }); 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..999e689ed9 100644 --- a/packages/agent/test/services/authorization/authorization.test.ts +++ b/packages/agent/test/services/authorization/authorization.test.ts @@ -83,94 +83,138 @@ 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, + "You are not allowed to run this live query segment on the 'books' collection " + + "(connection 'library').", + ); + }); - 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, + "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 () => { + (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', () => { diff --git a/packages/datasource-sequelize/src/datasource.ts b/packages/datasource-sequelize/src/datasource.ts index 9e9a642875..15360e2a61 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,17 @@ interface NativeQueryConnection { instance: Sequelize; } +const EPHEMERAL_SQLITE_STORAGES = [':memory:', '']; + +function isEphemeralSqlite(instance: Sequelize): boolean { + const { storage, host } = (instance as SequelizeExt).options; + + return ( + instance.getDialect() === 'sqlite' && + EPHEMERAL_SQLITE_STORAGES.includes(storage ?? (host || ':memory:')) + ); +} + export default class SequelizeDataSource extends BaseDataSource { /** * We can't directly use the Sequelize version we install in the package.json @@ -58,13 +70,32 @@ export default class SequelizeDataSource extends BaseDataSource( - query, - { - bind: contextVariables, - type: QueryTypes.SELECT, - raw: true, - }, - ); + const { instance } = this.nativeQueryConnections[connectionName] as NativeQueryConnection; + const selectOptions = { bind: contextVariables, type: QueryTypes.SELECT, raw: true } as const; + + // 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({ readOnly: true }); + let rows: R[]; + + try { + if (instance.getDialect() === 'postgres') { + await instance.query('SET TRANSACTION READ ONLY', { 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); + throw error; + } + + await transaction.rollback(); + + return rows; } } 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 2eb0f1e519..62a826735e 100644 --- a/packages/datasource-sequelize/test/datasource.unit.test.ts +++ b/packages/datasource-sequelize/test/datasource.unit.test.ts @@ -65,23 +65,145 @@ 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' | 'sqlite', storage?: string) => { + const sequelize = new Sequelize({ dialect, storage }); 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) }; + 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, spyTransaction, 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 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'); + + 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 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 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', ''); + + 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'); + + 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')); + + 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', () => { 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..ddc81e3420 --- /dev/null +++ b/packages/datasource-sequelize/test/integration/native-query/native-query.integration.test.ts @@ -0,0 +1,116 @@ +import type { Dialect } from 'sequelize'; + +import os from 'os'; +import path from 'path'; +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); +} + +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', + 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, + 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 }); + await server.getQueryInterface().dropDatabase(db); + await connection.query.createDatabase(server, db); + await server.close(); + + return new Sequelize(connection.uri(db), { logging: false }); + }, + })); + + const databases = [sqliteInMemory, sqliteTemporary, sqliteFile, ...servers]; + + describe.each(databases)('On $name', ({ open, rollsBackWrites }) => { + 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 roll back what a query writes unless its connection is shared', async () => { + 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); + }); + + 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); + }); + }); +}); 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, + ); + }); }); });