From 783cd649276c472aa3af97dd311eb4766ff3adfb Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Fri, 12 Apr 2019 09:50:21 +0200 Subject: [PATCH] Add option to disable refresh token invalidation (#6584) * Add option to disable refresh token invalidation Signed-off-by: Jonas Franz * Add integration tests and remove wrong todos Signed-off-by: Jonas Franz * Fix typo Signed-off-by: Jonas Franz * Fix tests and add documentation Signed-off-by: Jonas Franz --- custom/conf/app.ini.sample | 2 + .../doc/advanced/config-cheat-sheet.en-us.md | 1 + integrations/oauth_test.go | 41 +++++++++++++++++++ modules/auth/user_form.go | 1 - modules/setting/setting.go | 2 + routers/user/oauth.go | 21 +++++----- 6 files changed, 57 insertions(+), 11 deletions(-) diff --git a/custom/conf/app.ini.sample b/custom/conf/app.ini.sample index b527e2249..6dee39ed8 100644 --- a/custom/conf/app.ini.sample +++ b/custom/conf/app.ini.sample @@ -680,6 +680,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 diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 3f99f1d13..1e97f93e1 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -409,6 +409,7 @@ NB: You must `REDIRECT_MACARON_LOG` and have `DISABLE_ROUTER_LOG` set to `false` - `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`: **\**: OAuth2 authentication secret for access and refresh tokens, change this a unique string. ## i18n (`i18n`) diff --git a/integrations/oauth_test.go b/integrations/oauth_test.go index 9674146f8..2b5839dd7 100644 --- a/integrations/oauth_test.go +++ b/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) +} diff --git a/modules/auth/user_form.go b/modules/auth/user_form.go index 78dd75fa1..810a2f941 100644 --- a/modules/auth/user_form.go +++ b/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 diff --git a/modules/setting/setting.go b/modules/setting/setting.go index ab290fc4f..c3d57452c 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -304,12 +304,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 { diff --git a/routers/user/oauth.go b/routers/user/oauth.go index 110fa93b3..326bd0bc5 100644 --- a/routers/user/oauth.go +++ b/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",