Repository: supabase/gotrue
Pull request #470 (2022-05-04 22:28:46)

chore: rename rate limit token endpoint env var
## What kind of change does this PR introduce?
* Rename `GOTRUE_RATE_LIMIT_TOKEN` to `GOTRUE_RATE_LIMIT_TOKEN_REFRESH` for clarity
* This env var controls the rate limit on the `/token` endpoint and defaults to 30 requests over 5 mins with a burst of 30


Patch:
diff --git a/README.md b/README.md
index 322b8ef5..b0342ce7 100644
--- a/README.md
+++ b/README.md
@@ -66,6 +66,15 @@ Rate limit the number of emails sent per hr on the following endpoints: `/signup
 
 Minimum password length, defaults to 6.
 
+`GOTRUE_SECURITY_REFRESH_TOKEN_ROTATION_ENABLED` - `bool`
+
+If refresh token rotation is enabled, gotrue will automatically detect malicious attempts to reuse a revoked refresh token. When a malicious attempt is detected, gotrue immediately revokes all tokens that descended from the offending token.
+
+`GOTRUE_SECURITY_REFRESH_TOKEN_REUSE_INTERVAL` - `string`
+
+This setting is only applicable if `GOTRUE_SECURITY_REFRESH_TOKEN_ROTATION_ENABLED` is enabled. The reuse interval for a refresh token allows for exchanging the refresh token multiple times during the interval to support concurrency or offline issues. During the reuse interval, gotrue will not consider using a revoked token as a malicious attempt and will simply return the child refresh token. 
+
+Only the previous revoked token can be reused. Using an old refresh token way before the current valid refresh token will trigger the reuse detection. 
 ### API
 
 ```properties
diff --git a/api/api.go b/api/api.go
index 5a0bbf83..cb98b37f 100644
--- a/api/api.go
+++ b/api/api.go
@@ -124,7 +124,7 @@ func NewAPIWithVersion(ctx context.Context, globalConfig *conf.GlobalConfigurati
 
 		r.With(api.limitHandler(
 			// Allow requests at the specified rate per 5 minutes.
-			tollbooth.NewLimiter(api.config.RateLimitToken/(60*5), &limiter.ExpirableOptions{
+			tollbooth.NewLimiter(api.config.RateLimitTokenRefresh/(60*5), &limiter.ExpirableOptions{
 				DefaultExpirationTTL: time.Hour,
 			}).SetBurst(30),
 		)).Post("/token", api.Token)
diff --git a/api/token.go b/api/token.go
index edd86b83..1ce341be 100644
--- a/api/token.go
+++ b/api/token.go
@@ -273,41 +273,50 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h
 		return oauthError("invalid_grant", "Invalid Refresh Token")
 	}
 
-	if !(config.External.Email.Enabled && config.External.Phone.Enabled) {
-		providers, err := models.FindProvidersByUser(a.db, user)
-		if err != nil {
-			return internalServerError(err.Error())
-		}
-		for _, provider := range providers {
-			if provider == "email" && !config.External.Email.Enabled {
-				return badRequestError("Email logins are disabled")
+	var newToken *models.RefreshToken
+	if token.Revoked {
+		a.clearCookieTokens(config, w)
+		err = a.db.Transaction(func(tx *storage.Connection) error {
+			validToken, terr := models.GetValidChildToken(tx, token)
+			if terr != nil {
+				if errors.Is(terr, models.RefreshTokenNotFoundError{}) {
+					// revoked token has no descendants
+					return nil
+				}
+				return terr
 			}
-			if provider == "phone" && !config.External.Phone.Enabled {
-				return badRequestError("Phone logins are disabled")
+			// check if token is the last previous revoked token
+			if validToken.Parent == storage.NullString(token.Token) {
+				refreshTokenReuseWindow := token.UpdatedAt.Add(time.Second * time.Duration(config.Security.RefreshTokenReuseInterval))
+				if time.Now().Before(refreshTokenReuseWindow) {
+					newToken = validToken
+				}
 			}
+			return nil
+		})
+		if err != nil {
+			return internalServerError("Error validating reuse interval").WithInternalError(err)
 		}
-	}
 
-	if token.Revoked {
-		a.clearCookieTokens(config, w)
-		if config.Security.RefreshTokenRotationEnabled {
-			// Revoke all tokens in token family
-			err = a.db.Transaction(func(tx *storage.Connection) error {
-				var terr error
-				if terr = models.RevokeTokenFamily(tx, token); terr != nil {
-					return terr
+		if newToken == nil {
+			if config.Security.RefreshTokenRotationEnabled {
+				// Revoke all tokens in token family
+				err = a.db.Transaction(func(tx *storage.Connection) error {
+					var terr error
+					if terr = models.RevokeTokenFamily(tx, token); terr != nil {
+						return terr
+					}
+					return nil
+				})
+				if err != nil {
+					return internalServerError(err.Error())
 				}
-				return nil
-			})
-			if err != nil {
-				return internalServerError(err.Error())
 			}
+			return oauthError("invalid_grant", "Invalid Refresh Token").WithInternalMessage("Possible abuse attempt: %v", r)
 		}
-		return oauthError("invalid_grant", "Invalid Refresh Token").WithInternalMessage("Possible abuse attempt: %v", r)
 	}
 
 	var tokenString string
-	var newToken *models.RefreshToken
 	var newTokenResponse *AccessTokenResponse
 
 	err = a.db.Transaction(func(tx *storage.Connection) error {
@@ -316,9 +325,11 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h
 			return terr
 		}
 
-		newToken, terr = models.GrantRefreshTokenSwap(tx, user, token)
-		if terr != nil {
-			return internalServerError(terr.Error())
+		if newToken == nil {
+			newToken, terr = models.GrantRefreshTokenSwap(tx, user, token)
+			if terr != nil {
+				return internalServerError(terr.Error())
+			}
 		}
 
 		tokenString, terr = generateAccessToken(user, time.Second*time.Duration(config.JWT.Exp), config.JWT.Secret)
diff --git a/conf/configuration.go b/conf/configuration.go
index c99e840e..21a87d06 100644
--- a/conf/configuration.go
+++ b/conf/configuration.go
@@ -66,17 +66,17 @@ type GlobalConfiguration struct {
 		RequestIDHeader string `envconfig:"REQUEST_ID_HEADER"`
 		ExternalURL     string `json:"external_url" envconfig:"API_EXTERNAL_URL"`
 	}
-	DB                 DBConfiguration
-	External           ProviderConfiguration
-	Logging            LoggingConfig `envconfig:"LOG"`
-	OperatorToken      string        `split_words:"true" required:"false"`
-	MultiInstanceMode  bool
-	Tracing            TracingConfig
-	SMTP               SMTPConfiguration
-	RateLimitHeader    string  `split_words:"true"`
-	RateLimitEmailSent float64 `split_words:"true" default:"30"`
-	RateLimitVerify    float64 `split_words:"true" default:"30"`
-	RateLimitToken     float64 `split_words:"true" default:"30"`
+	DB                    DBConfiguration
+	External              ProviderConfiguration
+	Logging               LoggingConfig `envconfig:"LOG"`
+	OperatorToken         string        `split_words:"true" required:"false"`
+	MultiInstanceMode     bool
+	Tracing               TracingConfig
+	SMTP                  SMTPConfiguration
+	RateLimitHeader       string  `split_words:"true"`
+	RateLimitEmailSent    float64 `split_words:"true" default:"30"`
+	RateLimitVerify       float64 `split_words:"true" default:"30"`
+	RateLimitTokenRefresh float64 `split_words:"true" default:"30"`
 }
 
 // EmailContentConfiguration holds the configuration for emails, both subjects and template URLs.
@@ -181,6 +181,7 @@ type CaptchaConfiguration struct {
 type SecurityConfiguration struct {
 	Captcha                               CaptchaConfiguration `json:"captcha"`
 	RefreshTokenRotationEnabled           bool                 `json:"refresh_token_rotation_enabled" split_words:"true" default:"true"`
+	RefreshTokenReuseInterval             int                  `json:"refresh_token_reuse_interval" split_words:"true"`
 	UpdatePasswordRequireReauthentication bool                 `json:"update_password_require_reauthentication" split_words:"true"`
 }
 
diff --git a/example.env b/example.env
index e38ef11a..be0e6312 100644
--- a/example.env
+++ b/example.env
@@ -190,7 +190,9 @@ GOTRUE_EXTERNAL_SAML_SIGNING_KEY=""
 
 # Additional Security config
 GOTRUE_LOG_LEVEL="debug"
-GOTRUE_REFRESH_TOKEN_ROTATION_ENABLED="false"
+GOTRUE_SECURITY_REFRESH_TOKEN_ROTATION_ENABLED="false"
+GOTRUE_SECURITY_REFRESH_TOKEN_REUSE_INTERVAL="0"
+GOTRUE_SECURITY_UPDATE_PASSWORD_REQUIRE_REAUTHENTICATION="false"
 GOTRUE_OPERATOR_TOKEN="unused-operator-token"
 GOTRUE_RATE_LIMIT_HEADER="X-Forwarded-For"
 GOTRUE_RATE_LIMIT_EMAIL_SENT="100"
diff --git a/models/refresh_token.go b/models/refresh_token.go
index 054806e4..2af42c50 100644
--- a/models/refresh_token.go
+++ b/models/refresh_token.go
@@ -1,6 +1,7 @@
 package models
 
 import (
+	"database/sql"
 	"time"
 
 	"github.com/gobuffalo/pop/v5"
@@ -57,19 +58,33 @@ func GrantRefreshTokenSwap(tx *storage.Connection, user *User, token *RefreshTok
 
 // RevokeTokenFamily revokes all refresh tokens that descended from the provided token.
 func RevokeTokenFamily(tx *storage.Connection, token *RefreshToken) error {
+	tablename := (&pop.Model{Value: RefreshToken{}}).TableName()
 	err := tx.RawQuery(`
 	with recursive token_family as (
-		select id, user_id, token, revoked, parent from refresh_tokens where parent = ?
+		select id, user_id, token, revoked, parent from `+tablename+` where parent = ?
 		union
-		select r.id, r.user_id, r.token, r.revoked, r.parent from `+(&pop.Model{Value: RefreshToken{}}).TableName()+` r inner join token_family t on t.token = r.parent
+		select r.id, r.user_id, r.token, r.revoked, r.parent from `+tablename+` r inner join token_family t on t.token = r.parent
 	)
-	update `+(&pop.Model{Value: RefreshToken{}}).TableName()+` r set revoked = true from token_family where token_family.id = r.id;`, token.Token).Exec()
+	update `+tablename+` r set revoked = true from token_family where token_family.id = r.id;`, token.Token).Exec()
 	if err != nil {
 		return err
 	}
 	return nil
 }
 
+// GetValidChildToken returns the child token of the token provided if the child is not revoked.
+func GetValidChildToken(tx *storage.Connection, token *RefreshToken) (*RefreshToken, error) {
+	refreshToken := &RefreshToken{}
+	err := tx.Q().Where("parent = ? and revoked = false", token.Token).First(refreshToken)
+	if err != nil {
+		if errors.Cause(err) == sql.ErrNoRows {
+			return nil, RefreshTokenNotFoundError{}
+		}
+		return nil, err
+	}
+	return refreshToken, nil
+}
+
 // Logout deletes all refresh tokens for a user.
 func Logout(tx *storage.Connection, instanceID uuid.UUID, id uuid.UUID) error {
 	return tx.RawQuery("DELETE FROM "+(&pop.Model{Value: RefreshToken{}}).TableName()+" WHERE instance_id = ? AND user_id = ?", instanceID, id).Exec()


Test patch:
diff --git a/api/token_test.go b/api/token_test.go
index eaa5dcb5..ace039f9 100644
--- a/api/token_test.go
+++ b/api/token_test.go
@@ -57,7 +57,7 @@ func (ts *TokenTestSuite) SetupTest() {
 	require.NoError(ts.T(), err, "Error creating refresh token")
 }
 
-func (ts *TokenTestSuite) TestRateLimitToken() {
+func (ts *TokenTestSuite) TestRateLimitTokenRefresh() {
 	var buffer bytes.Buffer
 	req := httptest.NewRequest(http.MethodPost, "http://localhost/token", &buffer)
 	req.Header.Set("Content-Type", "application/json")
@@ -150,6 +150,95 @@ func (ts *TokenTestSuite) TestTokenRefreshTokenGrantFailure() {
 	assert.Equal(ts.T(), http.StatusBadRequest, w.Code)
 }
 
+func (ts *TokenTestSuite) TestTokenRefreshTokenRotation() {
+	u, err := models.NewUser(ts.instanceID, "foo@example.com", "password", ts.Config.JWT.Aud, nil)
+	require.NoError(ts.T(), err, "Error creating test user model")
+	t := time.Now()
+	u.EmailConfirmedAt = &t
+	require.NoError(ts.T(), ts.API.db.Create(u), "Error saving foo user")
+	first, err := models.GrantAuthenticatedUser(ts.API.db, u)
+	require.NoError(ts.T(), err)
+	second, err := models.GrantRefreshTokenSwap(ts.API.db, u, first)
+	require.NoError(ts.T(), err)
+	third, err := models.GrantRefreshTokenSwap(ts.API.db, u, second)
+	require.NoError(ts.T(), err)
+
+	cases := []struct {
+		desc                        string
+		refreshTokenRotationEnabled bool
+		reuseInterval               int
+		refreshToken                string
+		expectedCode                int
+		expectedBody                map[string]interface{}
+	}{
+		{
+			"Valid refresh within reuse interval",
+			true,
+			30,
+			second.Token,
+			http.StatusOK,
+			map[string]interface{}{
+				"refresh_token": third.Token,
+			},
+		},
+		{
+			"Invalid refresh, first token is not the previous revoked token",
+			true,
+			0,
+			first.Token,
+			http.StatusBadRequest,
+			map[string]interface{}{
+				"error":             "invalid_grant",
+				"error_description": "Invalid Refresh Token",
+			},
+		},
+		{
+			"Invalid refresh, revoked third token",
+			true,
+			0,
+			second.Token,
+			http.StatusBadRequest,
+			map[string]interface{}{
+				"error":             "invalid_grant",
+				"error_description": "Invalid Refresh Token",
+			},
+		},
+		{
+			"Invalid refresh, third token revoked by previous case",
+			true,
+			30,
+			third.Token,
+			http.StatusBadRequest,
+			map[string]interface{}{
+				"error":             "invalid_grant",
+				"error_description": "Invalid Refresh Token",
+			},
+		},
+	}
+
+	for _, c := range cases {
+		ts.Run(c.desc, func() {
+			ts.Config.Security.RefreshTokenRotationEnabled = c.refreshTokenRotationEnabled
+			ts.Config.Security.RefreshTokenReuseInterval = c.reuseInterval
+			var buffer bytes.Buffer
+			require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
+				"refresh_token": c.refreshToken,
+			}))
+			req := httptest.NewRequest(http.MethodPost, "http://localhost/token?grant_type=refresh_token", &buffer)
+			req.Header.Set("Content-Type", "application/json")
+			w := httptest.NewRecorder()
+			ts.API.handler.ServeHTTP(w, req)
+			assert.Equal(ts.T(), c.expectedCode, w.Code)
+
+			data := make(map[string]interface{})
+			require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data))
+			for k, v := range c.expectedBody {
+				require.Equal(ts.T(), v, data[k])
+			}
+		})
+	}
+}
+
 func (ts *TokenTestSuite) createBannedUser() *models.User {
 	u, err := models.NewUser(ts.instanceID, "banned@example.com", "password", ts.Config.JWT.Aud, nil)
 	require.NoError(ts.T(), err, "Error creating test user model")