Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: Rest API query parameters handling #25648

Merged
merged 3 commits into from
May 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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