Skip to content

Immediate token exchange and simplified storage API#34

Merged
koistya merged 2 commits intomainfrom
mcp
Jan 25, 2026
Merged

Immediate token exchange and simplified storage API#34
koistya merged 2 commits intomainfrom
mcp

Conversation

@koistya
Copy link
Member

@koistya koistya commented Jan 25, 2026

Problem

The MCP SDK's auth() flow creates a timing issue for CLI/desktop apps:

  1. SDK checks provider.tokens() — if valid, returns 'AUTHORIZED'
  2. If no tokens, calls redirectToAuthorization(url)
  3. Immediately returns 'REDIRECT' (without re-checking tokens)

For web OAuth, this works: the redirect happens and the SDK never sees a synchronous return. For CLI apps using browserAuth(), we capture the callback in-process, but the SDK has already decided to return 'REDIRECT', causing UnauthorizedError.

Solution

Exchange tokens inside redirectToAuthorization() and save them to storage. While the first connect() still throws UnauthorizedError (SDK constraint), subsequent calls find valid tokens:

const transport = new StreamableHTTPClientTransport(serverUrl, { authProvider });
try {
  await client.connect(transport);
} catch (error) {
  if (error.message === "Unauthorized") {
    // Tokens acquired during first attempt; retry succeeds
    await client.connect(
      new StreamableHTTPClientTransport(serverUrl, { authProvider })
    );
  } else throw error;
}

Changes

OAuth Flow

  • Complete token exchange synchronously within redirectToAuthorization()
  • Add CSRF protection via state parameter validation
  • Remove refresh_token grant type (SDK handles re-auth, see ADR-001)
  • New authServerUrl option for separate auth/token endpoint origins

Storage API (breaking)

  • Remove clear() from TokenStore interface
  • Add explicit deleteClient() and deleteCodeVerifier() to OAuthStore
  • Use symbol brand for type-safe OAuthStore detection vs duck typing
  • Atomic file writes via temp file + rename

Bug Fixes

  • invalidateCredentials('all') was clearing the entire store; now only clears the provider's own storeKey
  • Token expiry check was reading from store unnecessarily; now uses in-memory expiresAt

Auth Flow Diagram

sequenceDiagram
    participant App
    participant SDK as MCP SDK
    participant Provider as browserAuth
    participant Browser
    participant AuthServer as Auth Server
    
    App->>SDK: client.connect(transport)
    SDK->>Provider: tokens()
    Provider-->>SDK: undefined (no tokens)
    SDK->>Provider: redirectToAuthorization(url)
    Provider->>Browser: Open authorization URL
    Browser->>AuthServer: User authenticates
    AuthServer->>Provider: Callback with code
    Note over Provider: Validate state (CSRF)
    Provider->>AuthServer: Exchange code for tokens
    AuthServer-->>Provider: Access token
    Provider->>Provider: saveTokens()
    Provider-->>SDK: void
    SDK-->>App: throws UnauthorizedError
    Note over App: Tokens now in storage
    App->>SDK: client.connect(newTransport)
    SDK->>Provider: tokens()
    Provider-->>SDK: {access_token: "..."}
    SDK-->>App: Connected ✓
Loading

ADRs

This PR includes architectural decision records documenting the rationale:

ADR Decision
ADR-001 No refresh tokens—SDK handles re-auth
ADR-002 Immediate token exchange in redirectToAuthorization()
ADR-003 Client metadata fixed at construction
ADR-004 State validation when present in auth URL
ADR-005 Store only persists tokens, client, and PKCE verifier

https://www.linkedin.com/posts/koistya_opensource-typescript-oauth-activity-7421227824482734080-9Tyb

Complete OAuth flow synchronously within redirectToAuthorization() instead
of caching auth codes for SDK's separate token exchange. This addresses the
MCP SDK's design where auth() returns 'REDIRECT' immediately after the call
without re-checking for tokens.

Key changes:
- Exchange auth code for tokens inside redirectToAuthorization()
- Add state parameter validation (CSRF protection)
- Remove clear() from TokenStore, add explicit delete methods to OAuthStore
- Use symbol brand for type-safe OAuthStore detection
- Fix invalidateCredentials('all') to only clear own storeKey, not entire store
- Add authServerUrl option for separate auth/token endpoints
- Atomic file writes via temp file + rename

See ADR-001 through ADR-005 for detailed rationale.
@koistya koistya merged commit aa06439 into main Jan 25, 2026
4 checks passed
@koistya koistya deleted the mcp branch January 25, 2026 16:04
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