Create and apply admin rate limit middleware

[derrickmehaffy]

What this PR does:

• Creates a new admin middleware to rate limit 5 requests every 5 minutes
• Applies this new middleware to all authentication or password reset routes
• Creates a new RateLimitError class to properly throw an error that is readable by the login page
• Allows for user customization via the ./config/admin.js file
• Properly sets error status code to 429
• Documentation PR created to add docs for the new middleware configuration option

How to test

• Register a new admin user
• Logout of the new user
• Attempt to sign in using the wrong password more than 5 times in 5 minutes
• Wait 5 minutes or change your email to a different user
• Try to login again with proper creds

Preview

[derrickmehaffy]

For some reason I am unable to set the status code in the new error class and I'm not quite sure how this works, would appreciate a bit of help here before this is merged as the status code should be 429 not 400

[Convly]

For some reason I am unable to set the status code in the new error class and I'm not quite sure how this works, would appreciate a bit of help here before this is merged as the status code should be 429 not 400

What do you see if you log the ctx & error's status in there? https://github.com/strapi/strapi/blob/deits/poc/packages/core/strapi/lib/middlewares/errors.js#L19-L24

[petersg83]

Good work! You should come develop with us :p

[codecov]

Codecov Report

Base: 60.49% // Head: 60.49% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (ae7c609) compared to base (44c3c09).
Patch coverage: 22.22% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #14546      +/-   ##
==========================================
- Coverage   60.49%   60.49%   -0.01%     
==========================================
  Files        1352     1352              
  Lines       33174    33179       +5     
  Branches     6337     6339       +2     
==========================================
  Hits        20070    20070              
- Misses      11271    11274       +3     
- Partials     1833     1835       +2     

Flag	Coverage Δ
back	50.33% <22.22%> (-0.03%)	⬇️
front	65.05% <ø> (ø)
unit_back	50.33% <22.22%> (-0.03%)	⬇️
unit_front	65.05% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
packages/core/utils/lib/errors.js	60.86% <22.22%> (-7.43%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[petersg83]

Small things

[MattieBelt]

Hey @derrickmehaffy,
Sweet addition, I just had two thoughts on seeing this:

• Shouldn't there also be a way to have a limit per 'username' next to or instead of IP-based?
• What about adding the wait time to the front-end error message to give users a clearer message?

[petersg83]

Good catch @MattieBelt! Organizations behind a proxy or vpn could have issues loggin in.
+1 for the error message. I don't know if it would require frontend work though. Ping @yanniskadiri.

[derrickmehaffy]

Good catch @MattieBelt! Organizations behind a proxy or vpn could have issues loggin in.
+1 for the error message. I don't know if it would require frontend work though. Ping @yanniskadiri.

It shouldn't as we can just adjust the message sent in the thrown error. Downside is this would be more complex to implement in storage. Realistically a user should be using the redis connection option here but we can't do that by default.

[derrickmehaffy]

I added in the user's email (with a fallback, it would be rare an email wouldn't exist but didn't want to take the risk). That should allow multiple people to fail from the same IP. I also decreased the default down to 5 requests per 5 minutes.

[WalkingPizza]

Following up on what @MattieBelt said, if it's doable it'd be cool to have an header with the number of requests left and another with the number of seconds remaining before the rate limit resets.

Shopify does it and I like it quite a lot.
Here's some info from their docs about the headers.

[derrickmehaffy]

Yeah this is fairly common with rate limiting. I wanted to just get this one out ASAP and we can add more cool features later.

[gu-stav]

@derrickmehaffy I've added a small version bump in #14730 to avoid missing peer dependency warnings. It would be good if you could update the dependency here too 🙏🏼

[petersg83]

I updated the PR to only rate limit on the login. After discussing with Alex, we came to the conclusion that it is not necessary to rate limit on routes that use a token, we believe token are strong enough (same as jwt).
I removed the rate limit on SSO as I cannot filter on emails there and I believe it's the SSO provider that does the security, not us, do you confirm?
I used a lower case email for the prefixKey.

Wdyt? When we agree I'll ask Alex review

[derrickmehaffy]

Makes sense to me

[petersg83]

LGTM :)

[derrickmehaffy]

I want to do a bit more testing with the changes yet so I'll re-request later today probably.

Thank you both

[innerdvations]

LGTM!