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

Configure middleware position in Rails #491

Closed
wants to merge 2 commits into from
Closed

Conversation

pfac
Copy link

@pfac pfac commented Jul 1, 2020

Why:

  • The position of a middleware in the Rack attack has consequences. For
    instance, any instrumentation middleware like Skylight will measure
    and capture what happens in the middlewares below it, but not on those
    above.

    The current implementation forces the rack-attack middleware to always
    be appended to the list when initializing Rails, making it the last on
    the list when added, but often others will be added afterwards.

    While this behaviour is perfectly suitable by default, there are
    situations where one might need to control where the middleware is on
    the list. Given the lack of configuration points to tweak this
    behaviour, currently the only way to achieve this is to add the
    middleware again at the top. This results in a duplication of the
    Rack::Attack middleware in the stack.

This change addresses the need by:

  • Shamelessly copying the approach followed by skylight-ruby, where
    the Railtie initializes a configuration object that can be set up in
    config/application.rb, thus allowing the position of the middleware
    to be tweaked.

Why:

* The position of a middleware in the Rack attack has consequences. For
  instance, any instrumentation middleware like [Skylight] will measure
  and capture what happens in the middlewares below it, but not on those
  above.

  The current implementation forces the rack-attack middleware to always
  be appended to the list when initializing Rails, making it the last on
  the list when added, but often others will be added afterwards.

  While this behaviour is perfectly suitable by default, there are
  situations where one might need to control where the middleware is on
  the list. Given the lack of configuration points to tweak this
  behaviour, currently the only way to achieve this is to add the
  middleware again at the top. This results in a duplication of the
  `Rack::Attack` middleware in the stack.

This change addresses the need by:

* Shamelessly copying the approach followed by [skylight-ruby], where
  the Railtie initializes a configuration object that can be set up in
  `config/application.rb`, thus allowing the position of the middleware
  to be tweaked.

[Skylight]: https://skylight.io
[skylight-ruby]: https://github.com/skylightio/skylight-ruby
@pfac
Copy link
Author

pfac commented Jul 1, 2020

I guess this might be useful to address #459 too.

Why:

* The build is failing due to the use of `ActiveSupport::OrderedOptions`
  in some versions.

This change addresses the issue by:

* Adding `middleware_position` to the common
  `Rack::Attack.configuration`;
* Changing the approach to use the common configuration object instead
  of creating a new one;
* Moving the requirements of the Rails railtie to the end of the entry
  file, to ensure the configuration object is already defined when it is
  loaded.
@pfac
Copy link
Author

pfac commented Oct 15, 2020

Closing this due to lack of response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant