Add option to disable refresh token invalidation (#6584) (#6587)

* Add option to disable refresh token invalidation

Signed-off-by: Jonas Franz <[email protected]>

* Add integration tests and remove wrong todos

Signed-off-by: Jonas Franz <[email protected]>

* Fix typo

Signed-off-by: Jonas Franz <[email protected]>

* Fix tests and add documentation

Signed-off-by: Jonas Franz <[email protected]>

(cherry picked from commit 783cd64)
Signed-off-by: Jonas Franz <[email protected]>

Author: jonasfranz

custom/conf/app.ini.sample

@@ -661,6 +661,8 @@ ENABLED = true
 ACCESS_TOKEN_EXPIRATION_TIME=3600
 ; Lifetime of an OAuth2 access token in hours
 REFRESH_TOKEN_EXPIRATION_TIME=730
+; Check if refresh token got already used
+INVALIDATE_REFRESH_TOKENS=false
 ; OAuth2 authentication secret for access and refresh tokens, change this a unique string.
 JWT_SECRET=Bk0yK7Y9g_p56v86KaHqjSbxvNvu3SbKoOdOt2ZcXvU
 

docs/content/doc/advanced/config-cheat-sheet.en-us.md

@@ -350,6 +350,7 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
 - `ENABLED`: **true**: Enables OAuth2 provider.
 - `ACCESS_TOKEN_EXPIRATION_TIME`: **3600**: Lifetime of an OAuth2 access token in seconds
 - `REFRESH_TOKEN_EXPIRATION_TIME`: **730**: Lifetime of an OAuth2 access token in hours
+- `INVALIDATE_REFRESH_TOKEN`: **false**: Check if refresh token got already used
 - `JWT_SECRET`: **\<empty\>**: OAuth2 authentication secret for access and refresh tokens, change this a unique string.
 
 ## i18n (`i18n`)

integrations/oauth_test.go

@@ -8,6 +8,8 @@ import (
 	"encoding/json"
 	"testing"
 
+	"code.gitea.io/gitea/modules/setting"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -177,3 +179,42 @@ func TestAccessTokenExchangeWithBasicAuth(t *testing.T) {
 	})
 	resp = MakeRequest(t, req, 400)
 }
+
+func TestRefreshTokenInvalidation(t *testing.T) {
+	prepareTestEnv(t)
+	req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
+		"grant_type":    "authorization_code",
+		"client_id":     "da7da3ba-9a13-4167-856f-3899de0b0138",
+		"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
+		"redirect_uri":  "a",
+		"code":          "authcode",
+		"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", // test PKCE additionally
+	})
+	resp := MakeRequest(t, req, 200)
+	type response struct {
+		AccessToken  string `json:"access_token"`
+		TokenType    string `json:"token_type"`
+		ExpiresIn    int64  `json:"expires_in"`
+		RefreshToken string `json:"refresh_token"`
+	}
+	parsed := new(response)
+	assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsed))
+
+	// test without invalidation
+	setting.OAuth2.InvalidateRefreshTokens = false
+
+	refreshReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
+		"grant_type":    "refresh_token",
+		"client_id":     "da7da3ba-9a13-4167-856f-3899de0b0138",
+		"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
+		"redirect_uri":  "a",
+		"refresh_token": parsed.RefreshToken,
+	})
+	MakeRequest(t, refreshReq, 200)
+	MakeRequest(t, refreshReq, 200)
+
+	// test with invalidation
+	setting.OAuth2.InvalidateRefreshTokens = true
+	MakeRequest(t, refreshReq, 200)
+	MakeRequest(t, refreshReq, 400)
+}

modules/auth/user_form.go

@@ -172,7 +172,6 @@ type AccessTokenForm struct {
 	ClientID     string
 	ClientSecret string
 	RedirectURI  string
-	// TODO Specify authentication code length to prevent against birthday attacks
 	Code         string
 	RefreshToken string
 

modules/setting/setting.go

@@ -299,12 +299,14 @@ var (
 		Enable                     bool
 		AccessTokenExpirationTime  int64
 		RefreshTokenExpirationTime int64
+		InvalidateRefreshTokens    bool
 		JWTSecretBytes             []byte `ini:"-"`
 		JWTSecretBase64            string `ini:"JWT_SECRET"`
 	}{
 		Enable:                     true,
 		AccessTokenExpirationTime:  3600,
 		RefreshTokenExpirationTime: 730,
+		InvalidateRefreshTokens:    false,
 	}
 
 	U2F = struct {

routers/user/oauth.go

@@ -102,18 +102,19 @@ const (
 
 // AccessTokenResponse represents a successful access token response
 type AccessTokenResponse struct {
-	AccessToken string    `json:"access_token"`
-	TokenType   TokenType `json:"token_type"`
-	ExpiresIn   int64     `json:"expires_in"`
-	// TODO implement RefreshToken
-	RefreshToken string `json:"refresh_token"`
+	AccessToken  string    `json:"access_token"`
+	TokenType    TokenType `json:"token_type"`
+	ExpiresIn    int64     `json:"expires_in"`
+	RefreshToken string    `json:"refresh_token"`
 }
 
 func newAccessTokenResponse(grant *models.OAuth2Grant) (*AccessTokenResponse, *AccessTokenError) {
-	if err := grant.IncreaseCounter(); err != nil {
-		return nil, &AccessTokenError{
-			ErrorCode:        AccessTokenErrorCodeInvalidGrant,
-			ErrorDescription: "cannot increase the grant counter",
+	if setting.OAuth2.InvalidateRefreshTokens {
+		if err := grant.IncreaseCounter(); err != nil {
+			return nil, &AccessTokenError{
+				ErrorCode:        AccessTokenErrorCodeInvalidGrant,
+				ErrorDescription: "cannot increase the grant counter",
+			}
 		}
 	}
 	// generate access token to access the API
@@ -366,7 +367,7 @@ func handleRefreshToken(ctx *context.Context, form auth.AccessTokenForm) {
 	}
 
 	// check if token got already used
-	if grant.Counter != token.Counter || token.Counter == 0 {
+	if setting.OAuth2.InvalidateRefreshTokens && (grant.Counter != token.Counter || token.Counter == 0) {
 		handleAccessTokenError(ctx, AccessTokenError{
 			ErrorCode:        AccessTokenErrorCodeUnauthorizedClient,
 			ErrorDescription: "token was already used",