Hi @zguig52
Thanks for this
Do you think it would be possible to add basic tests to test/rateLimiter.test.ts to ensure that the module behaves as expected for some basic configuration values? (e.g. undefined, single ip, multiple ips, empty array, etc.)

Hi @vejja ,
I previously added a manual tests page, sorry, I hadn't seen the autotests folder.
I have added 3 pages + associated conf and tests. They all pass when I do yarn test, but I don't know how to get tests results details...
I would have liked to check timing impacts of the whitelist array presence regarding to no whitelist at all.

NB: for the undefined value test, it is already performed with existing tests as it is the default value configured

Hi @zguig52
You can test the pre-release package from the Stackblitz Continuous Release artefact here:
npm i https://pkg.pr.new/nuxt-modules/security/nuxt-security@524b003
I don't think the timing impacts will be measurable but let us know !

NB: The bot was unable to inline the PR preview because your branch is outside of this repo but I triggered it manually.

@zguig52 : Before we release this, I'd like to get @Baroshem's advice here because I'm not extremely knowledgeable about the Rate Limiter.

Do we agree that we are allowing ip whitelists here, as opposed to domain names ?
In other words, this is unlikely to be used in production, where the requesting IP is in general unknown.
If this assumption is correct, the main use case here is in development mode when we want to whitelist localhost by IP (e.g. 127.0.0.1 usually) ?

Let me know if I'm correct

@vejja , this is an IPs CLIENT whitelist that is managed here.

Main usages are for production:

• internal other components calls (where you could whitelist your internal network IPs)
• performance / load tests (where you could whitelist your traffic injectors IPs)

If you look in the function associated to the rateLimiter, it uses client IP to determine and store all calls performed to the endpoint by a specific client and determine if rates are normal:

function getIP (event: H3Event) {
  const ip = getRequestIP(event, { xForwardedFor: true }) || ''
  return ip
}

This is a h3 function that provide client IP https://github.com/unjs/h3/blob/77510f3c27e6884489189b300c52b5355751aefc/src/utils/request.ts#L297

So I reused this information to compare it to the whitelist and skip rateLimit checks and storage if it matches.

npm i https://pkg.pr.new/nuxt-modules/security/nuxt-security@524b003

I am deploying it in a qualification environment to check if it behaves as expected with public clients IPs

@vejja , this is an IPs CLIENT whitelist that is managed here.

Main usages are for production:

• internal other components calls (where you could whitelist your internal network IPs)
• performance / load tests (where you could whitelist your traffic injectors IPs)

If you look in the function associated to the rateLimiter, it uses client IP to determine and store all calls performed to the endpoint by a specific client and determine if rates are normal:

function getIP (event: H3Event) {
  const ip = getRequestIP(event, { xForwardedFor: true }) || ''
  return ip
}

This is a h3 function that provide client IP https://github.com/unjs/h3/blob/77510f3c27e6884489189b300c52b5355751aefc/src/utils/request.ts#L297

So I reused this information to compare it to the whitelist and skip rateLimit checks and storage if it matches.

Oh oh I think I missed your whole point effectively !
I initially thought that you wanted to whitelist XHR requests to the API made by some IP-whitelisted frontend client.

But if I understand correctly, the use cases are significantly larger than what I thought

• For people who deploy with SSR: whenever the SSR server prefetches data from an internal API route, avoid hitting rate limits. Assuming the SSR server is on a known internal IP, then we can whitelist it.
• For larger systems where third-party tools (e.g. testing suites) are used to hit the internal API routes, avoid hitting rate limits. Assuming the 3rd-party suite is a known IP, then we can whitelist.

Am I right here ?

@vejja , that's it! As I started to comment on initial discussion associated, the best would be to be allowed defining networks and not only specific IPs, but it is a bit more complicated to implement and require more thinking!
#517

npm i https://pkg.pr.new/nuxt-modules/security/nuxt-security@524b003

Just did some tests with this release, it works as expected (at least for a single public IP configured, which is for the moment my tests conditions). I used siege tool to overflow server API routes and it doesn't trigger the rateLimit.

When I am behind the whitelisted public IP, no 429. When I use another public IP, it trigger 429!

@vejja , that's it! As I started to comment on initial discussion associated, the best would be to be allowed defining networks and not only specific IPs, but it is a bit more complicated to implement and require more thinking! #517

ok clear saw #517 also - would implementation break the type definition of whitelist ?

ok clear
saw #517 also - would implementation break the type definition of whitelist ?

@Baroshem : whenever you think this is ready - I believe this is a feature upgrade, shall we bump to minor v2.2 ?

saw #517 also - would implementation break the type definition of whitelist ?

I have reviewed a bit my comments on this, as I think we might be able to add more capabilities with the same configuration format, without breaking this first implementation (or maybe we already change before release for basic input config format to be able to change it without breaking changes later).