Conversation
mitchellhenke
changed the title
config/initializers/rack_attack.rb
Outdated
There was a problem hiding this comment.
Random comment for a future potential change - Would be NICE if we could define these by controller so if we add new languages or adjust the URL structure the limits would just follow.
There was a problem hiding this comment.
Do we have access to URL helpers, or at least I18n.available_locales at this point in initialization?
| EMAIL_REGISTRATION_PATHS = ['/sign_up/enter_email', '/es/sign_up/enter_email', | |
| EMAIL_REGISTRATION_PATHS = [*I18n.available_locales, ''].map { |locale| sign_up_email_path(locale: locale) } |
There was a problem hiding this comment.
Also, isn't /en/sign_up/enter_email technically a valid path? Should it be included here?
There was a problem hiding this comment.
Good catch on /en, yeah, it should be there as well.
Initializers unfortunately happen before the application is loaded so routes/actions and i18n locales aren't available at this point. rack-attack has a short section on matching actions in Rails. It has a caveat at the top that doesn't inspire confidence, and I don't love the solution since it relies on rescue and doesn't take the HTTP method into account (though it looks like recognize_path supports it).
I messed around with recognize_path and more dynamic post-initialize solutions and didn't really like any of them. I also don't like the static path-based solution we have currently.
mitchellhenke
force-pushed
the
mitchellhenke/sign-up-rack-attack
branch
from
27fbc11 to
2acb8b2
Compare
* changelog: Internal, Rate Limit, Add rate limit for email registration requests * add non-US languages to sign in path rate limits * add /en paths
3 participants
🛠 Summary of changes
We identified a layer that is potentially missing from our rate limiting capabilities, and while this has some overlap with other rate limits that are already in place, this is a more precise one to be able to tune close to the application level.
(I also saw that we are only rate limiting the English sign-in path
/rather than all of them, so included changes for that as well)