Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
"editor.defaultFormatter": "esbenp.prettier-vscode",
"cSpell.words": [
"bunx",
"EADDRINUSE",
"konstantin",
"kriasoft",
"modelcontextprotocol",
"myapp",
"PKCE",
"publint",
"srcpack",
"streamable",
Expand Down
7 changes: 6 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# OAuth Callback Project Guide

## Documentation

**ADRs** (`docs/adr/NNN-slug.md`): Architectural decisions (reference as ADR-NNN)
**SPECs** (`docs/specs/slug.md`): Design specifications (reference as SPEC-slug)

## Project Structure

```bash
Expand Down Expand Up @@ -56,7 +61,7 @@ oauth-callback/
- `browserAuth()` - MCP SDK-compatible OAuth provider
- `inMemoryStore()` - Ephemeral token storage
- `fileStore()` - Persistent file-based token storage
- Type exports: `BrowserAuthOptions`, `Tokens`, `TokenStore`, `ClientInfo`, `OAuthSession`, `OAuthStore`
- Type exports: `BrowserAuthOptions`, `Tokens`, `TokenStore`, `ClientInfo`, `OAuthStore`

## Key Constraints

Expand Down
29 changes: 21 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ import { browserAuth, inMemoryStore } from "oauth-callback/mcp";
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js";

const serverUrl = new URL("https://mcp.notion.com/mcp");

// Create MCP-compatible OAuth provider
const authProvider = browserAuth({
port: 3000,
Expand All @@ -135,18 +137,29 @@ const authProvider = browserAuth({
store: inMemoryStore(), // Or fileStore() for persistence
});

// Use with MCP SDK transport
const transport = new StreamableHTTPClientTransport(
new URL("https://mcp.notion.com/mcp"),
{ authProvider },
);

const client = new Client(
{ name: "my-app", version: "1.0.0" },
{ capabilities: {} },
);

await client.connect(transport);
// Connect with OAuth retry: first attempt completes OAuth and saves tokens,
// but SDK returns before checking them. Second attempt succeeds.
async function connectWithOAuthRetry() {
const transport = new StreamableHTTPClientTransport(serverUrl, {
authProvider,
});
try {
await client.connect(transport);
} catch (error: any) {
if (error.message === "Unauthorized") {
await client.connect(
new StreamableHTTPClientTransport(serverUrl, { authProvider }),
);
} else throw error;
}
}

await connectWithOAuthRetry();
```

#### Token Storage Options
Expand Down Expand Up @@ -275,7 +288,7 @@ class OAuthError extends Error {

### `browserAuth(options)`

Available from `oauth-callback/mcp`. Creates an MCP SDK-compatible OAuth provider for browser-based flows. Handles Dynamic Client Registration (DCR), token storage, and automatic refresh.
Available from `oauth-callback/mcp`. Creates an MCP SDK-compatible OAuth provider for browser-based flows. Handles Dynamic Client Registration (DCR) and token storage. Expired tokens trigger re-authentication.

#### Parameters

Expand Down
3 changes: 2 additions & 1 deletion docs/.vitepress/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export default withMermaid(
{ text: "API", link: "/api/get-auth-code" },
{ text: "Examples", link: "/examples/notion" },
{
text: "v2.0.0",
text: "v2.2.0",
items: [
{
text: "Release Notes",
Expand All @@ -82,6 +82,7 @@ export default withMermaid(
},
{ text: "Getting Started", link: "/getting-started" },
{ text: "Core Concepts", link: "/core-concepts" },
{ text: "ADRs", link: "/adr/" },
],
},
{
Expand Down
28 changes: 28 additions & 0 deletions docs/adr/000-template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# ADR-NNN Title

**Status:** Proposed | Accepted | Deprecated | Superseded
**Date:** YYYY-MM-DD
**Tags:** tag1, tag2

## Problem

- One or two sentences on the decision trigger or constraint.

## Decision

- The chosen approach in a short paragraph.

## Alternatives (brief)

- Option A — why not.
- Option B — why not.

## Impact

- Positive:
- Negative/Risks:

## Links

- Code/Docs:
- Related ADRs:
37 changes: 37 additions & 0 deletions docs/adr/001-no-refresh-tokens.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# ADR-001: No Refresh Tokens in browserAuth

**Status:** Accepted
**Date:** 2025-01-25
**Tags:** oauth, mcp, security

## Problem

- Should `browserAuth` handle OAuth refresh tokens and automatic token renewal?

## Decision

- `browserAuth` intentionally does not request or handle refresh tokens.
- When tokens expire, `tokens()` returns `undefined`, signaling the MCP SDK to re-authenticate.
- The SDK's built-in retry logic handles re-auth transparently.

Rationale:

- **MCP SDK lifecycle**: The SDK expects auth providers to return `undefined` for expired tokens, triggering its standard re-auth flow.
- **CLI/desktop UX**: Interactive re-consent is acceptable and often preferred over silent background refresh.
- **Simplicity**: Avoiding refresh eliminates token rotation complexity, race conditions, and concurrent refresh handling.
- **Security**: No long-lived refresh tokens stored; each session requires explicit user consent.

## Alternatives (brief)

- **Implement refresh flow** — Adds complexity (token rotation, concurrency), conflicts with SDK's re-auth expectations, stores long-lived credentials.
- **Optional refresh via config** — Increases API surface, creates two divergent code paths to maintain.

## Impact

- Positive: Simpler implementation, predictable behavior, aligns with MCP SDK design.
- Negative/Risks: More frequent browser prompts for long-running sessions (acceptable for CLI tools).

## Links

- Code: `src/auth/browser-auth.ts`
- Related: MCP SDK auth interface in `@modelcontextprotocol/sdk/client/auth`
67 changes: 67 additions & 0 deletions docs/adr/002-immediate-token-exchange.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# ADR-002: Immediate Token Exchange in redirectToAuthorization()

**Status:** Accepted
**Date:** 2025-01-25
**Tags:** oauth, mcp, sdk-integration

## Problem

The MCP SDK's `auth()` flow works as follows:

1. Check `provider.tokens()` — if valid tokens exist, return `'AUTHORIZED'`
2. If no tokens, start authorization: call `redirectToAuthorization(url)`
3. **Immediately** return `'REDIRECT'` (without re-checking tokens)

For web-based OAuth, this makes sense: `redirectToAuthorization()` triggers a page redirect and control never returns synchronously. The SDK expects authentication to complete in a subsequent request.

For CLI/desktop apps using `browserAuth()`, control **does** return synchronously—we capture the callback in-process via a local HTTP server. We exchange tokens inside `redirectToAuthorization()`, but the SDK has already decided to return `'REDIRECT'`, causing `UnauthorizedError`.

## Decision

Exchange tokens **inside** `redirectToAuthorization()` and document the retry pattern as the expected usage:

```typescript
// First connect triggers OAuth flow and saves tokens, but SDK returns
// 'REDIRECT' before checking. Second connect finds valid tokens.
async function connectWithOAuthRetry(client, serverUrl, authProvider) {
const transport = new StreamableHTTPClientTransport(serverUrl, {
authProvider,
});
try {
await client.connect(transport);
} catch (error) {
if (error.message === "Unauthorized") {
await client.connect(
new StreamableHTTPClientTransport(serverUrl, { authProvider }),
);
} else throw error;
}
}
```

**Why a new transport on retry?** The transport caches connection state internally. A fresh transport ensures clean reconnection.

## Rationale

- **SDK constraint**: No hook exists between redirect completion and the `'REDIRECT'` return. The SDK interface (`Promise<void>`) cannot signal "auth completed."
- **In-process capture**: CLI apps don't have page redirects that would trigger a fresh auth check cycle.
- **Correctness over elegance**: The retry is unusual but reliable—tokens are always saved before the error.

## Alternatives Considered

| Alternative | Why Rejected |
| ---------------------------------------------- | ------------------------------------------------------------- |
| `transport.finishAuth(callbackUrl)` | Breaks provider encapsulation; doesn't fit in-process capture |
| Return tokens from `redirectToAuthorization()` | SDK interface expects `Promise<void>` |
| Upstream SDK change | Not viable for library consumers |

## Impact

- **Positive**: Self-contained auth flow; no external coordination needed
- **Negative**: First connection always throws `UnauthorizedError` after OAuth—must be documented clearly

## Links

- Code: `src/auth/browser-auth.ts` lines 254-368
- MCP SDK auth interface: `@modelcontextprotocol/sdk/client/auth.js`
- Related: ADR-001 (no refresh tokens)
31 changes: 31 additions & 0 deletions docs/adr/003-stable-client-metadata.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# ADR-003: Stable Client Metadata Across DCR

**Status:** Accepted
**Date:** 2025-01-25
**Tags:** oauth, dcr, security

## Problem

- During Dynamic Client Registration (DCR), the authorization server may return different capabilities than requested (e.g., `token_endpoint_auth_method`).
- If `clientMetadata` adapts to DCR responses, subsequent token requests may fail when the AS caches the original registration metadata.

## Decision

- `clientMetadata` is immutable after construction.
- `token_endpoint_auth_method` is determined at construction: `client_secret_post` if `clientSecret` is provided, `none` otherwise. DCR responses never change this value.
- DCR credentials (`client_id`, `client_secret`) are stored separately and never mutate the auth method.

## Alternatives (brief)

- **Dynamic metadata evolution** — Adapting to DCR responses seems flexible but causes cache mismatches with AS that remember original registration.
- **Per-request method detection** — Adds complexity and non-deterministic behavior across retries.

## Impact

- Positive: Predictable behavior with all AS implementations; eliminates cache-related auth failures.
- Negative/Risks: None identified; the fixed method (`client_secret_post`) has universal support.

## Links

- Code: `src/auth/browser-auth.ts`
- Related ADRs: ADR-001 (No Refresh Tokens), ADR-002 (Immediate Token Exchange)
40 changes: 40 additions & 0 deletions docs/adr/004-conditional-state-validation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# ADR-004: Conditional OAuth State Validation

**Status:** Accepted
**Date:** 2025-01-25
**Tags:** oauth, security, csrf

## Problem

- RFC 6749 recommends `state` for CSRF protection, but RFC 8252 (native apps) relies on loopback redirect for security.
- Some authorization servers don't echo `state` back; others require it.
- Strict validation breaks compatibility; no validation weakens security.

## Decision

- Validate `state` only if it was present in the authorization URL.
- If the auth URL includes `state` and the callback doesn't match, reject as CSRF.
- If the auth URL omits `state`, accept callbacks without state validation.

Rationale:

- **Defense-in-depth**: Loopback binding (127.0.0.1) prevents network CSRF, but state adds protection against local attacks (malicious apps, browser extensions intercepting localhost).
- **CLI threat model**: Unlike web apps, CLI tools face local machine threats—other processes can probe localhost ports. State validation detects if a callback arrives from an unrelated auth flow.
- **Compatibility**: Authorization servers have inconsistent state handling. Conditional validation works with all servers while providing protection when available.

## Alternatives (brief)

- **Always require state** — Breaks servers that don't echo state or don't support it.
- **Never validate state** — Loopback provides baseline security, but ignores state when the AS cooperates.
- **Generate state internally always** — Conflicts with auth URLs that already include state from the MCP SDK.

## Impact

- Positive: Maximum security when AS supports state; universal compatibility otherwise.
- Negative/Risks: If an AS echoes arbitrary state values without validation, the protection is weaker (rare edge case).

## Links

- Code: `src/auth/browser-auth.ts:297-300`
- RFC 6749 Section 10.12 (CSRF Protection)
- RFC 8252 Section 8.1 (Loopback Redirect)
36 changes: 36 additions & 0 deletions docs/adr/005-store-responsibility-reduction.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# ADR-005: OAuthStore Responsibility Reduction

**Status:** Accepted
**Date:** 2025-01-25
**Tags:** api, storage, simplification

## Problem

The store was accumulating OAuth flow state (session, nonce, state parameter) alongside persistent data (tokens, client registration). This blurred the line between "what survives a crash" and "what's ephemeral by design," making the API harder to reason about and test.

## Decision

The store is responsible **only** for data that must survive process restarts:

| Stored | Not Stored |
| --------------------- | ----------------- |
| `tokens` | `state` parameter |
| `client` (DCR result) | `nonce` |
| `codeVerifier` (PKCE) | session objects |

The `codeVerifier` is the sole flow artifact persisted—it enables completing an in-progress authorization if the process crashes after browser launch but before callback.

## Alternatives (brief)

- **Full session persistence** — Would enable crash-recovery at any point, but adds complexity for a rare edge case. Users can simply restart the flow.
- **No verifier persistence** — Simpler, but loses the most common crash scenario (user switches apps, process dies).

## Impact

- Positive: Cleaner mental model; store implementations are trivial to write and test.
- Negative: If the process crashes before `codeVerifier` is saved, the flow must restart. This is acceptable—it's a sub-second window.

## Links

- Code: `src/storage/`, `src/mcp-types.ts`
- Related: ADR-002 (Immediate Token Exchange)
11 changes: 11 additions & 0 deletions docs/adr/index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Architecture Decision Records

Key design decisions with context and rationale.

| ADR | Decision |
| ---------------------------------------------- | ----------------------------------------------------- |
| [001](./001-no-refresh-tokens.md) | No refresh tokens—rely on MCP SDK's re-auth flow |
| [002](./002-immediate-token-exchange.md) | Token exchange inside `redirectToAuthorization()` |
| [003](./003-stable-client-metadata.md) | Immutable client metadata across DCR |
| [004](./004-conditional-state-validation.md) | Validate `state` only when present in auth URL |
| [005](./005-store-responsibility-reduction.md) | Store persists only tokens, client, and PKCE verifier |
Loading