fix(oauth): bind token exchanges to the original client request (#37704) (#37740)

Backport #37704 

This PR hardens OAuth token exchange validation by binding exchanged
credentials to the client and redirect URI that originally obtained
them.

What it changes:

- reject refresh token exchanges when the refresh token belongs to a
different OAuth application
- reject authorization code exchanges when the `redirect_uri` in the
token request differs from the `redirect_uri` stored with the
authorization code
- add integration coverage for:
  - authorization code exchange with a mismatched redirect URI
- refresh token reuse across two different dynamically created OAuth
applications

Why:

OAuth authorization codes and refresh tokens must remain bound to the
client context that originally received them. Without those checks:
- a valid authorization code can be redeemed against a different
registered redirect URI of the same client
- a refresh token can be replayed by a different OAuth client

---------

Co-authored-by: Nicolas <bircni@icloud.com>

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{

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{