Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rate limiter driver options not being properly passed #433

Open
numselli opened this issue Apr 24, 2024 · 3 comments · Fixed by #452 · May be fixed by #448
Open

Rate limiter driver options not being properly passed #433

numselli opened this issue Apr 24, 2024 · 3 comments · Fixed by #452 · May be fixed by #448
Assignees
Labels
bug Something isn't working
Milestone

Comments

@numselli
Copy link

numselli commented Apr 24, 2024

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"
        }
      }
    }
  },
  1. run npm run dev
  2. 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.

@numselli numselli added the bug Something isn't working label Apr 24, 2024
@Baroshem
Copy link
Owner

Baroshem commented Apr 25, 2024

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
Copy link
Collaborator

vejja commented Apr 26, 2024

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
Copy link
Owner

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?

@Baroshem Baroshem added this to the 2.0.0 milestone May 6, 2024
@vejja vejja self-assigned this May 13, 2024
@vejja vejja linked a pull request May 13, 2024 that will close this issue
6 tasks
@Baroshem Baroshem linked a pull request May 22, 2024 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants