Add option to disable refresh token invalidation (#6584)
* Add option to disable refresh token invalidation Signed-off-by: Jonas Franz <info@jonasfranz.software> * Add integration tests and remove wrong todos Signed-off-by: Jonas Franz <info@jonasfranz.software> * Fix typo Signed-off-by: Jonas Franz <info@jonasfranz.software> * Fix tests and add documentation Signed-off-by: Jonas Franz <info@jonasfranz.software>
This commit is contained in:
		
							parent
							
								
									3ff0a126e1
								
							
						
					
					
						commit
						783cd64927
					
				
					 6 changed files with 57 additions and 11 deletions
				
			
		|  | @ -680,6 +680,8 @@ ENABLED = true | ||||||
| ACCESS_TOKEN_EXPIRATION_TIME=3600 | ACCESS_TOKEN_EXPIRATION_TIME=3600 | ||||||
| ; Lifetime of an OAuth2 access token in hours | ; Lifetime of an OAuth2 access token in hours | ||||||
| REFRESH_TOKEN_EXPIRATION_TIME=730 | 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. | ; OAuth2 authentication secret for access and refresh tokens, change this a unique string. | ||||||
| JWT_SECRET=Bk0yK7Y9g_p56v86KaHqjSbxvNvu3SbKoOdOt2ZcXvU | JWT_SECRET=Bk0yK7Y9g_p56v86KaHqjSbxvNvu3SbKoOdOt2ZcXvU | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -409,6 +409,7 @@ NB: You must `REDIRECT_MACARON_LOG` and have `DISABLE_ROUTER_LOG` set to `false` | ||||||
| - `ENABLED`: **true**: Enables OAuth2 provider. | - `ENABLED`: **true**: Enables OAuth2 provider. | ||||||
| - `ACCESS_TOKEN_EXPIRATION_TIME`: **3600**: Lifetime of an OAuth2 access token in seconds | - `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 | - `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. | - `JWT_SECRET`: **\<empty\>**: OAuth2 authentication secret for access and refresh tokens, change this a unique string. | ||||||
| 
 | 
 | ||||||
| ## i18n (`i18n`) | ## i18n (`i18n`) | ||||||
|  |  | ||||||
|  | @ -8,6 +8,8 @@ import ( | ||||||
| 	"encoding/json" | 	"encoding/json" | ||||||
| 	"testing" | 	"testing" | ||||||
| 
 | 
 | ||||||
|  | 	"code.gitea.io/gitea/modules/setting" | ||||||
|  | 
 | ||||||
| 	"github.com/stretchr/testify/assert" | 	"github.com/stretchr/testify/assert" | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
|  | @ -177,3 +179,42 @@ func TestAccessTokenExchangeWithBasicAuth(t *testing.T) { | ||||||
| 	}) | 	}) | ||||||
| 	resp = MakeRequest(t, req, 400) | 	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) | ||||||
|  | } | ||||||
|  |  | ||||||
|  | @ -172,7 +172,6 @@ type AccessTokenForm struct { | ||||||
| 	ClientID     string | 	ClientID     string | ||||||
| 	ClientSecret string | 	ClientSecret string | ||||||
| 	RedirectURI  string | 	RedirectURI  string | ||||||
| 	// TODO Specify authentication code length to prevent against birthday attacks
 |  | ||||||
| 	Code         string | 	Code         string | ||||||
| 	RefreshToken string | 	RefreshToken string | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -304,12 +304,14 @@ var ( | ||||||
| 		Enable                     bool | 		Enable                     bool | ||||||
| 		AccessTokenExpirationTime  int64 | 		AccessTokenExpirationTime  int64 | ||||||
| 		RefreshTokenExpirationTime int64 | 		RefreshTokenExpirationTime int64 | ||||||
|  | 		InvalidateRefreshTokens    bool | ||||||
| 		JWTSecretBytes             []byte `ini:"-"` | 		JWTSecretBytes             []byte `ini:"-"` | ||||||
| 		JWTSecretBase64            string `ini:"JWT_SECRET"` | 		JWTSecretBase64            string `ini:"JWT_SECRET"` | ||||||
| 	}{ | 	}{ | ||||||
| 		Enable:                     true, | 		Enable:                     true, | ||||||
| 		AccessTokenExpirationTime:  3600, | 		AccessTokenExpirationTime:  3600, | ||||||
| 		RefreshTokenExpirationTime: 730, | 		RefreshTokenExpirationTime: 730, | ||||||
|  | 		InvalidateRefreshTokens:    false, | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	U2F = struct { | 	U2F = struct { | ||||||
|  |  | ||||||
|  | @ -102,18 +102,19 @@ const ( | ||||||
| 
 | 
 | ||||||
| // AccessTokenResponse represents a successful access token response
 | // AccessTokenResponse represents a successful access token response
 | ||||||
| type AccessTokenResponse struct { | type AccessTokenResponse struct { | ||||||
| 	AccessToken string    `json:"access_token"` | 	AccessToken  string    `json:"access_token"` | ||||||
| 	TokenType   TokenType `json:"token_type"` | 	TokenType    TokenType `json:"token_type"` | ||||||
| 	ExpiresIn   int64     `json:"expires_in"` | 	ExpiresIn    int64     `json:"expires_in"` | ||||||
| 	// TODO implement RefreshToken
 | 	RefreshToken string    `json:"refresh_token"` | ||||||
| 	RefreshToken string `json:"refresh_token"` |  | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func newAccessTokenResponse(grant *models.OAuth2Grant) (*AccessTokenResponse, *AccessTokenError) { | func newAccessTokenResponse(grant *models.OAuth2Grant) (*AccessTokenResponse, *AccessTokenError) { | ||||||
| 	if err := grant.IncreaseCounter(); err != nil { | 	if setting.OAuth2.InvalidateRefreshTokens { | ||||||
| 		return nil, &AccessTokenError{ | 		if err := grant.IncreaseCounter(); err != nil { | ||||||
| 			ErrorCode:        AccessTokenErrorCodeInvalidGrant, | 			return nil, &AccessTokenError{ | ||||||
| 			ErrorDescription: "cannot increase the grant counter", | 				ErrorCode:        AccessTokenErrorCodeInvalidGrant, | ||||||
|  | 				ErrorDescription: "cannot increase the grant counter", | ||||||
|  | 			} | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
| 	// generate access token to access the API
 | 	// 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
 | 	// 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{ | 		handleAccessTokenError(ctx, AccessTokenError{ | ||||||
| 			ErrorCode:        AccessTokenErrorCodeUnauthorizedClient, | 			ErrorCode:        AccessTokenErrorCodeUnauthorizedClient, | ||||||
| 			ErrorDescription: "token was already used", | 			ErrorDescription: "token was already used", | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue