rajska
rajska copied to clipboard
Proposal: add function to reset a rate limit set by the Rajska.RateLimiter
Sometimes is useful to have a way to reset the RateLimiter, e.g. rate limiting the login and be able to clear the rate limit when user resets his password.
For this we can add a function reset(identifier)
to the Rajska.RateLimiter
module that calls the https://hexdocs.pm/hammer/Hammer.html#delete_buckets/2.
Besides this low level function, we could also have a middleware for this.
Also, another suggestion is to improve the RateLimiter identifier config. Today we have 3 ways of specifying a identifier:
- specifying an
id
option - specifying a
keys
option - not specifying anything and fallbacks to the IP
It would be nice, given the idea to have a middleware for limiting and reseting, do something like this:
field :login, :session do
...
middleware Rajska.RateLimiter, namespace: :login, limit_by: :ip
resolve &AccountsResolver.login/2
end
field :reset_password, :session do
...
resolve &AccountsResolver.reset_password/2
middleware Rajska.RateLimiter.Reset, namespace: :login, limit_by: :ip
end
With this, we are identifying by both an namespace and user IP. I see this useful for readability: we use a namespace to better uderstand that we are limiting in one mutation and reseting in another. Without this, it's still possible to implement, but it's harder to see why that reset is there:
field :login, :session do
...
middleware Rajska.RateLimiter
resolve &AccountsResolver.login/2
end
field :reset_password, :session do
...
resolve &AccountsResolver.reset_password/2
middleware Rajska.RateLimiter.Reset
end
The limit_by
could be:
-
:ip
-> uses user IP -
{:keys, list}
-> same waykeys
option today works -
{:id, any_fixed_id}
-> same wayid
option works
The Hammer id built with this, would be something like "query:#{namespace}:#{limit_by}"
What do you guys think?
Very nice proposal! I would break this into 2 tasks, though.
One to implement the Reset middleware without any breaking changes in the API, and later this general refactoring, which we could wait to bundle with other breaking changes before releasing a new major version.
Another option would be to have the namespace as the query/mutation name by default. That way, we could already implement the concept of namespaces without a breaking change
Yeah, I thought about making 2 PR's for this too. But I also was thinking implementing this new options without breaking changes. The approach would be supporting the old options (informing that they are deprecated) and the new options.
For the namespace I was thinking that if it's not informed, the identification would be something like "query:#{limit_by}"
(skipping the namespace concatenation). But your idea of using the name of the query/mutation as default is nice :)