Skip to content

fix (auth): refresh token expiry#680

Merged
alexhancock merged 1 commit into
modelcontextprotocol:mainfrom
ArcadeAI:fix/refresh-expiry
Feb 24, 2026
Merged

fix (auth): refresh token expiry#680
alexhancock merged 1 commit into
modelcontextprotocol:mainfrom
ArcadeAI:fix/refresh-expiry

Conversation

@wdawson
Copy link
Copy Markdown
Contributor

@wdawson wdawson commented Feb 23, 2026

OAuth token expiration was not being handled correctly.

Motivation and Context

Two bugs currently exist:

  1. Token expiry is never detected: expires_in() from the oauth2 crate returns the original duration from the token response (e.g., 3600s), not the time remaining. So the check if expires_in <= Duration::from_secs(0) is always false
  2. Refresh failures don't trigger re-auth: If refresh_token() fails (e.g., refresh token revoked), the error propagates as TokenRefreshFailed instead of AuthorizationRequired, so it's more difficult for the client to know when to re-prompt.

How Has This Been Tested?

  • Tested locally against an Arcade.dev MCP Gateway server
  • Added unit tests for the new behavior

Breaking Changes

Technically breaking, but should provide better behavior:

  • The AuthorizationRequired error now propagates instead of TokenRefreshFailed to provide a clear, consistent contract for clients

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Also contains the fix from @glicht in #678 (thanks!). That should merge first for proper attribution for that fix.

AI Disclosure: AI assisted with implementation and test authoring. All changes were reviewed and guided by hand.

@wdawson wdawson requested a review from a team as a code owner February 23, 2026 22:47
@github-actions github-actions Bot added T-core Core library changes T-transport Transport layer changes labels Feb 23, 2026
@glicht
Copy link
Copy Markdown
Contributor

glicht commented Feb 24, 2026

I also saw this behavior with the expiry. I wasn't fully sure if it was by design or a bug. My way to get around this was to implement at the credential store's load function to decrement the expire_in to reflect the actual number of seconds left for expiration (or zero once expired).

alexhancock
alexhancock approved these changes Feb 24, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@alexhancock alexhancock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good find, and thanks for preserving all the interaction with the credential store

@alexhancock alexhancock merged commit 83808d3 into modelcontextprotocol:main Feb 24, 2026
11 checks passed
@github-actions github-actions Bot mentioned this pull request Feb 24, 2026
Closed
@DaleSeo DaleSeo mentioned this pull request Feb 24, 2026
Merged
9 tasks
@github-actions github-actions Bot mentioned this pull request Feb 26, 2026
Merged
@david-engelmann david-engelmann mentioned this pull request Apr 29, 2026
Merged
2 tasks
peicodes pushed a commit to warpdotdev/warp that referenced this pull request May 12, 2026
@david-engelmann @claude
fix(mcp): persist token_received_at so OAuth refresh fires before exp…
edfd414 
…iry (#8863) (#9460)

## Description

Fixes #8863.

`PersistedCredentials` did not record when its token was received, and
`install_persisting_credential_store` hardcoded `token_received_at:
None` when seeding the rmcp credential store. The result: rmcp's
pre-emptive refresh check at
[`AuthorizationManager::get_access_token`](https://github.com/warpdotdev/rmcp/blob/c0f65dc/crates/rmcp/src/transport/auth.rs#L617-L633)
— which requires both `expires_in` and `token_received_at` — was skipped
for the lifetime of every OAuth-authenticated session.

After the first authorization, the cached access token was used past its
TTL. When a request finally hit a 401, rmcp's
[`handle_response`](https://github.com/warpdotdev/rmcp/blob/c0f65dc/crates/rmcp/src/transport/auth.rs#L710-L720)
returns `AuthorizationRequired` without attempting recovery — the user
sees their MCP server disconnect and is forced to re-authenticate. That
matches the reproducer in #8863 exactly: "after logging in the first
time if you wait 1hr… the MCP server disconnects and requires you to
login again."

The cached-credentials path partially masked the bug because of the
unconditional `auth_manager.refresh_token()` workaround at
[`oauth.rs:272`](app/src/ai/mcp/templatable_manager/oauth.rs#L272) —
that call writes back fresh credentials with `received_at` properly set,
papering over the missing seed value for the duration of that session.
The fresh-OAuth path has no such fallback, so users hit the bug on their
first session.

### Fix

1. Add `token_received_at: Option<u64>` to `PersistedCredentials`.
`#[serde(default)]` keeps existing on-disk credentials deserializable —
they come back with `None` and the next refresh populates the field.
2. Forward `token_received_at` from `StoredCredentials` through
`PersistingCredentialStore::save` to the persist channel so the value
reaches secure storage.
3. Thread `token_received_at` as a parameter into
`install_persisting_credential_store` and seed the inner store with it
instead of hardcoded `None`.
4. Cached-creds call site (`make_authenticated_client`): pass the value
loaded from `PersistedCredentials`.
5. Fresh-OAuth call site: stamp `token_received_at` to
`now_epoch_secs()` at the point we save the credentials, and pass the
same value into the seed.

Spec reference: [RFC 6749 §5.1
`expires_in`](https://datatracker.ietf.org/doc/html/rfc6749#section-5.1)
and [§6 refresh-token
semantics](https://datatracker.ietf.org/doc/html/rfc6749#section-6);
rmcp's pre-emptive refresh implements the recommended client behavior of
refreshing before expiry to avoid in-flight 401s.

### Things deliberately left alone

- The unconditional connect-time `auth_manager.refresh_token()` call at
[oauth.rs:272](app/src/ai/mcp/templatable_manager/oauth.rs#L272) and the
stale comment above it claiming rmcp's fork lacks expiry detection. The
fork at the pinned revision (c0f65dc, "Fixes for oauth expiry and
scopes") *does* now have proper detection per upstream
[rust-sdk#680](modelcontextprotocol/rust-sdk#680),
so the workaround is now redundant. Removing it is a behavior change
worth its own PR — out of scope here.
- 401-on-active-request retry. rmcp's `handle_response` still bubbles up
`AuthorizationRequired` rather than refresh-and-retry; pre-emptive
refresh covers the common case but not surprise expiries (clock skew,
server-side revocation). A reactive layer is a follow-up.

## Testing

Six new unit tests in `app/src/ai/mcp/templatable_manager/oauth.rs`:

- `persisted_credentials_round_trip_through_serde_preserves_received_at`
— value preservation.
- `persisted_credentials_deserializes_legacy_format_without_received_at`
— **regression-protects existing users on upgrade**: credentials
persisted by older Warp must still deserialize, with `received_at`
defaulting to `None`. Failing this test would mean existing users lose
their MCP OAuth tokens.
- `save_forwards_token_received_at_to_persist_channel` — the core
regression test for the fix.
- `save_forwards_none_when_received_at_is_none` — defensive: don't
substitute a fake value if rmcp passes `None`.
- `save_skips_persist_when_token_response_absent` — no regression of the
existing `if let Some(token_response)` branch.
- `save_carries_forward_refresh_token_and_preserves_received_at` —
combined check that the existing refresh-token carry-forward (RFC 6749
§6) and the new `received_at` propagation don't interfere.
- `now_epoch_secs_returns_recent_unix_time` — sanity check on the
helper.

I was not able to run `./script/presubmit` locally (`warpui` build needs
`xcrun metal` which requires full Xcode.app, only CommandLineTools is
installed here). Trusting CI for `cargo clippy --workspace --all-targets
--tests -- -D warnings` and `cargo nextest run --workspace`.

## Server API dependencies

N/A — client-only change.

- [x] Is this change necessary to make the client compatible with a
desired [server API breaking
change](https://www.notion.so/warpdev/How-to-safely-introduce-server-API-breaking-changes-0aa805ff5d5d41fd8834f3c95caba0b4):
**No**

## Agent Mode

- [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode

## Changelog Entries for Stable

CHANGELOG-BUG-FIX: Fix MCP OAuth servers disconnecting after
access-token TTL expires; refresh tokens are now used to obtain new
access tokens before expiry (#8863).

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stupidloud pushed a commit to stupidloud/warp-cn that referenced this pull request May 21, 2026
@david-engelmann @claude @stupidloud
fix(mcp): persist token_received_at so OAuth refresh fires before exp…
007d0ae 
…iry (#8863) (#9460)

## Description

Fixes #8863.

`PersistedCredentials` did not record when its token was received, and
`install_persisting_credential_store` hardcoded `token_received_at:
None` when seeding the rmcp credential store. The result: rmcp's
pre-emptive refresh check at
[`AuthorizationManager::get_access_token`](https://github.com/warpdotdev/rmcp/blob/c0f65dc/crates/rmcp/src/transport/auth.rs#L617-L633)
— which requires both `expires_in` and `token_received_at` — was skipped
for the lifetime of every OAuth-authenticated session.

After the first authorization, the cached access token was used past its
TTL. When a request finally hit a 401, rmcp's
[`handle_response`](https://github.com/warpdotdev/rmcp/blob/c0f65dc/crates/rmcp/src/transport/auth.rs#L710-L720)
returns `AuthorizationRequired` without attempting recovery — the user
sees their MCP server disconnect and is forced to re-authenticate. That
matches the reproducer in #8863 exactly: "after logging in the first
time if you wait 1hr… the MCP server disconnects and requires you to
login again."

The cached-credentials path partially masked the bug because of the
unconditional `auth_manager.refresh_token()` workaround at
[`oauth.rs:272`](app/src/ai/mcp/templatable_manager/oauth.rs#L272) —
that call writes back fresh credentials with `received_at` properly set,
papering over the missing seed value for the duration of that session.
The fresh-OAuth path has no such fallback, so users hit the bug on their
first session.

### Fix

1. Add `token_received_at: Option<u64>` to `PersistedCredentials`.
`#[serde(default)]` keeps existing on-disk credentials deserializable —
they come back with `None` and the next refresh populates the field.
2. Forward `token_received_at` from `StoredCredentials` through
`PersistingCredentialStore::save` to the persist channel so the value
reaches secure storage.
3. Thread `token_received_at` as a parameter into
`install_persisting_credential_store` and seed the inner store with it
instead of hardcoded `None`.
4. Cached-creds call site (`make_authenticated_client`): pass the value
loaded from `PersistedCredentials`.
5. Fresh-OAuth call site: stamp `token_received_at` to
`now_epoch_secs()` at the point we save the credentials, and pass the
same value into the seed.

Spec reference: [RFC 6749 §5.1
`expires_in`](https://datatracker.ietf.org/doc/html/rfc6749#section-5.1)
and [§6 refresh-token
semantics](https://datatracker.ietf.org/doc/html/rfc6749#section-6);
rmcp's pre-emptive refresh implements the recommended client behavior of
refreshing before expiry to avoid in-flight 401s.

### Things deliberately left alone

- The unconditional connect-time `auth_manager.refresh_token()` call at
[oauth.rs:272](app/src/ai/mcp/templatable_manager/oauth.rs#L272) and the
stale comment above it claiming rmcp's fork lacks expiry detection. The
fork at the pinned revision (c0f65dc, "Fixes for oauth expiry and
scopes") *does* now have proper detection per upstream
[rust-sdk#680](modelcontextprotocol/rust-sdk#680),
so the workaround is now redundant. Removing it is a behavior change
worth its own PR — out of scope here.
- 401-on-active-request retry. rmcp's `handle_response` still bubbles up
`AuthorizationRequired` rather than refresh-and-retry; pre-emptive
refresh covers the common case but not surprise expiries (clock skew,
server-side revocation). A reactive layer is a follow-up.

## Testing

Six new unit tests in `app/src/ai/mcp/templatable_manager/oauth.rs`:

- `persisted_credentials_round_trip_through_serde_preserves_received_at`
— value preservation.
- `persisted_credentials_deserializes_legacy_format_without_received_at`
— **regression-protects existing users on upgrade**: credentials
persisted by older Warp must still deserialize, with `received_at`
defaulting to `None`. Failing this test would mean existing users lose
their MCP OAuth tokens.
- `save_forwards_token_received_at_to_persist_channel` — the core
regression test for the fix.
- `save_forwards_none_when_received_at_is_none` — defensive: don't
substitute a fake value if rmcp passes `None`.
- `save_skips_persist_when_token_response_absent` — no regression of the
existing `if let Some(token_response)` branch.
- `save_carries_forward_refresh_token_and_preserves_received_at` —
combined check that the existing refresh-token carry-forward (RFC 6749
§6) and the new `received_at` propagation don't interfere.
- `now_epoch_secs_returns_recent_unix_time` — sanity check on the
helper.

I was not able to run `./script/presubmit` locally (`warpui` build needs
`xcrun metal` which requires full Xcode.app, only CommandLineTools is
installed here). Trusting CI for `cargo clippy --workspace --all-targets
--tests -- -D warnings` and `cargo nextest run --workspace`.

## Server API dependencies

N/A — client-only change.

- [x] Is this change necessary to make the client compatible with a
desired [server API breaking
change](https://www.notion.so/warpdev/How-to-safely-introduce-server-API-breaking-changes-0aa805ff5d5d41fd8834f3c95caba0b4):
**No**

## Agent Mode

- [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode

## Changelog Entries for Stable

CHANGELOG-BUG-FIX: Fix MCP OAuth servers disconnecting after
access-token TTL expires; refresh tokens are now used to obtain new
access tokens before expiry (#8863).

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lawsmd pushed a commit to lawsmd/cortex that referenced this pull request May 22, 2026
@david-engelmann @claude
fix(mcp): persist token_received_at so OAuth refresh fires before exp…
3407b25 
…iry (warpdotdev#8863) (warpdotdev#9460)

## Description

Fixes warpdotdev#8863.

`PersistedCredentials` did not record when its token was received, and
`install_persisting_credential_store` hardcoded `token_received_at:
None` when seeding the rmcp credential store. The result: rmcp's
pre-emptive refresh check at
[`AuthorizationManager::get_access_token`](https://github.com/warpdotdev/rmcp/blob/c0f65dc/crates/rmcp/src/transport/auth.rs#L617-L633)
— which requires both `expires_in` and `token_received_at` — was skipped
for the lifetime of every OAuth-authenticated session.

After the first authorization, the cached access token was used past its
TTL. When a request finally hit a 401, rmcp's
[`handle_response`](https://github.com/warpdotdev/rmcp/blob/c0f65dc/crates/rmcp/src/transport/auth.rs#L710-L720)
returns `AuthorizationRequired` without attempting recovery — the user
sees their MCP server disconnect and is forced to re-authenticate. That
matches the reproducer in warpdotdev#8863 exactly: "after logging in the first
time if you wait 1hr… the MCP server disconnects and requires you to
login again."

The cached-credentials path partially masked the bug because of the
unconditional `auth_manager.refresh_token()` workaround at
[`oauth.rs:272`](app/src/ai/mcp/templatable_manager/oauth.rs#L272) —
that call writes back fresh credentials with `received_at` properly set,
papering over the missing seed value for the duration of that session.
The fresh-OAuth path has no such fallback, so users hit the bug on their
first session.

### Fix

1. Add `token_received_at: Option<u64>` to `PersistedCredentials`.
`#[serde(default)]` keeps existing on-disk credentials deserializable —
they come back with `None` and the next refresh populates the field.
2. Forward `token_received_at` from `StoredCredentials` through
`PersistingCredentialStore::save` to the persist channel so the value
reaches secure storage.
3. Thread `token_received_at` as a parameter into
`install_persisting_credential_store` and seed the inner store with it
instead of hardcoded `None`.
4. Cached-creds call site (`make_authenticated_client`): pass the value
loaded from `PersistedCredentials`.
5. Fresh-OAuth call site: stamp `token_received_at` to
`now_epoch_secs()` at the point we save the credentials, and pass the
same value into the seed.

Spec reference: [RFC 6749 §5.1
`expires_in`](https://datatracker.ietf.org/doc/html/rfc6749#section-5.1)
and [§6 refresh-token
semantics](https://datatracker.ietf.org/doc/html/rfc6749#section-6);
rmcp's pre-emptive refresh implements the recommended client behavior of
refreshing before expiry to avoid in-flight 401s.

### Things deliberately left alone

- The unconditional connect-time `auth_manager.refresh_token()` call at
[oauth.rs:272](app/src/ai/mcp/templatable_manager/oauth.rs#L272) and the
stale comment above it claiming rmcp's fork lacks expiry detection. The
fork at the pinned revision (c0f65dc, "Fixes for oauth expiry and
scopes") *does* now have proper detection per upstream
[rust-sdk#680](modelcontextprotocol/rust-sdk#680),
so the workaround is now redundant. Removing it is a behavior change
worth its own PR — out of scope here.
- 401-on-active-request retry. rmcp's `handle_response` still bubbles up
`AuthorizationRequired` rather than refresh-and-retry; pre-emptive
refresh covers the common case but not surprise expiries (clock skew,
server-side revocation). A reactive layer is a follow-up.

## Testing

Six new unit tests in `app/src/ai/mcp/templatable_manager/oauth.rs`:

- `persisted_credentials_round_trip_through_serde_preserves_received_at`
— value preservation.
- `persisted_credentials_deserializes_legacy_format_without_received_at`
— **regression-protects existing users on upgrade**: credentials
persisted by older Warp must still deserialize, with `received_at`
defaulting to `None`. Failing this test would mean existing users lose
their MCP OAuth tokens.
- `save_forwards_token_received_at_to_persist_channel` — the core
regression test for the fix.
- `save_forwards_none_when_received_at_is_none` — defensive: don't
substitute a fake value if rmcp passes `None`.
- `save_skips_persist_when_token_response_absent` — no regression of the
existing `if let Some(token_response)` branch.
- `save_carries_forward_refresh_token_and_preserves_received_at` —
combined check that the existing refresh-token carry-forward (RFC 6749
§6) and the new `received_at` propagation don't interfere.
- `now_epoch_secs_returns_recent_unix_time` — sanity check on the
helper.

I was not able to run `./script/presubmit` locally (`warpui` build needs
`xcrun metal` which requires full Xcode.app, only CommandLineTools is
installed here). Trusting CI for `cargo clippy --workspace --all-targets
--tests -- -D warnings` and `cargo nextest run --workspace`.

## Server API dependencies

N/A — client-only change.

- [x] Is this change necessary to make the client compatible with a
desired [server API breaking
change](https://www.notion.so/warpdev/How-to-safely-introduce-server-API-breaking-changes-0aa805ff5d5d41fd8834f3c95caba0b4):
**No**

## Agent Mode

- [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode

## Changelog Entries for Stable

CHANGELOG-BUG-FIX: Fix MCP OAuth servers disconnecting after
access-token TTL expires; refresh tokens are now used to obtain new
access tokens before expiry (warpdotdev#8863).

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexhancock alexhancock alexhancock approved these changes

Assignees

No one assigned

Labels

T-core Core library changes T-transport Transport layer changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wdawson @glicht @alexhancock