Skip to content

Fix hourly MCP re-auth by correcting upstream expires_in - #16

Open
UsamaSadiq wants to merge 5 commits into
foss-sandboxfrom
usama/mcp-session-refresh
Open

Fix hourly MCP re-auth by correcting upstream expires_in#16
UsamaSadiq wants to merge 5 commits into
foss-sandboxfrom
usama/mcp-session-refresh

Conversation

@UsamaSadiq

Copy link
Copy Markdown
Collaborator

Description

MCP sessions were forced through a full interactive re-authorization every hour.

mpass-auth-proxy reports the browser session cookie lifetime as expires_in
from its /token endpoint, because oauth2-proxy is the session authority for the
browser flow and trusts that value for its cookie. The Cognito access token
underneath still expires in one hour. The MCP servers point
COGNITO_UPSTREAM_TOKEN_URL at that same endpoint.

On the MCP path the session authority is FastMCP OAuthProxy, which keys renewal
off expires_in: it stores expires_at = now + expires_in. With the inflated
value that stored expiry sits days out, so at the one hour mark JWKS validation of
the real token fails while the refresh gate stays shut. load_access_token
returns None, the request 401s, and the client re-authorizes from scratch.

This registers an authlib compliance hook on both the authorization code and the
refresh grant, deriving expires_in from the access token own exp claim so the
stored lifetime matches reality and the session renews through the refresh grant
instead. OAuthProxy._create_upstream_oauth_client is the single factory for the
authlib client used by both grants, so one registration covers every path that
stores an upstream token lifetime.

Scope is confined to this repo. /token, SESSION_COOKIE_MAX_AGE_SECONDS,
oauth2-proxy and the Traefik router rules are untouched, so the browser flow
cannot be affected. The hook is inert where the server talks straight to Cognito,
which already reports a truthful lifetime.

Note that the client facing FastMCP token now also tracks the upstream hour and is
renewed by the client refresh token. Decoupling the two would require
fastmcp_access_token_expiry_seconds, which is absent from the fastmcp 3.2.x line
the sibling MCP servers pin, so it is deliberately not used here to keep all three
servers behaving identically.

Testing

  • Full suite green (only pre-existing failures remain, confirmed against a stashed
    baseline). ruff check clean on every changed file.
  • New tests/test_token_expiry.py covers the correction itself, the pass through
    cases (missing, empty, non JWT or undecodable access token, missing or non
    numeric exp, non 200, non JSON and non dict bodies, an already truthful
    lifetime), the 60 second floor, non numeric expires_in rejection, and a round
    trip through a real AsyncOAuth2Client that exercises the hook on both grants.
  • Before promoting to prod, validate on staging: authenticate an MCP client, then
    confirm a tool call still succeeds past the one hour mark with no re-auth
    prompt, and that docker logs mpass-auth-proxy shows grant_type=refresh_token
    with no warning following it.
  • Worth watching on staging: FastMCP rotates its refresh token on single use, and
    this change makes that path run hourly where it previously never ran. Concurrent
    or retried refreshes would surface as invalid_grant.

mpass-auth-proxy reports the browser session cookie lifetime as expires_in,
but the Cognito token underneath still expires in an hour. FastMCP gates
transparent refresh on that value, so the refresh never fired and clients
were bounced through a full re-authorization every hour.

Register an authlib compliance hook on both the authorization-code and
refresh grants that derives expires_in from the access token's own exp.
Document the deliberate 60s floor window, drive the compliance hook through
a real authlib client on both grants, and wrap an over-length test line.
Treat sub-second truncation drift as an already-truthful lifetime so a
direct-to-Cognito response passes through untouched, document the deliberate
60s floor window, and correct the docstrings: renewal runs through the client's
refresh grant, not transparent refresh, because the client-facing token now
tracks the upstream hour. Cover the hook contract with a real authlib
round-trip on both grants.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant