algoan / nestjs-components

A list of useful components for NestJS applications
https://www.algoan.com
254 stars 40 forks source link

[LoggingInterceptor] Masking `authorization` header #818

Closed amarlankri closed 11 months ago

amarlankri commented 11 months ago

Description

Currently, all the headers of the request are logged: https://github.com/algoan/nestjs-components/blob/c0b9a71f332d35d0a2e7f7fcc588d322bae6a95c/packages/logging-interceptor/src/logging.interceptor.ts#L72-L80

However, those headers may contain sensitive data. Especially, the authorization header may contain a JWT which can encode sensitive data, readable by anyone once decoded. So, the logging interceptor should provide a way to mask request headers.

Solution proposed

Adding an option to the logging interceptor for masking a given header. This option would have the following interface:

interface Mask {
  requestHeader?: RequestHeaderMask;
}

interface RequestHeaderMask {
  [headerKey: string]: boolean | (headerValue: string) => unknown;
}

Each header could either be completely hidden if true or have a masking method which would allow to return any kind of value. If the header value is not defined, then the method is not called.

For instance, it could be used as follows for the authorization header:

new LoggingInterceptor({
  mask: {
     requestHeader: {
       authorization: (header: string | string[]) => {
         const safeHeader = ... // remove sensitive data from the header
         return safeHeader; // the result could be an object containing non sensitive data useful for debugging
       }
    }
  }
}),

FAQs

- Why not providing a default handling for the authorization header? The format of the header authorization depends on the app. Some applications implements OAuth 2.0 standard but it is not mandatory. By the way, it is a framework so it means that it can be implemented differently from an app to another.

- Why not just masking the whole header? The header may contain sensitive and non sensitive data (e.g. a JWT). The non sensitive data may be useful in the logs for debugging. Having a method for masking header value allows to handle each header specifically.

- Why the header masking should be provided to the logging interceptor (i.e. at the app level) rather than the log decorator (I.e. at the endpoint level)? Usually, headers are consistent across an application. For instance, if an endpoint requires a request header authorization with the value Bearer <JWT>, all endpoints requiring an authentication in the app will be defined the same way. Having the option at the endpoint level would imply passing the option for a lot of endpoints which would be really tedious. By the way, we could provide another option to override the header mask for a specific endpoint. This could be a solution if the feature is requested in the future.