[vMCP] Extend identity-refresh integration test to wire TokenValidator.Middleware + fake UpstreamTokenRefresher
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
Context
The fix for #5323 added a higher-level integration test (pkg/vmcp/session/internal/backend/mcp_session_identity_refresh_test.go) that exercises the full transport chain — headerForwardRoundTripper → identityRoundTripper → authRoundTripper (UpstreamInjectStrategy) → http.DefaultTransport — and verifies that a fresh identity placed on req.Context() drives the upstream Authorization header.
That test attaches freshIdentity to the request context directly via auth.WithIdentity(...), which is the same call TokenValidator.Middleware makes at pkg/auth/token.go:1245. The iteration 2 review of #5323 explicitly allowed this higher-level alternative, but noted that it does not verify the link from TokenValidator.Middleware → loadUpstreamTokens → UpstreamTokenRefresher → fresh map on identity.UpstreamTokens.
Gap
The user-visible "forced re-auth every ~24h" symptom can recur via multiple distinct failure modes:
- The transport overriding fresh identity (the bug fixed in #5323, now covered).
loadUpstreamTokensreturning a stale map becauseUpstreamTokenRefresheris broken.TokenValidator.Middlewarefailing to callWithIdentityon the refresh path.
The new test only catches (1). Modes (2) and (3) would still produce the same user-visible symptom.
Proposal
Extend mcp_session_identity_refresh_test.go (or create a sibling test in a higher-level package such as pkg/vmcp/server/...) with an end-to-end case that:
- Builds a minimal
TokenValidatorwired to a fakeUpstreamTokenRefresherwhose returned tokens differ between the first and second call (simulating transparent refresh). - Constructs a signed JWT carrying a
tsidclaim that the validator can resolve via the fakeUpstreamTokenRefresher. - Issues two requests through the validator's middleware into the backend session.
- Asserts the upstream observes the rotated token on the second call.
This is the AC5 acceptance criterion as literally written on #5323. Closing this gap is non-trivial test setup (signed JWT, fake refresher, real middleware wiring), which is why the iteration 2 review allowed deferring it.
Acceptance criteria
- A test exists that exercises
TokenValidator.Middleware → loadUpstreamTokens → UpstreamTokenRefresherend-to-end against a vMCP backend session. - The fake
UpstreamTokenRefresherrotates tokens between the two calls. - The upstream
httptest.Serverobserves the rotated token on the second call. - The test fails if any link in the chain (middleware, refresher, or transport) is broken in a way that would surface the original #5323 symptom.
Notes
Raised as a MEDIUM finding on the iteration 2 review of #5323. The reviewer noted that either filing this issue or extending the existing test is acceptable; this issue makes the gap visible rather than implicit.
Related: #5323, #3877, #3869
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with pkg/vmcp/session/internal/backend/mcp_session_identity_refresh_test.go and pkg/auth/token.go:1245, then trace the TokenValidator and UpstreamTokenRefresher wiring used by the existing vMCP session path. Run the current identity-refresh integration test first. Done means two middleware requests reach the httptest upstream and the second observes the fake refresher's rotated token, covering the full chain.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- authentication, backend, testing
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100