vh13294 / nestjs-url-generator

Dynamically generating & signing URL based on controller method reference.
MIT License
44 stars 16 forks source link

Needs ability to modify protocol check #7

Closed danimbrogno closed 2 years ago

danimbrogno commented 2 years ago

Great module but I ran into an issue when implementing it.

My app is deployed to Azure App Service, which terminates the SSL connection before it hits the application.

So even though the original request is made over https, request.protocol = 'http' by the time the request hits the app.

Therefore isSignatureValid method determines that the request is invalid. Azure App Service forwards the orginal protocol in the header 'x-forwarded-proto', so I hacked around this with this re-implementation of 'SignedUrlGuard'.

But a cleaner approach would be an ability to customize how the request is read. Would you accept a PR around this?

import { Injectable, CanActivate, ExecutionContext, MethodNotAllowedException } from '@nestjs/common';
import { RequestWithSignature, UrlGeneratorService } from 'nestjs-url-generator';
import { Observable } from 'rxjs';

@Injectable()
export class DebugSignedUrlGuard implements CanActivate {
  constructor(private readonly urlGeneratorService: UrlGeneratorService) {}

  canActivate(context: ExecutionContext): boolean | Promise<boolean> | Observable<boolean> {
    const request = context.switchToHttp().getRequest();

    return this.validateRequest(request);
  }

  private validateRequest(request: RequestWithSignature): boolean {
    if (!request.headers.host) {
      throw new MethodNotAllowedException('Unable to derive host name from request');
    }

    if (!request.path) {
      throw new MethodNotAllowedException('Unable to derive path from request');
    }

    if (!request.query) {
      throw new MethodNotAllowedException('Signed Query is invalid');
    }

    const protocol: string = request.headers['x-forwarded-proto']
      ? (request.headers['x-forwarded-proto'] as string)
      : request.protocol;

    console.log('checking params', {
      protocol,
      host: request.headers.host,
      routePath: request.path,
      query: request.query,
    });

    return this.urlGeneratorService.isSignatureValid({
      protocol,
      host: request.headers.host,
      routePath: request.path,
      query: request.query,
    });
  }
}
vh13294 commented 2 years ago

Have you tried to enable proxy?

This option will try to forward reverse proxy option to the app.

app.enable('trust proxy');
danimbrogno commented 2 years ago

Thanks for the tip, I'll look into that now.

danimbrogno commented 2 years ago

Hm, no in my situation that doesn't seem to help.

Even with the following initialization code, request.protocol is still "http".

import { Logger } from '@nestjs/common';
import { NestFactory } from '@nestjs/core';
import { NestExpressApplication } from '@nestjs/platform-express';
import { AppModule } from './app/app.module';
import { config } from './config';

async function bootstrap() {
  const app = await NestFactory.create<NestExpressApplication>(AppModule.forRoot(config));
  app.set('trust proxy');
  const port = process.env.PORT || 3333;

  await app.listen(port, () => {
    Logger.log('Application listening on: ' + port + '/');
  });
}

bootstrap();
vh13294 commented 2 years ago

This is the document related to that issue.

https://docs.microsoft.com/en-us/azure/app-service/configure-language-nodejs?pivots=platform-linux#detect-https-session

app.set('trust proxy', 1)
...
if (req.secure) {
  // Do something when HTTPS is used
}

We could use both request.protocol together with requset.secure to check for HTTPS, I think

lowbits commented 1 year ago

@vh13294 ran into the same issue today, with render.com. So what is the solution? @danimbrogno how did you solved it?

danimbrogno commented 1 year ago

@vh13294 ran into the same issue today, with render.com. So what is the solution? @danimbrogno how did you solved it?

Instead of using SignedUrlGuard exported by this module I use the code I posted in the first message: https://github.com/vh13294/nestjs-url-generator/issues/7#issue-1013684402