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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

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 "s"
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 <sdimitrovski@gmail.com>
@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

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