task(auth,gql): Configure default limiting rules #18952
Conversation
dschom
force-pushed
the
configure-remaining-ratelimit-rules
branch
8 times, most recently
from
c20f7d6 to
4eb714d
Compare
| * @param action - Optional action to target. If nothing is provided, all counts are cleared. | ||
| * @returns Number of keys deleted. | ||
| */ | ||
| async resetCounts(action?:string) { |
There was a problem hiding this comment.
Surprising that isn't auto-formatted by prettier on pre-commit.
There was a problem hiding this comment.
Somehow it got turned off. I'll have to look into this.
| request, | ||
| sessionToken.uid, | ||
| 'passwordChange' | ||
| 'authenticatedPasswordChanged' |
There was a problem hiding this comment.
That's not a defined action in the customs server.
There was a problem hiding this comment.
I've added a v2 check here and fallback accordingly.
| await customs.check(request, email, 'passwordForgotVerifyOtp'); | ||
|
|
||
| // Daily limit, will be checked if and only if the default limit above passes | ||
| await customs.check(request, email, 'passwordForgotVerifyOtpPerDay'); |
There was a problem hiding this comment.
Also not a defined action in customs.
There was a problem hiding this comment.
I'm going to guard this with a check about if v2 is enabled.
| request, | ||
| checkAuthenticatedUid, | ||
| 'accountLogin' | ||
| 'authenticatedAccountLogin' |
chenba
left a comment
chenba
left a comment
There was a problem hiding this comment.
A few customs calls were changed to pass actions that are not defined in customs. I don't think that'll work since customs checks grouped known actions.
dschom
force-pushed
the
configure-remaining-ratelimit-rules
branch
from
4eb714d to
535e953
Compare
|
@chenba I've adjusted the few actions names where this was problematic to be conditional depending on v1 vs v2. Thanks for catching this. In my testing it didn't actually appear problematic, but none the less. I'll update it. The reason these names were changed is that v2 is stricter about what you supply. So if there's a rule that checks UID on a given action, you must supply a UID when checking that action. Same goes for email or IP. I thought about relaxing this requirement at the library level, but this strict behavior actually saved my bacon in a couple spots when customs wasn't properly invoked. So for now, I'll keep the behavior and just decide what to do based on the version of customs being used. |
Because: - We are getting ready to swap in the new rate limiting service This Commit: - Recreates the policies current held in the customs server, but in a flat file config format.
dschom
force-pushed
the
configure-remaining-ratelimit-rules
branch
from
535e953 to
2068eb9
Compare
Because
This pull request
Issue that this pull request solves
Closes: FXA-11608
Checklist
Put an
xin the boxes that applyScreenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)