task(auth): Remove recoveryPhoneAvailable customs check #18241
Conversation
dschom
force-pushed
the
remove-recoveryPhoneAvailable-customs-check
branch
2 times, most recently
from
d318576 to
f4c88dd
Compare
| @@ -227,8 +227,6 @@ class RecoveryPhoneHandler { | |||
| throw AppError.invalidToken(); | |||
| } | |||
|
|
|||
| await this.customs.check(request, email, 'recoveryPhoneAvailable'); | |||
There was a problem hiding this comment.
If we want to add rate limiting for some authenticated endpoints, we could use checkAuthenticated api. This will only check the uid and not ip or email address record.
The rate limit is more generous though (100 checks every 15mins) per action.
| format: 'nat', | ||
| env: 'SMS_RATE_LIMIT_INTERVAL_SECONDS', | ||
| }, | ||
| maxSms: { | ||
| doc: 'Number of sms sent within rateLimitIntervalSeconds before throttling', | ||
| default: 3, | ||
| default: 9, |
There was a problem hiding this comment.
I think we want to set this to 6 based on Slack conversation:
https://mozilla.slack.com/archives/C07T1GHL46S/p1736963075491979?thread_ts=1736960348.342549&cid=C07T1GHL46S
In theory there were notes to set the limit to 9/day for recovery phone setup, and 6/day for using the recovery phone during sign-in, but that might be overly complicated considering the current customs setup.
| default: 9, | |
| default: 6, |
There was a problem hiding this comment.
Ah gotcha, I didn't see the lower limit. I'll change the default.
…s rate limit defaults Because: - We deemed this check wasn't necessary. This Commit: - Removes the recoveryPhoneAvailable check. - Removes the action from customs. - Leaving isTwilioAction, however. We might need it later. - Changes default config to 9 sms per 24 hour period - Fixes remote test to be inline with updated customs rate limits
dschom
force-pushed
the
remove-recoveryPhoneAvailable-customs-check
branch
from
f4c88dd to
e2fd3e0
Compare
Because
This pull request
Issue that this pull request solves
Closes: FXA-11006, FXA-11005
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)
We are leaving
isTwilioActionand theTWILIO_ACTIONgroup set in customs. Pretty sure we will need it at some point soon, but with different actions.