Optional API rate limiter introduced - #511
Conversation
|
The four failing specs are currently exercising the disabled path. |
|
|
||
| authorization = request.get_header('HTTP_AUTHORIZATION').presence | ||
| session = request.cookies['_app_session'].presence | ||
| identifier = authorization || session || request.remote_ip |
There was a problem hiding this comment.
This particular order can be spoofed. A crafted request may set both a valid session cookie and a bogus authorization header. For that matter an attack can simply rotate invalid session and auth credentials, thereby bypassing rackattack. In this scenario I would only trust the underlying IP address request.remote_ip that comes from TCP/IP and is thus harder to mess with. Can you get rid of authorization and session?
There was a problem hiding this comment.
Good point, now it's only IP based.
| errors: | ||
| api: | ||
| internal_server: Szerver hiba | ||
| too_many_requests: Túl sok kérés. Kérjük, próbálja újra később. |
There was a problem hiding this comment.
For future reference, we maintain non-English translations for application strings in Transifex. I'll add this particular one manually, but you can review the Hungarian ones here: https://app.transifex.com/primero-v2/primero-app-v2/language/hu/
There was a problem hiding this comment.
Thanks for helping out on this one
| end | ||
| end | ||
|
|
||
| describe 'Rack::Attack API rate limit' do |
There was a problem hiding this comment.
Can you move this new code to a separate new file spec/middleware/rack_attack_spec.rb?
There was a problem hiding this comment.
I moved it to a new file as advised.
No description provided.