Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

RFC: Issues with multiple Attack instances in the same process / Rack stack (class ivar binding) #178

Closed
julik opened this issue May 14, 2016 · 4 comments · May be fixed by #442
Closed

Comments

@julik
Copy link

julik commented May 14, 2016

I would like to use multiple instances of Rack::Attack in the same Rack stack, but in different places. Each one of these would accept it's own throttles, blacklists etc. Judging from the code, I assumed I could do

 class MyThrottle < Rack::Attack
    throttle(...) do |req|
      ...
    end
 end

It turned out, however, that the method performing the throttling still reaches into the attribute reader of the superclass, and thus finds throttles defined on the parent instance. I think the same is the case for blacklisting and whitelisting, but I don't necessarily need them in this specific application. I was only able to make throttles work in a subclass by doing

 class MyThrottle < Rack::Attack
    ...
    def throttled?(req)
      self.class.throttles.any? do |name, throttle|
        throttle[req]
      end
    end
 end

This stems from the fact that some smart machinery is involved in making the configuration available on the class level. I think that it might make more sense if the same configuration could be applied with, say, options (since block handling with middleware attachment is not that well understood):

 my_attack_config = Rack::Attack::Config.new do |c|
    c.throttle(...) do |req| # etc
      ...
    end
 end
 use Rack::Attack, config: my_attack_config

So I think there was some thought given to separating the various classes' configurations within one Ruby process, but this was not really tested. You could in theory alter the throttled? etc. methods to call into the right class for the variables and consider this a bug, but maybe the API can just be improved and handle this explicitly?

@ktheory
Copy link
Collaborator

ktheory commented Jun 22, 2016

I would like to use multiple instances of Rack::Attack in the same Rack stack, but in different places

Interesting idea, though I'm trying to understand the motivation. Can you explain your use-case a bit more?

@julik
Copy link
Author

julik commented Jun 22, 2016

We are using multiple Rack apps within our Rails monolith. Within some of these apps we would like to throttle based on some context that does not get set up globally, but gets set up only within that particular app (mostly API endpoints). This means that we need to have lots if ifs if we want to apply these throttles globally - to check for all the maybe-present custom rack environment keys.

@grzuy
Copy link
Collaborator

grzuy commented Mar 8, 2018

It sounds like something worth trying.

@jellybob
Copy link

jellybob commented Jul 5, 2021

We're seeing similar issues, in our case there are some throttles we can apply without any user details. We then hit an internal service to get additional information about the connecting user, then apply any rate limits which require things like the list of user roles applied to their account.

We've worked around it with a middleware inserted after the first phase which deletes the key rack.attack.called from the env hash before moving on, and a monkey patch to Rack::Attack which allows passing a pre-populated configuration object to the initializer. This is an unpleasant pile of hacks I'm convinced are going to break at any moment though so it would be nice to have something better supported.

@rack rack locked and limited conversation to collaborators Jan 29, 2022
@grzuy grzuy converted this issue into discussion #569 Jan 29, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants