sourcefuse / loopback4-ratelimiter

A Rate Limiting Extension for Loopback 4 Applications
https://www.npmjs.com/package/loopback4-ratelimiter
MIT License
37 stars 10 forks source link

ratelimit decorator is not working #19

Closed ruturajmore closed 2 years ago

ruturajmore commented 3 years ago

Hi there, ratelimit decorator is not working.

controller.ts

  @ratelimit(true, {
      max: 4
  })
  @post('/add_user', {
    responses: {
      '200': {
        description: 'Add User',
        content: {
          'application/json': {
            schema: {
              type: 'object',
              properties: {
                is_success: {
                  type: 'boolean',
                },
              },
            },
          },
        },
      },
    },
  })
  async addUser(): Promise<{is_success: boolean}> {
      return {is_success: true}
  }

application.ts

    this.component(RateLimiterComponent);
    this.bind(RateLimitSecurityBindings.CONFIG).to({
      name: 'redis',
      max: 10,
      windowMs: 900000, // #9 
      message: 'Too many requests, please try again later.'
    });

package.json

  "engines": {
      "node": ">=10.16"
  },
  "dependencies": {
      "@loopback/authentication": "^7.0.1",
      "@loopback/authentication-jwt": "^0.7.1",
      "@loopback/authorization": "0.7.1",
      "@loopback/boot": "^3.0.1",
      "@loopback/core": "^2.10.1",
      "@loopback/cron": "^0.4.1",
      "@loopback/repository": "^3.0.1",
      "@loopback/rest": "^7.0.1",
      "@loopback/rest-explorer": "^3.0.1",
      "@loopback/service-proxy": "^3.0.1",
      "loopback-connector-kv-redis": "3.0.0",
      "loopback4-ratelimiter": "^2.2.0",
   }
   "devDependencies": {
      "@loopback/build": "^6.2.4",
      "@types/node": "^10.17.35",
      "typescript": "~4.0.2"
    }

@samarpan-b can you please check? Thanks in advance!

samarpan-b commented 3 years ago

Let me check this @ruturajmore

samarpan-b commented 3 years ago

So basically you want rate limiter only for one api and disabled for all others ? @ruturajmore

ruturajmore commented 3 years ago

Yes, the intention behind using a decorator is to enable rate-limiting for some required APIs. But it is not working as expected. Requests are not getting blocked even after max attempts. There no error in logs. @samarpan-b

samarpan-b commented 3 years ago

Thats because you have removed it from sequence. Decorator is not meant to do the rate limiting. Decorator is meant to provide it the config. The actual rate-limiting is performed by sequence action during the request capture in sequence. That's what decorator pattern means actually. Can you enable it in sequence and try again ?

samarpan-b commented 3 years ago

This component also exposes a method decorator for cases where you want tp specify different rate limiting options at API method level. For example, you want to keep hard rate limit for unauthorized API requests and want to keep it softer for other API requests. In this case, the global config will be overwritten by the method decoration. Refer below.

Check it out in docs. Also for disabling rate limiting from apis you can just do @ratelimit(false)

ruturajmore commented 3 years ago

Yes, I checked it's working. One last thing, default windowMs is not getting overridden even through the decorator. Thanks for your support @samarpan-b !!!

samarpan-b commented 3 years ago

We'll check that tomorrow @ruturajmore when @akshatdubeysf is back. Will that be fine ?

ruturajmore commented 3 years ago

Yes, it would be great. Thanks! @samarpan-b

ruturajmore commented 3 years ago

Observed one more weird thing when findRoute() and parseParams() are called after rateLimitAction(), decorator's configs are ignored and global configs are used.

sequence.ts

    await this.rateLimitAction(request, response);

    const route = this.findRoute(request);
    const args = await this.parseParams(request, route);

Can you please check? @samarpan-b @akshatdubeysf

samarpan-b commented 3 years ago

@ruturajmore thats how loopback sequence works. The decorator config is only read when you parsed the params. If you try to do it before that, it will not work. So better to invoke it afterwards only. Parse params and find route is responsible for identifying the controller method for the API. Hence it is needed before.

akshatdubeysf commented 2 years ago

Fixed through a PR hence closing this