diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index 05931d8f596..b4f36d543c4 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -561,6 +561,13 @@ func handleRefreshToken(ctx *context.Context, form forms.AccessTokenForm, server }) return } + if grant.ApplicationID != app.ID { + handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{ + ErrorCode: oauth2_provider.AccessTokenErrorCodeInvalidGrant, + ErrorDescription: "refresh token belongs to a different client", + }) + return + } // check if token got already used if setting.OAuth2.InvalidateRefreshTokens && (grant.Counter != token.Counter || token.Counter == 0) { @@ -630,6 +637,13 @@ func handleAuthorizationCode(ctx *context.Context, form forms.AccessTokenForm, s }) return } + if authorizationCode.RedirectURI != "" && form.RedirectURI != authorizationCode.RedirectURI { + handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{ + ErrorCode: oauth2_provider.AccessTokenErrorCodeInvalidGrant, + ErrorDescription: "redirect_uri differs from the original authorization request", + }) + return + } // check if granted for this application if authorizationCode.Grant.ApplicationID != app.ID { handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{ diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 505688f86da..c423534e145 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -5,6 +5,7 @@ package integration import ( "bytes" + "crypto/sha256" "encoding/base64" "fmt" "image" @@ -25,6 +26,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/auth/source/oauth2" "code.gitea.io/gitea/services/oauth2_provider" @@ -37,6 +39,51 @@ import ( "github.com/stretchr/testify/require" ) +func createOAuthTestApplication(t *testing.T, userName, name string, redirectURIs []string) *api.OAuth2Application { + t.Helper() + req := NewRequestWithJSON(t, "POST", "/api/v1/user/applications/oauth2", &api.CreateOAuth2ApplicationOptions{ + Name: name, + RedirectURIs: redirectURIs, + ConfidentialClient: true, + }).AddBasicAuth(userName) + resp := MakeRequest(t, req, http.StatusCreated) + created := DecodeJSON(t, resp, &api.OAuth2Application{}) + require.NotEmpty(t, created.ClientID) + require.NotEmpty(t, created.ClientSecret) + return created +} + +func issueOAuthAuthorizationCode(t *testing.T, user *user_model.User, app *api.OAuth2Application, redirectURI, scope string) (string, string) { + t.Helper() + + grant := &auth_model.OAuth2Grant{ + ApplicationID: app.ID, + UserID: user.ID, + Scope: scope, + } + require.NoError(t, db.Insert(t.Context(), grant)) + + r1, err := util.CryptoRandomBytes(12) + require.NoError(t, err) + + verifier := "phase3-verifier-" + base64.RawURLEncoding.EncodeToString(r1) + challengeBytes := sha256.Sum256([]byte(verifier)) + r2, err := util.CryptoRandomBytes(10) + require.NoError(t, err) + code := "phase3-code-" + base64.RawURLEncoding.EncodeToString(r2) + + require.NoError(t, db.Insert(t.Context(), &auth_model.OAuth2AuthorizationCode{ + GrantID: grant.ID, + Code: code, + CodeChallenge: base64.RawURLEncoding.EncodeToString(challengeBytes[:]), + CodeChallengeMethod: "S256", + RedirectURI: redirectURI, + ValidUntil: timeutil.TimeStampNow() + 86400, + })) + + return code, verifier +} + func TestOAuth2Provider(t *testing.T) { defer tests.PrepareTestEnv(t)() @@ -46,6 +93,9 @@ func TestOAuth2Provider(t *testing.T) { t.Run("AuthorizeUnsupportedCodeChallengeMethod", testAuthorizeUnsupportedCodeChallengeMethod) t.Run("AuthorizeLoginRedirect", testAuthorizeLoginRedirect) + t.Run("AccessTokenExchangeRedirectURIMismatch", testAccessTokenExchangeRedirectURIMismatch) + t.Run("RefreshTokenCrossClientUsage", testRefreshTokenCrossClientUsage) + t.Run("OAuth2WellKnown", testOAuth2WellKnown) t.Run("OAuthSourceSpecialChars", testOAuthSourceSpecialChars) // TODO: move more tests as sub-tests here, avoid unnecessary PrepareTestEnv @@ -186,12 +236,43 @@ func TestAccessTokenExchange(t *testing.T) { assert.Greater(t, len(parsed.RefreshToken), 10) } +func testAccessTokenExchangeRedirectURIMismatch(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + redirectURIs := []string{"https://phase3.example/callback", "https://phase3.example/callback-alt"} + app := createOAuthTestApplication(t, user.Name, "phase3-redirect-uri-guard", redirectURIs) + code, verifier := issueOAuthAuthorizationCode(t, user, app, redirectURIs[0], "openid profile") + + req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": app.ClientID, + "client_secret": app.ClientSecret, + "redirect_uri": redirectURIs[1], + "code": code, + "code_verifier": verifier, + }) + resp := MakeRequest(t, req, http.StatusBadRequest) + parsedError := new(oauth2_provider.AccessTokenError) + assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError)) + assert.Equal(t, "invalid_grant", string(parsedError.ErrorCode)) + assert.Equal(t, "redirect_uri differs from the original authorization request", parsedError.ErrorDescription) + + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": app.ClientID, + "client_secret": app.ClientSecret, + "redirect_uri": redirectURIs[0], + "code": code, + "code_verifier": verifier, + }) + MakeRequest(t, req, http.StatusOK) +} + func TestAccessTokenExchangeWithPublicClient(t *testing.T) { defer tests.PrepareTestEnv(t)() req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ "grant_type": "authorization_code", "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", - "redirect_uri": "http://127.0.0.1", + "redirect_uri": "http://127.0.0.1/", "code": "authcodepublic", "code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt", }) @@ -486,6 +567,54 @@ func TestRefreshTokenInvalidation(t *testing.T) { assert.Equal(t, "token was already used", parsedError.ErrorDescription) } +func testRefreshTokenCrossClientUsage(t *testing.T) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + primaryApp := createOAuthTestApplication(t, user.Name, "phase3-refresh-token-primary", []string{"https://phase3.example/refresh-primary"}) + secondaryApp := createOAuthTestApplication(t, user.Name, "refresh-token-client-guard", []string{"https://alt-client.example/oauth/callback"}) + code, verifier := issueOAuthAuthorizationCode(t, user, primaryApp, primaryApp.RedirectURIs[0], "openid profile") + + req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": primaryApp.ClientID, + "client_secret": primaryApp.ClientSecret, + "redirect_uri": primaryApp.RedirectURIs[0], + "code": code, + "code_verifier": verifier, + }) + resp := MakeRequest(t, req, http.StatusOK) + 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)) + assert.NotEmpty(t, parsed.RefreshToken) + + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "refresh_token", + "client_id": secondaryApp.ClientID, + "client_secret": secondaryApp.ClientSecret, + "redirect_uri": secondaryApp.RedirectURIs[0], + "refresh_token": parsed.RefreshToken, + }) + resp = MakeRequest(t, req, http.StatusBadRequest) + parsedError := new(oauth2_provider.AccessTokenError) + assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsedError)) + assert.Equal(t, "invalid_grant", string(parsedError.ErrorCode)) + assert.Equal(t, "refresh token belongs to a different client", parsedError.ErrorDescription) + + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "refresh_token", + "client_id": primaryApp.ClientID, + "client_secret": primaryApp.ClientSecret, + "redirect_uri": primaryApp.RedirectURIs[0], + "refresh_token": parsed.RefreshToken, + }) + MakeRequest(t, req, http.StatusOK) +} + func TestOAuthIntrospection(t *testing.T) { defer tests.PrepareTestEnv(t)() req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{