♻️ fix: Reuse Existing MCP OAuth Client Registrations to Prevent client_id Mismatch#11925
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes OAuth client_id mismatch errors that occur during token exchange when concurrent connections or reconnections request different client registrations from the authorization server. The solution reuses existing OAuth client registrations stored in the database instead of calling the Dynamic Client Registration (DCR) endpoint on every OAuth flow initiation.
Changes:
- Adds client registration reuse logic to
initiateOAuthFlow()by checking for existing client info before calling/register - Passes
findTokenmethod through the call chain fromMCPConnectionFactoryto enable database lookups - Includes comprehensive test coverage for the new reuse behavior with fallback scenarios
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/api/src/mcp/oauth/handler.ts | Added optional findToken parameter and client registration reuse logic with error handling and fallback to new registration |
| packages/api/src/mcp/MCPConnectionFactory.ts | Passes findToken method to initiateOAuthFlow() at both call sites |
| packages/api/src/mcp/tests/handler.test.ts | Added 4 test cases covering client reuse, fallback scenarios, and error handling |
| packages/api/src/mcp/tests/MCPConnectionFactory.test.ts | Updated test expectation to include the new findToken parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
DenisPalnitsky
force-pushed
the
fix/reuse-oauth-client-registration
branch
from
7a9d5c0 to
b2c82f7
Compare
DenisPalnitsky
force-pushed
the
fix/reuse-oauth-client-registration
branch
from
b2c82f7 to
28cb774
Compare
|
Thanks for the PR. Worth noting that the bug in your reproduction steps ("Flow state not found") is actually a different issue from what this fixes. That error is thrown because This PR only helps on reconnection (when a prior completed flow left client info in the DB). On first-time auth, I will merge #11941 first once they address my feedback. Then let me know if the issue is still persisting. |
|
@danny-avila Yeah, you a right. I realised that I was working on outdated main that did not have you recent fixes in that area so I got confused about the error. |
DenisPalnitsky
force-pushed
the
fix/reuse-oauth-client-registration
branch
from
28cb774 to
3c784fa
Compare
DenisPalnitsky
force-pushed
the
fix/reuse-oauth-client-registration
branch
from
3c784fa to
5dcdc7a
Compare
|
@danny-avila we are keep seeing this issue from time to time |
Can you describe your setup more to find the root cause? Are you horizontally scaling LibreChat? Which MCP servers have exhibited this behavior? I'm investigating further, but having this information would be critical. |
|
Also, critical to know: if using redis or not, which is recommended in horizontally scaled deployments. |
|
Yes we use redis with 3 librechat replicas. |
danny-avila
force-pushed
the
fix/reuse-oauth-client-registration
branch
from
103c54a to
7c400f1
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c400f19c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
danny-avila
changed the title
client_id Mismatch
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f18b511e0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Re-reviewed the latest head (a68e2218a903049139594d083b43b64d39360e4a).
The stale-client cleanup logic now looks correct in both OAuth paths:
- blocking path threads
reusedStoredClientanderrorthroughhandleOAuthRequired() returnOnOAuthcleanup now happens in the backgroundcreateFlow(...).catch(...)while reuse context is still in scope- cleanup is selectively gated by
isClientRejection(...)instead of firing on every failure
The added coverage also closes the remaining confidence gap:
- direct tests for
isClientRejection(...) oauthTestServernow supportsenforceClientId- token-exchange tests now prove a mismatched
client_idis rejected and correctly classified
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a68e2218a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d93571bd0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
danny-avila
force-pushed
the
fix/reuse-oauth-client-registration
branch
2 times, most recently
from
1d529c8 to
0c8a5b4
Compare
…ismatch When using auto-discovered OAuth (DCR), LibreChat calls /register on every flow initiation, getting a new client_id each time. When concurrent connections or reconnections happen, the client_id used during /authorize differs from the one used during /token, causing the server to reject the exchange. Before registering a new client, check if a valid client registration already exists in the database and reuse it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ing flow state FlowStateManager.createFlow() deletes FAILED flow state before rejecting, so getFlowState() after handleOAuthRequired() returns null would find nothing — making the stale-client cleanup dead code. Fix: hoist reusedStoredClient flag from flowMetadata into a local variable, include it in handleOAuthRequired()'s return type (both success and catch paths), and use result.reusedStoredClient directly in the caller instead of a second getFlowState() round-trip.
The returnOnOAuth cleanup was unreliable: it depended on reading FAILED flow state, but FlowStateManager.monitorFlow() deletes FAILED state before rejecting. Move cleanup into createFlow's catch handler where flowMetadata.reusedStoredClient is still in scope. Make cleanup selective in both paths: add isClientRejection() helper that only matches errors indicating the OAuth server rejected the client_id (invalid_client, unauthorized_client, client not found). Timeouts, user-cancelled flows, and other transient failures no longer wipe valid stored registrations. Thread the error from handleOAuthRequired() through the return type so the blocking path can also check isClientRejection().
Narrow 'client_id' match to 'client_id mismatch' to avoid false-positive cleanup on unrelated errors that happen to mention client_id.
- Add isClientRejection unit tests: invalid_client, unauthorized_client, client_id mismatch, client not found, unknown client, and negative cases (timeout, flow state not found, user denied, null, undefined) - Enhance OAuth test server with enforceClientId option: binds auth codes to the client_id that initiated /authorize, rejects token exchange with mismatched or unregistered client_id (401 invalid_client) - Add integration tests proving the test server correctly rejects stale client_ids and accepts matching ones at /token
- Issuer check: re-register when storedIssuer is absent or non-string instead of silently reusing. Narrows unknown type with typeof guard and inverts condition so missing issuer → fresh DCR (safer default). - OAuth callback route: call failFlow with the OAuth error when the authorization server redirects back with error= parameter, so the waiting flow receives the actual rejection instead of timing out. This lets isClientRejection match stale-client errors correctly. - Extract duplicated cleanup block to clearStaleClientIfRejected() private method, called from both returnOnOAuth and blocking paths. - Test fixes: add issuer to stored metadata in reuse tests, reset server to undefined in afterEach to prevent double-close.
…Client on join - OAuth callback: move failFlow call to after CSRF/session/active-flow validation so an attacker with only a leaked state parameter cannot force-fail a flow without passing the same integrity checks required for legitimate callbacks - PENDING join path: propagate reusedStoredClient from flow metadata into the return object so joiners can trigger stale-client cleanup if the joined flow later fails with a client rejection
danny-avila
force-pushed
the
fix/reuse-oauth-client-registration
branch
from
0c8a5b4 to
fdfcf26
Compare
…nd CSRF The previous restructuring moved oauthError and missing-code checks behind CSRF validation, breaking tests that expect those redirects without cookies. The redirect itself is harmless (just shows an error page). Only the failFlow call needs CSRF gating to prevent DoS. Restructure: oauthError check stays early (redirects immediately), but failFlow inside it runs the full CSRF/session/active-flow validation before marking the flow as FAILED.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c39fffedf7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Client registration reuse without cleanup capability creates a permanent failure loop: if the reused client is stale, the code detects the rejection but cannot clear the stored registration because deleteTokens is missing, so every retry reuses the same broken client_id. - MCPConnectionFactory: only pass findToken to initiateOAuthFlow when deleteTokens is also available, ensuring reuse is only enabled when recovery is possible - api/server/services/MCP.js: add deleteTokens to the tokenMethods object (was the only MCP call site missing it)
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a78b8db3e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
When joining a PENDING flow, reusedStoredClient was only set on the success return but not before the await. If createFlow throws (e.g. invalid_client during token exchange), the outer catch returns the local variable which was still false, skipping stale-client cleanup.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fcb0f57eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| hasActiveFlow = pendingFlow?.status === 'PENDING' && pendingAge < PENDING_STALE_MS; | ||
| } | ||
| if (hasCsrf || hasSession || hasActiveFlow) { | ||
| await flowManager.failFlow(flowId, 'mcp_oauth', String(oauthError)); |
There was a problem hiding this comment.
The new oauthError path still allows failFlow when only hasActiveFlow is true, which means no CSRF or session proof is required. If an OAuth state value is leaked (for example via logs or browser history), any unauthenticated request to the callback can force the corresponding pending flow into FAILED, causing a user-visible denial of service for that auth attempt. To actually gate this operation, failFlow should require a browser/user binding (hasCsrf or hasSession) instead of accepting hasActiveFlow alone.
Useful? React with 👍 / 👎.
hasActiveFlow only proves a PENDING flow exists, not that the caller is the same browser that initiated it. An attacker with a leaked state could force-fail the flow without any user binding. Require hasCsrf or hasSession before calling failFlow on the oauthError path.
There was a problem hiding this comment.
Re-fetched and re-checked the latest PR head (2b09879fafb625f7acdbe64542f5617f53e1468b).
The previously reported callback DoS issue is resolved in the current remote state: the oauthError path in api/server/routes/mcp.js now gates failFlow(...) on hasCsrf || hasSession only, with no hasActiveFlow fallback in that destructive branch.
I also re-verified the other targeted risk areas from the focused audit:
handleOAuthRequired()callers correctly checkresult?.tokens- joiner path now preserves
reusedStoredClient - stale-client cleanup remains selectively gated and idempotent
- partial token-method call sites are guarded/fixed
|
@codex review |
Match the returnOnOAuth path's defense-in-depth: only enable client registration reuse when deleteTokens is also available, ensuring cleanup is possible if the reused client turns out to be stale.
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
- Add deleteTokens method to InMemoryTokenStore matching TokenMethods contract; update test call site from deleteToken to deleteTokens - Add MCPConnectionFactory test: returnOnOAuth flow fails with invalid_client → clearStaleClientIfRejected invoked automatically - Add mcp.spec.js tests: OAuth error with CSRF → failFlow called; OAuth error without cookies → failFlow NOT called (DoS prevention) - Add JSDoc to isClientRejection with RFC 6749 and vendor attribution - Add inline comment explaining findToken/deleteTokens coupling guard - Normalize issuer comparison: strip trailing slashes to prevent spurious re-registrations from URL formatting differences - Fix dead-code: use local reusedStoredClient variable in PENDING join return instead of re-reading flowMeta
- N1: Add session cookie failFlow test — validates the hasSession branch triggers failFlow on OAuth error callback - N2: Replace setTimeout(50) with setImmediate for microtask drain - N3: Add 'unknown client' attribution to isClientRejection JSDoc - N4: Remove dead getFlowState mock from failFlow tests
|
@codex review |
|
Final review pass against the latest PR head ( Reviewed areas:
Outcome:
I also wrote local markdown review artifacts for traceability:
Net: looks ready to merge from this review pass. |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…ent_id` Mismatch (danny-avila#11925) * fix: reuse existing OAuth client registrations to prevent client_id mismatch When using auto-discovered OAuth (DCR), LibreChat calls /register on every flow initiation, getting a new client_id each time. When concurrent connections or reconnections happen, the client_id used during /authorize differs from the one used during /token, causing the server to reject the exchange. Before registering a new client, check if a valid client registration already exists in the database and reuse it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Handle re-registration of OAuth clients when redirect_uri changes * Add undefined fields for logo_uri and tos_uri in OAuth metadata tests * test: add client registration reuse tests for horizontal scaling race condition Reproduces the client_id mismatch bug that occurs in multi-replica deployments where concurrent initiateOAuthFlow calls each register a new OAuth client. Tests verify that the findToken-based client reuse prevents re-registration. * fix: address review findings for client registration reuse - Fix empty redirect_uris bug: invert condition so missing/empty redirect_uris triggers re-registration instead of silent reuse - Revert undocumented config?.redirect_uri in auto-discovery path - Change DB error logging from debug to warn for operator visibility - Fix import order: move package type import to correct section - Remove redundant type cast and misleading JSDoc comment - Test file: remove dead imports, restore process.env.DOMAIN_SERVER, rename describe blocks, add empty redirect_uris edge case test, add concurrent reconnection test with pre-seeded token, scope documentation to reconnection stabilization * fix: resolve type check errors for OAuthClientInformation redirect_uris The SDK's OAuthClientInformation type lacks redirect_uris (only on OAuthClientInformationFull). Cast to the local OAuthClientInformation type in handler.ts when accessing deserialized client info from DB, and use intersection types in tests for clientInfo with redirect_uris. * fix: address follow-up review findings R1, R2, R3 - R1: Move `import type { TokenMethods }` to the type-imports section, before local types, per CLAUDE.md import order rules - R2: Add unit test for empty redirect_uris in handler.test.ts to verify the inverted condition triggers re-registration - R3: Use delete for process.env.DOMAIN_SERVER restoration when the original value was undefined to avoid coercion to string "undefined" * fix: clear stale client registration on OAuth flow failure When a stored client_id is no longer recognized by the OAuth server, the flow fails but the stale client stays in MongoDB, causing every retry to reuse the same invalid registration in an infinite loop. On OAuth failure, clear the stored client registration so the next attempt falls through to fresh Dynamic Client Registration. - Add MCPTokenStorage.deleteClientRegistration() for targeted cleanup - Call it from MCPConnectionFactory's OAuth failure path - Add integration test proving recovery from stale client reuse * fix: validate auth server identity and target cleanup to reused clients - Gate client reuse on authorization server identity: compare stored issuer against freshly discovered metadata before reusing, preventing wrong-client reuse when the MCP server switches auth providers - Add reusedStoredClient flag to MCPOAuthFlowMetadata so cleanup only runs when the failed flow actually reused a stored registration, not on unrelated failures (timeouts, user-denied consent, etc.) - Add cleanup in returnOnOAuth path: when a prior flow that reused a stored client is detected as failed, clear the stale registration before re-initiating - Add tests for issuer mismatch and reusedStoredClient flag assertions * fix: address minor review findings N3, N5, N6 - N3: Type deleteClientRegistration param as TokenMethods['deleteTokens'] instead of Promise<unknown> - N5: Elevate deletion failure logging from debug to warn for operator visibility when stale client cleanup fails - N6: Use getLogPrefix() instead of hardcoded log prefix to respect system-user privacy convention * fix: correct stale-client cleanup in both OAuth paths - Blocking path: remove result?.clientInfo guard that made cleanup unreachable (handleOAuthRequired returns null on failure, so result?.clientInfo was always false in the failure branch) - returnOnOAuth path: only clear stored client when the prior flow status is FAILED, not on COMPLETED or PENDING flows, to avoid deleting valid registrations during normal flow replacement * fix: remove redundant cast on clientMetadata clientMetadata is already typed as Record<string, unknown>; the as Record<string, unknown> cast was a no-op. * fix: thread reusedStoredClient through return type instead of re-reading flow state FlowStateManager.createFlow() deletes FAILED flow state before rejecting, so getFlowState() after handleOAuthRequired() returns null would find nothing — making the stale-client cleanup dead code. Fix: hoist reusedStoredClient flag from flowMetadata into a local variable, include it in handleOAuthRequired()'s return type (both success and catch paths), and use result.reusedStoredClient directly in the caller instead of a second getFlowState() round-trip. * fix: selective stale-client cleanup in returnOnOAuth path The returnOnOAuth cleanup was unreliable: it depended on reading FAILED flow state, but FlowStateManager.monitorFlow() deletes FAILED state before rejecting. Move cleanup into createFlow's catch handler where flowMetadata.reusedStoredClient is still in scope. Make cleanup selective in both paths: add isClientRejection() helper that only matches errors indicating the OAuth server rejected the client_id (invalid_client, unauthorized_client, client not found). Timeouts, user-cancelled flows, and other transient failures no longer wipe valid stored registrations. Thread the error from handleOAuthRequired() through the return type so the blocking path can also check isClientRejection(). * fix: tighten isClientRejection heuristic Narrow 'client_id' match to 'client_id mismatch' to avoid false-positive cleanup on unrelated errors that happen to mention client_id. * test: add isClientRejection tests and enforced client_id on test server - Add isClientRejection unit tests: invalid_client, unauthorized_client, client_id mismatch, client not found, unknown client, and negative cases (timeout, flow state not found, user denied, null, undefined) - Enhance OAuth test server with enforceClientId option: binds auth codes to the client_id that initiated /authorize, rejects token exchange with mismatched or unregistered client_id (401 invalid_client) - Add integration tests proving the test server correctly rejects stale client_ids and accepts matching ones at /token * fix: issuer validation, callback error propagation, and cleanup DRY - Issuer check: re-register when storedIssuer is absent or non-string instead of silently reusing. Narrows unknown type with typeof guard and inverts condition so missing issuer → fresh DCR (safer default). - OAuth callback route: call failFlow with the OAuth error when the authorization server redirects back with error= parameter, so the waiting flow receives the actual rejection instead of timing out. This lets isClientRejection match stale-client errors correctly. - Extract duplicated cleanup block to clearStaleClientIfRejected() private method, called from both returnOnOAuth and blocking paths. - Test fixes: add issuer to stored metadata in reuse tests, reset server to undefined in afterEach to prevent double-close. * fix: gate failFlow behind callback validation, propagate reusedStoredClient on join - OAuth callback: move failFlow call to after CSRF/session/active-flow validation so an attacker with only a leaked state parameter cannot force-fail a flow without passing the same integrity checks required for legitimate callbacks - PENDING join path: propagate reusedStoredClient from flow metadata into the return object so joiners can trigger stale-client cleanup if the joined flow later fails with a client rejection * fix: restore early oauthError/code redirects, gate only failFlow behind CSRF The previous restructuring moved oauthError and missing-code checks behind CSRF validation, breaking tests that expect those redirects without cookies. The redirect itself is harmless (just shows an error page). Only the failFlow call needs CSRF gating to prevent DoS. Restructure: oauthError check stays early (redirects immediately), but failFlow inside it runs the full CSRF/session/active-flow validation before marking the flow as FAILED. * fix: require deleteTokens for client reuse, add missing import in MCP.js Client registration reuse without cleanup capability creates a permanent failure loop: if the reused client is stale, the code detects the rejection but cannot clear the stored registration because deleteTokens is missing, so every retry reuses the same broken client_id. - MCPConnectionFactory: only pass findToken to initiateOAuthFlow when deleteTokens is also available, ensuring reuse is only enabled when recovery is possible - api/server/services/MCP.js: add deleteTokens to the tokenMethods object (was the only MCP call site missing it) * fix: set reusedStoredClient before createFlow in joined-flow path When joining a PENDING flow, reusedStoredClient was only set on the success return but not before the await. If createFlow throws (e.g. invalid_client during token exchange), the outer catch returns the local variable which was still false, skipping stale-client cleanup. * fix: require browser binding (CSRF/session) for failFlow on OAuth error hasActiveFlow only proves a PENDING flow exists, not that the caller is the same browser that initiated it. An attacker with a leaked state could force-fail the flow without any user binding. Require hasCsrf or hasSession before calling failFlow on the oauthError path. * fix: guard findToken with deleteTokens check in blocking OAuth path Match the returnOnOAuth path's defense-in-depth: only enable client registration reuse when deleteTokens is also available, ensuring cleanup is possible if the reused client turns out to be stale. * fix: address review findings — tests, types, normalization, docs - Add deleteTokens method to InMemoryTokenStore matching TokenMethods contract; update test call site from deleteToken to deleteTokens - Add MCPConnectionFactory test: returnOnOAuth flow fails with invalid_client → clearStaleClientIfRejected invoked automatically - Add mcp.spec.js tests: OAuth error with CSRF → failFlow called; OAuth error without cookies → failFlow NOT called (DoS prevention) - Add JSDoc to isClientRejection with RFC 6749 and vendor attribution - Add inline comment explaining findToken/deleteTokens coupling guard - Normalize issuer comparison: strip trailing slashes to prevent spurious re-registrations from URL formatting differences - Fix dead-code: use local reusedStoredClient variable in PENDING join return instead of re-reading flowMeta * fix: address final review nits N1-N4 - N1: Add session cookie failFlow test — validates the hasSession branch triggers failFlow on OAuth error callback - N2: Replace setTimeout(50) with setImmediate for microtask drain - N3: Add 'unknown client' attribution to isClientRejection JSDoc - N4: Remove dead getFlowState mock from failFlow tests --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Danny Avila <danny@librechat.ai>
Summary
/register(DCR) in the auto-discovery OAuth path, check if a valid client registration already exists in the database and reuse itclient_idvaluesfindTokenfromMCPConnectionFactorytoinitiateOAuthFlowto enable the lookupContext
LibreChat calls
/registeron every OAuth flow initiation, getting a newclient_ideach time. When concurrent connections or reconnections happen, theclient_idused during/authorizediffers from the one used during/token, causing the authorization server to reject the exchange with a "client ID mismatch" error. This is especially frequent with Okta-backed OAuth servers.Steps to reproduce
Expected Result
The Oauth flow passes in one of the tabs and conversation continues
Actual results
Both conversations fail and there is an error in log:
And there is an
client id does not matcherror in MCP itselfChanges
packages/api/src/mcp/oauth/handler.ts— Added optionalfindTokenparameter toinitiateOAuthFlow(). Before registering a new client, checksMCPTokenStorage.getClientInfoAndMetadata()for an existing registration. Falls back gracefully to new registration on errors.packages/api/src/mcp/MCPConnectionFactory.ts— Passesthis.tokenMethods?.findTokento bothinitiateOAuthFlow()call sites (returnOnOAuth and normal paths).packages/api/src/mcp/__tests__/handler.test.ts— Added 5 new test cases covering client reuse, fallback to registration, backward compatibility, and error handling.packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts— Updated call expectation for the new parameter.Test plan
handler.test.ts— 34 tests pass (4 new)MCPConnectionFactory.test.ts— 14 tests passclient_idis used (check logs for "Reusing existing client registration")