fix: key STS token cache by acting subject, not session alone

[QuentinBisson]

What

The STS token-propagation plugin cached the exchanged (delegated) token keyed
by session ID alone. A session that carries messages from more than one subject
therefore reused whichever subject's token seeded the cache first, so every
later caller's MCP tool calls ran under that first subject's delegated identity.
On cache expiry the next request re-seeded the entry, so the acting identity
could also drift nondeterministically.

This keys the cache by (sessionID, subject), where subject is the acting
bearer's issuer-scoped sub claim (iss + sub), falling back to a hash of
the raw token for opaque or sub-less tokens. Each caller in a shared session now
gets and reuses its own exchanged token.

Why

Shared sessions already deliver per-caller identity everywhere except this
cache. The session-sharing feature (#1935)
pins X-User-Id to the session owner for history, but deliberately leaves
Authorization as the caller's own token and records the real caller as
initiated_by. So in a shared session a visitor's tool calls are meant to run
under the visitor's own delegated token. The sessionID-only cache defeated
that: the owner's (or first visitor's) exchanged token was reused for everyone,
collapsing per-caller identity and leaking one user's delegated authority to
another.

The condition is reproducible entirely within the in-tree share feature: two
users in one shared session, each authenticated with their own token, produce
tool calls that all execute under the first-seen subject.

Changes

• Cache keyed by cacheKey{sessionID, subject}. subjectKey() derives the
  discriminator from the bearer as iss + sub (sub is only unique within an
  issuer), or a hash of the raw token when there is no sub. The token is parsed
  unverified and used only to partition the cache, never to gate a security
  decision; it is validated server-side on exchange.
• BeforeRunCallback and HeaderProvider both recover the acting bearer through
  actingBearer(), which prefers the value the executor stored
  (models.BearerTokenKey) and falls back to the A2A CallContext Authorization
  header. The fallback is the same source the round-tripper's propagateToken
  path reads, so the acting subject is recoverable at the MCP transport layer
  even when BearerTokenKey is not threaded onto the MCP request context;
  without it a lookup could silently miss and inject no token.
• AfterRunCallback sweeps all expired entries rather than a single session key.
• Drops GetTokenForSession, which had no callers and could not gain one: the
  subject half of the key is derived by the unexported subjectKey.

Behavior change

For existing read-write shared sessions, a visitor's tool calls move from the
owner's exchanged token to the visitor's own token. This corrects an identity
collapse (one user's authority was borrowed by others), but it is externally
observable for anyone who relied on the prior behavior.

Second, bearer recovery now happens before the cache lookup instead of after it,
so a request carrying no bearer no longer reuses a token the session cached
earlier. It skips propagation instead. That is the fail-closed half of keying by
subject (an unidentified caller must not inherit an identified one's token), but
it means any path that fails to thread the bearer loses token injection silently
rather than falling back. actingBearer's CallContext fallback exists to keep
that from happening on the MCP transport path.

Open question (does not block this fix)

This fix makes the cache honor whichever acting token arrives; it does not
choose an identity model. Whether a shared session should run tools per-caller
or under a single pinned identity (owner/initiator) is orthogonal, and is
decided by which token the fronting proxy forwards, not by this cache.

Worth confirming the intended direction: single-identity tool pinning cannot be
provided in-cluster for UI shares, because no component holds a non-speaking
principal's live token to exchange or refresh. It is only realizable where a
credential-forwarding layer exists (for example an A2A gateway that can forward
the pinned principal's token every turn). The controller already owns the
history half via the share feature. If you agree, kagent stays mode-neutral here
and this fix stands on correctness alone.

Fixes #2181

[Copilot]

Pull request overview

This PR fixes an identity-collapsing bug in the STS token-propagation plugin where exchanged tokens were cached by session ID only, causing shared sessions to reuse the first subject’s delegated token for later callers. It updates the cache keying strategy to scope exchanged tokens by both session ID and the acting subject (derived from the caller’s bearer), so shared sessions preserve per-caller delegated identity.

Changes:

• Key STS token cache entries by (sessionID, subject) rather than sessionID alone, using sub (or a hash fallback) as the subject discriminator.
• Update BeforeRunCallback and HeaderProvider to derive the acting subject from the request’s bearer before cache lookup/injection.
• Sweep all expired cache entries in AfterRunCallback instead of only a single session entry, and add tests covering shared-session multi-subject behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
go/adk/pkg/sts/plugin.go	Introduces subject-aware cache keys and updates token exchange + header injection to preserve per-caller identity in shared sessions.
go/adk/pkg/sts/plugin_test.go	Adds tests for per-subject token behavior in shared sessions and same-subject cache reuse.

Comments suppressed due to low confidence (1)

go/adk/pkg/sts/plugin_test.go:78

• TestHeaderProvider_UsesSessionIDMethod currently seeds the cache with an empty subject and calls HeaderProvider with a context that has no bearer token. With the new session+subject cache keying, this scenario is not representative of production behavior (HeaderProvider is intended to select the cached token based on the request’s bearer-derived subject). Updating the test to include a bearer and cache entry for that bearer’s subject will better validate the new behavior and avoid implying that token injection works without a bearer.

func TestHeaderProvider_UsesSessionIDMethod(t *testing.T) {
	t.Parallel()
	plugin := NewTokenPropagationPlugin(nil, logr.Discard())
	plugin.setCachedToken("sess-123", "", "token-abc", 0)

	headers := plugin.HeaderProvider(fakeSessionContext{
		Context:   context.Background(),
		sessionID: "sess-123",
	})

	if headers["Authorization"] != "Bearer token-abc" {
		t.Fatalf("Authorization header = %q, want %q", headers["Authorization"], "Bearer token-abc")
	}
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mesutoezdil]

lgtm