fix(oauth): headless server support + non-standard OAuth providers (Granola)#72
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c89f3f61ad
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const cachedClient = await persistence.readClientInfo(); | ||
| if (cachedClient && Array.isArray((cachedClient as Record<string, unknown>).redirect_uris)) { | ||
| const cachedRedirect = ((cachedClient as Record<string, unknown>).redirect_uris as string[])[0]; |
There was a problem hiding this comment.
Close callback server on persistence errors
This stale-client check executes after server.listen(...) has already bound a local port, so if readClientInfo() (or the later clear('client')) throws due to malformed cache JSON or filesystem permissions, PersistentOAuthClientProvider.create exits without closing the HTTP server. In that failure path the process can be left with an open callback listener and may hang instead of terminating cleanly; wrap this section so failures close the server before rethrowing.
Useful? React with 👍 / 👎.
|
Yes, need that as well. Thanks @mgonto |
|
Hey @mgonto, nice work on this — removing the hardcoded scope and letting the SDK derive it is definitely the cleaner fix. The headless xdg-open and stale registration fixes are a bonus. One thing that might be worth tacking on: servers that don't advertise {
"mcpServers": {
"granola": {
"baseUrl": "https://mcp.granola.ai/mcp",
"auth": "oauth",
"oauthScope": "openid email profile offline_access"
}
}
}The metadata change would just be: ...(definition.oauthScope \!== undefined
? { scope: definition.oauthScope || undefined }
: {}), // no scope set → SDK derives it as normalTotally additive on top of what you have. Happy to add it to this branch or follow up after merge — whatever works for you. |
|
that's awesome. feel free to add it to this branch |
|
Tried to push directly to your branch but don't have write access to your fork. Here's the diff — should be a straightforward apply: ```diff
diff --git a/src/config-schema.ts b/src/config-schema.ts
diff --git a/src/oauth.ts b/src/oauth.ts
Three files touched, all additive. Typechecks clean. |
|
@steipete Can you please merge this in, or at least cherry pick the dynamic port issue fix? |
…roviders 1. Remove hardcoded scope='mcp:tools' from client metadata. Providers like Granola reject this scope at the authorize endpoint (invalid_scope). Let the MCP SDK derive scope from server metadata instead, falling back to the auth server's scopes_supported. 2. Swallow xdg-open ENOENT on headless Linux servers. On VPS/CI environments without a desktop, spawning xdg-open throws an unhandled error event that crashes the process before the OAuth callback server can receive the redirect. 3. Clear stale client registration when dynamic callback port changes. With dynamic ports (the default), each run picks a different port. If a previous client registration is cached with a different redirect_uri, the auth server rejects subsequent requests with invalid_redirect_uri. Now detects the mismatch and re-registers. Fixes steipete#67
…check Address review feedback: wrap the redirect URI mismatch check in try/catch so that if readClientInfo() or clear() throws (malformed cache JSON, filesystem permissions), the already-bound HTTP callback server is closed before rethrowing. Prevents the process from hanging with an open listener in failure paths.
eb8d057 to
779d560
Compare
Problem
Three related issues prevent mcporter OAuth from working on headless Linux servers and with providers like Granola:
scope=mcp:toolsrejected by Granola — The hardcodedscope: 'mcp:tools'in client metadata is not in Granola'sscopes_supported(email,offline_access,openid,profile), causing aninvalid_scopeerror at the authorize endpoint.xdg-opencrash on headless servers — On VPS/CI environments without a desktop, spawningxdg-openemits an unhandledENOENTerror event that crashes the Node process before the OAuth callback server can receive the redirect.Stale client registration with dynamic ports — With dynamic callback ports (the default), each run picks a different port. The previous client registration is cached with the old
redirect_uri, so the auth server rejects subsequent requests withinvalid_redirect_uri.Fix
scopefrom client metadata. The MCP SDK already derives scope from the server's resource metadata or auth server'sscopes_supported.xdg-openspawn in a try/catch and attach an error handler to swallowENOENTgracefully.Testing
Tested against Granola's official MCP endpoint (
https://mcp.granola.ai/mcp) on a headless Ubuntu VPS. All three issues are resolved and the full OAuth flow completes successfully.Fixes #67