diff --git a/packages/api/src/mcp/MCPConnectionFactory.ts b/packages/api/src/mcp/MCPConnectionFactory.ts index 00d381a437..a305cb00a6 100644 --- a/packages/api/src/mcp/MCPConnectionFactory.ts +++ b/packages/api/src/mcp/MCPConnectionFactory.ts @@ -413,7 +413,17 @@ export class MCPConnectionFactory { if (result?.tokens) { connection.emit('oauthHandled'); } else { - // OAuth failed, emit oauthFailed to properly reject the promise + // OAuth failed — clear stored client registration so the next attempt + // does a fresh DCR instead of reusing a potentially stale client_id + if (this.tokenMethods?.deleteTokens) { + await MCPTokenStorage.deleteClientRegistration({ + userId: this.userId!, + serverName: this.serverName, + deleteTokens: this.tokenMethods.deleteTokens, + }).catch((err) => { + logger.debug(`${this.logPrefix} Failed to clear stale client registration`, err); + }); + } logger.warn(`${this.logPrefix} OAuth failed, emitting oauthFailed event`); connection.emit('oauthFailed', new Error('OAuth authentication failed')); } diff --git a/packages/api/src/mcp/__tests__/MCPOAuthClientRegistrationReuse.test.ts b/packages/api/src/mcp/__tests__/MCPOAuthClientRegistrationReuse.test.ts index 92ca017430..6a79880fa4 100644 --- a/packages/api/src/mcp/__tests__/MCPOAuthClientRegistrationReuse.test.ts +++ b/packages/api/src/mcp/__tests__/MCPOAuthClientRegistrationReuse.test.ts @@ -283,6 +283,66 @@ describe('MCPOAuthHandler - client registration reuse on reconnection', () => { expect(result.flowMetadata.clientInfo?.client_id).toBeTruthy(); }); + it('should not reuse a stale client on retry after a failed flow', async () => { + server = await createOAuthMCPServer({ tokenTTLMs: 60000 }); + const tokenStore = new InMemoryTokenStore(); + + // Seed a stored client with a client_id that the OAuth server doesn't recognize + await MCPTokenStorage.storeTokens({ + userId: 'user-1', + serverName: 'test-server', + tokens: { access_token: 'old-token', token_type: 'Bearer' }, + createToken: tokenStore.createToken, + updateToken: tokenStore.updateToken, + findToken: tokenStore.findToken, + clientInfo: { + client_id: 'stale-client-that-oauth-server-deleted', + client_secret: 'stale-secret', + redirect_uris: ['http://localhost:3080/api/mcp/test-server/oauth/callback'], + } as OAuthClientInformation & { redirect_uris: string[] }, + }); + + // First attempt: reuses the stale client (this is expected — we don't know it's stale yet) + const firstResult = await MCPOAuthHandler.initiateOAuthFlow( + 'test-server', + server.url, + 'user-1', + {}, + undefined, + undefined, + tokenStore.findToken, + ); + expect(firstResult.flowMetadata.clientInfo?.client_id).toBe( + 'stale-client-that-oauth-server-deleted', + ); + expect(server.registeredClients.size).toBe(0); + + // Simulate flow failure: the OAuth server rejected the stale client_id, + // so the operator clears the stored client registration + await MCPTokenStorage.deleteClientRegistration({ + userId: 'user-1', + serverName: 'test-server', + deleteTokens: tokenStore.deleteToken, + }); + + // Second attempt (retry after failure): should do a fresh DCR + const secondResult = await MCPOAuthHandler.initiateOAuthFlow( + 'test-server', + server.url, + 'user-1', + {}, + undefined, + undefined, + tokenStore.findToken, + ); + + // Now it registered a new client + expect(server.registeredClients.size).toBe(1); + expect(secondResult.flowMetadata.clientInfo?.client_id).not.toBe( + 'stale-client-that-oauth-server-deleted', + ); + }); + it('should not call getClientInfoAndMetadata when findToken is not provided', async () => { server = await createOAuthMCPServer({ tokenTTLMs: 60000 }); const spy = jest.spyOn(MCPTokenStorage, 'getClientInfoAndMetadata'); diff --git a/packages/api/src/mcp/oauth/tokens.ts b/packages/api/src/mcp/oauth/tokens.ts index 1e31a64511..d470ace103 100644 --- a/packages/api/src/mcp/oauth/tokens.ts +++ b/packages/api/src/mcp/oauth/tokens.ts @@ -476,6 +476,27 @@ export class MCPTokenStorage { }; } + /** Deletes only the stored client registration for a specific user and server */ + static async deleteClientRegistration({ + userId, + serverName, + deleteTokens, + }: { + userId: string; + serverName: string; + deleteTokens: (query: { userId: string; type: string; identifier: string }) => Promise; + }): Promise { + const identifier = `mcp:${serverName}`; + await deleteTokens({ + userId, + type: 'mcp_oauth_client', + identifier: `${identifier}:client`, + }); + logger.debug( + `[MCPTokenStorage] Cleared stored client registration for ${serverName} (userId: ${userId})`, + ); + } + /** * Deletes all OAuth-related tokens for a specific user and server */