FoalTS / foal

Full-featured Node.js framework, with no complexity. 🚀 Simple and easy to use, TypeScript-based and well-documented.
https://foalts.org/
MIT License
1.89k stars 139 forks source link

Hiding properties from JSON #533

Closed OrkhanAlikhanov closed 3 years ago

OrkhanAlikhanov commented 5 years ago

The problem

@Entity()
export class User extends BaseEntity {
  @PrimaryGeneratedColumn()
  id: number;

  @Column()
  name: string;

  @Column({ unique: true })
  email: string;

  @Column()
  password: string;

  @Column({ nullable: true })
  accessToken?: string;

  // tslint:disable-next-line:variable-name
  private _client?: ThridPartyClient;

  get thridPartyClient(): ThridPartyClient | undefined {
    if (!this._client && this.accessToken) {
      this._client = createThridPartyClient({accessToken: this.accessToken});
    }

    return this._client;
  }
}

Currently, when we return an instance of the above entity (or anything that contains it) from an API endpoint the entire model gets serialized into something like:

{
  "id": 1,
  "name": "Orkhan",
  "email": "myemail@me.com",
  "password": "password-hash",
  "accessToken": "blah213",
  "_client": {...}
}

The last 3 fields must not be inside json. We can of course create a new object and return it but there should be a better solution in general. Does foalts have something to offer in this case?

Workaround

We can override [toJSON](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#toJSON()_behavior) method in the User entity class which is used by JSON.stringify method:

function hidden(hidden: string[]) {
  return function (this: object) {
    const result = {};
    for (const x in this) {
      if (!hidden.includes(x)) {
        result[x] = this[x];
      }
    }
    return result;
  };
}

@Entity()
export class User extends BaseEntity {
 ...
  toJSON = hidden(['password', 'accessToken', '_client']);
}

Where we call hidden method with the array of field names to hide and it will return a method that returns a new copy of object by not copying specified fields. This works for arrays and nested objects too.

Proposed solution

Again I put forward laravel's approach as an example. FoalTS can use a custom replacer to serialize json response, where we can hook in and conditionally hide fields based on some sort of a reserved field on the provided entity (like foal: { hidden: ['password'] }). This would work for arrays and nested objects too.

squareloop1 commented 5 years ago

You can set a TypeORM configuration on the column like this:

@Column({ select: false })
password: string;

But if it is not a TypeORM column you'd have to manually remove it from the response I guess.

OrkhanAlikhanov commented 5 years ago

Hey! Thank you for your suggestion! I already have a working workaround. Also I want the column to be selected from database.

squareloop1 commented 5 years ago

Yes I agree, its a bit unconvenient. You'd have to built a custom query with .addSelect('password') to get the columns with { select: false } from the database.

yosbelms commented 4 years ago

I think the solution from @squareloop1 is great, another one could be, to write custom toJSON method like the following:

toJSON() {
  return {
    id: this.id,
    name: this.name,
    email: this.email,
  }
}

With Lodash:

toJSON() {
  return _.pick(this, ['id', 'name', 'email']);
}

It is more secure, because if you want to include a new field you will do it explicitly, and not accidentally leave it without {select: false} showing the value to the world. As you can see, above very simple solutions, the framework should not be responsible of this, keep it simple.

PS: This topic can be included in the docs. cc @LoicPoullain

OrkhanAlikhanov commented 4 years ago

Hey @yosbelms! I think we both have used toJSON. However, I think a user friendly framework should provide the most out of the box. There might be some cases where we want to conditionally hide some properties, laravel does a great job handling these cases which you can find a link in the description of this issue.

yosbelms commented 4 years ago

I think we both have used toJSON

Sure, but you wrote a custom function with the hidden logic, with O(n) complexity, my toJSON proposal(only show policy), is simple, O(1) complexity.

However, I think a user friendly framework should provide the most out of the box.

Bloated frameworks provides the most out of the box. However, JS has toJSON in the box. So, you have it at language level.

Laravel does that way because it doesn't recommend to override toJSON function because it has its own custom logic, see: https://github.com/illuminate/database/blob/fba945616f6f80cf91ea3bf73c55f43ffe3cc6ba/Eloquent/Model.php#L1099, so it provide tools for those specific cases, but in the case of TypeORM, it is different, it lives in JS land from where JSON comes.

As I said, I think this topic should be addressed in the docs.

squareloop1 commented 4 years ago

There is also the possibility to use the @Exclude() and @Expose() decorators of the class-validator / class-transformer packages.

OrkhanAlikhanov commented 4 years ago

Sure, but you wrote a custom function with the hidden logic, with O(n) complexity, my toJSON proposal(only show policy), is simple, O(1) complexity.

Well, you've only moved it to lodash, complexity still is the same. Also, my above code is just for the sake of showing an example approach. I've come up with your solution before moving it to have more simpler API

LoicPoullain commented 4 years ago

Another approach could also be to use a "post hook":

function pick(obj, propKeys: string[]) {
  const o: any = {}; 
  for (const key of propKeys) {
    o[key] = obj[key];
  }
  return o;
}

function Pick(...propKeys: string[]) {
  return Hook(() => response => {
    if (Array.isArray(response.body)) {
     response.body = response.body.map(user => pick(user, propKeys));
     return;
    }
    response.body = pick(response.body, propKeys);
  });
}

@Pick('id', 'name', 'email')
export class UserController {
  @Get('/users')
  readUser() { ... }

  @Get('/users/me')
  readMe() { ... }

  @Post('/users')
  createUser () { ... }
}

What do you think?

Edo78 commented 4 years ago

I like the 'post hook' approach. It looks elegant and it's easy to create two different methods to whitelist and blacklist properties and apply each one to specific classes.

sinclairnick commented 4 years ago

Could this not be solved by using the @Exclude hook from the class-transformer repo and call the classToPlain method on the instance to convert to JSON?

This is what I've been using and it works fine.

yosbelms commented 4 years ago

This can be solved at ORM level with single table inheritance. Split User in two logic entities but both will live in the same table:

@Entity()
@TableInheritance({ column: { type: "varchar", name: "type" } })
export class User extends BaseEntity {
  @PrimaryGeneratedColumn()
  id: number;

  @Column()
  name: string;

  @Column({ unique: true })
  email: string;
}

@Entity()
export class SecuredUser extends User {
    @Column()
    password: string;

    @Column({ nullable: true })
    accessToken?: string;

    // tslint:disable-next-line:variable-name
    private _client?: ThridPartyClient;

    get thridPartyClient(): ThridPartyClient | undefined {
      if (!this._client && this.accessToken) {
        this._client = createThridPartyClient({accessToken: this.accessToken});
      }

      return this._client;
    }
}

Now you can query SecuredUser for security tasks (authentication, authorization) and User for the rest of the logic, manipulate, and serialize to JSON without worrying about to disclose security properties by mistake.

Edo78 commented 4 years ago

I think @yosbelms approach is the better one so far. It's a bit overkill for a single field but I often have more than a single field to hide from the client.

LoicPoullain commented 3 years ago

It looks like the issue has been answered. Feel free to re-open if you still need help.