Baroshem / nuxt-security

🛡 Automatically configure your app to follow OWASP security patterns and principles by using HTTP Headers and Middleware
https://nuxt-security.vercel.app/
MIT License
737 stars 56 forks source link

Rate limiter driver options not being properly passed #433

Closed numselli closed 3 weeks ago

numselli commented 2 months ago

Version

nuxt-security: V1.3.2 nuxt: v3.11.2

Steps to reproduce

  1. Install nuxt-security npx nuxi@latest module add security

  2. add configuration

    security: {
    rateLimiter: {
      driver: {
        name: "redis",
        options: {
          url: "redis://192.168.0.23:6379"
        }
      }
    }
    },
  3. run npm run dev

  4. see error trying to connect to 127.0.0.1:6379 instead of the url provided in config.

What is Expected?

Redis attempts to connect to url provided in config

What is actually happening?

Redis attempts to connect to the default url of 127.0.0.1:6379 because the storage options are not properly passed to unstorage's redis driver. By adding console.log(opts) to node_modules/unstorage/drivers/redis.mjs it shows that the redis driver is reciving

{
  "driver": "redis", 
  "options": { 
    "url": "redis://192.168.0.23:6379"
  }
}

when it is expecting a structure like

{
  "driver": "redis", 
   "url": "redis://192.168.0.23:6379"
}

I do not think this is an issue with unstorage since nuxt-session uses unstorage in a similar way and does not have this issue.

Baroshem commented 2 months ago

Hey, I think that redis then can have a bit different interface for unstorage.

How it works currently is that with rate limiter you select a driver (memory, lru, redis, etc) and you pass options as a second parameter. These options are then passed to the unstorage driver.

So the thing is that we may need to add a condition there so that if the driver is redis, we need to spread the options instead of passing them as options

https://github.com/Baroshem/nuxt-security/blob/main/src/module.ts#L270C15-L270C22

vejja commented 2 months ago

I'm not too sure, but shall we always spread options ? Looks like the unstorage drivers always need this. lruCache looks like the only one for which options has a property name that is options - or am I mistaken here ?

Baroshem commented 1 month ago

I think as well but maybe @pi0 would be able to help here?

Pooya, what would be your recommended approach to passing options to underlying unstorage in such case?