From 5bdde4717d6157e5b1ddcdffb82f8f94cfc21cf3 Mon Sep 17 00:00:00 2001 From: WoLfulus Date: Tue, 13 Oct 2020 20:09:37 -0300 Subject: [PATCH 1/5] Allows default exports to be used in extensions --- api/src/extensions.ts | 22 ++++++++++++++++++---- api/src/types/extensions.ts | 2 +- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/api/src/extensions.ts b/api/src/extensions.ts index 94b3481747..6586359b67 100644 --- a/api/src/extensions.ts +++ b/api/src/extensions.ts @@ -5,7 +5,7 @@ import { ServiceUnavailableException } from './exceptions'; import express, { Router } from 'express'; import emitter from './emitter'; import logger from './logger'; -import { HookRegisterFunction, EndpointRegisterFunction } from './types'; +import { ExtensionContext, HookRegisterFunction, EndpointRegisterFunction } from './types'; import { ensureDir } from 'fs-extra'; import * as exceptions from './exceptions'; @@ -78,9 +78,16 @@ function registerHooks(hooks: string[]) { function registerHook(hook: string) { const hookPath = path.resolve(extensionsPath, 'hooks', hook, 'index.js'); - const register: HookRegisterFunction = require(hookPath); - const events = register({ services, exceptions, env, database }); + const hookInstance: HookRegisterFunction | { default?: HookRegisterFunction } = require(hookPath); + let register: HookRegisterFunction = () => ({}); + if (typeof hookInstance !== "function") { + if (hookInstance.default) { + register = hookInstance.default; + } + } + + let events = register({ services, exceptions, env, database }); for (const [event, handler] of Object.entries(events)) { emitter.on(event, handler); } @@ -101,7 +108,14 @@ function registerEndpoints(endpoints: string[], router: Router) { function registerEndpoint(endpoint: string) { const endpointPath = path.resolve(extensionsPath, 'endpoints', endpoint, 'index.js'); - const register: EndpointRegisterFunction = require(endpointPath); + const endpointInstance: EndpointRegisterFunction | { default?: EndpointRegisterFunction } = require(endpointPath); + + let register: EndpointRegisterFunction = () => ({}); + if (typeof endpointInstance !== "function") { + if (endpointInstance.default) { + register = endpointInstance.default; + } + } const scopedRouter = express.Router(); router.use(`/${endpoint}/`, scopedRouter); diff --git a/api/src/types/extensions.ts b/api/src/types/extensions.ts index 1d60d25bfa..030819cb1c 100644 --- a/api/src/types/extensions.ts +++ b/api/src/types/extensions.ts @@ -5,7 +5,7 @@ import env from '../env'; import Knex from 'knex'; import { Router } from 'express'; -type ExtensionContext = { +export type ExtensionContext = { services: typeof services; exceptions: typeof exceptions; database: Knex; From 4572fc8041cfcf25450abf5296eebc4c3fe8f795 Mon Sep 17 00:00:00 2001 From: WoLfulus Date: Tue, 13 Oct 2020 20:12:56 -0300 Subject: [PATCH 2/5] Remove unused import/export --- api/src/extensions.ts | 2 +- api/src/types/extensions.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/extensions.ts b/api/src/extensions.ts index 6586359b67..b0dd6a1deb 100644 --- a/api/src/extensions.ts +++ b/api/src/extensions.ts @@ -5,7 +5,7 @@ import { ServiceUnavailableException } from './exceptions'; import express, { Router } from 'express'; import emitter from './emitter'; import logger from './logger'; -import { ExtensionContext, HookRegisterFunction, EndpointRegisterFunction } from './types'; +import { HookRegisterFunction, EndpointRegisterFunction } from './types'; import { ensureDir } from 'fs-extra'; import * as exceptions from './exceptions'; diff --git a/api/src/types/extensions.ts b/api/src/types/extensions.ts index 030819cb1c..1d60d25bfa 100644 --- a/api/src/types/extensions.ts +++ b/api/src/types/extensions.ts @@ -5,7 +5,7 @@ import env from '../env'; import Knex from 'knex'; import { Router } from 'express'; -export type ExtensionContext = { +type ExtensionContext = { services: typeof services; exceptions: typeof exceptions; database: Knex; From 484bcbb5987565e1814555e2724fa25f3808a47f Mon Sep 17 00:00:00 2001 From: WoLfulus Date: Tue, 13 Oct 2020 20:14:51 -0300 Subject: [PATCH 3/5] Fix default extension function --- api/src/extensions.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/extensions.ts b/api/src/extensions.ts index b0dd6a1deb..8b31d8326b 100644 --- a/api/src/extensions.ts +++ b/api/src/extensions.ts @@ -80,7 +80,7 @@ function registerHooks(hooks: string[]) { const hookPath = path.resolve(extensionsPath, 'hooks', hook, 'index.js'); const hookInstance: HookRegisterFunction | { default?: HookRegisterFunction } = require(hookPath); - let register: HookRegisterFunction = () => ({}); + let register: HookRegisterFunction = hookInstance as HookRegisterFunction; if (typeof hookInstance !== "function") { if (hookInstance.default) { register = hookInstance.default; @@ -110,7 +110,7 @@ function registerEndpoints(endpoints: string[], router: Router) { const endpointPath = path.resolve(extensionsPath, 'endpoints', endpoint, 'index.js'); const endpointInstance: EndpointRegisterFunction | { default?: EndpointRegisterFunction } = require(endpointPath); - let register: EndpointRegisterFunction = () => ({}); + let register: EndpointRegisterFunction = endpointInstance as EndpointRegisterFunction; if (typeof endpointInstance !== "function") { if (endpointInstance.default) { register = endpointInstance.default; From 1e628d89152c0d23ccca0fa1ff63a78b6dacabb8 Mon Sep 17 00:00:00 2001 From: rijkvanzanten Date: Wed, 14 Oct 2020 15:31:47 -0400 Subject: [PATCH 4/5] Validate the used query before running it --- api/src/middleware/sanitize-query.ts | 6 +- api/src/utils/parse-filter.ts | 7 +- api/src/utils/sanitize-query.ts | 11 ++- api/src/utils/validate-query.ts | 110 +++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 api/src/utils/validate-query.ts diff --git a/api/src/middleware/sanitize-query.ts b/api/src/middleware/sanitize-query.ts index 24e6fea692..4d2ebbb323 100644 --- a/api/src/middleware/sanitize-query.ts +++ b/api/src/middleware/sanitize-query.ts @@ -5,6 +5,7 @@ import { RequestHandler } from 'express'; import { sanitizeQuery } from '../utils/sanitize-query'; +import { validateQuery } from '../utils/validate-query'; const sanitizeQueryMiddleware: RequestHandler = (req, res, next) => { req.sanitizedQuery = {}; @@ -13,15 +14,16 @@ const sanitizeQueryMiddleware: RequestHandler = (req, res, next) => { req.sanitizedQuery = sanitizeQuery( { fields: req.query.fields || '*', - ...req.query + ...req.query, }, req.accountability || null ); Object.freeze(req.sanitizedQuery); + validateQuery(req.sanitizedQuery); + return next(); }; export default sanitizeQueryMiddleware; - diff --git a/api/src/utils/parse-filter.ts b/api/src/utils/parse-filter.ts index a1eaf1ded6..92c8888648 100644 --- a/api/src/utils/parse-filter.ts +++ b/api/src/utils/parse-filter.ts @@ -2,7 +2,12 @@ import { Filter, Accountability } from '../types'; import { deepMap } from './deep-map'; export function parseFilter(filter: Filter, accountability: Accountability | null) { - return deepMap(filter, (val: any) => { + return deepMap(filter, (val: any, key: string) => { + if (val === 'true') return true; + if (val === 'false') return false; + + if (key === '_in' || key === '_nin') return val.split(',').filter((val: any) => val); + if (val === '$NOW') return new Date(); if (val === '$CURRENT_USER') return accountability?.user || null; if (val === '$CURRENT_ROLE') return accountability?.role || null; diff --git a/api/src/utils/sanitize-query.ts b/api/src/utils/sanitize-query.ts index 67b17cd5be..9597ce5a2a 100644 --- a/api/src/utils/sanitize-query.ts +++ b/api/src/utils/sanitize-query.ts @@ -2,7 +2,10 @@ import { Accountability, Query, Sort, Filter, Meta } from '../types'; import logger from '../logger'; import { parseFilter } from '../utils/parse-filter'; -export function sanitizeQuery(rawQuery: Record, accountability: Accountability | null) { +export function sanitizeQuery( + rawQuery: Record, + accountability: Accountability | null +) { const query: Query = {}; if (rawQuery.limit !== undefined) { @@ -49,11 +52,7 @@ export function sanitizeQuery(rawQuery: Record, accountability: Acc query.search = rawQuery.search; } - if ( - rawQuery.export && - typeof rawQuery.export === 'string' && - ['json', 'csv'].includes(rawQuery.export) - ) { + if (rawQuery.export) { query.export = rawQuery.export as 'json' | 'csv'; } diff --git a/api/src/utils/validate-query.ts b/api/src/utils/validate-query.ts new file mode 100644 index 0000000000..3b66e5dda3 --- /dev/null +++ b/api/src/utils/validate-query.ts @@ -0,0 +1,110 @@ +import { Query } from '../types'; +import Joi from 'joi'; +import { InvalidQueryException } from '../exceptions'; + +const querySchema = Joi.object({ + fields: Joi.array().items(Joi.string()), + sort: Joi.array().items( + Joi.object({ + column: Joi.string(), + order: Joi.string().valid('asc', 'desc'), + }) + ), + filter: Joi.object({}).unknown(), + limit: Joi.number(), + offset: Joi.number(), + page: Joi.number(), + single: Joi.boolean(), + meta: Joi.array().items(Joi.string().valid('total_count', 'result_count')), + search: Joi.string(), + export: Joi.string().valid('json', 'csv'), + deep: Joi.link('#query'), +}).id('query'); + +export function validateQuery(query: Query) { + const { error } = querySchema.validate(query); + + if (query.filter && Object.keys(query.filter).length > 0) { + validateFilter(query.filter); + } + + if (error) { + throw new InvalidQueryException(error.message); + } + + return query; +} + +function validateFilter(filter: Query['filter']) { + if (!filter) throw new InvalidQueryException('Invalid filter object'); + + for (const [key, nested] of Object.entries(filter)) { + if (key === '_and' || key === '_or') { + nested.forEach(validateFilter); + } else if (key.startsWith('_')) { + const value = nested; + + switch (key) { + case '_eq': + case '_neq': + case '_contains': + case '_ncontains': + case '_gt': + case '_gte': + case '_lt': + case '_lte': + default: + validateFilterPrimitive(value, key); + break; + case '_in': + case '_nin': + validateList(value, key); + break; + case '_null': + case '_nnull': + case '_empty': + case '_nempty': + validateBoolean(value, key); + break; + } + } else { + validateFilter(nested); + } + } +} + +function validateFilterPrimitive(value: any, key: string) { + if ((typeof value === 'string' || typeof value === 'number') === false) { + throw new InvalidQueryException( + `The filter value for "${key}" has to be a string or a number` + ); + } + + if (Number.isNaN(value)) { + throw new InvalidQueryException(`The filter value for "${key}" is not a valid number`); + } + + if (typeof value === 'string' && value.length === 0) { + throw new InvalidQueryException( + `You can't filter for an empty string in "${key}". Use "_empty" or "_nempty" instead` + ); + } + + return true; +} + +function validateList(value: any, key: string) { + if (Array.isArray(value) === false || value.length === 0) { + throw new InvalidQueryException(`"${key}" has to be an array of values`); + } + + return true; +} + +function validateBoolean(value: any, key: string) { + if (typeof value !== 'boolean') { + throw new InvalidQueryException(`"${key}" has to be a boolean`); + } + + return true; +} From efe98c01885a0b51800f3166212896dbbd69edd1 Mon Sep 17 00:00:00 2001 From: rijkvanzanten Date: Wed, 14 Oct 2020 15:37:18 -0400 Subject: [PATCH 5/5] Update advanced filter drawer detail to match filter query check --- app/src/utils/filters-to-query/filters-to-query.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/src/utils/filters-to-query/filters-to-query.ts b/app/src/utils/filters-to-query/filters-to-query.ts index d7b2c61b77..a9418900a4 100644 --- a/app/src/utils/filters-to-query/filters-to-query.ts +++ b/app/src/utils/filters-to-query/filters-to-query.ts @@ -1,11 +1,17 @@ import { Filter } from '@/types/'; -import { set } from 'lodash'; +import { set, clone } from 'lodash'; export default function filtersToQuery(filters: readonly Filter[]) { const filterQuery: Record = {}; for (const filter of filters) { - const { field, operator, value } = filter; + let { field, operator, value } = clone(filter) as any; + + if (['empty', 'nempty', 'null', 'nnull'].includes(operator)) { + value = true; + } + + if (!value) continue; set(filterQuery, field, { [`_${operator}`]: value }); }