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

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"
@@ -58,6 +59,46 @@ func testOAuth2PrepareTestCode(t *testing.T) {
 	require.NoError(t, err)
 }
 
+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))
+
+	verifier := "phase3-verifier-" + util.FastCryptoRandomHex(12)
+	challengeBytes := sha256.Sum256([]byte(verifier))
+	code := "phase3-code-" + util.FastCryptoRandomHex(10)
+
+	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 TestOAuth2(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
@@ -72,12 +113,14 @@ func TestOAuth2(t *testing.T) {
 		t.Run("AuthorizeRedirectWithExistingGrant", testAuthorizeRedirectWithExistingGrant)
 		t.Run("AuthorizePKCERequiredForPublicClient", testAuthorizePKCERequiredForPublicClient)
 		t.Run("AccessTokenExchange", testAccessTokenExchange)
+		t.Run("AccessTokenExchangeRedirectURIMismatch", testAccessTokenExchangeRedirectURIMismatch)
 		t.Run("AccessTokenExchangeWithPublicClient", testAccessTokenExchangeWithPublicClient)
 		t.Run("AccessTokenExchangeJSON", testAccessTokenExchangeJSON)
 		t.Run("AccessTokenExchangeWithoutPKCE", testAccessTokenExchangeWithoutPKCE)
 		t.Run("AccessTokenExchangeWithInvalidCredentials", testAccessTokenExchangeWithInvalidCredentials)
 		t.Run("AccessTokenExchangeWithBasicAuth", testAccessTokenExchangeWithBasicAuth)
 		t.Run("RefreshTokenInvalidation", testRefreshTokenInvalidation)
+		t.Run("RefreshTokenCrossClientUsage", testRefreshTokenCrossClientUsage)
 		t.Run("OAuthIntrospection", testOAuthIntrospection)
 		t.Run("OAuthGrantScopesReadUserFailRepos", testOAuthGrantScopesReadUserFailRepos)
 		t.Run("OAuthGrantScopesBasicRespectsWriteUser", testOAuthGrantScopesBasicRespectsWriteUser)
@@ -225,12 +268,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) {
 	testOAuth2PrepareTestCode(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",
 	})
@@ -525,6 +599,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) {
 	testOAuth2PrepareTestCode(t)
 	req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{