136/sanitize returned api data

[esteban-gs]

Dev Summary

Changes:

• add class-transform serialization interceptor. This will use the class-transformer annotations to explicitly expose what we want to expose on data going out.
• sanitize return user-dto vals. Apply the transform interceptor to the account controller register endpoint.

Test Plan

repro steps:

See the the test that verifies the password isn't returned:

web-app/server/test/account-manager/account-manager.controller.e2e-spec.ts

Line 87 in 50b1240

expect(body.password).toBeUndefined();

Resources

• https://github.com/typestack/class-transformer

[marvinsjsu]

As mentioned in a previous comment, I've verified that the password data is not in the response and sanitizing looks to be working. I just have a few questions, but this LGTM.

[marvinsjsu]

Do you think it's worth to add a migration to update field names to follow a naming convention with camel-case instead of snake-case, or vice-versa? I think my preference is camelCase as it aligns with the front-end, but I'm not sure what is more common with Node/Nest.js?

[esteban-gs]

That's a good point. This was done, I believe to make table column names conform to the Postgres standard, but there should be a way to tell Typeorm to use camel case in JS.

[marvinsjsu]

Is an interceptor similar to a middleware? (I should go to the link and read up on class-transformer :D)

[marvinsjsu]

In class-transformer documentation, it looks like we may also need reflect-metadata and don't see it as a dependency.

[esteban-gs]

It's satisfied by the nestjs/common peer dependencies, which uses it as well.

[esteban-gs]

Is an interceptor similar to a middleware?

In NestJs an interceptor is middle-ware. In this specific case, we're chaining a class-transformer operation on the outgoing handler response. It's applied to any handler (controller method) using the UseInterceptors(new SerializeInterceptor(dto)) decorator, but in our case, I just wrapped it in a shorter decorator MapTo.

[marvinsjsu]

I've verified locally that password is not included in the returned payload even with this line removed. I'm not 100% clear on how this is happening, so describing here what I think is going on. In UsersService.create method, we've replaced ReturnUserDto as the output type with User, but in the controller, we use annotation @MapTo(ReturnUserDto), which I'm assuming is doing the sanitizing. Since password is not one of the properties of ReturnUserDto, what does the @Expose() annotation do and why do we use it?

[esteban-gs]

Yes, in a way. changing the return type actually has no bearing on the returned data (thanks to Javascript). Thankfully, class-transformer is very good at this. The class-transformer interceptor is getting set to

 return plainToInstance(this.dto, data, {
          excludeExtraneousValues: true,
          strategy: 'excludeAll',
        });

The excludeAll strategy means that any ReturnDto property not decorated with Expose will be removed from the response.

[esteban-gs]

I went with excludeAll so that we have to explicitly list all the data we want to expose, but we can change it if it becomes to cumbersome.