From c9920b7bd0f6ec1f7590f104711b09d55917f9e8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 28 Jun 2026 01:06:33 -0700 Subject: [PATCH] fix(oauth): restrict introspection to the token's client (#38042) Bind OAuth token introspection responses to the authenticated client. Return an inactive response when the token grant belongs to a different OAuth application to avoid leaking token metadata across clients. Add integration coverage for cross-client introspection attempts against both access tokens and refresh tokens. Assisted-by: GPT-5.4 --- routers/web/auth/oauth2_provider.go | 52 ++++++++++++++------ tests/integration/oauth_test.go | 76 +++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 16 deletions(-) diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index 0ed20329b46..62a86bfc0c8 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -128,7 +128,7 @@ func InfoOAuth(ctx *context.Context) { // IntrospectOAuth introspects an oauth token func IntrospectOAuth(ctx *context.Context) { - clientIDValid := false + var introspectingApp *auth.OAuth2Application authHeader := ctx.Req.Header.Get("Authorization") if parsed, ok := httpauth.ParseAuthorizationHeader(authHeader); ok && parsed.BasicAuth != nil { clientID, clientSecret := parsed.BasicAuth.Username, parsed.BasicAuth.Password @@ -139,9 +139,14 @@ func IntrospectOAuth(ctx *context.Context) { ctx.HTTPError(http.StatusInternalServerError) return } - clientIDValid = err == nil && app.ValidateClientSecret([]byte(clientSecret)) + clientIDValid := err == nil && app.ValidateClientSecret([]byte(clientSecret)) + if clientIDValid { + introspectingApp = app + } } - if !clientIDValid { + if introspectingApp == nil { + // RFC 7662 requires the caller to authenticate to the introspection endpoint. + // https://www.rfc-editor.org/rfc/rfc7662.html#section-2.1 ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="Gitea OAuth2"`) ctx.PlainText(http.StatusUnauthorized, "no valid authorization") return @@ -156,21 +161,36 @@ func IntrospectOAuth(ctx *context.Context) { form := web.GetForm(ctx).(*forms.IntrospectTokenForm) token, err := oauth2_provider.ParseToken(form.Token, oauth2_provider.DefaultSigningKey) - if err == nil { - grant, err := auth.GetOAuth2GrantByID(ctx, token.GrantID) - if err == nil && grant != nil { - app, err := auth.GetOAuth2ApplicationByID(ctx, grant.ApplicationID) - if err == nil && app != nil { - response.Active = true - response.Scope = grant.Scope - response.RegisteredClaims = oauth2_provider.NewJwtRegisteredClaimsFromUser(app.ClientID, grant.UserID, nil /*exp*/) - } - if user, err := user_model.GetUserByID(ctx, grant.UserID); err == nil { - response.Username = user.Name - } - } + if err != nil { + // RFC 7662 returns inactive token metadata for invalid/unknown tokens. + // https://www.rfc-editor.org/rfc/rfc7662.html#section-2.2 + log.Trace("Ignoring invalid token during introspection: %v", err) + ctx.JSON(http.StatusOK, response) + return } + grant, err := auth.GetOAuth2GrantByID(ctx, token.GrantID) + if err != nil { + ctx.ServerError("GetOAuth2GrantByID", err) + return + } + if grant == nil || grant.ApplicationID != introspectingApp.ID { + // RFC 7662 allows the server to reply inactive when the caller must not learn more. + // https://www.rfc-editor.org/rfc/rfc7662.html#section-2.2 + ctx.JSON(http.StatusOK, response) + return + } + + response.Active = true + response.Scope = grant.Scope + response.RegisteredClaims = oauth2_provider.NewJwtRegisteredClaimsFromUser(introspectingApp.ClientID, grant.UserID, nil /*exp*/) + user, err := user_model.GetUserByID(ctx, grant.UserID) + if err != nil { + ctx.ServerError("GetUserByID", err) + return + } + response.Username = user.Name + ctx.JSON(http.StatusOK, response) } diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index ee9eb838a35..b42860763ab 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -14,6 +14,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strconv" "strings" "testing" @@ -33,6 +34,7 @@ import ( "gitea.dev/tests" "github.com/PuerkitoBio/goquery" + jwt "github.com/golang-jwt/jwt/v5" "github.com/markbates/goth" "github.com/markbates/goth/gothic" "github.com/stretchr/testify/assert" @@ -122,6 +124,7 @@ func TestOAuth2(t *testing.T) { t.Run("RefreshTokenInvalidation", testRefreshTokenInvalidation) t.Run("RefreshTokenCrossClientUsage", testRefreshTokenCrossClientUsage) t.Run("OAuthIntrospection", testOAuthIntrospection) + t.Run("OAuthIntrospectionCrossClientIsolation", testOAuthIntrospectionCrossClientIsolation) t.Run("OAuthGrantScopesReadUserFailRepos", testOAuthGrantScopesReadUserFailRepos) t.Run("OAuthGrantScopesBasicRespectsWriteUser", testOAuthGrantScopesBasicRespectsWriteUser) t.Run("OAuthGrantScopesReadRepositoryFailOrganization", testOAuthGrantScopesReadRepositoryFailOrganization) @@ -705,6 +708,79 @@ func testOAuthIntrospection(t *testing.T) { assert.Contains(t, resp.Body.String(), "no valid authorization") } +func testOAuthIntrospectionCrossClientIsolation(t *testing.T) { + resourceOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + clientA := createOAuthTestApplication(t, "user1", "introspection-primary-client", []string{"https://primary.example/oauth/callback"}) + clientB := createOAuthTestApplication(t, "user2", "introspection-secondary-client", []string{"https://secondary.example/oauth/callback"}) + code, verifier := issueOAuthAuthorizationCode(t, resourceOwner, clientA, clientA.RedirectURIs[0], "openid profile") + + req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "grant_type": "authorization_code", + "client_id": clientA.ClientID, + "client_secret": clientA.ClientSecret, + "redirect_uri": clientA.RedirectURIs[0], + "code": code, + "code_verifier": verifier, + }) + resp := MakeRequest(t, req, http.StatusOK) + type tokenResponse struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + } + tokenParsed := new(tokenResponse) + require.NoError(t, json.Unmarshal(resp.Body.Bytes(), tokenParsed)) + require.NotEmpty(t, tokenParsed.AccessToken) + require.NotEmpty(t, tokenParsed.RefreshToken) + + type introspectResponse struct { + Active bool `json:"active"` + Scope string `json:"scope,omitempty"` + Username string `json:"username,omitempty"` + jwt.RegisteredClaims + } + + assertBlockedIntrospection := func(token string) { + t.Helper() + + req = NewRequestWithValues(t, "POST", "/login/oauth/introspect", map[string]string{ + "token": token, + }) + req.SetBasicAuth(clientB.ClientID, clientB.ClientSecret) + resp = MakeRequest(t, req, http.StatusOK) + + blocked := new(introspectResponse) + require.NoError(t, json.Unmarshal(resp.Body.Bytes(), blocked)) + assert.False(t, blocked.Active) + assert.Empty(t, blocked.Scope) + assert.Empty(t, blocked.Username) + assert.Empty(t, blocked.Subject) + assert.Empty(t, blocked.Audience) + } + + assertAllowedIntrospection := func(token string) { + t.Helper() + + req = NewRequestWithValues(t, "POST", "/login/oauth/introspect", map[string]string{ + "token": token, + }) + req.SetBasicAuth(clientA.ClientID, clientA.ClientSecret) + resp = MakeRequest(t, req, http.StatusOK) + + allowed := new(introspectResponse) + require.NoError(t, json.Unmarshal(resp.Body.Bytes(), allowed)) + assert.True(t, allowed.Active) + assert.Equal(t, "openid profile", allowed.Scope) + assert.Equal(t, resourceOwner.Name, allowed.Username) + assert.Equal(t, strconv.FormatInt(resourceOwner.ID, 10), allowed.Subject) + assert.Equal(t, jwt.ClaimStrings{clientA.ClientID}, allowed.Audience) + } + + assertBlockedIntrospection(tokenParsed.AccessToken) + assertAllowedIntrospection(tokenParsed.AccessToken) + assertBlockedIntrospection(tokenParsed.RefreshToken) + assertAllowedIntrospection(tokenParsed.RefreshToken) +} + func testOAuthGrantScopesReadUserFailRepos(t *testing.T) { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) accessToken := issueOAuthAccessTokenForScope(t, user, "openid read:user")