fix(oauth): allow client secrets shorter than 8 characters#3283
Conversation
validateClientSecret rejected any non-empty client secret under 8 characters, even though the OAuth spec doesn't mandate a minimum length and the value is issued by the authorization server, not chosen here. Server-side validation only requires the secret be non-empty. The client-side check now matches that contract: it only rejects a secret that's present but trims to nothing. Fixes MCPJam#1723
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
dosubot
Bot
added
size:XS
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughUpdated Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
mcpjam-inspector/client/src/components/connection/hooks/__tests__/use-server-form.test.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
mcpjam-inspector/client/src/components/connection/hooks/use-server-form.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Hey @Jatlus , thanks for the PR! While reviewing it, I noticed that we trim leading and trailing whitespace from client secrets. For example, " secret " gets saved as "secret", which changes the secret and could cause authentication to fail. You didn’t introduce this behavior, but since you’re already working in this area, could you please fix this issue as well? |
ignaciojimenezr
left a comment
ignaciojimenezr
left a comment
There was a problem hiding this comment.
Hey @Jatlus , thanks for the PR! While reviewing it, I noticed that we trim leading and trailing whitespace from client secrets. For example, " secret " gets saved as "secret", which changes the secret and could cause authentication to fail.
You didn’t introduce this behavior, but since you’re already working in this area, could you please fix this issue as well?
3 participants
Summary
Fixes #1723.
validateClientSecretrejected any non-empty client secret shorter than 8 characters, even though nothing about a short secret is actually invalid. The OAuth spec doesn't mandate a minimum length, and the value is issued by the identity provider, not chosen in the Inspector. Server-side validation (server/routes/v1/oauth.ts,server/routes/mcp/xaa.ts) only ever required the secret be non-empty (min(1)).Fix
validateClientSecretnow only rejects a secret that's present but trims to nothing (whitespace-only). This matches the server-side contract exactly. Any non-empty, non-whitespace value is accepted regardless of length.Testing
mcpjam-inspector/client/src/components/connection/hooks/__tests__/use-server-form.test.ts— newdescribe("validateClientSecret")block:"allows an empty client secret (public/PKCE client)""rejects a whitespace-only client secret""allows a single-character client secret""allows the reported repro value ('banana', 6 characters)""still allows client secrets 8+ characters long""does not affect validateClientId's own minimum-length rule", confirming this fix is scoped to the secret, not the separate client ID minimum.44/44 passing on the affected test file. Manually verified the inline "Client Secret must be at least 8 characters if provided" error no longer appears when typing a short secret like
banana.Summary by cubic
Accepts OAuth client secrets shorter than 8 characters by aligning client-side validation with the server’s rules. Resolves Linear #1723 where the Inspector blocked valid, short secrets.
validateClientSecretnow only rejects secrets that trim to empty; empty is allowed for public/PKCE clients; updated message to “Client Secret cannot be only whitespace.”Written for commit fdec12f. Summary will update on new commits.