Skip to content
Permalink
Browse files

fix(find): fix sql injection issue (#225)

* fix(find): fix sql injection issue

* more fixes

* more fixes

* wip

* more fixes
  • Loading branch information...
goldcaddy77 committed Oct 31, 2019
1 parent ceb5401 commit 8f68e42ee9f2c91bbd44cfb2112f1f8c598f1590
@@ -1,13 +1,18 @@
import { validate } from 'class-validator';
import { ArgumentValidationError } from 'type-graphql';
import { DeepPartial, FindManyOptions, FindOperator, getRepository, Repository } from 'typeorm';
import { DeepPartial, getRepository, Repository } from 'typeorm';
import { ColumnMetadata } from 'typeorm/metadata/ColumnMetadata';

import { StandardDeleteResponse } from '../tgql';
import { getFindOperator } from '../torm';
import { addQueryBuilderWhereItem } from '../torm';

import { BaseModel, WhereInput } from '..';
import { StringMap } from './types';

export class BaseService<E extends BaseModel> {
columnMap: StringMap;
klass: string;

// TODO: need to figure out why we couldn't type this as Repository<E>
constructor(protected entityClass: any, protected repository: Repository<any>) {
if (!entityClass) {
@@ -22,6 +27,17 @@ export class BaseService<E extends BaseModel> {
if (!repository) {
throw new Error(`BaseService requires a valid repository, class ${entityClass}`);
}

// Need a mapping of camelCase field name to the modified case using the naming strategy. For the standard
// SnakeNamingStrategy this would be something like { id: 'id', stringField: 'string_field' }
this.columnMap = this.repository.metadata.columns.reduce(
(prev: StringMap, column: ColumnMetadata) => {
prev[column.propertyPath] = column.databasePath;
return prev;
},
{}
);
this.klass = this.repository.metadata.name.toLowerCase();
}

async find<W extends WhereInput>(
@@ -31,36 +47,39 @@ export class BaseService<E extends BaseModel> {
offset?: number,
fields?: string[]
): Promise<E[]> {
const findOptions: FindManyOptions = {};
let qb = this.repository.createQueryBuilder(this.klass);

if (limit) {
findOptions.take = limit;
qb = qb.take(limit);
}
if (offset) {
findOptions.skip = offset;
qb = qb.skip(offset);
}

if (fields) {
// We always need to select ID or dataloaders will not function properly
if (fields.indexOf('id') === -1) {
fields.push('id');
}
findOptions.select = fields;
// Querybuilder requires you to prefix all fields with the table alias. It also requires you to
// specify the field name using it's TypeORM attribute name, not the camel-cased DB column name
const selection = fields.map(field => `${this.klass}.${field}`);
qb = qb.select(selection);
}
if (orderBy) {
// TODO: allow multiple sorts
// See https://github.com/typeorm/typeorm/blob/master/docs/select-query-builder.md#adding-order-by-expression
const parts = orderBy.toString().split('_');
// TODO: ensure attr is one of the properties on the model
const attr = parts[0];
const direction: 'ASC' | 'DESC' = parts[1] as 'ASC' | 'DESC';
// TODO: ensure key is one of the properties on the model
findOptions.order = {
[attr]: direction
};

qb = qb.orderBy(this.attrToDBColumn(attr), direction);
}

// Soft-deletes are filtered out by default, setting `deletedAt_all` is the only way to
// turn this off
where = where || {};

// Soft-deletes are filtered out by default, setting `deletedAt_all` is the only way to turn this off
const hasDeletedAts = Object.keys(where).find(key => key.indexOf('deletedAt_') === 0);
// If no deletedAt filters specified, hide them by default
if (!hasDeletedAts) {
@@ -75,9 +94,19 @@ export class BaseService<E extends BaseModel> {
// do nothing because the specific deleted at filters will be added by processWhereOptions
}

findOptions.where = this.processWhereOptions<W>(where);
if (Object.keys(where).length) {
// where is of shape { userName_contains: 'a' }
Object.keys(where).forEach(k => {
const key = k as keyof W; // userName
const parts = key.toString().split('_'); // ['userName', 'contains']
const attr = parts[0]; // userName
const operator = parts.length > 1 ? parts[1] : 'eq'; // contains

return this.repository.find(findOptions);
qb = addQueryBuilderWhereItem(qb, attr, this.attrToDBColumn(attr), operator, where[key]);
});
}

return qb.getMany();
}

// TODO: fix - W extends Partial<E>
@@ -175,14 +204,11 @@ export class BaseService<E extends BaseModel> {
return { id: found.id };
}

// extends WhereInput
processWhereOptions<W extends any>(where: W) {
const whereOptions: { [key: string]: FindOperator<any> } = {};
Object.keys(where).forEach(k => {
const key = k as keyof W;
const [attr, operator] = getFindOperator(String(key), where[key]);
whereOptions[attr] = operator;
});
return whereOptions;
}
attrsToDBColumns = (attrs: string[]): string[] => {
return attrs.map(this.attrToDBColumn);
};

attrToDBColumn = (attr: string): string => {
return `${this.klass}.${this.columnMap[attr]}`;
};
}
@@ -30,7 +30,6 @@ describe('Server', () => {
expect(server.schema).toBeTruthy();
expect(appListenSpy).toHaveBeenCalledTimes(0);
expect(hasGraphQlRoute(server.expressApp._router)).toBeTruthy();
expect(server.expressApp.settings.env).toEqual('test');
});

test('start a server with a custom express app', async () => {
@@ -44,7 +43,6 @@ describe('Server', () => {
expect(server.schema).toBeTruthy();
expect(appListenSpy).toHaveBeenCalledTimes(1);
expect(hasGraphQlRoute(server.expressApp._router)).toBeTruthy();
expect(server.expressApp.settings.env).toEqual('test');
});
});

@@ -29,14 +29,14 @@ describe('server', () => {
// Make sure to clean up server
beforeAll(async done => {
jest.setTimeout(20000);
setTestServerEnvironmentVariables();

await cleanUpTestData();

// build a custom express app with a dummy endpoint
customExpressApp = buildCustomExpressApp();

try {
setTestServerEnvironmentVariables();

server = getTestServer({
apolloConfig: { playground: false },
expressApp: customExpressApp,
@@ -1,6 +1,7 @@
// This file has been auto-generated by Warthog. Do not update directly as it
// will be re-written. If you need to change this file, update models or add
// new TypeGraphQL objects
import { GraphQLDateTime as DateTime } from "graphql-iso-date";
import { GraphQLID as ID } from "graphql";
import {
ArgsType,
@@ -24,6 +24,7 @@ export function getStandardEnvironmentVariables(): StringMap {
WARTHOG_APP_PROTOCOL: 'http',
WARTHOG_AUTO_GENERATE_FILES: 'true',
WARTHOG_AUTO_OPEN_PLAYGROUND: 'false',
WARTHOG_DB_CONNECTION: 'sqlite',
WARTHOG_DB_DATABASE: 'warthog-test',
WARTHOG_DB_ENTITIES: 'src/test/modules/**/*.model.ts',
WARTHOG_DB_HOST: 'localhost',
@@ -21,7 +21,6 @@ export function getTestServer(options: ServerOptions<any>) {
};
},
introspection: true,
mockDBConnection: true,
openPlayground: false,
...options
});
@@ -1,35 +1,43 @@
import { Equal, FindOperator, In, IsNull, LessThan, MoreThan, Not, Raw } from 'typeorm';

export function getFindOperator(key: string, value: any): [string, FindOperator<any>] {
const parts = key.toString().split('_');
const attr = parts[0];
const operator = parts.length > 1 ? parts[1] : 'eq';
import { SelectQueryBuilder } from 'typeorm';

export function addQueryBuilderWhereItem<E>(
qb: SelectQueryBuilder<E>,
dbColumn: string,
columnWithAlias: string,
operator: string,
value: any
): SelectQueryBuilder<E> {
switch (operator) {
case 'eq':
if (value === null) {
return [attr, IsNull()];
return qb.andWhere(`${columnWithAlias} IS NULL`);
}
return [attr, Equal(value)];
// // Not using "not" yet
// case 'not':
// return [attr, Not(value)];
return qb.andWhere(`${columnWithAlias} = :${dbColumn}`, { [dbColumn]: value });
case 'not':
return qb.andWhere(`${columnWithAlias} != :${dbColumn}`, { [dbColumn]: value });
case 'lt':
return [attr, LessThan(value)];
return qb.andWhere(`${columnWithAlias} < :${dbColumn}`, { [dbColumn]: value });
case 'lte':
return [attr, Not(MoreThan(value))];
return qb.andWhere(`${columnWithAlias} <= :${dbColumn}`, { [dbColumn]: value });
case 'gt':
return [attr, MoreThan(value)];
return qb.andWhere(`${columnWithAlias} > :${dbColumn}`, { [dbColumn]: value });
case 'gte':
return [attr, Not(LessThan(value))];
return qb.andWhere(`${columnWithAlias} >= :${dbColumn}`, { [dbColumn]: value });
case 'in':
return [attr, In(value)];
// IN (:... is the syntax for exploding array params into (?, ?, ?) in QueryBuilder
return qb.andWhere(`${columnWithAlias} IN (:...${dbColumn})`, { [dbColumn]: value });
case 'contains':
return [attr, Raw(alias => `LOWER(${alias}) LIKE LOWER('%${value}%')`)];
return qb.andWhere(`LOWER(${columnWithAlias}) LIKE :${dbColumn}`, {
[dbColumn]: `%${value}%`
});
case 'startsWith':
return [attr, Raw(alias => `LOWER(${alias}) LIKE LOWER('${value}%')`)];
return qb.andWhere(`LOWER(${columnWithAlias}) LIKE :${dbColumn}`, {
[dbColumn]: `${value}%`
});
case 'endsWith':
return [attr, Raw(alias => `LOWER(${alias}) LIKE LOWER('%${value}')`)];
return qb.andWhere(`LOWER(${columnWithAlias}) LIKE :${dbColumn}`, {
[dbColumn]: `%${value}`
});
default:
throw new Error(`Can't find operator ${operator}`);
}

0 comments on commit 8f68e42

Please sign in to comment.
You can’t perform that action at this time.