Skip to content

Commit

Permalink
fix(json-mapper): add option to disable unsecured constructor mapper
Browse files Browse the repository at this point in the history
Closes: #1942
  • Loading branch information
Romakita committed Aug 27, 2022
1 parent ce90b22 commit 999abb5
Show file tree
Hide file tree
Showing 17 changed files with 141 additions and 42 deletions.
35 changes: 35 additions & 0 deletions docs/docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,41 @@ A list of response filters must be called before returning a response to the con

Object configure Multer. See more on [Upload file](/tutorials/serve-static-files.md).

## jsonMapper

```typescript
@Configuration({
jsonMapper: {
additionalProperties: false,
disableUnsecureConstructor: false,
}
})
```

### jsonMapper.additionalProperties

Enable additional properties on model. By default, `false`. Enable this option is dangerous and may be a potential security issue.

### jsonMapper.disableUnsecureConstructor

Pass the plain object to the model constructor. By default, `false`.

It may be a potential security issue if you have as constructor with this followings code:

```typescript
class MyModel {
constructor(obj: any = {}) {
Object.assign(this, obj); // potential prototype pollution
}
}
```

::: tip Note
Recommended: Set this options to `true` in your new project.

In v7 this option will be set to true by default.
:::

## Platform Options

These options are specific for each framework (Express.js, Koa.js, etc...):
Expand Down
44 changes: 34 additions & 10 deletions docs/docs/converters.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,40 @@ It uses all decorators from `@tsed/schema` package and TypeScript metadata to wo
Ts.ED use this package to transform any input parameters sent by your consumer to a class and transform returned value by your endpoint
to a plain javascript object to your consumer.

::: warning Breaking change
For v5 developer, `@tsed/json-mapper` package is the new API under the @@ConverterService@@. There are some breaking changes between the previous service implementation:
## Configuration

- The `@Converter` decorator have been removed in favor of @@JsonMapper@@ decorator.
- Classes like `ArrayConverter`, `SetConverter`, etc... replaced by his equivalents Types mapper: @@ArrayMapper@@, @@SetMapper@@, etc...
- Type mapper classes are no longer injectable services.
- ConverterService is always available and can be injected to another provider, but now, ConverterService doesn't perform data validation. Validation is performed by [`@tsed/ajv`](/tutorials/ajv.md) package or any other validation library.
- `PropertyDeserialize` and `PropertySerialize` have been removed and replaced by @@OnDeserialize@@ and @@OnSerialize@@.
- Methods Signatures of Type mapper (like ArrayConverter) have changed.
:::
```typescript
@Configuration({
jsonMapper: {
additionalProperties: false,
disableUnsecureConstructor: false,
}
})
```

### jsonMapper.additionalProperties

Enable additional properties on model. By default, `false`. Enable this option is dangerous and may be a potential security issue.

### jsonMapper.disableUnsecureConstructor

Pass the plain object to the model constructor. By default, `false`.

It may be a potential security issue if you have as constructor with this followings code:

```typescript
class MyModel {
constructor(obj: any = {}) {
Object.assign(this, obj); // potential prototype pollution
}
}
```

::: tip Note
Recommended: Set this options to `true` in your new project.

In v7 this option will be set to true by default.
:::

## Usage

Expand Down Expand Up @@ -414,7 +438,7 @@ export class Server {}

[Moment.js](https://momentjs.com) is a powerful library to transform any formatted date string to a Moment instance.

You can change the Date converter behavior to transform string to a Moment instance.
You can change the Date mapper behavior to transform string to a Moment instance.

<Tabs class="-code">
<Tab label="MomentMapper">
Expand Down
5 changes: 2 additions & 3 deletions docs/docs/providers.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ handle HTTP requests and delegate complex tasks to the **providers**.

The providers are plain javascript class and use one of these decorators on top of them. Here the list:

<ApiList query="['Injectable', 'Module', 'Service', 'Controller', 'Interceptor', 'Converter', 'Middleware', 'Filter', 'Protocol'].indexOf(symbolName) > -1" />
<ApiList query="['Injectable', 'Module', 'Service', 'Controller', 'Interceptor', 'JsonMapper', 'Middleware', 'Filter', 'Protocol'].indexOf(symbolName) > -1" />

## Services

Expand Down Expand Up @@ -102,8 +102,7 @@ These decorators can be used with:
- [Service](/docs/services.md),
- [Controller](/docs/controllers.md),
- [Middleware](/docs/middlewares.md),
- [Filter](/docs/filters.md)
- [Converter](/docs/converters.md).
- [Filter](/docs/filters.md).

@@Constant@@ and @@Value@@ accept an expression as parameter to inspect the configuration object and return the value.

Expand Down
13 changes: 9 additions & 4 deletions packages/orm/adapters/src/adapters/LowDbAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {cleanObject} from "@tsed/core";
import isMatch from "lodash/isMatch";
import low from "lowdb";
import {v4 as uuid} from "uuid";
import {deserialize} from "v8";
import {Adapter} from "../domain/Adapter";

export interface AdapterModel {
Expand Down Expand Up @@ -42,10 +41,14 @@ export class LowDbAdapter<T extends AdapterModel> extends Adapter<T> {
let item = await this.findById(id);

if (!item) {
payload = {...payload, _id: id || uuid(), expires_at: expiresAt};
payload = {...payload, _id: id || uuid()};

await this.validate(payload as T);
await this.collection.push(this.serialize(payload) as T).write();

const item = this.serialize(payload);
item.expires_at = expiresAt;

await this.collection.push(item).write();

return this.deserialize(payload);
}
Expand All @@ -67,13 +70,15 @@ export class LowDbAdapter<T extends AdapterModel> extends Adapter<T> {
let item = this.deserialize(this.collection.get(index).value());

item = {
expires_at: expiresAt || item.expires_at,
...item,
...payload,
_id: item._id
};

await this.validate(item as T);

item.expires_at = expiresAt || item.expires_at;

await this.collection.set(index, item).write();

return this.deserialize(item);
Expand Down
1 change: 1 addition & 0 deletions packages/orm/mongoose/src/utils/createSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export function createSchema(target: Type<any>, options: MongooseSchemaOptions =
type: target,
useAlias: false,
additionalProperties: true,
disabledUnsecureConstructor: false,
groups: false
}
);
Expand Down
2 changes: 1 addition & 1 deletion packages/platform/common/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ to make your code more readable and less error-prone.
- Define class as Controller,
- Define class as Service (IoC),
- Define class as Middleware and MiddlewareError,
- Define class as Converter (POJ to Model and Model to POJ),
- Define class as Json Mapper (POJ to Model and Model to POJ),
- Define root path for an entire controller and versioning your Rest API,
- Define as sub-route path for a method,
- Define routes on GET, POST, PUT, DELETE and HEAD verbs,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export interface PlatformJsonMapperSettings {
/**
* JsonMapper additional property policy. (see [JsonMapper](/docs/converters.md))
*/
additionalProperties?: "error" | "accept" | "ignore" | boolean;
/**
* Disable the unsecure constructor injection when the deserialize function is used (by default: false)
*/
disableUnsecureConstructor?: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe("PlatformConfiguration", () => {
settings.routers = {mergeParams: true};
settings.exclude = ["./**/*.spec.ts"];
settings.debug = true;
settings.converter = {};
settings.jsonMapper = {};

settings.setHttpPort({address: "address", port: 8081});
settings.setHttpsPort({address: "address", port: 8080});
Expand Down Expand Up @@ -153,6 +153,10 @@ describe("PlatformConfiguration", () => {
expect(settings.converter).toEqual({});
});

it("should return jsonMapper settings", () => {
expect(settings.jsonMapper).toEqual({});
});

it("should return controllerScope", () => {
expect(settings.controllerScope).toEqual("singleton");
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {Env, getHostInfoFromPort} from "@tsed/core";
import {getHostInfoFromPort, isBoolean} from "@tsed/core";
import {DIConfiguration, Injectable, ProviderScope, TokenProvider} from "@tsed/di";
import {JsonMapperSettings} from "@tsed/json-mapper";
import Https from "https";
import {ConverterSettings} from "../interfaces/ConverterSettings";
import {PlatformJsonMapperSettings} from "../interfaces/PlatformJsonMapperSettings";

const rootDir = process.cwd();

Expand Down Expand Up @@ -73,12 +74,32 @@ export class PlatformConfiguration extends DIConfiguration {
this.setRaw("acceptMimes", value || []);
}

get converter(): Partial<ConverterSettings> {
return this.get("converter") || {};
/**
* @deprecated
*/
get converter(): Partial<PlatformJsonMapperSettings> {
return this.jsonMapper;
}

set converter(options: Partial<ConverterSettings>) {
this.setRaw("converter", options);
/**
* @deprecated
*/
// istanbul ignore next
set converter(options: Partial<PlatformJsonMapperSettings>) {
this.jsonMapper = options;
}

get jsonMapper(): Partial<PlatformJsonMapperSettings> {
return this.get("jsonMapper") || {};
}

set jsonMapper(options: Partial<PlatformJsonMapperSettings>) {
this.setRaw("jsonMapper", options);

JsonMapperSettings.disableUnsecureConstructor = Boolean(options.disableUnsecureConstructor);
JsonMapperSettings.additionalProperties = Boolean(
isBoolean(options.additionalProperties) ? options.additionalProperties : options.additionalProperties === "accept"
);
}

get additionalProperties() {
Expand Down
2 changes: 1 addition & 1 deletion packages/platform/common/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

export * from "./exports";
export * from "./builder/PlatformBuilder";
export * from "./config/interfaces/ConverterSettings";
export * from "./config/interfaces/PlatformJsonMapperSettings";
export * from "./config/interfaces/PlatformLoggerSettings";
export * from "./config/interfaces/PlatformMulterSettings";
export * from "./config/interfaces/PlatformStaticsSettings";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,9 @@ import {JsonParameterStore, PipeMethods} from "@tsed/schema";

@Injectable()
export class DeserializerPipe implements PipeMethods {
@Constant("converter", {})
private settings: {
additionalProperties?: "error" | "accept" | "ignore";
};

transform(value: any, param: JsonParameterStore) {
return deserialize(value, {
useAlias: true,
additionalProperties: this.settings.additionalProperties === "accept",
type: param.type,
collectionType: param.collectionType,
groups: param.parameter.groups,
Expand Down
2 changes: 1 addition & 1 deletion packages/specs/json-mapper/coverage.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"statements": 100,
"branches": 95,
"branches": 94.48,
"functions": 100,
"lines": 100
}
4 changes: 4 additions & 0 deletions packages/specs/json-mapper/src/domain/JsonMapperSettings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const JsonMapperSettings = {
disableUnsecureConstructor: false,
additionalProperties: false
};
1 change: 1 addition & 0 deletions packages/specs/json-mapper/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export * from "./decorators/jsonMapper";
export * from "./decorators/onDeserialize";
export * from "./decorators/onSerialize";
export * from "./domain/JsonMapperContext";
export * from "./domain/JsonMapperSettings";
export * from "./domain/JsonMapperTypesContainer";
export * from "./hooks/alterAfterDeserialize";
export * from "./hooks/alterBeforeDeserialize";
Expand Down
11 changes: 9 additions & 2 deletions packages/specs/json-mapper/src/utils/deserialize.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {isArray, isBoolean, isClass, isEmpty, isNil, MetadataTypes, nameOf, objectKeys, Type} from "@tsed/core";
import {getValue, isArray, isBoolean, isClass, isEmpty, isNil, MetadataTypes, nameOf, objectKeys, Type} from "@tsed/core";
import {alterIgnore, getProperties, JsonEntityStore, JsonHookContext, JsonPropertyStore, JsonSchema} from "@tsed/schema";
import "../components/ArrayMapper";
import "../components/DateMapper";
Expand All @@ -7,6 +7,7 @@ import "../components/PrimitiveMapper";
import "../components/SetMapper";
import "../components/SymbolMapper";
import {JsonMapperContext} from "../domain/JsonMapperContext";
import {JsonMapperSettings} from "../domain/JsonMapperSettings";
import {getJsonMapperTypes} from "../domain/JsonMapperTypesContainer";
import {alterAfterDeserialize} from "../hooks/alterAfterDeserialize";
import {alterBeforeDeserialize} from "../hooks/alterBeforeDeserialize";
Expand All @@ -25,6 +26,10 @@ export interface JsonDeserializerOptions<T = any, C = any> extends MetadataTypes
* Accept additionalProperties or ignore it
*/
additionalProperties?: boolean;
/**
*
*/
disableUnsecureConstructor?: boolean;
/**
* Use the store which have all metadata to deserialize correctly the model. This
* property is useful when you deal with metadata parameters.
Expand Down Expand Up @@ -132,7 +137,7 @@ export function plainObjectToClass<T = any>(src: any, options: JsonDeserializerO
const additionalProperties = getAdditionalProperties(propertiesMap.size, store, options);
src = alterBeforeDeserialize(src, store.schema, options);

const out: any = new type({});
const out: any = new type(options.disableUnsecureConstructor ? {} : src);

propertiesMap.forEach((propStore) => {
const key = options.useAlias
Expand Down Expand Up @@ -199,6 +204,8 @@ function buildOptions(options: JsonDeserializerOptions<any, any>): any {
groups: false,
useAlias: true,
...options,
additionalProperties: getValue(options, "additionalProperties", JsonMapperSettings.additionalProperties),
disableUnsecureConstructor: getValue(options, "disableUnsecureConstructor", JsonMapperSettings.disableUnsecureConstructor),
partial: options.groups ? options.groups.includes("partial") : false,
type: options.type ? options.type : undefined,
types: options.types ? options.types : getJsonMapperTypes()
Expand Down
2 changes: 1 addition & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ It provides a lot of decorators and guideline to make your code more readable an
- Define class as Controller,
- Define class as Service (IoC),
- Define class as Middleware and MiddlewareError,
- Define class as Converter (POJ to Model and Model to POJ),
- Define class as Json Mapper (POJ to Model and Model to POJ),
- Define root path for an entire controller and versioning your Rest API,
- Define as sub-route path for a method,
- Define routes on GET, POST, PUT, DELETE and HEAD verbs,
Expand Down

0 comments on commit 999abb5

Please sign in to comment.