Skip to content

Commit

Permalink
Chore: Rest API query parameters handling (#25648)
Browse files Browse the repository at this point in the history
  • Loading branch information
ggazzo committed May 26, 2022
1 parent 052858d commit 31ae30f
Show file tree
Hide file tree
Showing 9 changed files with 445 additions and 124 deletions.
2 changes: 1 addition & 1 deletion apps/meteor/.mocharc.api.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ module.exports = {
timeout: 10000,
bail: true,
file: 'tests/end-to-end/teardown.js',
spec: ['tests/end-to-end/api/*.js', 'tests/end-to-end/api/*.ts', 'tests/end-to-end/apps/*.js'],
spec: ['tests/unit/app/api/server/v1/*.spec.ts', 'tests/end-to-end/api/*.js', 'tests/end-to-end/api/*.ts', 'tests/end-to-end/apps/*.js'],
};
2 changes: 1 addition & 1 deletion apps/meteor/app/api/server/api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ type Operations<TPathPattern extends PathPattern, TOptions extends Options = {}>
};

declare class APIClass<TBasePath extends string = '/'> {
fieldSeparator(fieldSeparator: unknown): void;
fieldSeparator: string;

limitedUserFieldsToExclude(fields: { [x: string]: unknown }, limitedUserFieldsToExclude: unknown): { [x: string]: unknown };

Expand Down
3 changes: 3 additions & 0 deletions apps/meteor/app/api/server/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,9 @@ export class APIClass extends Restivus {
connection,
});

this.queryOperations = options.queryOperations;
this.queryFields = options.queryFields;

result = DDP._CurrentInvocation.withValue(invocation, () => Promise.await(originalAction.apply(this))) || API.v1.success();

log.http({
Expand Down
119 changes: 0 additions & 119 deletions apps/meteor/app/api/server/helpers/parseJsonQuery.js

This file was deleted.

142 changes: 142 additions & 0 deletions apps/meteor/app/api/server/helpers/parseJsonQuery.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
import { Meteor } from 'meteor/meteor';
import { EJSON } from 'meteor/ejson';

import { hasPermission } from '../../../authorization/server';
import { isValidQuery } from '../lib/isValidQuery';
import { clean } from '../lib/cleanQuery';
import { API } from '../api';

const pathAllowConf = {
'/api/v1/users.list': ['$or', '$regex', '$and'],
'def': ['$or', '$and', '$regex'],
};

API.helperMethods.set(
'parseJsonQuery',
function _parseJsonQuery(this: {
request: {
route: string;
};
queryParams: {
query?: string;
sort?: string;
fields?: string;
};
logger: any;
userId: string;
queryFields: string[];
queryOperations: string[];
}) {
let sort;
if (this.queryParams.sort) {
try {
sort = JSON.parse(this.queryParams.sort);
Object.entries(sort).forEach(([key, value]) => {
if (value !== 1 && value !== -1) {
throw new Meteor.Error('error-invalid-sort-parameter', `Invalid sort parameter: ${key}`, {
helperMethod: 'parseJsonQuery',
});
}
});
} catch (e) {
this.logger.warn(`Invalid sort parameter provided "${this.queryParams.sort}":`, e);
throw new Meteor.Error('error-invalid-sort', `Invalid sort parameter provided: "${this.queryParams.sort}"`, {
helperMethod: 'parseJsonQuery',
});
}
}

let fields: Record<string, 0 | 1> | undefined;
if (this.queryParams.fields) {
try {
fields = JSON.parse(this.queryParams.fields) as Record<string, 0 | 1>;

Object.entries(fields).forEach(([key, value]) => {
if (value !== 1 && value !== 0) {
throw new Meteor.Error('error-invalid-sort-parameter', `Invalid fields parameter: ${key}`, {
helperMethod: 'parseJsonQuery',
});
}
});
} catch (e) {
this.logger.warn(`Invalid fields parameter provided "${this.queryParams.fields}":`, e);
throw new Meteor.Error('error-invalid-fields', `Invalid fields parameter provided: "${this.queryParams.fields}"`, {
helperMethod: 'parseJsonQuery',
});
}
}

// Verify the user's selected fields only contains ones which their role allows
if (typeof fields === 'object') {
let nonSelectableFields = Object.keys(API.v1.defaultFieldsToExclude);
if (this.request.route.includes('/v1/users.')) {
const getFields = () =>
Object.keys(
hasPermission(this.userId, 'view-full-other-user-info')
? API.v1.limitedUserFieldsToExcludeIfIsPrivilegedUser
: API.v1.limitedUserFieldsToExclude,
);
nonSelectableFields = nonSelectableFields.concat(getFields());
}

Object.keys(fields).forEach((k) => {
if (nonSelectableFields.includes(k) || nonSelectableFields.includes(k.split(API.v1.fieldSeparator)[0])) {
fields && delete fields[k as keyof typeof fields];
}
});
}

// Limit the fields by default
fields = Object.assign({}, fields, API.v1.defaultFieldsToExclude);
if (this.request.route.includes('/v1/users.')) {
if (hasPermission(this.userId, 'view-full-other-user-info')) {
fields = Object.assign(fields, API.v1.limitedUserFieldsToExcludeIfIsPrivilegedUser);
} else {
fields = Object.assign(fields, API.v1.limitedUserFieldsToExclude);
}
}

let query: Record<string, any> = {};
if (this.queryParams.query) {
this.logger.warn('attribute query is deprecated');
try {
query = EJSON.parse(this.queryParams.query);
query = clean(query, pathAllowConf.def);
} catch (e) {
this.logger.warn(`Invalid query parameter provided "${this.queryParams.query}":`, e);
throw new Meteor.Error('error-invalid-query', `Invalid query parameter provided: "${this.queryParams.query}"`, {
helperMethod: 'parseJsonQuery',
});
}
}

// Verify the user has permission to query the fields they are
if (typeof query === 'object') {
let nonQueryableFields = Object.keys(API.v1.defaultFieldsToExclude);

if (this.request.route.includes('/v1/users.')) {
if (hasPermission(this.userId, 'view-full-other-user-info')) {
nonQueryableFields = nonQueryableFields.concat(Object.keys(API.v1.limitedUserFieldsToExcludeIfIsPrivilegedUser));
} else {
nonQueryableFields = nonQueryableFields.concat(Object.keys(API.v1.limitedUserFieldsToExclude));
}
}

if (this.queryFields && !isValidQuery(query, this.queryFields || ['*'], this.queryOperations ?? pathAllowConf.def)) {
throw new Meteor.Error('error-invalid-query', isValidQuery.errors.join('\n'));
}

Object.keys(query).forEach((k) => {
if (nonQueryableFields.includes(k) || nonQueryableFields.includes(k.split(API.v1.fieldSeparator)[0])) {
query && delete query[k];
}
});
}

return {
sort,
fields,
query,
};
},
);
4 changes: 2 additions & 2 deletions apps/meteor/app/api/server/lib/cleanQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ type Query = { [k: string]: any };

const denyList = ['constructor', '__proto__', 'prototype'];

const removeDangerousProps = (v: Query): Query => {
export const removeDangerousProps = (v: Query): Query => {
const query = Object.create(null);
for (const key in v) {
if (v.hasOwnProperty(key) && !denyList.includes(key)) {
Expand All @@ -12,7 +12,7 @@ const removeDangerousProps = (v: Query): Query => {

return query;
};

/* @deprecated */
export function clean(v: Query, allowList: string[] = []): Query {
const typedParam = removeDangerousProps(v);
if (v instanceof Object) {
Expand Down
58 changes: 58 additions & 0 deletions apps/meteor/app/api/server/lib/isValidQuery.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { removeDangerousProps } from './cleanQuery';

type Query = { [k: string]: any };

export const isValidQuery: {
(query: Query, allowedAttributes: string[], allowedOperations: string[]): boolean;
errors: string[];
} = Object.assign(
(query: Query, allowedAttributes: string[], allowedOperations: string[]): boolean => {
isValidQuery.errors = [];
if (!(query instanceof Object)) {
throw new Error('query must be an object');
}

// eslint-disable-next-line @typescript-eslint/no-use-before-define
return verifyQuery(query, allowedAttributes, allowedOperations);
},
{
errors: [],
},
);

export const verifyQuery = (query: Query, allowedAttributes: string[], allowedOperations: string[], parent = ''): boolean => {
return Object.entries(removeDangerousProps(query)).every(([key, value]) => {
const path = parent ? `${parent}.${key}` : key;
if (parent === '' && path.startsWith('$')) {
if (!allowedOperations.includes(path)) {
isValidQuery.errors.push(`Invalid operation: ${path}`);
return false;
}
if (!Array.isArray(value)) {
isValidQuery.errors.push(`Invalid parameter for operation: ${path} : ${value}`);
return false;
}
return value.every((v) => verifyQuery(v, allowedAttributes, allowedOperations));
}

if (
!allowedAttributes.some((allowedAttribute) => {
if (allowedAttribute.endsWith('.*')) {
return path.startsWith(allowedAttribute.replace('.*', ''));
}
if (allowedAttribute.endsWith('*') && !allowedAttribute.endsWith('.*')) {
return true;
}
return path === allowedAttribute;
})
) {
isValidQuery.errors.push(`Invalid attribute: ${path}`);
return false;
}

if (value instanceof Object) {
return verifyQuery(value, allowedAttributes, allowedOperations, path);
}
return true;
});
};
Loading

0 comments on commit 31ae30f

Please sign in to comment.