-
Notifications
You must be signed in to change notification settings - Fork 17
feat: add OAuth support for external MCP servers in the Playground #1323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This implements OAuth 2.1 Dynamic Client Registration (DCR) flow support for external MCP servers in Gram Elements, following the RFC for MCP-Compliant Playground. Backend changes: - Add user_oauth_tokens table for storing encrypted OAuth tokens - Create external_oauth.go with authorize, callback, status, disconnect, and token endpoints - Implement PKCE (S256) for secure authorization code flow - Add GetToolsetByID query for toolset lookup Elements changes: - Add ExternalOAuthConfig and OAuthApiConfig types - Create useOAuthStatus hook for checking auth status - Create useOAuthToken hook for fetching access tokens - Integrate OAuth status into ElementsProvider context - Handle OAuth callback URL parameters Dashboard changes: - Update PlaygroundElements to pass OAuth config when toolset has external OAuth server configured Resolves AGE-1150 Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 3a3757d The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Add visual OAuth connection status and connect/disconnect buttons in the Playground Authentication section, matching the RFC mockups. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Replace Spinner from moonshine with Loader2 from lucide-react. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Update PlaygroundAuth to detect OAuth requirements from external MCP tools discovered via the MCP protocol, in addition to the existing external OAuth server configuration. - Add getExternalMcpOAuthConfig() to extract OAuth config from rawTools - Add ExternalMcpOAuthConnection component for MCP OAuth 2.1/2.0 flows - Support both legacy externalOauthServer and MCP-discovered OAuth - Display "MCP OAuth 2.1" or "OAuth 2.0" label based on discovered version This aligns with the RFC architecture where the Playground acts as an MCP client that self-discovers OAuth requirements from imported catalog tools. Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add external OAuth endpoints at /oauth-external/* to avoid route conflicts - Implement Dynamic Client Registration (DCR) per RFC 7591 for MCP OAuth 2.1 - Add session header authentication fallback for cross-origin requests - Fix OAuth state cache key consistency using StateID field - Update frontend to pass Gram-Session header for OAuth status/disconnect - Add external_oauth_client_registrations table for storing DCR credentials The OAuth flow now works for external MCP tools like Linear. Note: Agent/chat path does not yet support external MCP tool execution (returns error for ToolKindExternalMCP). This PR focuses on the authentication flow. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Use refetch() instead of invalidateQueries() for more reliable status update after OAuth flow completes. Added small delay to ensure server has processed the callback. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add comprehensive logging to help debug OAuth status check issues: - Log user_id, organization_id, and issuer when checking status - Log whether token was found or not found - Log final status (authenticated/needs_auth/disconnected), connected, and expired flags Also add new slog helpers to attr package: - SlogOAuthStatus for OAuth status string - SlogOAuthConnected for connection boolean - SlogOAuthExpired for expiration boolean Co-Authored-By: Claude Opus 4.5 <[email protected]>
After OAuth authorization completes, instead of redirecting to the full
dashboard URL (which loaded the entire app in the popup), now show a
minimal success page with:
- Checkmark icon
- "Connected to {provider}" message
- Auto-close after 1.5 seconds
This provides better UX as users see immediate feedback that the
connection succeeded, and the popup closes automatically.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Added OAuth support for external MCP servers in the Playground.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // Check if this toolset requires external MCP OAuth | ||
| const mcpOAuthConfig = useMemo( | ||
| () => | ||
| toolset?.tools ? getExternalMcpOAuthConfig(toolset.tools) : undefined, | ||
| [toolset?.tools], | ||
| ); | ||
| console.log({ mcpOAuthConfig }); | ||
|
|
||
| const { data: oauthStatus, isLoading: oauthStatusLoading } = | ||
| useExternalMcpOAuthStatus(toolset?.id, mcpOAuthConfig); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 OAuth status hook called with ExternalMCPToolDefinition instead of options object, breaking query key/enabled logic
useExternalMcpOAuthStatus expects its second argument to be an options object { slug?: string; enabled?: boolean }, but both callers pass an ExternalMCPToolDefinition object instead.
- Actual:
options?.slug/options?.enabledread from an unrelated object, so the query key uniqueness and theenabledflag are effectively wrong/undefined. - Expected: pass something like
{ slug: mcpOAuthConfig.slug }(and optionallyenabled) so the hook behaves deterministically.
Click to expand
Hook signature: client/dashboard/src/pages/playground/PlaygroundAuth.tsx:80-86
Caller sites:
client/dashboard/src/pages/playground/PlaygroundElements.tsx:96-105useExternalMcpOAuthStatus(toolset?.id, mcpOAuthConfig);
client/dashboard/src/pages/playground/PlaygroundAuth.tsx:187-193useExternalMcpOAuthStatus(toolset?.id, mcpOAuthConfig);
Because the hook builds its cache key as getExternalMcpOAuthStatusQueryKey(toolsetId, options?.slug) (PlaygroundAuth.tsx:90-91), passing the wrong shape can cause cache collisions/staleness and unexpected refetch/enable behavior.
Recommendation: Change calls to useExternalMcpOAuthStatus(toolset?.id, { slug: mcpOAuthConfig.slug, enabled: !!toolset?.id }) (and update ExternalMcpOAuthConnection similarly).
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Attempt to look for a stored OAuth credential if the requests comes | ||
| // from a Gram app (eg: Dashboard/Playground) | ||
| if gramSession, _ := r.Cookie(constants.SessionCookie); gramSession != nil { | ||
| resolvedToken, err := s.resolveExternalMcpOAuthToken(ctx, fullToolset) | ||
| if err != nil { | ||
| w.Header().Set("WWW-Authenticate", wwwAuth) | ||
| return oops.E(oops.CodeUnauthorized, err, "unauthorized") | ||
| } | ||
|
|
||
| tokenInputs = append(tokenInputs, oauthTokenInputs{ | ||
| securityKeys: []string{}, | ||
| Token: resolvedToken, | ||
| }) | ||
|
|
||
| break | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 External MCP OAuth token lookup always authenticates with empty session token (cookie presence check doesn’t populate context)
When handling public MCP requests that require external MCP OAuth, the server attempts to resolve a stored OAuth token if a gram_session cookie exists. However resolveExternalMcpOAuthToken calls s.sessions.Authenticate(ctx, "", false) with an empty key and does not read cookies itself, so it relies on contextvalues.GetSessionTokenFromContext(ctx) being set upstream. The prior check only confirms the cookie exists (r.Cookie(...)), but does not put the cookie value into the context.
- Actual: requests from the dashboard/playground can hit the
hasExternalMCPOAuthbranch, see the cookie, then fail token lookup with unauthorized becauseAuthenticatereceives"". - Expected: pass the cookie value into
Authenticate(or set the session token into context) so stored OAuth tokens can be found.
Click to expand
Call site checks cookie:
server/internal/mcp/impl.go:478-485if gramSession, _ := r.Cookie(constants.SessionCookie); gramSession != nil { resolvedToken, err := s.resolveExternalMcpOAuthToken(ctx, fullToolset) }
But token lookup authenticates with empty key:
server/internal/mcp/impl.go:985-989sessionCtx, err := s.sessions.Authenticate(ctx, "", false)
sessions.Manager.Authenticate only falls back to context token if present:
server/internal/auth/sessions/sessions.go:82-85if key == "" { key, _ = contextvalues.GetSessionTokenFromContext(ctx) }
So without middleware that injects the cookie value into context, this flow fails.
Recommendation: Pass the cookie value into resolveExternalMcpOAuthToken and call s.sessions.Authenticate(ctx, gramSession.Value, false) (or set the session token into context before calling).
Was this helpful? React with 👍 or 👎 to provide feedback.
| if token.ExpiresAt.Valid && token.ExpiresAt.Time.Before(time.Now()) { | ||
| return "", oops.E(oops.CodeUnauthorized, err, "OAuth token has expired") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Expired OAuth token path returns error with nil cause (uses stale err variable)
In resolveExternalMcpOAuthToken, the expiry check returns an error that wraps err, but err is nil at that point (it was last set by GetUserOAuthToken and already checked).
- Actual: error is constructed with a nil cause, losing diagnostic information.
- Expected: return a new error without reusing
err, or include a meaningful cause.
Click to expand
server/internal/mcp/impl.go:1016-1018
if token.ExpiresAt.Valid && token.ExpiresAt.Time.Before(time.Now()) {
return "", oops.E(oops.CodeUnauthorized, err, "OAuth token has expired")
}At this point err is nil.
Recommendation: Return oops.E(oops.CodeUnauthorized, nil, "OAuth token has expired") or create a dedicated sentinel error (e.g. errors.New("oauth token expired")).
Was this helpful? React with 👍 or 👎 to provide feedback.
| }); | ||
|
|
||
| // If toolsets have loaded and there are none, show full-page empty state | ||
| if (toolsets !== undefined && toolsets.length === 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (toolsets !== undefined && toolsets.length === 0) { | |
| if (!toolsets) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An empty array still evaluates to true.
const toolset: any[] = [];
console.log(!toolset) // false| const params = new URLSearchParams({ | ||
| toolset_id: toolset?.id ?? "", | ||
| external_mcp_slug: mcpOAuthConfig.slug, | ||
| redirect_uri: window.location.href.split("?")[0], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be worth having an assertion that window.location.href matches site url and throwing if it isn't or falling back to site url. maybe i'm being paranoid...
Co-authored-by: Georges Haidar <[email protected]>
…-api/gram into feat/oauth-support-in-elements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| proxy: serverUrl | ||
| ? { | ||
| "/rpc": serverUrl, | ||
| "/chat": serverUrl, | ||
| "/mcp": serverUrl, | ||
| "/oauth": serverUrl, | ||
| "/.well-known": serverUrl, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Missing /oauth-external proxy route in Vite dev config causes OAuth status requests to fail
The Vite proxy configuration proxies /oauth but not /oauth-external, causing OAuth status/authorize/disconnect requests to fail in development.
Click to expand
Issue
The proxy configuration at vite.config.ts:85-92 includes:
proxy: serverUrl
? {
"/rpc": serverUrl,
"/chat": serverUrl,
"/mcp": serverUrl,
"/oauth": serverUrl,
"/.well-known": serverUrl,
}
: undefined,However, the OAuth external endpoints use /oauth-external prefix (see PlaygroundAuth.tsx:102, PlaygroundAuth.tsx:205, PlaygroundAuth.tsx:250):
/oauth-external/status/oauth-external/disconnect/oauth-external/authorize
Impact
In development mode, requests to these endpoints will not be proxied to the backend server, causing:
- OAuth status checks to fail with network errors or 404s
- OAuth connect/disconnect operations to fail
- The feature to be completely non-functional during local development
Recommendation: Add /oauth-external to the proxy configuration: "/oauth-external": serverUrl,
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| data: oauthStatus, | ||
| isLoading: statusLoading, | ||
| refetch: refetchStatus, | ||
| } = useExternalMcpOAuthStatus(toolset?.id, mcpOAuthConfig); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 OAuth status query never runs due to incorrect options parameter type
The useExternalMcpOAuthStatus hook is called with mcpOAuthConfig (type ExternalMCPToolDefinition) as the second argument, but the hook expects options?: { slug?: string; enabled?: boolean }.
Click to expand
Root Cause
The hook signature at PlaygroundAuth.tsx:80-85 expects:
options?: {
slug?: string;
enabled?: boolean;
}But it's called with mcpOAuthConfig which is ExternalMCPToolDefinition | undefined. While ExternalMCPToolDefinition has a slug property, it does NOT have an enabled property.
Impact
At line 128, the query's enabled option is computed as:
enabled: options?.enabled && !!toolsetId,Since ExternalMCPToolDefinition doesn't have an enabled property, options?.enabled evaluates to undefined, making the entire expression falsy. This means the query will never be enabled and OAuth status will never be fetched.
Actual vs Expected
- Actual: OAuth status query never runs; users always appear unauthenticated even when connected
- Expected: OAuth status should be fetched when
mcpOAuthConfigexists andtoolsetIdis available
Recommendation: Change the hook call to pass the correct options object:
} = useExternalMcpOAuthStatus(toolset?.id, {
slug: mcpOAuthConfig?.slug,
enabled: !!mcpOAuthConfig,
});Also fix the same issue in PlaygroundElements.tsx:104.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ); | ||
|
|
||
| const { data: oauthStatus, isLoading: oauthStatusLoading } = | ||
| useExternalMcpOAuthStatus(toolset?.id, mcpOAuthConfig); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 OAuth status query never runs in PlaygroundElements due to same incorrect parameter
Same issue as BUG-0001 but in PlaygroundElements.tsx. The useExternalMcpOAuthStatus hook is called with mcpOAuthConfig as the second argument instead of the expected options object.
Click to expand
Impact
The check at PlaygroundElements.tsx:147-158 that blocks rendering when OAuth is required but user is not authenticated will never work correctly because the OAuth status query never runs.
Since oauthStatus will never be populated, the condition oauthStatus?.status !== "authenticated" will always be true when mcpOAuthConfig exists, causing the OAuthRequiredNotice to always be shown even after the user has successfully authenticated.
Recommendation: Change the hook call to:
const { data: oauthStatus, isLoading: oauthStatusLoading } =
useExternalMcpOAuthStatus(toolset?.id, {
slug: mcpOAuthConfig?.slug,
enabled: !!mcpOAuthConfig,
});Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Implements MCP OAuth 2.1 with Dynamic Client Registration (DCR) per RFC 7591 for external MCP servers in the Gram Playground.
The MCP service will detect requests to external MCP servers (from the catalog) that originate from the playground. If a token for that MCP server has been stored, then Gram will inject it into the request headers before forwarding it to the underlying MCP server.
UI/UX Updates
When viewing a toolset that does not have an OAuth token available yet, the user won't be able to interact with the playground chat.
Clicking "Connect" spawns a popup that allows the user to connect their 3rd party account.
Known Limitations
Related
Resolves AGE-1150