Skip to content

fix(mcp): use dynamic client registration for OAuth instead of hardcoded client ID #1313

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.

Sign up for GitHub

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

Merged
Kitenite merged 1 commit into from
Feb 8, 2026
+13 −10
Merged

fix(mcp): use dynamic client registration for OAuth instead of hardcoded client ID #1313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions .mcp.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,7 @@
"mcpServers": {
"superset": {
"type": "http",
"url": "https://api.superset.sh/api/agent/mcp",
"oauth": {
"clientId": "claude-code",
"authorizationUrl": "https://api.superset.sh/api/auth/mcp/authorize",
"tokenUrl": "https://api.superset.sh/api/auth/mcp/token",
"scopes": ["openid", "profile", "email"]
}
"url": "https://api.superset.sh/api/agent/mcp"
},
"expo-mcp": {
"type": "http",
Expand Down
8 changes: 5 additions & 3 deletions apps/web/src/app/oauth/consent/components/ConsentForm/ConsentForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ interface Organization {
interface ConsentFormProps {
consentCode: string;
clientId: string;
clientName?: string;
scopes: string[];
userName: string;
organizations: Organization[];
Expand Down Expand Up @@ -57,6 +58,7 @@ const SCOPE_DESCRIPTIONS: Record<
export function ConsentForm({
consentCode,
clientId,
clientName,
scopes,
userName,
organizations,
Expand Down Expand Up @@ -134,16 +136,16 @@ export function ConsentForm({
}
};

const clientName = getClientDisplayName(clientId);
const displayName = clientName ?? getClientDisplayName(clientId);

return (
<div className="mx-auto flex w-full flex-col justify-center space-y-6 sm:w-[400px]">
<div className="flex flex-col space-y-2 text-center">
<h1 className="text-2xl font-semibold tracking-tight">
Authorize {clientName}
Authorize {displayName}
</h1>
<p className="text-muted-foreground text-sm">
<span className="font-medium text-foreground">{clientName}</span> is
<span className="font-medium text-foreground">{displayName}</span> is
requesting access to your Superset account
</p>
Comment on lines +145 to 150
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Consider whether displaying an attacker-controlled name needs sanitization or a visual trust indicator.

With dynamic client registration, any caller can register an application with an arbitrary name (e.g., "Superset Desktop" or "Google"). The consent screen currently renders that name without any trust differentiation. This could be used to impersonate a known/trusted application.

Consider either:

  • Showing the raw clientId alongside the display name so users can distinguish dynamically registered apps.
  • Adding a "verified" badge for known/trusted clients only.
🤖 Prompt for AI Agents 
In `@apps/web/src/app/oauth/consent/components/ConsentForm/ConsentForm.tsx` around
lines 145 - 150, The consent screen currently renders the attacker-controlled
displayName in ConsentForm; update ConsentForm (where displayName is used) to
show the raw clientId alongside displayName and/or a visual "verified" badge for
trusted clients: fetch or accept a isVerified flag for the client and render a
small badge component next to displayName when true, and render clientId in
muted text under or beside the name when not verified; ensure
displayName/clientId are output as plain text (React JSX escaping) or explicitly
sanitize them before rendering to avoid any HTML injection.

</div>
Expand Down
7 changes: 7 additions & 0 deletions apps/web/src/app/oauth/consent/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { auth } from "@superset/auth/server";
import { db } from "@superset/db/client";
import { headers } from "next/headers";
import Image from "next/image";
import { redirect } from "next/navigation";
Expand Down Expand Up @@ -63,6 +64,11 @@ export default async function ConsentPage({ searchParams }: ConsentPageProps) {
const trpc = await api();
const userOrganizations = await trpc.user.myOrganizations.query();

const oauthApp = await db.query.oauthApplications.findFirst({
where: (table, { eq }) => eq(table.clientId, client_id),
columns: { name: true },
});

const extendedSession = session.session as typeof session.session & {
activeOrganizationId?: string | null;
};
Expand All @@ -86,6 +92,7 @@ export default async function ConsentPage({ searchParams }: ConsentPageProps) {
<ConsentForm
consentCode={consent_code}
clientId={client_id}
clientName={oauthApp?.name ?? undefined}
scopes={scopes}
userName={session.user.name}
organizations={userOrganizations.map((org) => ({
Expand Down