Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: configurable email and sms rate limiting #1800

Merged
merged 3 commits into from
Oct 14, 2024

Conversation

cstockton
Copy link
Contributor

@cstockton cstockton commented Oct 10, 2024

Adds two new configuration values for rate limiting the sending of emails and sms messages:

  • GOTRUE_RATE_LIMIT_EMAIL_SENT
  • GOTRUE_RATE_LIMIT_SMS_SENT

It is implemented with a simple rate limiter that resets a counter at a regular interval. The first intervals start time is set when the counter is initialized. It will be reset when the server is restarted, but preserved when the config is reloaded.

Syntax examples:

1.5       # Allow 1.5 events over 1 hour (legacy format)
100       # Allow 100 events over 1 hour (1h is default)
100/1h    # Allow 100 events over 1 hour (explicit duration)
100/24h   # Allow 100 events over 24 hours
100/72h   # Allow 100 events over 72 hours (use hours for days)
10/30m    # Allow 10  events over 30 minutes
3/10s     # Allow 3   events over 10 seconds

Syntax in ABNF to express the format as value:

value = count / rate
count = 1*DIGIT ["." 1*DIGIT]
rate = 1*DIGIT "/" ival
ival = ival-sec / ival-min / ival-hr
ival-sec = 1*DIGIT "s"
ival-min = 1*DIGIT "m"
ival-hr = 1*DIGIT "h"

This change was a continuation of #1746 adapted to support the recent preservation of rate limiters across server reloads.

Adds two new configuration values for rate limiting the sending
of emails and sms messages:

 - GOTRUE_RATE_LIMIT_EMAIL_SENT
 - GOTRUE_RATE_LIMIT_SMS_SENT

It is implemented with a simple rate limiter that resets a counter
at a regular interval. The first intervals start time is set when
the counter is initialized. It will be reset when the server is
restarted, but preserved when the config is reloaded.

Syntax examples:
  1.5       # Allow 1.5 events over 1 hour (legacy format)
  100       # Allow 100 events over 1 hour (1h is default)
  100/1h    # Allow 100 events over 1 hour (explicit duration)
  100/24h   # Allow 100 events over 24 hours
  100/72h   # Allow 100 events over 72 hours (use hours for days)
  10/30m    # Allow 10  events over 30 minutes
  3/10s     # Allow 3   events over 10 seconds

Syntax in ABNF to express the format as value:
  value = count / rate
  count = 1*DIGIT ["." 1*DIGIT]
  rate = 1*DIGIT "/" ival
  ival = ival-sec / ival-min / ival-hr
  ival-sec = 1*DIGIT "s"
  ival-min = 1*DIGIT "s"
  ival-hr = 1*DIGIT "h"

Co-authored-by: Stojan Dimitrovski <[email protected]>
@cstockton cstockton requested a review from a team as a code owner October 10, 2024 22:37
Copy link
Contributor

@hf hf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just remove that function that's unused and good to go!

internal/api/middleware_test.go Outdated Show resolved Hide resolved
internal/api/middleware_test.go Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Oct 11, 2024

Pull Request Test Coverage Report for Build 11292135273

Details

  • 61 of 75 (81.33%) changed or added relevant lines in 6 files are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.008%) to 57.979%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/api/phone.go 1 2 50.0%
internal/api/mail.go 1 7 14.29%
internal/conf/rate.go 25 32 78.13%
Files with Coverage Reduction New Missed Lines %
internal/api/phone.go 1 81.51%
internal/api/mail.go 1 57.4%
Totals Coverage Status
Change from base Build 11277025862: 0.008%
Covered Lines: 9381
Relevant Lines: 16180

💛 - Coveralls

@hf hf merged commit 5e94047 into master Oct 14, 2024
2 checks passed
@hf hf deleted the cs/feat-email-and-sms-rate-limiting branch October 14, 2024 09:14
cstockton pushed a commit that referenced this pull request Oct 15, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.163.0](v2.162.2...v2.163.0)
(2024-10-15)


### Features

* add mail header support via `GOTRUE_SMTP_HEADERS` with `$messageType`
([#1804](#1804))
([99d6a13](99d6a13))
* add MFA for WebAuthn
([#1775](#1775))
([8cc2f0e](8cc2f0e))
* configurable email and sms rate limiting
([#1800](#1800))
([5e94047](5e94047))
* mailer logging ([#1805](#1805))
([9354b83](9354b83))
* preserve rate limiters in memory across configuration reloads
([#1792](#1792))
([0a3968b](0a3968b))


### Bug Fixes

* add twilio verify support on mfa
([#1714](#1714))
([aeb5d8f](aeb5d8f))
* email header setting no longer misleading
([#1802](#1802))
([3af03be](3af03be))
* enforce authorized address checks on send email only
([#1806](#1806))
([c0c5b23](c0c5b23))
* fix `getExcludedColumns` slice allocation
([#1788](#1788))
([7f006b6](7f006b6))
* Fix reqPath for bypass check for verify EP
([#1789](#1789))
([646dc66](646dc66))
* inline mailme package for easy development
([#1803](#1803))
([fa6f729](fa6f729))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
hf pushed a commit that referenced this pull request Oct 22, 2024
## 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants