Repository: supabase/gotrue
Pull request #860 (2022-12-14 00:07:20)

fix: add separate config for sms rate limits
## What kind of change does this PR introduce?
* Separate the email and sms rate limits by introducing a new env var `GOTRUE_RATE_LIMIT_SMS_SENT`
* This env var controls the global limit on the total number of sms-es that can be sent per hour. It is a global rate limit that defaults to 30 sms-es per hour


Patch:
diff --git a/api/middleware.go b/api/middleware.go
index b132c0f3..b83ecef4 100644
--- a/api/middleware.go
+++ b/api/middleware.go
@@ -70,10 +70,17 @@ func (a *API) limitHandler(lmt *limiter.Limiter) middlewareHandler {
 
 func (a *API) limitEmailOrPhoneSentHandler() middlewareHandler {
 	// limit per hour
-	freq := a.config.RateLimitEmailSent / (60 * 60)
-	lmt := tollbooth.NewLimiter(freq, &limiter.ExpirableOptions{
+	emailFreq := a.config.RateLimitEmailSent / (60 * 60)
+	smsFreq := a.config.RateLimitSmsSent / (60 * 60)
+
+	emailLimiter := tollbooth.NewLimiter(emailFreq, &limiter.ExpirableOptions{
 		DefaultExpirationTTL: time.Hour,
 	}).SetBurst(int(a.config.RateLimitEmailSent)).SetMethods([]string{"PUT", "POST"})
+
+	phoneLimiter := tollbooth.NewLimiter(smsFreq, &limiter.ExpirableOptions{
+		DefaultExpirationTTL: time.Hour,
+	}).SetBurst(int(a.config.RateLimitSmsSent)).SetMethods([]string{"PUT", "POST"})
+
 	return func(w http.ResponseWriter, req *http.Request) (context.Context, error) {
 		c := req.Context()
 		config := a.config
@@ -94,14 +101,14 @@ func (a *API) limitEmailOrPhoneSentHandler() middlewareHandler {
 				}
 
 				if requestBody.Email != "" {
-					if err := tollbooth.LimitByKeys(lmt, []string{"email_functions"}); err != nil {
-						return c, httpError(http.StatusTooManyRequests, "Rate limit exceeded")
+					if err := tollbooth.LimitByKeys(emailLimiter, []string{"email_functions"}); err != nil {
+						return c, httpError(http.StatusTooManyRequests, "Email rate limit exceeded")
 					}
 				}
 
 				if requestBody.Phone != "" {
-					if err := tollbooth.LimitByKeys(lmt, []string{"phone_functions"}); err != nil {
-						return c, httpError(http.StatusTooManyRequests, "Rate limit exceeded")
+					if err := tollbooth.LimitByKeys(phoneLimiter, []string{"phone_functions"}); err != nil {
+						return c, httpError(http.StatusTooManyRequests, "Sms rate limit exceeded")
 					}
 				}
 			}
diff --git a/conf/configuration.go b/conf/configuration.go
index ad5822c5..3d390440 100644
--- a/conf/configuration.go
+++ b/conf/configuration.go
@@ -100,6 +100,7 @@ type GlobalConfiguration struct {
 	SMTP                  SMTPConfiguration
 	RateLimitHeader       string  `split_words:"true"`
 	RateLimitEmailSent    float64 `split_words:"true" default:"30"`
+	RateLimitSmsSent      float64 `split_words:"true" default:"30"`
 	RateLimitVerify       float64 `split_words:"true" default:"30"`
 	RateLimitTokenRefresh float64 `split_words:"true" default:"30"`
 	RateLimitSso          float64 `split_words:"true" default:"30"`


Test patch:
diff --git a/api/middleware_test.go b/api/middleware_test.go
index fa95e1a5..c5154319 100644
--- a/api/middleware_test.go
+++ b/api/middleware_test.go
@@ -27,7 +27,7 @@ type MiddlewareTestSuite struct {
 	Config *conf.GlobalConfiguration
 }
 
-func TestHCaptcha(t *testing.T) {
+func TestMiddlewareFunctions(t *testing.T) {
 	api, config, err := setupAPIForTest()
 	require.NoError(t, err)
 
@@ -150,6 +150,54 @@ func (ts *MiddlewareTestSuite) TestVerifyCaptchaInvalid() {
 	}
 }
 
+func (ts *MiddlewareTestSuite) TestLimitEmailOrPhoneSentHandler() {
+	// Set up rate limit config for this test
+	ts.Config.RateLimitEmailSent = 5
+	ts.Config.RateLimitSmsSent = 5
+
+	cases := []struct {
+		desc             string
+		expectedErrorMsg string
+		requestBody      map[string]interface{}
+	}{
+		{
+			desc:             "Email rate limit exceeded",
+			expectedErrorMsg: "429: Email rate limit exceeded",
+			requestBody: map[string]interface{}{
+				"email": "test@example.com",
+			},
+		},
+		{
+			desc:             "Sms rate limit exceeded",
+			expectedErrorMsg: "429: Sms rate limit exceeded",
+			requestBody: map[string]interface{}{
+				"phone": "+1233456789",
+			},
+		},
+	}
+
+	limiter := ts.API.limitEmailOrPhoneSentHandler()
+	for _, c := range cases {
+		ts.Run(c.desc, func() {
+			var buffer bytes.Buffer
+			require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(c.requestBody))
+			req := httptest.NewRequest(http.MethodPost, "http://localhost", &buffer)
+			req.Header.Set("Content-Type", "application/json")
+			w := httptest.NewRecorder()
+
+			for i := 0; i < 5; i++ {
+				_, err := limiter(w, req)
+				require.NoError(ts.T(), err)
+			}
+
+			// should exceed rate limit on 5th try
+			_, err := limiter(w, req)
+			require.Error(ts.T(), err)
+			require.Equal(ts.T(), c.expectedErrorMsg, err.Error())
+		})
+	}
+}
+
 func TestFunctionHooksUnmarshalJSON(t *testing.T) {
 	tests := []struct {
 		in string