Skip to content

Commit 1a3d97f

Browse files
peterkarman1geelen
authored andcommitted
feat(auth-timeout): make callback timeout configurable
1 parent 01e240f commit 1a3d97f

File tree

7 files changed

+196
-7
lines changed

7 files changed

+196
-7
lines changed

README.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,17 @@ You can specify multiple `--ignore-tool` flags to ignore different patterns. Exa
153153
- `*account` - ignores all tools ending with "account" (e.g., `getAccount`, `updateAccount`)
154154
- `exactTool` - ignores only the tool named exactly "exactTool"
155155

156+
* To change the timeout for the OAuth callback (by default `30` seconds), add the `--auth-timeout` flag with a value in seconds. This is useful if the authentication process on the server side takes a long time.
157+
158+
```json
159+
"args": [
160+
"mcp-remote",
161+
"https://remote.mcp.server/sse",
162+
"--auth-timeout",
163+
"60"
164+
]
165+
```
166+
156167
### Transport Strategies
157168

158169
MCP Remote supports different transport strategies when connecting to an MCP server. This allows you to control whether it uses Server-Sent Events (SSE) or HTTP transport, and in what order it tries them.

src/client.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ async function runClient(
3636
host: string,
3737
staticOAuthClientMetadata: StaticOAuthClientMetadata,
3838
staticOAuthClientInfo: StaticOAuthClientInformationFull,
39+
authTimeoutMs: number,
3940
) {
4041
// Set up event emitter for auth flow
4142
const events = new EventEmitter()
@@ -44,7 +45,7 @@ async function runClient(
4445
const serverUrlHash = getServerUrlHash(serverUrl)
4546

4647
// Create a lazy auth coordinator
47-
const authCoordinator = createLazyAuthCoordinator(serverUrlHash, callbackPort, events)
48+
const authCoordinator = createLazyAuthCoordinator(serverUrlHash, callbackPort, events, authTimeoutMs)
4849

4950
// Create the OAuth client provider
5051
const authProvider = new NodeOAuthClientProvider({
@@ -159,8 +160,8 @@ async function runClient(
159160

160161
// Parse command-line arguments and run the client
161162
parseCommandLineArgs(process.argv.slice(2), 'Usage: npx tsx client.ts <https://server-url> [callback-port] [--debug]')
162-
.then(({ serverUrl, callbackPort, headers, transportStrategy, host, staticOAuthClientMetadata, staticOAuthClientInfo }) => {
163-
return runClient(serverUrl, callbackPort, headers, transportStrategy, host, staticOAuthClientMetadata, staticOAuthClientInfo)
163+
.then(({ serverUrl, callbackPort, headers, transportStrategy, host, staticOAuthClientMetadata, staticOAuthClientInfo, authTimeoutMs }) => {
164+
return runClient(serverUrl, callbackPort, headers, transportStrategy, host, staticOAuthClientMetadata, staticOAuthClientInfo, authTimeoutMs)
164165
})
165166
.catch((error) => {
166167
console.error('Fatal error:', error)

src/lib/coordination.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,12 @@ export async function waitForAuthentication(port: number): Promise<boolean> {
129129
* @param events The event emitter to use for signaling
130130
* @returns An AuthCoordinator object with an initializeAuth method
131131
*/
132-
export function createLazyAuthCoordinator(serverUrlHash: string, callbackPort: number, events: EventEmitter): AuthCoordinator {
132+
export function createLazyAuthCoordinator(
133+
serverUrlHash: string,
134+
callbackPort: number,
135+
events: EventEmitter,
136+
authTimeoutMs: number,
137+
): AuthCoordinator {
133138
let authState: { server: Server; waitForAuthCode: () => Promise<string>; skipBrowserAuth: boolean } | null = null
134139

135140
return {
@@ -144,7 +149,7 @@ export function createLazyAuthCoordinator(serverUrlHash: string, callbackPort: n
144149
if (DEBUG) debugLog('Initializing auth coordination on-demand', { serverUrlHash, callbackPort })
145150

146151
// Initialize auth using the existing coordinateAuth logic
147-
authState = await coordinateAuth(serverUrlHash, callbackPort, events)
152+
authState = await coordinateAuth(serverUrlHash, callbackPort, events, authTimeoutMs)
148153
if (DEBUG) debugLog('Auth coordination completed', { skipBrowserAuth: authState.skipBrowserAuth })
149154
return authState
150155
},
@@ -162,6 +167,7 @@ export async function coordinateAuth(
162167
serverUrlHash: string,
163168
callbackPort: number,
164169
events: EventEmitter,
170+
authTimeoutMs: number,
165171
): Promise<{ server: Server; waitForAuthCode: () => Promise<string>; skipBrowserAuth: boolean }> {
166172
if (DEBUG) debugLog('Coordinating authentication', { serverUrlHash, callbackPort })
167173

@@ -228,6 +234,7 @@ export async function coordinateAuth(
228234
port: callbackPort,
229235
path: '/oauth/callback',
230236
events,
237+
authTimeoutMs,
231238
})
232239

233240
// Get the actual port the server is running on

src/lib/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ export interface OAuthCallbackServerOptions {
4343
path: string
4444
/** Event emitter to signal when auth code is received */
4545
events: EventEmitter
46+
/** Timeout in milliseconds for the auth callback server's long poll */
47+
authTimeoutMs?: number
4648
}
4749

4850
// optional tatic OAuth client information

src/lib/utils.test.ts

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { describe, it, expect, vi } from 'vitest'
22
import { parseCommandLineArgs, shouldIncludeTool, mcpProxy } from './utils'
33
import { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'
4+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
5+
import { parseCommandLineArgs, setupOAuthCallbackServerWithLongPoll } from './utils'
6+
import { EventEmitter } from 'events'
7+
import express from 'express'
48

59
// All sanitizeUrl tests have been moved to the strict-url-sanitise package
610

@@ -773,3 +777,150 @@ describe('Feature: MCP Proxy', () => {
773777
)
774778
})
775779
})
780+
781+
describe('parseCommandLineArgs', () => {
782+
const mockUsage = 'Usage: test <url>'
783+
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
784+
throw new Error('process.exit called')
785+
})
786+
787+
beforeEach(() => {
788+
vi.clearAllMocks()
789+
})
790+
791+
afterEach(() => {
792+
mockExit.mockReset()
793+
})
794+
795+
describe('--auth-timeout parsing', () => {
796+
it('should use default timeout of 30000ms when no --auth-timeout flag is provided', async () => {
797+
const args = ['https://example.com']
798+
const result = await parseCommandLineArgs(args, mockUsage)
799+
800+
expect(result.authTimeoutMs).toBe(30000)
801+
})
802+
803+
it('should parse valid timeout in seconds and convert to milliseconds', async () => {
804+
const args = ['https://example.com', '--auth-timeout', '60']
805+
const result = await parseCommandLineArgs(args, mockUsage)
806+
807+
expect(result.authTimeoutMs).toBe(60000)
808+
})
809+
810+
it('should parse another valid timeout value', async () => {
811+
const args = ['https://example.com', '--auth-timeout', '120']
812+
const result = await parseCommandLineArgs(args, mockUsage)
813+
814+
expect(result.authTimeoutMs).toBe(120000)
815+
})
816+
817+
it('should use default timeout when invalid timeout value is provided', async () => {
818+
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
819+
820+
const args = ['https://example.com', '--auth-timeout', 'invalid']
821+
const result = await parseCommandLineArgs(args, mockUsage)
822+
823+
expect(result.authTimeoutMs).toBe(30000)
824+
expect(consoleSpy).toHaveBeenCalledWith(
825+
expect.stringContaining('Warning: Ignoring invalid auth timeout value: invalid. Must be a positive number.')
826+
)
827+
828+
consoleSpy.mockRestore()
829+
})
830+
831+
it('should use default timeout when negative timeout value is provided', async () => {
832+
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
833+
834+
const args = ['https://example.com', '--auth-timeout', '-30']
835+
const result = await parseCommandLineArgs(args, mockUsage)
836+
837+
expect(result.authTimeoutMs).toBe(30000)
838+
expect(consoleSpy).toHaveBeenCalledWith(
839+
expect.stringContaining('Warning: Ignoring invalid auth timeout value: -30. Must be a positive number.')
840+
)
841+
842+
consoleSpy.mockRestore()
843+
})
844+
845+
it('should use default timeout when zero timeout value is provided', async () => {
846+
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
847+
848+
const args = ['https://example.com', '--auth-timeout', '0']
849+
const result = await parseCommandLineArgs(args, mockUsage)
850+
851+
expect(result.authTimeoutMs).toBe(30000)
852+
expect(consoleSpy).toHaveBeenCalledWith(
853+
expect.stringContaining('Warning: Ignoring invalid auth timeout value: 0. Must be a positive number.')
854+
)
855+
856+
consoleSpy.mockRestore()
857+
})
858+
859+
it('should use default timeout when --auth-timeout flag has no value', async () => {
860+
const args = ['https://example.com', '--auth-timeout']
861+
const result = await parseCommandLineArgs(args, mockUsage)
862+
863+
expect(result.authTimeoutMs).toBe(30000)
864+
})
865+
866+
it('should log when using custom timeout', async () => {
867+
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
868+
869+
const args = ['https://example.com', '--auth-timeout', '45']
870+
const result = await parseCommandLineArgs(args, mockUsage)
871+
872+
expect(result.authTimeoutMs).toBe(45000)
873+
expect(consoleSpy).toHaveBeenCalledWith(
874+
expect.stringContaining('Using auth callback timeout: 45 seconds')
875+
)
876+
877+
consoleSpy.mockRestore()
878+
})
879+
})
880+
})
881+
882+
describe('setupOAuthCallbackServerWithLongPoll', () => {
883+
let server: any
884+
let events: EventEmitter
885+
886+
beforeEach(() => {
887+
events = new EventEmitter()
888+
})
889+
890+
afterEach(() => {
891+
if (server) {
892+
server.close()
893+
server = null
894+
}
895+
})
896+
897+
it('should use custom timeout when authTimeoutMs is provided', async () => {
898+
const customTimeout = 5000
899+
const result = setupOAuthCallbackServerWithLongPoll({
900+
port: 0, // Use any available port
901+
path: '/oauth/callback',
902+
events,
903+
authTimeoutMs: customTimeout
904+
})
905+
906+
server = result.server
907+
908+
// Test that the server was created
909+
expect(server).toBeDefined()
910+
expect(typeof result.waitForAuthCode).toBe('function')
911+
})
912+
913+
it('should use default timeout when authTimeoutMs is not provided', async () => {
914+
const result = setupOAuthCallbackServerWithLongPoll({
915+
port: 0, // Use any available port
916+
path: '/oauth/callback',
917+
events
918+
})
919+
920+
server = result.server
921+
922+
// Test that the server was created with defaults
923+
expect(server).toBeDefined()
924+
expect(typeof result.waitForAuthCode).toBe('function')
925+
})
926+
})

src/lib/utils.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ export function setupOAuthCallbackServerWithLongPoll(options: OAuthCallbackServe
475475
const longPollTimeout = setTimeout(() => {
476476
log('Long poll timeout reached, responding with 202')
477477
res.status(202).send('Authentication in progress')
478-
}, 30000)
478+
}, options.authTimeoutMs || 30000)
479479

480480
// If auth completes while we're waiting, send the response immediately
481481
authCompletedPromise
@@ -716,6 +716,19 @@ export async function parseCommandLineArgs(args: string[], usage: string) {
716716
j++
717717
}
718718

719+
// Parse auth timeout
720+
let authTimeoutMs = 30000 // Default 30 seconds
721+
const authTimeoutIndex = args.indexOf('--auth-timeout')
722+
if (authTimeoutIndex !== -1 && authTimeoutIndex < args.length - 1) {
723+
const timeoutSeconds = parseInt(args[authTimeoutIndex + 1], 10)
724+
if (!isNaN(timeoutSeconds) && timeoutSeconds > 0) {
725+
authTimeoutMs = timeoutSeconds * 1000
726+
log(`Using auth callback timeout: ${timeoutSeconds} seconds`)
727+
} else {
728+
log(`Warning: Ignoring invalid auth timeout value: ${args[authTimeoutIndex + 1]}. Must be a positive number.`)
729+
}
730+
}
731+
719732
if (!serverUrl) {
720733
log(usage)
721734
process.exit(1)
@@ -791,6 +804,7 @@ export async function parseCommandLineArgs(args: string[], usage: string) {
791804
staticOAuthClientInfo,
792805
authorizeResource,
793806
ignoredTools,
807+
authTimeoutMs,
794808
}
795809
}
796810

src/proxy.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ async function runProxy(
3737
staticOAuthClientInfo: StaticOAuthClientInformationFull,
3838
authorizeResource: string,
3939
ignoredTools: string[],
40+
authTimeoutMs: number,
4041
) {
4142
// Set up event emitter for auth flow
4243
const events = new EventEmitter()
@@ -45,7 +46,7 @@ async function runProxy(
4546
const serverUrlHash = getServerUrlHash(serverUrl)
4647

4748
// Create a lazy auth coordinator
48-
const authCoordinator = createLazyAuthCoordinator(serverUrlHash, callbackPort, events)
49+
const authCoordinator = createLazyAuthCoordinator(serverUrlHash, callbackPort, events, authTimeoutMs)
4950

5051
// Create the OAuth client provider
5152
const authProvider = new NodeOAuthClientProvider({
@@ -158,6 +159,7 @@ parseCommandLineArgs(process.argv.slice(2), 'Usage: npx tsx proxy.ts <https://se
158159
staticOAuthClientInfo,
159160
authorizeResource,
160161
ignoredTools,
162+
authTimeoutMs,
161163
}) => {
162164
return runProxy(
163165
serverUrl,
@@ -169,6 +171,7 @@ parseCommandLineArgs(process.argv.slice(2), 'Usage: npx tsx proxy.ts <https://se
169171
staticOAuthClientInfo,
170172
authorizeResource,
171173
ignoredTools,
174+
authTimeoutMs,
172175
)
173176
},
174177
)

0 commit comments

Comments
 (0)