From f7454cf94ee2d2f52d5ae7ffb80057bf8a65ee56 Mon Sep 17 00:00:00 2001 From: Danny White <3104761+dnywh@users.noreply.github.com> Date: Mon, 3 Aug 2026 09:57:27 +1000 Subject: [PATCH] feat(studio): oauth impersonation warning on authorize (#48162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What kind of change does this PR introduce? Feature + docs. Stacked on #48161 (logo contract / [DEPR-604](https://linear.app/supabase/issue/DEPR-604/define-connect-logo-asset-and-variant-contract)). ## What is the current behavior? After #48161, curated logos only resolve from allowlisted `redirect_uri` hosts. A requester can still present a trusted partner **name** (e.g. Claude) while redirecting to an unrelated remote host; the UI shows Supabase alone but does not call out the mismatch. ## What is the new behavior? - Shows a caution admonition when the requester name looks like a trusted partner (Claude, Cursor, ChatGPT/OpenAI, Perplexity) but `redirect_uri` is a **remote** host outside that partner's allowlist. - Skips localhost / loopback redirects for the caution (common for local MCP clients); those still get curated logos when the name matches a trusted partner. - Highlights the footer redirect URL in warning colour when the caution is shown. - Documents the behaviour in the Connect interstitials pattern. ### To test Real MCP clients (Claude, Cursor, etc.) only send users to **production** `/authorize`, so you cannot drive a local or preview Studio build from those tools. Use a Network override instead: 1. Start Studio and sign in (`pnpm dev:studio`, or use the [Vercel preview](https://studio-staging-git-danny-oauth-impersonation-warning-supabase.vercel.app/)). 2. Open `/dashboard/authorize?auth_id=foo` (any `auth_id` is fine; the real response may 404) ([Vercel preview](https://studio-staging-git-danny-oauth-impersonation-warning-supabase.vercel.app/dashboard/authorize?auth_id=foo)). 3. DevTools → **Network** → find `GET …/platform/oauth/authorizations/foo` (or whatever id you used). 4. Right-click → **Override content** (enable Local Overrides / pick a folder if prompted). 5. Paste one of the payloads below (status **200**), save, then reload the authorize page. 6. Keep `expires_at` in the future so the request does not look expired. #### Impersonation caution (trusted name + remote non-allowlisted redirect) Expect: - Supabase alone (no curated Claude mark) - Caution: “Redirect does not match this app name” - Footer redirect URL in warning colour ```json { "name": "Claude", "website": "https://claude.ai", "icon": null, "domain": "claude.ai", "redirect_uri": "https://evil.com/callback", "expires_at": "2099-01-01T00:00:00.000Z", "scopes": ["organizations:read", "projects:read"], "approved_at": null, "registration_type": "dynamic" } ``` | Preview | | --- | | Authorize Claude Supabase | #### Localhost MCP: no caution Expect curated Claude + Supabase pair (name match + loopback), **no** caution, normal footer colour. Local MCP clients often use loopback redirects. ```json { "name": "Claude", "website": "https://claude.ai", "icon": null, "domain": "claude.ai", "redirect_uri": "http://127.0.0.1:42813/callback", "expires_at": "2099-01-01T00:00:00.000Z", "scopes": ["organizations:read", "projects:read"], "approved_at": null, "registration_type": "dynamic" } ``` | Preview | | --- | | Authorize Claude Supabase | #### Legitimate curated partner: no caution Expect curated Cursor + Supabase pair, no admonition, normal footer colour. ```json { "name": "Cursor", "website": "https://cursor.com", "icon": null, "domain": "cursor.com", "redirect_uri": "https://cursor.com/callback", "expires_at": "2099-01-01T00:00:00.000Z", "scopes": ["organizations:read", "projects:read"], "approved_at": null, "registration_type": "dynamic" } ``` | Preview | | --- | | 56164 | #### Unrelated name + remote redirect: no caution Expect Supabase alone (no icon), no admonition. ```json { "name": "Acme Tools", "website": "https://evil.com", "icon": null, "domain": "evil.com", "redirect_uri": "https://evil.com/callback", "expires_at": "2099-01-01T00:00:00.000Z", "scopes": ["organizations:read", "projects:read"], "approved_at": null, "registration_type": "dynamic" } ``` | Preview | | --- | | Authorize Acme Tools Supabase | ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Added an OAuth caution when a requester name matches a known partner but uses an unapproved remote redirect host. - Improved trusted partner logo selection for localhost/loopback redirects while preserving safe fallbacks for untrusted redirects. - **Documentation** - Updated Connect interstitial guidance for redirect mismatches and localhost/loopback behavior. - **Tests** - Expanded coverage for caution visibility, messaging, localhost logo pairing, and trusted redirect scenarios. --- .../ui-patterns/connect-interstitials.mdx | 11 ++- .../ApiAuthorization.Form.tsx | 18 +++- .../OAuthApps/AuthorizeRequesterDetails.tsx | 25 ++++- .../OAuthApps/OAuthApps.utils.test.ts | 89 ++++++++++++++++-- .../Organization/OAuthApps/OAuthApps.utils.ts | 91 +++++++++++++++++-- .../components/ApiAuthorization.test.tsx | 41 ++++++++- 6 files changed, 249 insertions(+), 26 deletions(-) diff --git a/apps/design-system/content/docs/ui-patterns/connect-interstitials.mdx b/apps/design-system/content/docs/ui-patterns/connect-interstitials.mdx index 4457f7ef69c..0ef1e2da938 100644 --- a/apps/design-system/content/docs/ui-patterns/connect-interstitials.mdx +++ b/apps/design-system/content/docs/ui-patterns/connect-interstitials.mdx @@ -165,12 +165,17 @@ the pair should use the dark set together. **Where logos come from on `/authorize`** -- Curated partner logos resolve from allowlisted `redirect_uri` hosts only, - not from self-asserted `name` or `website`. Those pairs may use theme tiles - and dark assets when the partner has them. +- Curated partner logos resolve from allowlisted `redirect_uri` hosts, or from + a trusted partner name when `redirect_uri` is localhost / loopback (local MCP + clients). Do not resolve curated logos from self-asserted `name` or `website` + on a remote host. Those pairs may use theme tiles and dark assets when the + partner has them. - Published organisation OAuth app icons uploaded in Studio remain trusted remote images, paired with forced-light tiles on both sides. - Everything else falls back to `SupabaseLogo` alone. +- If the requester name looks like a known partner but `redirect_uri` is a + remote host outside that partner's allowlist, show a caution admonition. + Localhost MCP redirects are excluded. ## Account row diff --git a/apps/studio/components/interfaces/ApiAuthorization/ApiAuthorization.Form.tsx b/apps/studio/components/interfaces/ApiAuthorization/ApiAuthorization.Form.tsx index 8a858ec839d..c5269d6ecfb 100644 --- a/apps/studio/components/interfaces/ApiAuthorization/ApiAuthorization.Form.tsx +++ b/apps/studio/components/interfaces/ApiAuthorization/ApiAuthorization.Form.tsx @@ -21,8 +21,10 @@ import { ShimmeringLoader } from 'ui-patterns/ShimmeringLoader' import type { ApprovalState, IApprovalFormSchema } from './ApiAuthorization.Schema' import { AuthorizeConnectLogo, + AuthorizeImpersonationWarning, AuthorizeRequesterDetails, } from '@/components/interfaces/Organization/OAuthApps/AuthorizeRequesterDetails' +import { getOAuthImpersonationWarning } from '@/components/interfaces/Organization/OAuthApps/OAuthApps.utils' import { InterstitialActionError, InterstitialLayout, @@ -115,6 +117,10 @@ export function ApiAuthorizationMainView({ ) : ( <> + {organizations._tag === 'loading' && } {organizations._tag === 'error' && ( @@ -312,6 +318,13 @@ function FormFooter({ onDecline, onApprove, }: FormFooterProps): ReactNode { + const hasImpersonationWarning = Boolean( + getOAuthImpersonationWarning({ + name: requester.name, + redirectUri: requester.redirect_uri, + }) + ) + return (

- Authorizing will redirect you to {redirectUrl} + Authorizing will redirect you to{' '} + + {redirectUrl} +

)} diff --git a/apps/studio/components/interfaces/Organization/OAuthApps/AuthorizeRequesterDetails.tsx b/apps/studio/components/interfaces/Organization/OAuthApps/AuthorizeRequesterDetails.tsx index ddff1416f86..ea477f87494 100644 --- a/apps/studio/components/interfaces/Organization/OAuthApps/AuthorizeRequesterDetails.tsx +++ b/apps/studio/components/interfaces/Organization/OAuthApps/AuthorizeRequesterDetails.tsx @@ -11,10 +11,11 @@ import { CollapsibleContent, CollapsibleTrigger, } from 'ui' +import { Admonition } from 'ui-patterns/Admonition' import { InfoTooltip } from 'ui-patterns/info-tooltip' import { PERMISSIONS_DESCRIPTIONS } from './OAuthApps.constants' -import { getRequesterLogo } from './OAuthApps.utils' +import { getOAuthImpersonationWarning, getRequesterLogo } from './OAuthApps.utils' import { CONNECT_LOGO_LIGHT_TILE_CLASSNAME, LogoBox, @@ -196,10 +197,11 @@ export const AuthorizeConnectLogo = ({ () => getRequesterLogo({ icon, + name, redirectUri, useDarkVariant: resolvedTheme === 'dark', }), - [icon, redirectUri, resolvedTheme] + [icon, name, redirectUri, resolvedTheme] ) const hasUsableLogo = Boolean(logo.src) && failedIcon !== logo.src @@ -227,6 +229,25 @@ export const AuthorizeConnectLogo = ({ ) } +export const AuthorizeImpersonationWarning = ({ + name, + redirectUri, +}: { + name: string + redirectUri?: string | null +}) => { + const warning = getOAuthImpersonationWarning({ name, redirectUri }) + if (!warning) return null + + return ( + + ) +} + export const AuthorizeRequesterDetails = ({ name, scopes, diff --git a/apps/studio/components/interfaces/Organization/OAuthApps/OAuthApps.utils.test.ts b/apps/studio/components/interfaces/Organization/OAuthApps/OAuthApps.utils.test.ts index cce0b1b7fe9..46f5f78ceeb 100644 --- a/apps/studio/components/interfaces/Organization/OAuthApps/OAuthApps.utils.test.ts +++ b/apps/studio/components/interfaces/Organization/OAuthApps/OAuthApps.utils.test.ts @@ -3,6 +3,7 @@ import { describe, expect, test } from 'vitest' import { findTrustedPartnerByRedirectUri, + getOAuthImpersonationWarning, getRedirectHostname, getRequesterLogo, hostMatchesAllowlist, @@ -60,9 +61,10 @@ describe('findTrustedPartnerByRedirectUri', () => { }) describe('getRequesterLogo', () => { - test('uses curated assets only when redirect host is allowlisted', () => { + test('uses curated assets when redirect host is allowlisted', () => { const trusted = getRequesterLogo({ icon: null, + name: 'Claude', redirectUri: 'https://claude.ai/api/mcp/auth_callback', useDarkVariant: false, }) @@ -70,22 +72,97 @@ describe('getRequesterLogo', () => { src: getMcpClientIconSrc({ icon: 'claude', useDarkVariant: false }), isKnownClient: true, }) + }) - const namedOnly = getRequesterLogo({ - icon: null, - redirectUri: 'https://evil.com/callback', - useDarkVariant: false, + test('uses curated assets for localhost when the name matches a trusted partner', () => { + expect( + getRequesterLogo({ + icon: null, + name: 'Claude', + redirectUri: 'http://127.0.0.1:42813/callback', + useDarkVariant: false, + }) + ).toEqual({ + src: getMcpClientIconSrc({ icon: 'claude', useDarkVariant: false }), + isKnownClient: true, }) - expect(namedOnly).toEqual({ src: '', isKnownClient: false }) + }) + + test('does not use curated assets from name alone on a remote host', () => { + expect( + getRequesterLogo({ + icon: null, + name: 'Claude', + redirectUri: 'https://evil.com/callback', + useDarkVariant: false, + }) + ).toEqual({ src: '', isKnownClient: false }) }) test('falls back to the supplied icon URL when redirect is not trusted', () => { expect( getRequesterLogo({ icon: 'https://example.com/icon.png', + name: 'Acme', redirectUri: 'https://evil.com/callback', useDarkVariant: false, }) ).toEqual({ src: 'https://example.com/icon.png', isKnownClient: false }) }) }) + +describe('getOAuthImpersonationWarning', () => { + test('warns when a trusted name redirects to a remote non-allowlisted host', () => { + expect( + getOAuthImpersonationWarning({ + name: 'Claude Desktop', + redirectUri: 'https://evil.com/callback', + }) + ).toEqual({ + brandDisplayName: 'Claude', + redirectHost: 'evil.com', + }) + }) + + test('skips localhost MCP redirects', () => { + expect( + getOAuthImpersonationWarning({ + name: 'Claude', + redirectUri: 'http://127.0.0.1:42813/callback', + }) + ).toBe(null) + }) + + test('skips when redirect host matches the named partner', () => { + expect( + getOAuthImpersonationWarning({ + name: 'Claude', + redirectUri: 'https://claude.ai/api/mcp/auth_callback', + }) + ).toBe(null) + }) + + test('skips when the name does not match a trusted partner', () => { + expect( + getOAuthImpersonationWarning({ + name: 'Acme Tools', + redirectUri: 'https://evil.com/callback', + }) + ).toBe(null) + }) + + test('skips missing or unparsable redirect URIs', () => { + expect( + getOAuthImpersonationWarning({ + name: 'Claude', + redirectUri: null, + }) + ).toBe(null) + expect( + getOAuthImpersonationWarning({ + name: 'Claude', + redirectUri: 'not-a-url', + }) + ).toBe(null) + }) +}) diff --git a/apps/studio/components/interfaces/Organization/OAuthApps/OAuthApps.utils.ts b/apps/studio/components/interfaces/Organization/OAuthApps/OAuthApps.utils.ts index bc0df5ef05e..e8f942cf7a0 100644 --- a/apps/studio/components/interfaces/Organization/OAuthApps/OAuthApps.utils.ts +++ b/apps/studio/components/interfaces/Organization/OAuthApps/OAuthApps.utils.ts @@ -1,6 +1,8 @@ import { getMcpClientIconSrc } from 'ui-patterns/McpUrlBuilder' export type TrustedOAuthPartner = { + /** Substrings matched against the requester name (case-insensitive). */ + nameMatchers: readonly string[] displayName: string icon: string hasDistinctDarkIcon: boolean @@ -10,28 +12,34 @@ export type TrustedOAuthPartner = { /** * High-traffic MCP / OAuth partners with curated Connect logos. - * Logos resolve from redirect_uri host only — never from self-asserted name/website. + * Logos resolve from allowlisted redirect_uri hosts, or from a trusted name when + * redirect_uri is localhost / loopback (common for local MCP clients). + * Never from self-asserted name alone on a remote host. */ export const TRUSTED_OAUTH_PARTNERS: readonly TrustedOAuthPartner[] = [ { + nameMatchers: ['claude'], displayName: 'Claude', icon: 'claude', hasDistinctDarkIcon: false, redirectHosts: ['claude.ai', 'anthropic.com'], }, { + nameMatchers: ['cursor'], displayName: 'Cursor', icon: 'cursor', hasDistinctDarkIcon: true, redirectHosts: ['cursor.com', 'cursor.sh'], }, { + nameMatchers: ['chatgpt', 'openai'], displayName: 'ChatGPT', icon: 'openai', hasDistinctDarkIcon: true, redirectHosts: ['chatgpt.com', 'openai.com'], }, { + nameMatchers: ['perplexity'], displayName: 'Perplexity', icon: 'perplexity', hasDistinctDarkIcon: true, @@ -78,24 +86,89 @@ export function findTrustedPartnerByRedirectUri( ) } +export function findTrustedPartnerByName(name: string): TrustedOAuthPartner | null { + const searchable = name.toLowerCase() + return ( + TRUSTED_OAUTH_PARTNERS.find((partner) => + partner.nameMatchers.some((matcher) => searchable.includes(matcher)) + ) ?? null + ) +} + +function curatedLogoForPartner( + partner: TrustedOAuthPartner, + useDarkVariant: boolean +): { src: string; isKnownClient: boolean } | null { + const customLogoUrl = getMcpClientIconSrc({ + icon: partner.icon, + useDarkVariant, + hasDistinctDarkIcon: partner.hasDistinctDarkIcon, + }) + if (!customLogoUrl) return null + return { src: customLogoUrl, isKnownClient: true } +} + export function getRequesterLogo({ icon, + name, redirectUri, useDarkVariant, }: { icon: string | null + name?: string | null redirectUri: string | null | undefined useDarkVariant: boolean }): { src: string; isKnownClient: boolean } { - const trusted = findTrustedPartnerByRedirectUri(redirectUri) - if (trusted) { - const customLogoUrl = getMcpClientIconSrc({ - icon: trusted.icon, - useDarkVariant, - hasDistinctDarkIcon: trusted.hasDistinctDarkIcon, - }) - if (customLogoUrl) return { src: customLogoUrl, isKnownClient: true } + const byRedirect = findTrustedPartnerByRedirectUri(redirectUri) + if (byRedirect) { + const curated = curatedLogoForPartner(byRedirect, useDarkVariant) + if (curated) return curated + } + + // Local MCP clients (Claude Desktop, Cursor, etc.) use loopback redirects. + // Name match is enough there — remote hosts still require the allowlist. + const hostname = getRedirectHostname(redirectUri) + if (hostname && isLocalRedirectHost(hostname) && name) { + const byName = findTrustedPartnerByName(name) + if (byName) { + const curated = curatedLogoForPartner(byName, useDarkVariant) + if (curated) return curated + } } return { src: icon || '', isKnownClient: false } } + +export type OAuthImpersonationWarning = { + /** Trusted partner label used in the caution copy. */ + brandDisplayName: string + redirectHost: string +} + +/** + * Warn when the requester name looks like a known partner but redirect_uri is a + * remote host outside that partner's allowlist. Localhost redirects are skipped + * (common for local MCP clients). Missing or malformed redirect URIs are skipped. + */ +export function getOAuthImpersonationWarning({ + name, + redirectUri, +}: { + name: string + redirectUri: string | null | undefined +}): OAuthImpersonationWarning | null { + const namedPartner = findTrustedPartnerByName(name) + if (!namedPartner) return null + + const hostname = getRedirectHostname(redirectUri) + if (!hostname || isLocalRedirectHost(hostname)) return null + + if (hostMatchesAllowlist(hostname, namedPartner.redirectHosts)) { + return null + } + + return { + brandDisplayName: namedPartner.displayName, + redirectHost: hostname, + } +} diff --git a/apps/studio/tests/components/ApiAuthorization.test.tsx b/apps/studio/tests/components/ApiAuthorization.test.tsx index 4f88fadf43e..c1046c17aa3 100644 --- a/apps/studio/tests/components/ApiAuthorization.test.tsx +++ b/apps/studio/tests/components/ApiAuthorization.test.tsx @@ -148,6 +148,22 @@ describe('AuthorizeConnectLogo', () => { expect(screen.queryByAltText('Claude')).not.toBeInTheDocument() }) + test('pairs curated logos for localhost when the name matches a trusted partner', () => { + customRender( + + ) + + expect(screen.getByAltText('Claude')).toHaveAttribute( + 'src', + getMcpClientIconSrc({ icon: 'claude', useDarkVariant: false }) + ) + expect(screen.getByAltText('Supabase')).toBeInTheDocument() + }) + test('shows Supabase alone when the requester has no icon', () => { customRender() @@ -346,9 +362,12 @@ describe('ApiAuthorizationScreen', () => { await screen.findByText('Authorize API access for Cursor') expect(screen.getByAltText('Cursor')).toBeInTheDocument() expect(screen.getByAltText('Supabase')).toBeInTheDocument() + expect( + screen.queryByText('Check this redirect before authorizing') + ).not.toBeInTheDocument() }) - test('shows Supabase alone when name looks trusted but redirect host is not allowlisted', async () => { + test('warns when a trusted name redirects to a non-allowlisted host', async () => { mockBothEndpoints( createMockAuthResponse({ name: 'Claude', @@ -357,7 +376,14 @@ describe('ApiAuthorizationScreen', () => { }) ) renderScreen() - await screen.findByText('Authorize API access for Claude') + expect( + await screen.findByText('Check this redirect before authorizing') + ).toBeInTheDocument() + expect( + screen.getByText( + 'This request uses the name Claude, but after you authorize you will be redirected to evil.com, not Claude.' + ) + ).toBeInTheDocument() expect(screen.queryByAltText('Claude')).not.toBeInTheDocument() expect(screen.getByAltText('Supabase')).toBeInTheDocument() }) @@ -382,14 +408,19 @@ describe('ApiAuthorizationScreen', () => { describe('expiration', () => { test('shows expiration warning and hides action buttons when request has expired', async () => { mockBothEndpoints( - createMockAuthResponse({ expires_at: dayjs().subtract(1, 'hour').toISOString() }) + createMockAuthResponse({ + name: 'Claude', + redirect_uri: 'https://evil.com/callback', + expires_at: dayjs().subtract(1, 'hour').toISOString(), + }) ) renderScreen() await screen.findByText('Authorization request expired') - expect(screen.queryByRole('button', { name: 'Cancel' })).not.toBeInTheDocument() expect( - screen.queryByRole('button', { name: /Authorize Test App/ }) + screen.queryByText('Check this redirect before authorizing') ).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Cancel' })).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: /Authorize Claude/ })).not.toBeInTheDocument() }) test('does not show expiration warning when request has not expired', async () => {