fix(oauth): introspection describes only the caller's own client's access tokens (BUG-3398) (#1791)

* fix(oauth): introspection describes only the caller's own client's access tokens (BUG-3398)

Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS

* test(oauth): two clients share the test redirect URI (BUG-3398)

Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS

* test(server): the census names the shared introspection caller reader (BUG-3398)

Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS

internal/server/app_tokens.go

@@ -221,13 +221,26 @@ func (s *Server) introspectAppToken(ctx context.Context, token string) (*AppToke
 // (fosite.AccessTokenFromRequest) first, and only without one, the Basic
 // credentials with the client id percent-decoded.
 func (s *Server) introspectionCallerIsInstall(r *http.Request) bool {
+	id := s.introspectionCallerClientID(r)
+	if id == "" {
+		return false
+	}
+	c, err := s.store.GetOAuthClient(id)
+	return err == nil && c.IsInstallClient()
+}
+
+// introspectionCallerClientID is the client a public /oauth/introspect
+// request authenticates as, read as fosite's NewIntrospectionRequest reads
+// it: the client of a bearer access token (header, form or query) first,
+// and only without one, the Basic or form client id. "" when it cannot be
+// read; fosite then refuses the request on its own terms.
+func (s *Server) introspectionCallerClientID(r *http.Request) string {
 	if bearer := fosite.AccessTokenFromRequest(r); bearer != "" {
 		ar, _, err := s.oauthServer.IntrospectToken(r.Context(), bearer)
-		if err != nil || ar == nil {
-			return false
+		if err != nil || ar == nil || ar.GetClient() == nil {
+			return ""
 		}
-		c, cerr := s.store.GetOAuthClient(ar.GetClient().GetID())
-		return cerr == nil && c.IsInstallClient()
+		return ar.GetClient().GetID()
 	}
-	return s.installClientFor(r) != nil
+	return requestClientID(r)
 }

internal/server/body_reader_census_test.go

@@ -469,9 +469,10 @@ func TestEveryRequestHandoffIsClassified(t *testing.T) {
 		"handlers_oauth.go::Server.handleOAuthRevoke::github.com/ory/fosite.OAuth2Provider.NewRevocationRequest":         1,
 		"handlers_oauth.go::Server.handleOAuthIntrospect::github.com/ory/fosite.OAuth2Provider.NewIntrospectionRequest":  1,
 		// TASK-3394: fosite's own bearer extraction (header, then the form's
-		// access_token, parsing multipart), so the install-caller refusal
-		// reads the caller exactly as NewIntrospectionRequest will.
-		"app_tokens.go::Server.introspectionCallerIsInstall::github.com/ory/fosite.AccessTokenFromRequest": 1,
+		// access_token, parsing multipart), so the caller is read exactly as
+		// NewIntrospectionRequest will. Shared since BUG-3398 by the
+		// install-caller refusal and the token-ownership check.
+		"app_tokens.go::Server.introspectionCallerClientID::github.com/ory/fosite.AccessTokenFromRequest": 1,
 	}
 
 	handoffs := collectRequestHandoffs(t, ".")

internal/server/bug3398_introspect_test.go

@@ -0,0 +1,57 @@
+package server
+
+import (
+	"net/http"
+	"net/url"
+	"testing"
+)
+
+// BUG-3398: the public /oauth/introspect describes a token only to that
+// token's own client, and never describes a refresh token. Either refusal is
+// the bare {"active": false} an unknown token gets, so neither is an oracle.
+func TestBug3398_IntrospectionIsOwnClientAccessTokensOnly(t *testing.T) {
+	t.Parallel()
+	srv, _ := oauthEnabledTestServer(t)
+	_, sessionToken := loginTestUser(t, srv)
+	csrfTok := readCSRFFromCookie(t, srv, sessionToken)
+	clientA := registerTestClient(t, srv, "https://app.test/cb")
+	clientB := registerTestClient(t, srv, "https://app.test/cb")
+
+	a1 := runAuthCodeFlow(t, srv, sessionToken, csrfTok, clientA, "verifier-b3398-a1-quick-brown-fox-1234567890")
+	a2 := runAuthCodeFlow(t, srv, sessionToken, csrfTok, clientA, "verifier-b3398-a2-quick-brown-fox-1234567890")
+	b1 := runAuthCodeFlow(t, srv, sessionToken, csrfTok, clientB, "verifier-b3398-b1-quick-brown-fox-1234567890")
+	str := func(m map[string]any, k string) string {
+		v, _ := m[k].(string)
+		if v == "" {
+			t.Fatalf("token response lacks %s: %v", k, m)
+		}
+		return v
+	}
+
+	introspect := func(token, bearer string) map[string]any {
+		t.Helper()
+		rr := postOAuthFormBearer(srv, "/oauth/introspect", url.Values{"token": {token}}, bearer)
+		if rr.Code != http.StatusOK {
+			t.Fatalf("introspect: %d %s", rr.Code, rr.Body.String())
+		}
+		var out map[string]any
+		parseJSON(t, rr, &out)
+		return out
+	}
+	inactive := func(where string, got map[string]any) {
+		t.Helper()
+		if len(got) != 1 || got["active"] != false {
+			t.Errorf("%s: want exactly {\"active\": false}, got %v", where, got)
+		}
+	}
+
+	// The control: the same client's other access token is described.
+	if got := introspect(str(a1, "access_token"), str(a2, "access_token")); got["active"] != true || got["client_id"] != clientA {
+		t.Fatalf("same client: want active with client_id %s, got %v", clientA, got)
+	}
+	// Another client's token, though the caller's own token is active.
+	inactive("another client's access token", introspect(str(a1, "access_token"), str(b1, "access_token")))
+	// A refresh token, even the caller's own client's.
+	inactive("own client's refresh token", introspect(str(a1, "refresh_token"), str(a2, "access_token")))
+	inactive("another client's refresh token", introspect(str(a1, "refresh_token"), str(b1, "access_token")))
+}

internal/server/handlers_oauth.go

@@ -1440,6 +1440,10 @@ func (s *Server) handleOAuthRevoke(w http.ResponseWriter, r *http.Request) {
 // bearer is itself active and not the same token being introspected
 // (avoid trivial "introspect yourself with yourself" auth).
 //
+// What is described (BUG-3398): only an ACCESS token, and only to that
+// token's own client, read from the bearer's client (or the Basic client
+// id). Any other token reads as {"active": false}, as an unknown one does.
+//
 // In practice the consumer of this endpoint is sub-PR E's
 // MCPBearerAuth — but that integration uses fosite.IntrospectToken
 // directly (server-side, no HTTP roundtrip), so this public
@@ -1468,8 +1472,8 @@ func (s *Server) handleOAuthIntrospect(w http.ResponseWriter, r *http.Request) {
 
 	// Installed apps' clients do not use the public endpoint, and it never
 	// describes their tokens (lead ruling R2 on TASK-3394): the app API
-	// checks its tokens in-process. The general caller-ownership gap is
-	// BUG-3398.
+	// checks its tokens in-process. Every other caller is told only about
+	// its own client's access tokens (BUG-3398, below).
 	if err := r.ParseForm(); err != nil {
 		http.Error(w, "Invalid form body", http.StatusBadRequest)
 		return
@@ -1481,8 +1485,23 @@ func (s *Server) handleOAuthIntrospect(w http.ResponseWriter, r *http.Request) {
 
 	ir, err := s.oauthServer.Provider().NewIntrospectionRequest(ctx, r, session)
 	if err == nil && ir.IsActive() {
-		if c, cerr := s.store.GetOAuthClient(ir.GetAccessRequester().GetClient().GetID()); cerr == nil && c.IsInstallClient() {
+		inspected := ir.GetAccessRequester().GetClient().GetID()
+		switch {
+		case ir.GetTokenUse() != fosite.AccessToken:
+			// A refresh token is never described (BUG-3398): it is not a
+			// bearer anywhere (/mcp refuses one), and describing it tells a
+			// holder of the string that it still mints access tokens.
 			err = fosite.ErrInactiveToken
+		case inspected != s.introspectionCallerClientID(r):
+			// A token is described only to its own client (BUG-3398). RFC
+			// 7662 §2.2 lets the server answer inactive when the caller may
+			// not know about the token, and that answer is the same as for
+			// an unknown one, so this is not an oracle.
+			err = fosite.ErrInactiveToken
+		default:
+			if c, cerr := s.store.GetOAuthClient(inspected); cerr == nil && c.IsInstallClient() {
+				err = fosite.ErrInactiveToken
+			}
 		}
 	}
 	if err != nil {