DennisSnijder / nestjs-bull-board

NestJS module for bull-board
https://www.npmjs.com/package/nestjs-bull-board
1 stars 0 forks source link

Apply middleware using Fastify #1

Open simplenotezy opened 1 year ago

simplenotezy commented 1 year ago

The middleware does not seem to work, not sure why. I tried applying this middleware:

/* eslint-disable @typescript-eslint/no-floating-promises */
import { Injectable, NestMiddleware } from '@nestjs/common';
import { FastifyReply, FastifyRequest } from 'fastify';

@Injectable()
export class BasicAuthMiddleware implements NestMiddleware {
  use(req: FastifyRequest, res: FastifyReply, next: () => void) {
    const authHeader = req.headers.authorization;
    if (!authHeader || !authHeader.startsWith('Basic ')) {
      res.statusCode = 401;
      res.header('WWW-Authenticate', 'Basic realm="Restricted"');
      res.send('Unauthorized');
      return;
    }

    const credentials = Buffer.from(authHeader.split(' ')[1], 'base64').toString().split(':');
    const username = credentials[0];
    const password = credentials[1];

    // Replace this with your own authentication logic
    const isValid = username === 'admin' && password === 'password';
    if (!isValid) {
      res.statusCode = 401;
      res.send('Unauthorized');
      return;
    }

    next();
  }
}

Using:

BullBoardModule.forRoot({
  route: '/queues',
  middleware: [BasicAuthMiddleware],
  adapter: FastifyAdapter,
}),
simplenotezy commented 1 year ago

This worked:

BullBoardModule.forRoot({
-  route: '/queues',
+  route: '/queues/',
  middleware: [BasicAuthMiddleware],
  adapter: FastifyAdapter,
}),
simplenotezy commented 1 year ago

Oh, while that will have the middleware register, it will cause a 404. Also, if you access /queues/ you essentially bypass the middleware. If you access /queues you get the middleware, but you'll get the 404

simplenotezy commented 1 year ago

This is a nasty workaround:


    consumer.apply(BasicAuthMiddleware).forRoutes(
      {
        path: 'queues(.*)',
        method: RequestMethod.ALL,
      },
      {
        path: 'queues/',
        method: RequestMethod.ALL,
      },
      {
        path: 'queues',
        method: RequestMethod.ALL,
      },
      {
        path: '/queues/',
        method: RequestMethod.ALL,
      },
      {
        path: '(.*)queues(.*)',
        method: RequestMethod.ALL,
      },
    );
DennisSnijder commented 1 year ago

Hi @simplenotezy, thanks for the report and the research/potential fix you already pointed out! I'll look into this tomorrow morning!

I already tested out some forRoutes using Fastify, and gotta admit, it's kinda weird on how it works / why it's so inconsistent when compared to the Express middleware.

simplenotezy commented 1 year ago

I already tested out some forRoutes using Fastify, and gotta admit, it's kinda weird on how it works / why it's so inconsistent when compared to the Express middleware.

Totally agree, it was rather frustrating trying to do something so simple, yet find it so hard. Not even ChatGPT with 100 trillion parameters managed to solve it.

DennisSnijder commented 1 year ago

@simplenotezy

Okay, I'm trying to debug why the route isn't matching, but oddly enough some of the routes you provide require some authentication. Soooo.... what I found is the following.

When i log req.url in the middleware, I get /?activeQueue=&page=1&jobsPerPage=10 when visiting /queues. which is odd. Notice the / url instead of /queues, seems like it's completely ignoring the prefix.

Might also explain why I'm able to to enter the dashboard on /queues but the API calls using the term queue are failing since they match one of the described routes applied for the middleware.

I posted a question on the NestJS Discord regarding this. Perhaps it's a bug on NestJS side / unclear instructions on how to deal with Fastify and using middlewares.

I'll keep you updated!

Edit: seems like this is an issue for NestJS.... https://github.com/nestjs/nest/issues/11585

simplenotezy commented 1 year ago

Nice! Thanks for the update. Fingers crossed

simplenotezy commented 1 year ago

Something perhaps unrelated, but I couldn't rename the path to anything but "queues"

DennisSnijder commented 1 year ago

Something perhaps unrelated, but I couldn't rename the path to anything but "queues"

Hmm that's odd, I can't seem to replicate that issue. Do you have an example of your setup? Or a url you're trying to use?

DennisSnijder commented 1 year ago

@simplenotezy https://github.com/nestjs/nest/pull/11718

Fix for the issue described above should be on it's way! πŸ˜„

simplenotezy commented 1 year ago

@simplenotezy https://github.com/nestjs/nest/pull/11718

Fix for the issue described above should be on it's way! πŸ˜„

Nice! Looking forward

DennisSnijder commented 1 year ago

@simplenotezy it has been merged! when you update @nestjs/platform-fastify to 9.4.3 it works perfectly fine!

Since you're using (or still implementing) this module/library, I want to give you a quick heads up: The creator of bull-board asked me to create a PR to integrate this library into the bull-board monorepo and create a new package which will most likely be called @bull-board/nestjs πŸ₯³. I'll ping you on this issue and update this repository when it's available πŸ˜„

simplenotezy commented 1 year ago

@simplenotezy it has been merged! when you update @nestjs/platform-fastify to 9.4.3 it works perfectly fine!

Since you're using (or still implementing) this module/library, I want to give you a quick heads up: The creator of bull-board asked me to create a PR to integrate this library into the bull-board monorepo and create a new package which will most likely be called @bull-board/nestjs πŸ₯³. I'll ping you on this issue and update this repository when it's available πŸ˜„

Hmm - I upgraded @nestjs/platform-fastify to 9.4.3 but the same problem appears. The middleware is not applied.

p.s. Sounds great with a dedicated @bull-board/nestjs package!

DennisSnijder commented 1 year ago

@simplenotezy it has been merged! when you update @nestjs/platform-fastify to 9.4.3 it works perfectly fine! Since you're using (or still implementing) this module/library, I want to give you a quick heads up: The creator of bull-board asked me to create a PR to integrate this library into the bull-board monorepo and create a new package which will most likely be called @bull-board/nestjs πŸ₯³. I'll ping you on this issue and update this repository when it's available πŸ˜„

Hmm - I upgraded @nestjs/platform-fastify to 9.4.3 but the same problem appears. The middleware is not applied.

p.s. Sounds great with a dedicated @bull-board/nestjs package!

If that doesn't fix the problem lemme reopen this issue, I'll take a look into it. About the route, are you using any router other than /queue ?

DennisSnijder commented 1 year ago

ugh, @simplenotezy sorry for the false hope, seems like the merged PR didn't quite make it into 9.4.3. I think I tested the middleware on a development build where I had a wildcard enabled.... πŸ˜“. Let's wait for NestJS to release a new version.

simplenotezy commented 1 year ago

Ah okay, thanks for the update! 😊

DennisSnijder commented 1 year ago

@simplenotezy the new @bull-board/nestjs package has been released and published! πŸ₯³

Github: https://github.com/felixmosh/bull-board/tree/master/packages/nestjs NPM: https://www.npmjs.com/package/@bull-board/nestjs

I'll open this issue on the bull-board github and reference this issue in there.