diff --git a/.claude/skills/react-hook-form/SKILL.md b/.claude/skills/react-hook-form/SKILL.md index 29571cb24c742..c08c5037a2b4b 100644 --- a/.claude/skills/react-hook-form/SKILL.md +++ b/.claude/skills/react-hook-form/SKILL.md @@ -235,6 +235,12 @@ schema, `onChange`, rendering, and the submit mapping. - Gate Save on `isDirty`; show Cancel only when dirty. In the owner, destructure from `form.formState`; anywhere else, `useFormState({ control })`. +- When the form lives in a Sheet or Dialog, also wire dirty dismissal: + `useConfirmOnClose` + `DiscardChangesConfirmationDialog`. Route Cancel, + Escape, and backdrop through the guard; call the raw `onClose` on successful + submit so you do not prompt after save. Details: + `apps/design-system/content/docs/ui-patterns/modality.mdx` (Dirty form + dismissal) and the studio-ui-patterns skill Sheets section. - To show _which_ fields changed (review/summary dialogs), read `dirtyFields` from the same subscription instead of hand-comparing `defaultValues.x !== watchedX`. RHF already does that comparison correctly; diff --git a/.claude/skills/studio-ui-patterns/SKILL.md b/.claude/skills/studio-ui-patterns/SKILL.md index cc875ec57104a..3f8740ec5040b 100644 --- a/.claude/skills/studio-ui-patterns/SKILL.md +++ b/.claude/skills/studio-ui-patterns/SKILL.md @@ -125,6 +125,10 @@ Forms in sheets: - `layout="horizontal"` for wider sheets - `layout="vertical"` for narrow sheets (`size="sm"` or below) +- When the sheet contains a form, wire dirty dismissal with `useConfirmOnClose` + + `DiscardChangesConfirmationDialog` (Cancel, Escape, and backdrop). Source of + truth: `apps/design-system/content/docs/ui-patterns/modality.mdx` (Dirty form + dismissal). Also see the react-hook-form skill for `isDirty` destructuring. ## Copy 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 99c0671ea699e..0ef1e2da938da 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 @@ -217,20 +222,14 @@ Match feedback to its scope: feedback for a failure the user needs to resolve on the current card. ```tsx -{ - actionError && ( -
-

- {actionError} -

-
- ) -} + ``` Clear stale action feedback when the user retries or changes a relevant selection. Error copy should say what failed and, when it is not obvious, what -the user can do next. +the user can do next. When passive supporting copy occupies the same footer +region, replace it with the action error until the error is cleared instead of +stacking both messages. Cancel -
-

- Failed to authorize Stripe Projects. Please try again. -

-
+ diff --git a/apps/design-system/registry/default/example/connect-interstitial-shared.tsx b/apps/design-system/registry/default/example/connect-interstitial-shared.tsx index e9100175ae04e..49c53778e8032 100644 --- a/apps/design-system/registry/default/example/connect-interstitial-shared.tsx +++ b/apps/design-system/registry/default/example/connect-interstitial-shared.tsx @@ -120,6 +120,18 @@ export function InterstitialShell({ ) } +export function InterstitialActionError({ error }: { error?: React.ReactNode }) { + if (!error) return null + + return ( +
+

+ {error} +

+
+ ) +} + export function SignOutButton() { return - {redirectUrl && ( + + {!actionError && redirectUrl && (

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

)} diff --git a/apps/studio/components/interfaces/ApiAuthorization/ApiAuthorization.Valid.tsx b/apps/studio/components/interfaces/ApiAuthorization/ApiAuthorization.Valid.tsx index 621e49ea9d7ce..6d1ab488c80f4 100644 --- a/apps/studio/components/interfaces/ApiAuthorization/ApiAuthorization.Valid.tsx +++ b/apps/studio/components/interfaces/ApiAuthorization/ApiAuthorization.Valid.tsx @@ -142,38 +142,57 @@ export function ApiAuthorizationValidScreen({ } = useApiAuthorizationQuery({ id: auth_id }) const isApproved = (requester?.approved_at ?? null) !== null - const { mutate: approveRequest } = useApiAuthorizationApproveMutation({ + const { + mutate: approveRequest, + error: approveError, + reset: resetApproveError, + } = useApiAuthorizationApproveMutation({ onSuccess: (res) => { window.location.href = res.url }, + onError: () => { + setApprovalState('indeterminate') + }, }) - const { mutate: declineRequest } = useApiAuthorizationDeclineMutation({ + const { + mutate: declineRequest, + error: declineError, + reset: resetDeclineError, + } = useApiAuthorizationDeclineMutation({ onSuccess: () => { toast.success('Declined API authorization request') navigate('/organizations') }, + onError: () => { + setApprovalState('indeterminate') + }, }) + const actionError = approveError + ? `Failed to authorize request: ${approveError.message}` + : declineError + ? `Failed to cancel authorization request: ${declineError.message}` + : undefined + const resetActionError = () => { + resetApproveError() + resetDeclineError() + } const onApproveRequest = form.handleSubmit((values) => { if (approvalState !== 'indeterminate') { return } + resetActionError() setApprovalState('approving') - approveRequest( - { id: auth_id, slug: values.selectedOrgSlug }, - { onError: () => setApprovalState('indeterminate') } - ) + approveRequest({ id: auth_id, slug: values.selectedOrgSlug }) }) const onDeclineRequest = form.handleSubmit((values) => { if (approvalState !== 'indeterminate') { return } + resetActionError() setApprovalState('declining') - declineRequest( - { id: auth_id, slug: values.selectedOrgSlug }, - { onError: () => setApprovalState('indeterminate') } - ) + declineRequest({ id: auth_id, slug: values.selectedOrgSlug }) }) if (isLoading) { @@ -224,6 +243,8 @@ export function ApiAuthorizationValidScreen({ requester={effectiveRequester} requestedOrganizationSlug={effectiveOrganizationSlug} organizations={effectiveOrganizationsState} + actionError={actionError} + onOrganizationChange={resetActionError} onApprove={onApproveRequest} onDecline={onDeclineRequest} /> diff --git a/apps/studio/components/interfaces/Auth/CustomAuthProviders/CreateOrUpdateCustomProviderSheet.tsx b/apps/studio/components/interfaces/Auth/CustomAuthProviders/CreateOrUpdateCustomProviderSheet.tsx index c46a10f81e478..7a910e1e79026 100644 --- a/apps/studio/components/interfaces/Auth/CustomAuthProviders/CreateOrUpdateCustomProviderSheet.tsx +++ b/apps/studio/components/interfaces/Auth/CustomAuthProviders/CreateOrUpdateCustomProviderSheet.tsx @@ -250,12 +250,7 @@ export const CreateOrUpdateCustomProviderSheet = ({ return ( - +
diff --git a/apps/studio/components/interfaces/ConnectSheet/ConnectSheet.tsx b/apps/studio/components/interfaces/ConnectSheet/ConnectSheet.tsx index 6ea2ce855cdb5..a6684e24f0af4 100644 --- a/apps/studio/components/interfaces/ConnectSheet/ConnectSheet.tsx +++ b/apps/studio/components/interfaces/ConnectSheet/ConnectSheet.tsx @@ -162,7 +162,6 @@ export const ConnectSheet = () => { Connect to your project diff --git a/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/NewPublicationPanel.tsx b/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/NewPublicationPanel.tsx index 9e5ca2799ddf5..57991eda2e6fd 100644 --- a/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/NewPublicationPanel.tsx +++ b/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/NewPublicationPanel.tsx @@ -20,10 +20,12 @@ import { FormItemLayout } from 'ui-patterns/form/FormItemLayout/FormItemLayout' import { MultiSelector } from 'ui-patterns/multi-select' import { z } from 'zod' +import { DiscardChangesConfirmationDialog } from '@/components/ui-patterns/Dialogs/DiscardChangesConfirmationDialog' import { useCreatePublicationMutation } from '@/data/replication/publication-create-mutation' import { useReplicationSourceId } from '@/data/replication/sources-query' import { useReplicationTablesQuery } from '@/data/replication/tables-query' import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject' +import { useConfirmOnClose } from '@/hooks/ui/useConfirmOnClose' interface NewPublicationPanelProps { visible: boolean @@ -37,21 +39,12 @@ export const NewPublicationPanel = ({ visible, onClose }: NewPublicationPanelPro const { data: tables } = useReplicationTablesQuery({ projectRef, sourceId }, { enabled: visible }) - const { mutate: createPublication, isPending: creatingPublication } = - useCreatePublicationMutation({ - onSuccess: (_, vars) => { - toast.success('Successfully created publication') - form.reset(defaultValues) - onClose(vars.name) - }, - }) - const formId = 'publication-editor' const FormSchema = z.object({ name: z.string().min(1, 'Name is required'), tables: z.array(z.string()).min(1, 'At least one table is required'), }) - const defaultValues = { + const defaultValues: z.infer = { name: '', tables: [], } @@ -62,6 +55,28 @@ export const NewPublicationPanel = ({ visible, onClose }: NewPublicationPanelPro defaultValues, }) + // Always destructure formState values otherwise they won't be updated + // See https://react-hook-form.com/docs/useform/formstate + const { isDirty } = form.formState + + const closePanel = (newPublication?: string) => { + form.reset(defaultValues) + onClose(newPublication) + } + + const { confirmOnClose, handleOpenChange, modalProps } = useConfirmOnClose({ + checkIsDirty: () => isDirty, + onClose: () => closePanel(), + }) + + const { mutate: createPublication, isPending: creatingPublication } = + useCreatePublicationMutation({ + onSuccess: (_, vars) => { + toast.success('Successfully created publication') + closePanel(vars.name) + }, + }) + const onSubmit = async (data: z.infer) => { if (!projectRef) return console.error('Project ref is required') if (!project) return console.error('Project is required') @@ -82,80 +97,83 @@ export const NewPublicationPanel = ({ visible, onClose }: NewPublicationPanelPro } return ( - onClose()}> - -
- - Create a new publication - Choose which tables to replicate to destinations. - - - - - ( - - - - - - )} - /> - ( - - - - - - - {tables?.map((table) => ( - - {`${table.schema}.${table.name}`} - - ))} - - - - - - )} - /> - - - - - - - -
- - + <> + + +
+ + Create a new publication + Choose which tables to replicate to destinations. + + +
+ + ( + + + + + + )} + /> + ( + + + + + + + {tables?.map((table) => ( + + {`${table.schema}.${table.name}`} + + ))} + + + + + + )} + /> + + +
+ + + + +
+ + + + ) } diff --git a/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/index.tsx b/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/index.tsx index a88710f4b5365..69b6b7b1e8fa1 100644 --- a/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/index.tsx +++ b/apps/studio/components/interfaces/Database/Replication/DestinationPanel/DestinationForm/index.tsx @@ -3,7 +3,7 @@ import { PermissionAction } from '@supabase/shared-types/out/constants' import { useParams } from 'common' import { AnimatePresence, motion } from 'framer-motion' import { Loader2 } from 'lucide-react' -import { useEffect, useMemo, useRef, useState, type ReactNode } from 'react' +import { RefObject, useEffect, useMemo, useRef, useState, type ReactNode } from 'react' import { useForm, useWatch } from 'react-hook-form' import { AWS_REGIONS } from 'shared-data' import { toast } from 'sonner' @@ -83,7 +83,9 @@ interface DestinationFormProps { visible: boolean existingDestination?: ExistingDestination typeSelection?: ReactNode + checkIsDirtyRef?: RefObject<() => boolean> onClose: () => void + onCancel?: () => void } export const DestinationForm = ({ @@ -91,7 +93,9 @@ export const DestinationForm = ({ visible, existingDestination, typeSelection, + checkIsDirtyRef, onClose, + onCancel = onClose, }: DestinationFormProps) => { const { ref: projectRef } = useParams() @@ -261,6 +265,10 @@ export const DestinationForm = ({ defaultValues, }) + // Always destructure formState values otherwise they won't be updated + // See https://react-hook-form.com/docs/useform/formstate + const { isDirty } = form.formState + const publicationName = useWatch({ control: form.control, name: 'publicationName' }) const publicationNames = useMemo(() => publications?.map((pub) => pub.name) ?? [], [publications]) @@ -409,11 +417,22 @@ export const DestinationForm = ({ } useEffect(() => { - if (visible && !form.formState.isDirty) { + if (!checkIsDirtyRef) return + + checkIsDirtyRef.current = () => isDirty + return () => { + checkIsDirtyRef.current = () => false + } + }, [checkIsDirtyRef, isDirty]) + + useEffect(() => { + // Reset when closed (including after discard) so reopening does not restore + // discarded values, and when open but pristine so async defaults can apply. + if (!visible || !isDirty) { form.reset(defaultValues) resetValidation() } - }, [visible, defaultValues, form, resetValidation]) + }, [visible, defaultValues, form, isDirty, resetValidation]) useEffect(() => { if (visible && projectRef && sourceId) { @@ -567,7 +586,7 @@ export const DestinationForm = ({ )}
-
- -

- - Learn more - {' '} - about billing through AWS. -

+
+ + +
+ {!linkError && ( +

+ + Learn more + {' '} + about billing through AWS. +

+ )}
diff --git a/apps/studio/components/interfaces/Organization/OAuthApps/AuthorizeRequesterDetails.tsx b/apps/studio/components/interfaces/Organization/OAuthApps/AuthorizeRequesterDetails.tsx index ddff1416f86a7..ea477f8749466 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 cce0b1b7fe911..46f5f78ceeb1d 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 bc0df5ef05ee4..e8f942cf7a0ca 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/components/interfaces/OrganizationInvite/OrganizationInvite.tsx b/apps/studio/components/interfaces/OrganizationInvite/OrganizationInvite.tsx index 7e62eb43785b2..bdd1c39c301d1 100644 --- a/apps/studio/components/interfaces/OrganizationInvite/OrganizationInvite.tsx +++ b/apps/studio/components/interfaces/OrganizationInvite/OrganizationInvite.tsx @@ -13,6 +13,7 @@ import { import { OrganizationInviteError } from './OrganizationInviteError' import { InterstitialAccountRow, + InterstitialActionError, InterstitialLayout, SupabaseLogo, } from '@/components/layouts/InterstitialLayout' @@ -197,13 +198,9 @@ export const OrganizationInvite = () => { - {joinError && ( -
-

- Failed to join organization: {joinError.message} -

-
- )} + ) diff --git a/apps/studio/components/interfaces/Storage/StorageExplorer/FileExplorerRow.tsx b/apps/studio/components/interfaces/Storage/StorageExplorer/FileExplorerRow.tsx index 48c7bc68b5d64..f63c45e77ea86 100644 --- a/apps/studio/components/interfaces/Storage/StorageExplorer/FileExplorerRow.tsx +++ b/apps/studio/components/interfaces/Storage/StorageExplorer/FileExplorerRow.tsx @@ -268,6 +268,10 @@ export const FileExplorerRow = ({ const mimeType = item.metadata ? item.metadata.mimetype : '-' const createdAt = item.created_at ? new Date(item.created_at).toLocaleString() : '-' const updatedAt = item.updated_at ? new Date(item.updated_at).toLocaleString() : '-' + const isFile = item.type === STORAGE_ROW_TYPES.FILE + // Files: checkbox replaces icon on hover, keyboard focus, and when selected. + // Folders: icon only (no selection checkbox). + const showRowIcon = !isFile || !isSelected const nameWidth = view === STORAGE_VIEWS.LIST && item.isCorrupted @@ -290,12 +294,14 @@ export const FileExplorerRow = ({ >
{ event.stopPropagation() @@ -313,12 +319,14 @@ export const FileExplorerRow = ({ view === STORAGE_VIEWS.LIST ? 'w-[40%] min-w-[250px]' : 'w-[90%]' )} > -
event.stopPropagation()}> - {!isSelected && ( +
+ {showRowIcon && (
)} - { - onCheckItem(event.nativeEvent.shiftKey) - }} - aria-label="Check to select this item" - /> + {isFile ? ( + { + event.stopPropagation() + onCheckItem(event.nativeEvent.shiftKey) + }} + aria-label="Check to select this item" + /> + ) : ( + // Reserve the same slot as the file checkbox without a focusable control + + )}

{item.name} @@ -382,7 +398,7 @@ export const FileExplorerRow = ({ /> ) : ( - +

{item.name} actions diff --git a/apps/studio/components/interfaces/Storage/VectorBuckets/VectorBucketDetails/VectorBucketTableExamplesSheet.tsx b/apps/studio/components/interfaces/Storage/VectorBuckets/VectorBucketDetails/VectorBucketTableExamplesSheet.tsx index 75b8469bc02b2..171d331a4c7a5 100644 --- a/apps/studio/components/interfaces/Storage/VectorBuckets/VectorBucketDetails/VectorBucketTableExamplesSheet.tsx +++ b/apps/studio/components/interfaces/Storage/VectorBuckets/VectorBucketDetails/VectorBucketTableExamplesSheet.tsx @@ -62,7 +62,7 @@ export const VectorBucketTableExamplesSheet = ({ index }: VectorBucketTableExamp Insert vectors - +
diff --git a/apps/studio/components/layouts/InterstitialLayout.tsx b/apps/studio/components/layouts/InterstitialLayout.tsx index 1764b546c7259..d7597dd32a450 100644 --- a/apps/studio/components/layouts/InterstitialLayout.tsx +++ b/apps/studio/components/layouts/InterstitialLayout.tsx @@ -204,3 +204,15 @@ export const InterstitialAccountRow = ({ ) + +export const InterstitialActionError = ({ error }: { error?: ReactNode }) => { + if (!error) return null + + return ( +
+

+ {error} +

+
+ ) +} diff --git a/apps/studio/tests/components/ApiAuthorization.test.tsx b/apps/studio/tests/components/ApiAuthorization.test.tsx index 477bc847fd108..c1046c17aa3f1 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 () => { @@ -419,6 +450,25 @@ describe('ApiAuthorizationScreen', () => { await user.click(screen.getByRole('button', { name: /Authorize Test App/ })) await waitFor(() => expect(approveHandler).toHaveBeenCalled()) }) + + test('shows an approval failure inline and keeps the action available', async () => { + const user = userEvent.setup() + mockBothEndpoints() + addAPIMock({ + method: 'post', + path: '/platform/organizations/:slug/oauth/authorizations/:id', + response: () => + HttpResponse.json({ message: 'Authorization failed' }, { status: 500 }), + }) + renderScreen() + + await user.click(await screen.findByRole('button', { name: /Authorize Test App/ })) + + expect(await screen.findByRole('alert')).toHaveTextContent( + 'Failed to authorize request: Authorization failed' + ) + expect(screen.getByRole('button', { name: /Authorize Test App/ })).toBeEnabled() + }) }) describe('decline action', () => { @@ -439,6 +489,25 @@ describe('ApiAuthorizationScreen', () => { await waitFor(() => expect(declineHandler).toHaveBeenCalled()) await waitFor(() => expect(navigate).toHaveBeenCalledWith('/organizations')) }) + + test('shows a cancellation failure inline and keeps the action available', async () => { + const user = userEvent.setup() + mockBothEndpoints() + addAPIMock({ + method: 'delete', + path: '/platform/organizations/:slug/oauth/authorizations/:id', + response: () => + HttpResponse.json({ message: 'Cancellation failed' }, { status: 500 }), + }) + renderScreen() + + await user.click(await screen.findByRole('button', { name: 'Cancel' })) + + expect(await screen.findByRole('alert')).toHaveTextContent( + 'Failed to cancel authorization request: Cancellation failed' + ) + expect(screen.getByRole('button', { name: 'Cancel' })).toBeEnabled() + }) }) describe('form validation', () => { diff --git a/apps/studio/tests/pages/aws-marketplace-onboarding.test.tsx b/apps/studio/tests/pages/aws-marketplace-onboarding.test.tsx index 1422fe9d8b92f..466f16385bb27 100644 --- a/apps/studio/tests/pages/aws-marketplace-onboarding.test.tsx +++ b/apps/studio/tests/pages/aws-marketplace-onboarding.test.tsx @@ -3,6 +3,7 @@ import userEvent from '@testing-library/user-event' import { platformComponents as components } from 'api-types' import { LOCAL_STORAGE_KEYS } from 'common' import { http, HttpResponse } from 'msw' +import { toast } from 'sonner' import { beforeEach, describe, expect, test, vi } from 'vitest' import { AwsMarketplaceOnboardingScreen } from '@/components/interfaces/Organization/CloudMarketplace/AwsMarketplaceOnboarding' @@ -17,6 +18,10 @@ import { createMockOrganizationResponse } from '@/tests/helpers' import { customRender } from '@/tests/lib/custom-render' import { addAPIMock, mswServer } from '@/tests/lib/msw' +vi.mock('sonner', () => ({ + toast: { error: vi.fn() }, +})) + type OrganizationResponse = components['schemas']['OrganizationResponse'] const DEFAULT_PROFILE_CONTEXT: ProfileContextType = { @@ -170,6 +175,28 @@ describe('AwsMarketplaceOnboardingScreen', () => { await screen.findByText('Organization linked') }) + test('renders a link failure inline and keeps the action available', async () => { + const user = userEvent.setup() + mockAwsEndpoints() + mswServer.use( + http.put(`${API_URL}/platform/organizations/:slug/cloud-marketplace/link`, () => + HttpResponse.json({ message: 'Marketplace link failed' }, { status: 500 }) + ) + ) + + renderScreen() + + await user.click(await screen.findByRole('button', { name: /Acme Production/ })) + await user.click(screen.getByRole('button', { name: 'Link organization' })) + + expect(await screen.findByRole('alert')).toHaveTextContent( + 'Failed to link organization: Marketplace link failed' + ) + expect(toast.error).not.toHaveBeenCalled() + expect(screen.queryByText(/Learn more/)).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Link organization' })).toBeEnabled() + }) + test('creates an AWS-managed organization with buyerId and returns to linked state', async () => { const user = userEvent.setup() let createRequest: unknown diff --git a/packages/ui/src/components/shadcn/ui/sheet.test.tsx b/packages/ui/src/components/shadcn/ui/sheet.test.tsx new file mode 100644 index 0000000000000..92cb5d8190dea --- /dev/null +++ b/packages/ui/src/components/shadcn/ui/sheet.test.tsx @@ -0,0 +1,58 @@ +import { render, screen, waitFor } from '@testing-library/react' +import { describe, expect, it } from 'vitest' + +import { Sheet, SheetContent, SheetDescription, SheetTitle } from './sheet' + +const TestSheet = ({ tabIndex }: { tabIndex?: number }) => { + const tabIndexProps = tabIndex === undefined ? {} : { tabIndex } + + return ( + + + Sheet title + Sheet description + + + + ) +} + +describe('SheetContent', () => { + it('focuses its first interactive child without making the sheet focusable', async () => { + render() + + const sheet = screen.getByRole('dialog') + const input = screen.getByRole('textbox', { name: 'First field' }) + + await waitFor(() => expect(input).toHaveFocus()) + expect(sheet).not.toHaveAttribute('tabindex') + }) + + it('allows the sheet to be made programmatically focusable explicitly', () => { + render() + + expect(screen.getByRole('dialog')).toHaveAttribute('tabindex', '-1') + }) + + it('does not move focus to the sheet when a focused child is removed', async () => { + const renderSheet = (showFirstButton: boolean) => ( + + + Sheet title + Sheet description + {showFirstButton && } + + + + ) + const { rerender } = render(renderSheet(true)) + const button = screen.getByRole('button', { name: 'Remove me' }) + + await waitFor(() => expect(button).toHaveFocus()) + rerender(renderSheet(false)) + + await waitFor(() => expect(button).not.toBeInTheDocument()) + await new Promise((resolve) => queueMicrotask(resolve)) + expect(screen.getByRole('dialog')).not.toHaveFocus() + }) +}) diff --git a/packages/ui/src/components/shadcn/ui/sheet.tsx b/packages/ui/src/components/shadcn/ui/sheet.tsx index 4f822924b7083..586d56febe023 100644 --- a/packages/ui/src/components/shadcn/ui/sheet.tsx +++ b/packages/ui/src/components/shadcn/ui/sheet.tsx @@ -174,6 +174,7 @@ const SheetContent = React.forwardRef< {children}