diff --git a/packages/admin-panel/src/routes/visualisations/mapOverlayGroupRelations.js b/packages/admin-panel/src/routes/visualisations/mapOverlayGroupRelations.js index ba07e79bcf..056380b0aa 100644 --- a/packages/admin-panel/src/routes/visualisations/mapOverlayGroupRelations.js +++ b/packages/admin-panel/src/routes/visualisations/mapOverlayGroupRelations.js @@ -7,56 +7,84 @@ const RESOURCE_NAME = { singular: 'map overlay group relation' }; export const RELATION_ENDPOINT = 'mapOverlayGroupRelations'; -const FIELDS = [ - { - Header: 'Map overlay group code', - source: 'map_overlay_group.code', - - editConfig: { - optionsEndpoint: 'mapOverlayGroups', - optionLabelKey: 'map_overlay_group.code', - optionValueKey: 'map_overlay_group.id', - sourceKey: 'map_overlay_group_id', - }, +const mapOverlayGroupCode = { + Header: 'Map overlay group code', + source: 'map_overlay_group.code', + editConfig: { + optionsEndpoint: 'mapOverlayGroups', + optionLabelKey: 'map_overlay_group.code', + optionValueKey: 'map_overlay_group.id', + sourceKey: 'map_overlay_group_id', }, - { - Header: 'Child ID', - source: 'child_id', +}; - editConfig: { - optionsEndpoint: 'mapOverlays', - optionLabelKey: 'mapOverlay.id', - optionValueKey: 'mapOverlay.id', - canCreateNewOptions: true, - sourceKey: 'child_id', - }, +const childType = { + Header: 'Child type', + width: 160, + source: 'child_type', + editConfig: { + options: [ + { + label: 'Map overlay', + value: 'mapOverlay', + }, + { + label: 'Map overlay group', + value: 'mapOverlayGroup', + }, + ], }, - { - Header: 'Child type', - width: 160, - source: 'child_type', +}; - editConfig: { - options: [ - { - label: 'Map overlay', - value: 'mapOverlay', - }, - { - label: 'Map overlay group', - value: 'mapOverlayGroup', - }, - ], - }, +const childCode = { + Header: 'Child Code', + source: 'child_code', +}; + +const childMapOverlayCode = { + Header: 'Child map overlay code', + id: 'child_map_overlay_code', + source: 'child_code', + editConfig: { + optionsEndpoint: 'mapOverlays', + optionLabelKey: 'mapOverlay.code', + optionValueKey: 'mapOverlay.id', + sourceKey: 'child_id', + visibilityCriteria: { child_type: 'mapOverlay' }, }, - { - Header: 'Sort order', - source: 'sort_order', +}; + +const childMapOverlayGroupCode = { + Header: 'Child map overlay group code', + id: 'child_map_overlay_group_code', + source: 'child_code', + editConfig: { + optionsEndpoint: 'mapOverlayGroups', + optionLabelKey: 'mapOverlayGroups.code', + optionValueKey: 'mapOverlayGroups.id', + sourceKey: 'child_id', + visibilityCriteria: { child_type: 'mapOverlayGroup' }, }, +}; + +const sortOrder = { + Header: 'Sort order', + source: 'sort_order', +}; + +const EDIT_FIELDS = [ + mapOverlayGroupCode, + childType, + childMapOverlayCode, + childMapOverlayGroupCode, + sortOrder, ]; const COLUMNS = [ - ...FIELDS, + mapOverlayGroupCode, + childType, + childCode, + sortOrder, { Header: 'Edit', type: 'edit', @@ -64,7 +92,7 @@ const COLUMNS = [ actionConfig: { title: `Edit ${RESOURCE_NAME.singular}`, editEndpoint: 'mapOverlayGroupRelations', - fields: FIELDS, + fields: EDIT_FIELDS, }, }, { @@ -79,7 +107,7 @@ const COLUMNS = [ const CREATE_CONFIG = { actionConfig: { editEndpoint: RELATION_ENDPOINT, - fields: FIELDS, + fields: EDIT_FIELDS, }, }; diff --git a/packages/central-server/examples.http b/packages/central-server/examples.http index 75a4fa96af..559912818d 100644 --- a/packages/central-server/examples.http +++ b/packages/central-server/examples.http @@ -49,6 +49,24 @@ GET {{baseUrl}}/export/optionSet/5c625e01f013d60db42a5763 HTTP/1.1 Authorization: {{user-authorization}} Content-Type: application/vnd.openxmlformats-officedocument.spreadsheetml.sheet +### Get all map overlay groups + +GET {{baseUrl}}/mapOverlayGroups +content-type: {{contentType}} +Authorization: {{user-authorization}} + +### Get all map overlay group relations + +GET {{baseUrl}}/mapOverlayGroupRelations +content-type: {{contentType}} +Authorization: {{user-authorization}} + +### Get map overlay group relation by ID + +GET {{baseUrl}}/mapOverlayGroupRelations/5f2c7ddb61f76a513a0000bf +content-type: {{contentType}} +Authorization: {{user-authorization}} + ### Get survey responses GET {{baseUrl}}/surveyResponses?pageSize=5&columns=["entity.name", "country.name"] HTTP/2.0 diff --git a/packages/central-server/src/apiV2/GETHandler/GETHandler.js b/packages/central-server/src/apiV2/GETHandler/GETHandler.js index 6749d16291..92a006f6da 100644 --- a/packages/central-server/src/apiV2/GETHandler/GETHandler.js +++ b/packages/central-server/src/apiV2/GETHandler/GETHandler.js @@ -7,8 +7,8 @@ import { respond } from '@tupaia/utils'; import { CRUDHandler } from '../CRUDHandler'; import { getQueryOptionsForColumns, - fullyQualifyColumnSelector, processColumns, + processColumnSelector, processColumnSelectorKeys, generateLinkHeader, } from './helpers'; @@ -78,14 +78,14 @@ export class GETHandler extends CRUDHandler { // add any user requested sorting to the start of the sort clause if (sortString) { const sortKeys = JSON.parse(sortString); - const fullyQualifiedSortKeys = sortKeys.map(sortKey => - fullyQualifyColumnSelector(this.models, sortKey, this.recordType), + const processedSortKeys = sortKeys.map(sortKey => + processColumnSelector(this.models, sortKey, this.recordType), ); // if 'distinct', we can't order by any columns that aren't included in the distinct selection if (distinct) { - dbQueryOptions.sort = fullyQualifiedSortKeys; + dbQueryOptions.sort = processedSortKeys; } else { - dbQueryOptions.sort.unshift(...fullyQualifiedSortKeys); + dbQueryOptions.sort.unshift(...processedSortKeys); } } diff --git a/packages/central-server/src/apiV2/GETHandler/helpers.js b/packages/central-server/src/apiV2/GETHandler/helpers.js index 0b057fc85b..523692fc1a 100644 --- a/packages/central-server/src/apiV2/GETHandler/helpers.js +++ b/packages/central-server/src/apiV2/GETHandler/helpers.js @@ -42,7 +42,10 @@ export const generateLinkHeader = (resource, pageString, lastPage, originalQuery return formatLinkHeader(linkHeader); }; -export const fullyQualifyColumnSelector = (models, unprocessedColumnSelector, baseRecordType) => { +/** + * Used only by {@link processColumnSelector}, so not exported. + */ +const fullyQualifyColumnSelector = (models, unprocessedColumnSelector, baseRecordType) => { const [resource, column] = unprocessedColumnSelector.includes('.') ? unprocessedColumnSelector.split('.') : [baseRecordType, unprocessedColumnSelector]; @@ -55,7 +58,7 @@ export const fullyQualifyColumnSelector = (models, unprocessedColumnSelector, ba export const processColumnSelectorKeys = (models, object, recordType) => { const processedObject = {}; Object.entries(object).forEach(([columnSelector, value]) => { - processedObject[fullyQualifyColumnSelector(models, columnSelector, recordType)] = value; + processedObject[processColumnSelector(models, columnSelector, recordType)] = value; }); return processedObject; }; diff --git a/packages/central-server/src/apiV2/mapOverlayGroupRelations/GETMapOverlayGroupRelations.js b/packages/central-server/src/apiV2/mapOverlayGroupRelations/GETMapOverlayGroupRelations.js index cf825d1674..38eeabbffb 100644 --- a/packages/central-server/src/apiV2/mapOverlayGroupRelations/GETMapOverlayGroupRelations.js +++ b/packages/central-server/src/apiV2/mapOverlayGroupRelations/GETMapOverlayGroupRelations.js @@ -30,10 +30,6 @@ export class GETMapOverlayGroupRelations extends GETHandler { nearTableKey: 'map_overlay_group_relation.map_overlay_group_id', farTableKey: 'map_overlay_group.id', }, - map_overlay: { - nearTableKey: 'map_overlay_group_relation.child_id', - farTableKey: 'map_overlay.id', - }, }; async findSingleRecord(mapOverlayGroupRelationId, options) { @@ -54,6 +50,17 @@ export class GETMapOverlayGroupRelations extends GETHandler { return mapOverlayGroupRelation; } + async getDbQueryOptions() { + const { multiJoin, sort, ...restOfOptions } = await super.getDbQueryOptions(); + return { + ...restOfOptions, + // Strip table prefix from `child_code` as it’s a `customColumn` + sort: sort.map(s => s.replace('map_overlay_group_relation.child_code', 'child_code')), + // Appending the multi-join from the Record class so that we can fetch the `child_code` + multiJoin: multiJoin.concat(this.models.mapOverlayGroupRelation.DatabaseRecordClass.joins), + }; + } + async getPermissionsFilter(criteria, options) { const dbConditions = await createMapOverlayGroupRelationDBFilter( this.accessPolicy, diff --git a/packages/database/src/DatabaseModel.js b/packages/database/src/DatabaseModel.js index 75a7b60295..6911b36884 100644 --- a/packages/database/src/DatabaseModel.js +++ b/packages/database/src/DatabaseModel.js @@ -4,6 +4,7 @@ */ import { DatabaseError, reduceToDictionary } from '@tupaia/utils'; import { runDatabaseFunctionInBatches } from './utilities/runDatabaseFunctionInBatches'; +import { QUERY_CONJUNCTIONS } from './TupaiaDatabase'; export class DatabaseModel { otherModels = {}; @@ -100,16 +101,26 @@ export class DatabaseModel { // A helper for the 'xById' methods, which disambiguates the id field to ensure joins are handled getIdClause(id) { return { - [`${this.databaseRecord}.id`]: id, + [this.fullyQualifyColumn('id')]: id, }; } + // A helper function to ensure that we're using fully qualified column names to avoid ambiguous references when joins are being used + fullyQualifyColumn(column) { + if (column.includes('.')) { + // Already fully qualified + return column; + } + + return `${this.databaseRecord}.${column}`; + } + async getColumnsForQuery() { // Alias field names to the table to prevent errors when joining other tables // with same column names. const fieldNames = await this.fetchFieldNames(); return fieldNames.map(fieldName => { - const qualifiedName = `${this.databaseRecord}.${fieldName}`; + const qualifiedName = this.fullyQualifyColumn(fieldName); const customSelector = this.customColumnSelectors && this.customColumnSelectors[fieldName]; if (customSelector) { return { [fieldName]: customSelector(qualifiedName) }; @@ -136,6 +147,30 @@ export class DatabaseModel { return { ...options, ...customQueryOptions }; } + async getDbConditions(dbConditions = {}) { + const fieldNames = await this.fetchFieldNames(); + const fullyQualifiedConditions = {}; + + const whereClauses = Object.entries(dbConditions); + for (let i = 0; i < whereClauses.length; i++) { + const [field, value] = whereClauses[i]; + if (field === QUERY_CONJUNCTIONS.AND || field === QUERY_CONJUNCTIONS.OR) { + // Recursively proccess AND and OR conditions + fullyQualifiedConditions[field] = await this.getDbConditions(value); + } else if (field === QUERY_CONJUNCTIONS.RAW) { + // Don't touch RAW conditions + fullyQualifiedConditions[field] = value; + } else { + const fullyQualifiedField = fieldNames.includes(field) + ? this.fullyQualifyColumn(field) + : field; + fullyQualifiedConditions[fullyQualifiedField] = value; + } + } + + return fullyQualifiedConditions; + } + /** * @param {...any} args * @returns {Promise} Count of records matching args @@ -171,7 +206,12 @@ export class DatabaseModel { async findOne(dbConditions, customQueryOptions = {}) { const queryOptions = await this.getQueryOptions(customQueryOptions); - const result = await this.database.findOne(this.databaseRecord, dbConditions, queryOptions); + const processedDbConditions = await this.getDbConditions(dbConditions); + const result = await this.database.findOne( + this.databaseRecord, + processedDbConditions, + queryOptions, + ); if (!result) return null; return this.generateInstance(result); } @@ -184,7 +224,12 @@ export class DatabaseModel { */ async find(dbConditions, customQueryOptions = {}) { const queryOptions = await this.getQueryOptions(customQueryOptions); - const dbResults = await this.database.find(this.databaseRecord, dbConditions, queryOptions); + const processedDbConditions = await this.getDbConditions(dbConditions); + const dbResults = await this.database.find( + this.databaseRecord, + processedDbConditions, + queryOptions, + ); return Promise.all(dbResults.map(result => this.generateInstance(result))); } diff --git a/packages/database/src/DatabaseRecord.js b/packages/database/src/DatabaseRecord.js index a4a23d4bd9..fdfc32de11 100644 --- a/packages/database/src/DatabaseRecord.js +++ b/packages/database/src/DatabaseRecord.js @@ -2,7 +2,7 @@ * Tupaia * Copyright (c) 2017 - 2020 Beyond Essential Systems Pty Ltd */ -import { TypeValidationError, stripFields } from '@tupaia/utils'; +import { stripFields, TypeValidationError } from '@tupaia/utils'; export class DatabaseRecord { static databaseRecord = null; // The database table name @@ -33,42 +33,41 @@ export class DatabaseRecord { */ static fieldValidators = new Map(); - /* - Joins are executed on every model query and give developers the ability to - add extra fields that are necessary for a model to be meaningful. - - format: - static joins = [ - { - fields: { - ['field code in joined database']: 'field code to map to in model', - }, - joinWith: 'table to join with', - joinCondition: [`{table to join with}.id`, `${this.databaseRecord}.${field to join on}`], - } - ] - - eg: - static joins = [ - { - fields: { - code: 'country_code', - }, - joinWith: RECORDS.COUNTRY, - joinCondition: [`${RECORDS.COUNTRY}.id`, `${this.databaseRecord}.country_id`], - }, - { - fields: { - code: 'foo_bar_field', - }, - joinWith: RECORDS.FOO_BAR, - joinCondition: [`${RECORDS.FOO_BAR}.id`, `${this.databaseRecord}.foo_bar_id`], - } - ] - - Will add the fields `country_code` and `foo_bar_field` to the model using the value - from the join query. - */ + /** + * Joins are executed on every model query and give developers the ability to + * add extra fields that are necessary for a model to be meaningful. + * + * Format: + * ```js + * static joins = [ + * { + * fields: { + * ['field code in joined database']: 'field code to map to in model', + * }, + * joinWith: 'table to join with', + * joinCondition: [`{table to join with}.id`, `${this.databaseRecord}.${field to join on}`], + * } + * ] + * ``` + * + * @example Add the fields `country_code` and `foo_bar_field` to the model using the value from the join query. + * static joins = [ + * { + * fields: { + * code: 'country_code', + * }, + * joinWith: RECORDS.COUNTRY, + * joinCondition: [`${RECORDS.COUNTRY}.id`, `${this.databaseRecord}.country_id`], + * }, + * { + * fields: { + * code: 'foo_bar_field', + * }, + * joinWith: RECORDS.FOO_BAR, + * joinCondition: [`${RECORDS.FOO_BAR}.id`, `${this.databaseRecord}.foo_bar_id`], + * } + * ] + */ static joins = []; constructor(model, fieldValues) { diff --git a/packages/database/src/TupaiaDatabase.js b/packages/database/src/TupaiaDatabase.js index 7d4be24721..149fee76bb 100644 --- a/packages/database/src/TupaiaDatabase.js +++ b/packages/database/src/TupaiaDatabase.js @@ -58,6 +58,18 @@ const COMPARATORS = { ILIKE: 'ilike', }; +/** + * We only support specific functions in SELECT statements to avoid SQL injection. + * + * @privateRemarks Because of the (appropriately) conservative way Knex handles identifiers in + * parameterised queries, supported functions may need custom handling in {@link getColSelector}. + * Otherwise, Knex will attempt to interpret function calls as identifiers. For example, + * `COALESCE(foo, bar)` would otherwise become `"COALESCE(FOO", "bar)"`. + * + * @see getColSelector + */ +const supportedFunctions = ['ST_AsGeoJSON', 'COALESCE']; + // no math here, just hand-tuned to be as low as possible while // keeping all the tests passing const HANDLER_DEBOUNCE_DURATION = 250; @@ -536,11 +548,16 @@ function buildQuery(connection, queryConfig, where = {}, options = {}) { return { [alias]: connection.raw(`??::${castAs}`, [alias]) }; } - // special case to handle selecting geojson - avoid generic handling of functions to keep - // out sql injection vulnerabilities - if (selector.includes('ST_AsGeoJSON')) { - const [, columnSelector] = selector.match(/ST_AsGeoJSON\((.*)\)/); - return { [alias]: connection.raw('ST_AsGeoJSON(??)', [columnSelector]) }; + // Special case to handle allowlisted SQL functions, namely for selecting GeoJSON and COALESCE + // attributes. Avoid generic handling of functions to keep out SQL injection vulnerabilities. + for (const func of supportedFunctions) { + if (selector.includes(func)) { + const [, argsString] = selector.match(new RegExp(`${func}\\((.*)\\)`)); + const args = argsString.split(',').map(arg => arg.trim()); + return { + [alias]: connection.raw(`${func}(${args.map(() => '??').join(',')})`, [...args]), + }; + } } return { [alias]: connection.raw('??', [selector]) }; @@ -683,17 +700,39 @@ function addJoin(baseQuery, recordType, joinOptions) { }); } +/** + * @privateRemarks + * This sanitisation step fails if the input uses both JSON operators and the `COALESCE` function. + * + * @see supportedFunctions + */ function getColSelector(connection, inputColStr) { const jsonOperatorPattern = /->>?/g; - if (!jsonOperatorPattern.test(inputColStr)) return inputColStr; + if (jsonOperatorPattern.test(inputColStr)) { + const params = inputColStr.split(jsonOperatorPattern); + const allButFirst = params.slice(1); + const lastIndexOfLookupAsText = inputColStr.lastIndexOf('->>'); + const lastIndexOfLookupAsJson = inputColStr.lastIndexOf('->'); + const selector = lastIndexOfLookupAsText >= lastIndexOfLookupAsJson ? '#>>' : '#>'; - const params = inputColStr.split(jsonOperatorPattern); - const allButFirst = params.slice(1); - const lastIndexOfLookupAsText = inputColStr.lastIndexOf('->>'); - const lastIndexOfLookupAsJson = inputColStr.lastIndexOf('->'); - const selector = lastIndexOfLookupAsText >= lastIndexOfLookupAsJson ? '#>>' : '#>'; + // Turn `config->item->>colour` into `config #>> '{item,colour}'` + // For some reason, Knex fails when we try to convert it to `config->'item'->>'colour'` + return connection.raw(`?? ${selector} '{${allButFirst.map(() => '??').join(',')}}'`, params); + } + + /** + * Special handling of COALESCE() - one of the {@link supportedFunctions} - to treat its arguments + * as identifiers individually rather than trying to treat ‘COALESCE(foo, bar)’ as a single + * identifier. + */ + const coalescePattern = /^COALESCE\(.+\)$/; + if (coalescePattern.test(inputColStr)) { + const [, argsString] = inputColStr.match(/^COALESCE\((.+)\)$/); + const bindings = argsString.split(',').map(arg => arg.trim()); + const identifiers = bindings.map(() => '??'); + + return connection.raw(`COALESCE(${identifiers})`, bindings); + } - // Turn `config->item->>colour` into `config #>> '{item,colour}'` - // For some reason, Knex fails when we try to convert it to `config->'item'->>'colour'` - return connection.raw(`?? ${selector} '{${allButFirst.map(() => '??').join(',')}}'`, params); + return inputColStr; } diff --git a/packages/database/src/modelClasses/MapOverlayGroupRelation.js b/packages/database/src/modelClasses/MapOverlayGroupRelation.js index 5d15d9efdd..08ddad8d06 100644 --- a/packages/database/src/modelClasses/MapOverlayGroupRelation.js +++ b/packages/database/src/modelClasses/MapOverlayGroupRelation.js @@ -1,11 +1,12 @@ -/** +/* * Tupaia - * Copyright (c) 2017 - 2020 Beyond Essential Systems Pty Ltd + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ import { DatabaseModel } from '../DatabaseModel'; import { DatabaseRecord } from '../DatabaseRecord'; import { RECORDS } from '../records'; +import { JOIN_TYPES } from '../TupaiaDatabase'; const MAP_OVERLAY = 'mapOverlay'; const MAP_OVERLAY_GROUP = 'mapOverlayGroup'; @@ -17,6 +18,26 @@ const RELATION_CHILD_TYPES = { export class MapOverlayGroupRelationRecord extends DatabaseRecord { static databaseRecord = RECORDS.MAP_OVERLAY_GROUP_RELATION; + static joins = [ + { + joinType: JOIN_TYPES.LEFT, + joinWith: RECORDS.MAP_OVERLAY, + joinAs: 'child_map_overlay', + joinCondition: [`${RECORDS.MAP_OVERLAY_GROUP_RELATION}.child_id`, `child_map_overlay.id`], + fields: { code: 'code' }, + }, + { + joinType: JOIN_TYPES.LEFT, + joinWith: RECORDS.MAP_OVERLAY_GROUP, + joinAs: 'child_map_overlay_group', + joinCondition: [ + `${RECORDS.MAP_OVERLAY_GROUP_RELATION}.child_id`, + `child_map_overlay_group.id`, + ], + fields: { code: 'code' }, + }, + ]; + async findChildRelations() { return this.model.find({ map_overlay_group_id: this.child_id }); } @@ -31,6 +52,10 @@ export class MapOverlayGroupRelationModel extends DatabaseModel { return RELATION_CHILD_TYPES; } + customColumnSelectors = { + child_code: () => 'COALESCE(child_map_overlay.code, child_map_overlay_group.code)', + }; + async findTopLevelMapOverlayGroupRelations() { const rootMapOverlayGroup = await this.otherModels.mapOverlayGroup.findRootMapOverlayGroup();