Skip to content

Commit 9ce2340

Browse files
authored
fix: ignore rate limits for autoconfirm (#1810)
## What kind of change does this PR introduce? * #1800 introduced a bug which applied the rate limiting setting even if `AUTOCONFIRM` was enabled. Although this is unintended, it is a breaking change so we need to give users time to update their rate limit settings before applying it ## What is the current behavior? Please link any relevant issues here. ## What is the new behavior? Feel free to include screenshots if it includes visual changes. ## Additional context Add any other context or screenshots.
1 parent 39459c1 commit 9ce2340

File tree

2 files changed

+17
-11
lines changed

2 files changed

+17
-11
lines changed

internal/api/mail.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -627,14 +627,17 @@ func (a *API) sendEmail(r *http.Request, tx *storage.Connection, u *models.User,
627627
}
628628
}
629629

630-
// apply rate limiting before the email is sent out
631-
if ok := a.limiterOpts.Email.Allow(); !ok {
632-
emailRateLimitCounter.Add(
633-
ctx,
634-
1,
635-
metric.WithAttributeSet(attribute.NewSet(attribute.String("path", r.URL.Path))),
636-
)
637-
return EmailRateLimitExceeded
630+
// TODO(km): Deprecate this behaviour - rate limits should still be applied to autoconfirm
631+
if !config.Mailer.Autoconfirm {
632+
// apply rate limiting before the email is sent out
633+
if ok := a.limiterOpts.Email.Allow(); !ok {
634+
emailRateLimitCounter.Add(
635+
ctx,
636+
1,
637+
metric.WithAttributeSet(attribute.NewSet(attribute.String("path", r.URL.Path))),
638+
)
639+
return EmailRateLimitExceeded
640+
}
638641
}
639642

640643
if config.Hook.SendEmail.Enabled {

internal/api/phone.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,12 @@ func (a *API) sendPhoneConfirmation(r *http.Request, tx *storage.Connection, use
8686

8787
// not using test OTPs
8888
if otp == "" {
89-
// apply rate limiting before the sms is sent out
90-
if ok := a.limiterOpts.Phone.Allow(); !ok {
91-
return "", tooManyRequestsError(ErrorCodeOverSMSSendRateLimit, "SMS rate limit exceeded")
89+
// TODO(km): Deprecate this behaviour - rate limits should still be applied to autoconfirm
90+
if !config.Sms.Autoconfirm {
91+
// apply rate limiting before the sms is sent out
92+
if ok := a.limiterOpts.Phone.Allow(); !ok {
93+
return "", tooManyRequestsError(ErrorCodeOverSMSSendRateLimit, "SMS rate limit exceeded")
94+
}
9295
}
9396
otp, err = crypto.GenerateOtp(config.Sms.OtpLength)
9497
if err != nil {

0 commit comments

Comments
 (0)