Skip to content

Commit 79cc494

Browse files
committed
fix: keep request pending during OAuth redirect and fix e2e tests
* StreamableHTTPClientTransport._send and _startOrAuthSse now wait for finishAuth() rather than rejecting immediately with UnauthorizedError when a REDIRECT occurs. * Refactored OAuth test suite to support asynchronous redirect flows. Tests now hang gracefully during REDIRECT, waiting for finishAuth() rather than asserting immediate rejections. * Updated simpleOAuthClient example to accurately reflect the new asynchronous flow. * Fixed sporadic vitest timeouts and FakeTimers leakage by properly managing unhandled transport promises during tests.
1 parent d041db4 commit 79cc494

5 files changed

Lines changed: 139 additions & 67 deletions

File tree

examples/oauth/client.ts

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,11 @@ const client = new Client(
9696
// `redirectToAuthorization` — in `simpleOAuthClient.ts` that opens a browser;
9797
// here we just capture it.
9898
let capturedAuthorizationUrl: URL | undefined;
99+
let urlCapturedCallback: () => void;
100+
const urlCapturedPromise = new Promise<void>(resolve => {
101+
urlCapturedCallback = resolve;
102+
});
103+
99104
const clientMetadata: OAuthClientMetadata = {
100105
client_name: 'Headless OAuth MCP Client (CI)',
101106
redirect_uris: [CALLBACK_URL],
@@ -106,24 +111,20 @@ const clientMetadata: OAuthClientMetadata = {
106111
};
107112
const provider = new InMemoryOAuthClientProvider(CALLBACK_URL, clientMetadata, authUrl => {
108113
capturedAuthorizationUrl = authUrl;
114+
urlCapturedCallback();
109115
});
110116

111117
const firstTransport = new StreamableHTTPClientTransport(new globalThis.URL(url), { authProvider: provider });
112-
let challenged = false;
113-
try {
114-
await client.connect(firstTransport);
115-
} catch (error) {
116-
// Both `--legacy` and `mode: 'auto'` surface the original
117-
// `UnauthorizedError` directly (the negotiation probe propagates it
118-
// unchanged; older releases wrapped it as the `data.cause` of an
119-
// EraNegotiationFailed `SdkError`, which the unwrap below still
120-
// tolerates). Either way the auth driver has already run by the time we
121-
// land here — DCR done, auth URL captured.
122-
const root = error instanceof UnauthorizedError ? error : (error as { data?: { cause?: unknown } }).data?.cause;
123-
if (!(root instanceof UnauthorizedError)) throw error;
124-
challenged = true;
125-
}
126-
check.ok(challenged, 'first connect must 401 and throw UnauthorizedError');
118+
const connectPromise = client.connect(firstTransport).catch(error => {
119+
// Both `--legacy` and `mode: 'auto'` surfaced the original
120+
// `UnauthorizedError` directly before the pending contract change.
121+
// Now they should just succeed after finishAuth completes!
122+
throw error;
123+
});
124+
125+
// Give the async connect time to hit the auth wall and trigger the callback
126+
await urlCapturedPromise;
127+
127128
check.ok(capturedAuthorizationUrl, 'SDK auth driver should have produced an authorization URL');
128129
check.ok(provider.clientInformation()?.client_id, 'dynamic client registration should have run');
129130

@@ -139,6 +140,9 @@ const callbackParams = await followAuthorizationRedirects(capturedAuthorizationU
139140
// (+ PKCE `code_verifier`) to the AS `/token` endpoint and saves the tokens
140141
// on `provider`.
141142
await firstTransport.finishAuth(callbackParams);
143+
144+
// The original connect should now complete!
145+
await connectPromise;
142146
const tokens = provider.tokens();
143147
check.ok(tokens?.access_token, 'token exchange should have yielded an access_token');
144148
check.equal(tokens?.token_type, 'Bearer');

examples/oauth/simpleOAuthClient.ts

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -136,32 +136,14 @@ class InteractiveOAuthClient {
136136
});
137137
}
138138

139-
private async attemptConnection(oauthProvider: InMemoryOAuthClientProvider): Promise<void> {
140-
console.log('🚢 Creating transport with OAuth provider...');
141-
const baseUrl = new URL(this.serverUrl);
142-
const transport = new StreamableHTTPClientTransport(baseUrl, {
143-
authProvider: oauthProvider
144-
});
145-
console.log('🚢 Transport created');
146-
139+
private async attemptConnection(transport: StreamableHTTPClientTransport): Promise<void> {
147140
try {
148141
console.log('🔌 Attempting connection (this will trigger OAuth redirect)...');
149142
await this.client!.connect(transport);
150143
console.log('✅ Connected successfully');
151144
} catch (error) {
152-
if (error instanceof UnauthorizedError) {
153-
console.log('🔐 OAuth required - waiting for authorization...');
154-
const callbackParams = await this.waitForOAuthCallback();
155-
// Pass the whole callback query — the SDK extracts `code` and validates
156-
// `iss` against the recorded issuer (RFC 9207) before exchanging the code.
157-
await transport.finishAuth(callbackParams);
158-
console.log('🔐 Authorization code received:', callbackParams.get('code'));
159-
console.log('🔌 Reconnecting with authenticated transport...');
160-
await this.attemptConnection(oauthProvider);
161-
} else {
162-
console.error('❌ Connection failed with non-auth error:', error);
163-
throw error;
164-
}
145+
console.error('❌ Connection failed:', error);
146+
throw error;
165147
}
166148
}
167149

@@ -180,18 +162,35 @@ class InteractiveOAuthClient {
180162
token_endpoint_auth_method: 'client_secret_post'
181163
};
182164

165+
let currentTransport: StreamableHTTPClientTransport;
166+
183167
console.log('🔐 Creating OAuth provider...');
184168
const oauthProvider = new InMemoryOAuthClientProvider(
185169
CALLBACK_URL,
186170
clientMetadata,
187171
(redirectUrl: URL) => {
188172
console.log(`📌 OAuth redirect handler called - opening browser`);
189173
console.log(`Opening browser to: ${redirectUrl.toString()}`);
174+
175+
console.log('🔐 OAuth required - waiting for authorization...');
176+
this.waitForOAuthCallback().then(async callbackParams => {
177+
console.log('🔐 Authorization code received:', callbackParams.get('code'));
178+
await currentTransport.finishAuth(callbackParams);
179+
console.log('🔌 Authentication complete!');
180+
}).catch(err => {
181+
console.error("❌ OAuth flow failed:", err);
182+
});
183+
190184
this.openBrowser(redirectUrl.toString());
191185
},
192186
this.clientMetadataUrl
193187
);
194188
console.log('🔐 OAuth provider created');
189+
190+
const baseUrl = new URL(this.serverUrl);
191+
currentTransport = new StreamableHTTPClientTransport(baseUrl, {
192+
authProvider: oauthProvider
193+
});
195194

196195
console.log('👤 Creating MCP client...');
197196
this.client = new Client(
@@ -205,7 +204,7 @@ class InteractiveOAuthClient {
205204

206205
console.log('🔐 Starting OAuth flow...');
207206

208-
await this.attemptConnection(oauthProvider);
207+
await this.attemptConnection(currentTransport);
209208

210209
// Start interactive loop
211210
await this.interactiveLoop();

packages/client/src/client/streamableHttp.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,17 @@ export class StreamableHTTPClientTransport implements Transport {
616616
{ scope, resourceMetadataUrl, errorDescription, statusText: response.statusText, text },
617617
stepUpRetries
618618
);
619-
if (result !== 'AUTHORIZED') {
619+
if (result === 'REDIRECT') {
620+
if (!this._pendingAuthPromise) {
621+
this._pendingAuthPromise = new Promise<void>((resolve, reject) => {
622+
this._authResolve = resolve;
623+
this._authReject = reject;
624+
});
625+
this._pendingAuthPromise.catch(() => {});
626+
}
627+
await this._pendingAuthPromise;
628+
return this._startOrAuthSse(options, isAuthRetry, stepUpRetries + 1);
629+
} else if (result !== 'AUTHORIZED') {
620630
throw markAuthSeamEscape(new UnauthorizedError());
621631
}
622632
return this._startOrAuthSse(options, isAuthRetry, stepUpRetries + 1);
@@ -1124,7 +1134,17 @@ export class StreamableHTTPClientTransport implements Transport {
11241134
{ scope, resourceMetadataUrl, errorDescription, statusText: response.statusText, text },
11251135
stepUpRetries
11261136
);
1127-
if (result !== 'AUTHORIZED') {
1137+
if (result === 'REDIRECT') {
1138+
if (!this._pendingAuthPromise) {
1139+
this._pendingAuthPromise = new Promise<void>((resolve, reject) => {
1140+
this._authResolve = resolve;
1141+
this._authReject = reject;
1142+
});
1143+
this._pendingAuthPromise.catch(() => {});
1144+
}
1145+
await this._pendingAuthPromise;
1146+
return this._send(message, options, isAuthRetry, stepUpRetries + 1);
1147+
} else if (result !== 'AUTHORIZED') {
11281148
throw markAuthSeamEscape(new UnauthorizedError());
11291149
}
11301150
return this._send(message, options, isAuthRetry, stepUpRetries + 1);

0 commit comments

Comments
 (0)