From e1d60667fa99377caf0dc8f7948c778d5fc6311a Mon Sep 17 00:00:00 2001 From: Bill Leoutsakos Date: Sat, 22 Aug 2026 16:56:45 -0700 Subject: [PATCH 1/6] fix(oauth): bind update access to selected credential --- .../connect-oauth-modal.test.tsx | 226 ++++++++++++++++++ .../connect-oauth-modal.tsx | 18 +- .../connectors-section/connectors-section.tsx | 22 +- .../credential-selector.tsx | 9 + .../components/tools/credential-selector.tsx | 9 + .../lib/credentials/draft-processor.test.ts | 27 +++ 6 files changed, 303 insertions(+), 8 deletions(-) create mode 100644 apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.test.tsx diff --git a/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.test.tsx b/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.test.tsx new file mode 100644 index 00000000000..31a82cd10ce --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.test.tsx @@ -0,0 +1,226 @@ +/** + * @vitest-environment jsdom + */ +import { act, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + createDraft: vi.fn(), + connectOAuthService: vi.fn(), + onConnect: vi.fn(), +})) + +vi.mock('@sim/emcn', () => ({ + Badge: ({ children }: { children?: ReactNode }) => {children}, + ChipModal: ({ open, children }: { open: boolean; children?: ReactNode }) => + open ?
{children}
: null, + ChipModalBody: ({ children }: { children?: ReactNode }) =>
{children}
, + ChipModalError: ({ children }: { children?: ReactNode }) =>
{children}
, + ChipModalField: ({ title, children }: { title: string; children?: ReactNode }) => ( +
+ {title} + {children} +
+ ), + ChipModalFooter: ({ + primaryAction, + }: { + primaryAction: { label: string; onClick: () => void; disabled: boolean } + }) => ( + + ), + ChipModalHeader: ({ children }: { children?: ReactNode }) =>
{children}
, + InfoCard: ({ children }: { children?: ReactNode }) =>
{children}
, + InfoCardItem: ({ children }: { children?: ReactNode }) =>
{children}
, + InfoCardList: ({ children }: { children?: ReactNode }) =>
{children}
, +})) + +vi.mock('@/lib/auth/auth-client', () => ({ + useSession: () => ({ data: { user: { name: 'Test User' } } }), +})) + +vi.mock('@/lib/credentials/client-state', () => ({ + ADD_CONNECTOR_SEARCH_PARAM: 'addConnector', + writeOAuthReturnContext: vi.fn(), +})) + +vi.mock('@/lib/credentials/display-name', () => ({ + defaultCredentialDisplayName: () => 'Test credential', +})) + +vi.mock('@/lib/oauth', () => ({ + getProviderIdFromServiceId: (serviceId: string) => serviceId, + OAUTH_PROVIDERS: { + slack: { + name: 'Slack', + icon: null, + services: {}, + }, + }, + parseProvider: (provider: string) => ({ baseProvider: provider }), +})) + +vi.mock('@/lib/oauth/utils', () => ({ + getScopeDescription: (scope: string) => scope, + getServiceConfigByProviderId: () => null, +})) + +vi.mock('@/blocks/brand-icon', () => ({ + withBrandIcon: () => null, +})) + +vi.mock('@/hooks/queries/credentials', () => ({ + useCreateCredentialDraft: () => ({ + mutateAsync: mocks.createDraft, + isPending: false, + }), + useWorkspaceCredentials: () => ({ + data: [], + isPending: false, + }), +})) + +vi.mock('@/hooks/queries/oauth/oauth-connections', () => ({ + useConnectOAuthService: () => ({ + mutateAsync: mocks.connectOAuthService, + isPending: false, + }), +})) + +import { ConnectOAuthModal } from '@/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal' + +let container: HTMLDivElement +let root: Root + +function renderReauthorizeModal({ + reconnectTarget, + onConnect, +}: { + reconnectTarget?: { + workspaceId: string + credentialId: string + displayName: string + } + onConnect?: () => Promise | void +} = {}) { + act(() => { + root.render( + + ) + }) +} + +async function clickConnect() { + const button = container.querySelector('[data-testid="connect"]') + expect(button).not.toBeNull() + await act(async () => { + button?.click() + }) +} + +describe('ConnectOAuthModal reauthorization', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.createDraft.mockResolvedValue({ success: true, draftId: 'draft-exact' }) + mocks.connectOAuthService.mockResolvedValue({ success: true }) + mocks.onConnect.mockResolvedValue(undefined) + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + }) + + afterEach(() => { + act(() => root.unmount()) + container.remove() + }) + + it('binds the selected credential draft to the OAuth launch', async () => { + renderReauthorizeModal({ + reconnectTarget: { + workspaceId: 'workspace-1', + credentialId: 'credential-slack', + displayName: 'Team Slack', + }, + }) + + await clickConnect() + + expect(mocks.createDraft).toHaveBeenCalledWith({ + workspaceId: 'workspace-1', + providerId: 'slack', + credentialId: 'credential-slack', + displayName: 'Team Slack', + }) + expect(mocks.connectOAuthService).toHaveBeenCalledWith({ + providerId: 'slack', + callbackURL: window.location.href, + draftId: 'draft-exact', + }) + expect(mocks.createDraft.mock.invocationCallOrder[0]).toBeLessThan( + mocks.connectOAuthService.mock.invocationCallOrder[0] + ) + }) + + it('does not launch OAuth when the reconnect draft cannot be created', async () => { + mocks.createDraft.mockRejectedValue(new Error('Draft creation failed')) + renderReauthorizeModal({ + reconnectTarget: { + workspaceId: 'workspace-1', + credentialId: 'credential-slack', + displayName: 'Team Slack', + }, + }) + + await clickConnect() + + expect(mocks.connectOAuthService).not.toHaveBeenCalled() + expect(container).toHaveTextContent('Draft creation failed') + }) + + it('preserves provider-only reauthorization without creating a draft', async () => { + renderReauthorizeModal() + + await clickConnect() + + expect(mocks.createDraft).not.toHaveBeenCalled() + expect(mocks.connectOAuthService).toHaveBeenCalledWith({ + providerId: 'slack', + callbackURL: window.location.href, + draftId: undefined, + }) + }) + + it('keeps an onConnect override ahead of credential-bound reauthorization', async () => { + renderReauthorizeModal({ + reconnectTarget: { + workspaceId: 'workspace-1', + credentialId: 'credential-slack', + displayName: 'Team Slack', + }, + onConnect: mocks.onConnect, + }) + + await clickConnect() + + expect(mocks.onConnect).toHaveBeenCalledOnce() + expect(mocks.createDraft).not.toHaveBeenCalled() + expect(mocks.connectOAuthService).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx b/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx index d18cbf39970..552637626c8 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx @@ -112,6 +112,11 @@ interface ConnectOAuthModalReauthorizeProps extends ConnectOAuthModalBaseProps { toolName: string requiredScopes?: readonly string[] newScopes?: readonly string[] + reconnectTarget?: { + workspaceId: string + credentialId: string + displayName: string + } onConnect?: () => Promise | void } @@ -316,6 +321,16 @@ export function ConnectOAuthModal(props: ConnectOAuthModalProps) { handleClose() return } else { + if (props.reconnectTarget) { + const draft = await createDraft.mutateAsync({ + workspaceId: props.reconnectTarget.workspaceId, + providerId, + credentialId: props.reconnectTarget.credentialId, + displayName: props.reconnectTarget.displayName, + }) + draftId = draft.draftId + } + logger.info('Reauthorizing OAuth2', { providerId, requiredScopes, @@ -341,7 +356,8 @@ export function ConnectOAuthModal(props: ConnectOAuthModalProps) { } } - const isPending = (isConnect && createDraft.isPending) || connectOAuthService.isPending + const createsDraft = isConnect || (!isConnect && Boolean(props.reconnectTarget)) + const isPending = (createsDraft && createDraft.isPending) || connectOAuthService.isPending const isDisabled = isConnect ? !displayName.trim() || isPending || Boolean(existingCredential) : isPending diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.tsx index 10b0a8b6e4a..3caf9b9cd00 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.tsx @@ -286,12 +286,15 @@ function ConnectorCard({ useCredentialRefreshTriggers(refetchCredentials, providerId ?? '', workspaceId) - const missingScopes = useMemo(() => { - if (!credentials || !connector.credentialId) return [] - const credential = credentials.find((c) => c.id === connector.credentialId) - if (!credential) return [] - return getMissingRequiredScopes(credential, requiredScopes) - }, [credentials, connector.credentialId, requiredScopes]) + const selectedCredential = useMemo(() => { + if (!credentials || !connector.credentialId) return undefined + return credentials.find((credential) => credential.id === connector.credentialId) + }, [credentials, connector.credentialId]) + + const missingScopes = useMemo( + () => (selectedCredential ? getMissingRequiredScopes(selectedCredential, requiredScopes) : []), + [selectedCredential, requiredScopes] + ) const { data: detail, isLoading: detailLoading } = useConnectorDetail( expanded ? knowledgeBaseId : undefined, @@ -614,7 +617,12 @@ function ConnectorCard({ requiredScopes={getCanonicalScopesForProvider(providerId)} newScopes={missingScopes} serviceId={serviceId} - providerId={providerId} + providerId={selectedCredential?.provider ?? providerId} + reconnectTarget={{ + workspaceId, + credentialId: connector.credentialId, + displayName: selectedCredential?.name ?? connectorDef?.name ?? connector.connectorType, + }} /> )} diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/credential-selector/credential-selector.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/credential-selector/credential-selector.tsx index 0ab6e2fab39..68ce14d1acf 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/credential-selector/credential-selector.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/credential-selector/credential-selector.tsx @@ -516,6 +516,15 @@ export function CredentialSelector({ // the credential — deriving it from the service id would send a // sandbox user to production, where they cannot sign in at all. providerId={selectedCredential?.provider ?? effectiveProviderId} + reconnectTarget={ + selectedCredential + ? { + workspaceId, + credentialId: selectedCredential.id, + displayName: selectedCredential.name, + } + : undefined + } /> )} diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/credential-selector.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/credential-selector.tsx index 17be9d48172..3e121f6e6e8 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/credential-selector.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/credential-selector.tsx @@ -298,6 +298,15 @@ export function ToolCredentialSelector({ // the credential — deriving it from the service id would send a // sandbox user to production, where they cannot sign in at all. providerId={selectedCredential?.provider ?? effectiveProviderId} + reconnectTarget={ + selectedCredential + ? { + workspaceId, + credentialId: selectedCredential.id, + displayName: selectedCredential.name, + } + : undefined + } /> )} diff --git a/apps/sim/lib/credentials/draft-processor.test.ts b/apps/sim/lib/credentials/draft-processor.test.ts index 579f02375c4..7abf958b084 100644 --- a/apps/sim/lib/credentials/draft-processor.test.ts +++ b/apps/sim/lib/credentials/draft-processor.test.ts @@ -70,6 +70,33 @@ describe('processCredentialDraft', () => { expect(dbChainMockFns.delete).toHaveBeenCalledWith(schemaMock.pendingCredentialDraft) }) + it('reconnects the credential bound to an exact Slack draft regardless of account identity', async () => { + const draft = { + ...credentialDraft('draft-slack', 'workspace-1'), + providerId: 'slack', + displayName: 'Team Slack', + credentialId: 'credential-slack', + } + queueTableRows(schemaMock.pendingCredentialDraft, [draft]) + + await processCredentialDraft({ + draftId: 'draft-slack', + userId: 'user-1', + providerId: 'slack', + accountId: 'T01234567-usr_U01234567-new-account-id', + }) + + expect(mockHandleReconnectCredential).toHaveBeenCalledWith({ + draft, + newAccountId: 'T01234567-usr_U01234567-new-account-id', + workspaceId: 'workspace-1', + userId: 'user-1', + now: expect.any(Date), + }) + expect(mockHandleCreateCredentialFromDraft).not.toHaveBeenCalled() + expect(dbChainMockFns.delete).toHaveBeenCalledWith(schemaMock.pendingCredentialDraft) + }) + it('fails closed when a legacy callback has multiple active drafts', async () => { queueTableRows(schemaMock.pendingCredentialDraft, [ credentialDraft('draft-1', 'workspace-1'), From ac2e3d6af6e244450d3f9b12410bff142e9741a2 Mon Sep 17 00:00:00 2001 From: Bill Leoutsakos Date: Sat, 22 Aug 2026 17:07:35 -0700 Subject: [PATCH 2/6] fix(oauth): guard unresolved connector credentials --- .../connectors-section.test.tsx | 135 ++++++++++++++++-- .../connectors-section/connectors-section.tsx | 55 +++---- 2 files changed, 157 insertions(+), 33 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.test.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.test.tsx index 41d2bc89da2..12b21225bbd 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.test.tsx @@ -1,17 +1,21 @@ /** * @vitest-environment jsdom */ -import type { ReactNode, SVGProps } from 'react' +import type { ButtonHTMLAttributes, ReactNode, SVGProps } from 'react' import { act } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, describe, expect, it, vi } from 'vitest' import type { SyncLogData } from '@/lib/api/contracts/knowledge/connectors' import { CONNECTOR_SYNC_STALE_LOCK_TTL_MS } from '@/lib/knowledge/connectors/sync-limits' -const { icon } = vi.hoisted(() => ({ +const { connectOAuthModalMock, icon, oauthCredentialsState } = vi.hoisted(() => ({ + connectOAuthModalMock: vi.fn(), icon: (name: string) => (props: SVGProps) => ( ), + oauthCredentialsState: { + current: [] as Array<{ id: string; name: string; provider: string }>, + }, })) vi.mock('@sim/emcn/icons', () => ({ @@ -30,7 +34,16 @@ vi.mock('@sim/emcn/icons', () => ({ vi.mock('@sim/emcn', () => ({ Badge: ({ children }: { children?: ReactNode }) => {children}, - Button: ({ children }: { children?: ReactNode }) => , + Button: ({ + children, + variant: _variant, + size: _size, + ...props + }: ButtonHTMLAttributes & { variant?: string; size?: string }) => ( + + ), Checkbox: () => , ChipConfirmModal: () => null, cn: (...classes: unknown[]) => classes.filter(Boolean).join(' '), @@ -38,7 +51,11 @@ vi.mock('@sim/emcn', () => ({ DropdownMenuContent: ({ children }: { children?: ReactNode }) =>
{children}
, DropdownMenuItem: ({ children }: { children?: ReactNode }) =>
{children}
, DropdownMenuTrigger: ({ children }: { children?: ReactNode }) =>
{children}
, - Tooltip: ({ children }: { children?: ReactNode }) =>
{children}
, + Tooltip: { + Root: ({ children }: { children?: ReactNode }) =>
{children}
, + Trigger: ({ children }: { children?: ReactNode }) =>
{children}
, + Content: ({ children }: { children?: ReactNode }) =>
{children}
, + }, })) vi.mock('@/lib/credentials/client-state', () => ({ @@ -47,11 +64,14 @@ vi.mock('@/lib/credentials/client-state', () => ({ })) vi.mock('@/lib/oauth', () => ({ getCanonicalScopesForProvider: vi.fn(() => []), - getProviderIdFromServiceId: vi.fn(() => undefined), + getProviderIdFromServiceId: vi.fn(() => 'slack'), })) vi.mock('@/lib/oauth/utils', () => ({ getMissingRequiredScopes: vi.fn(() => []) })) vi.mock('@/app/workspace/[workspaceId]/components/connect-oauth-modal', () => ({ - ConnectOAuthModal: () => null, + ConnectOAuthModal: (props: unknown) => { + connectOAuthModalMock(props) + return null + }, })) vi.mock( '@/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/edit-connector-modal', @@ -59,21 +79,37 @@ vi.mock( ) vi.mock('@/blocks', () => ({ getBlock: vi.fn(() => undefined) })) vi.mock('@/blocks/icon-color', () => ({ getTileIconColorClass: vi.fn(() => '') })) -vi.mock('@/connectors/registry', () => ({ CONNECTOR_META_REGISTRY: {} })) +vi.mock('@/connectors/registry', () => ({ + CONNECTOR_META_REGISTRY: { + slack: { + id: 'slack', + name: 'Slack', + auth: { mode: 'oauth', provider: 'slack', requiredScopes: ['channels:read'] }, + }, + }, +})) vi.mock('@/hooks/queries/kb/connectors', () => ({ + isConnectorSyncingOrPending: vi.fn( + (connector: { status: string }) => + connector.status === 'pending' || connector.status === 'syncing' + ), useConnectorDetail: vi.fn(() => ({ data: undefined, isLoading: false })), useDeleteConnector: vi.fn(() => ({ mutate: vi.fn(), isPending: false })), useTriggerSync: vi.fn(() => ({ mutate: vi.fn() })), useUpdateConnector: vi.fn(() => ({ mutate: vi.fn() })), })) vi.mock('@/hooks/queries/oauth/oauth-credentials', () => ({ - useOAuthCredentials: vi.fn(() => ({ data: [] })), + useOAuthCredentials: vi.fn(() => ({ data: oauthCredentialsState.current })), })) vi.mock('@/hooks/use-credential-refresh-triggers', () => ({ useCredentialRefreshTriggers: vi.fn(), })) -import { SyncHistory } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section' +import { + ConnectorsSection, + SyncHistory, +} from '@/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section' +import type { ConnectorData } from '@/hooks/queries/kb/connectors' let root: Root | null = null @@ -102,6 +138,46 @@ function render(log: SyncLogData) { return container } +function makeConnector(overrides: Partial = {}): ConnectorData { + return { + id: 'connector-1', + knowledgeBaseId: 'knowledge-1', + connectorType: 'slack', + credentialId: 'credential-1', + sourceConfig: {}, + syncMode: null, + syncIntervalMinutes: 60, + status: 'disabled', + lastSyncAt: null, + lastSyncError: 'invalid_auth', + lastSyncDocCount: null, + nextSyncAt: null, + consecutiveFailures: 3, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + ...overrides, + } +} + +function renderSection(connector: ConnectorData) { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + const container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + act(() => + root?.render( + + ) + ) + return container +} + function icons(container: HTMLElement) { return Array.from(container.querySelectorAll('[data-testid^="icon-"]')).map((node) => node.getAttribute('data-testid') @@ -112,9 +188,50 @@ afterEach(() => { act(() => root?.unmount()) root = null document.body.innerHTML = '' + oauthCredentialsState.current = [] vi.clearAllMocks() }) +describe('Connector credential reauthorization', () => { + it('fails closed when the connector credential cannot be resolved', () => { + const container = renderSection(makeConnector()) + const reconnectButton = Array.from(container.querySelectorAll('button')).find( + (button) => button.textContent === 'Reconnect' + ) + + expect(reconnectButton?.disabled).toBe(true) + + act(() => reconnectButton?.click()) + + expect(connectOAuthModalMock).not.toHaveBeenCalled() + }) + + it('reauthorizes with the resolved credential provider and identity', () => { + oauthCredentialsState.current = [ + { id: 'credential-1', name: 'Workspace Slack', provider: 'slack-custom' }, + ] + const container = renderSection(makeConnector()) + const reconnectButton = Array.from(container.querySelectorAll('button')).find( + (button) => button.textContent === 'Reconnect' + ) + + expect(reconnectButton?.disabled).toBe(false) + + act(() => reconnectButton?.click()) + + expect(connectOAuthModalMock).toHaveBeenCalledWith( + expect.objectContaining({ + providerId: 'slack-custom', + reconnectTarget: { + workspaceId: 'workspace-1', + credentialId: 'credential-1', + displayName: 'Workspace Slack', + }, + }) + ) + }) +}) + describe('SyncHistory', () => { it('renders a fresh "started" row as in progress, not as a success', () => { const container = render(makeLog({ status: 'started' })) diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.tsx index 3caf9b9cd00..3f1e1f9da73 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.tsx @@ -518,13 +518,15 @@ function ConnectorCard({ {canEdit && serviceId && providerId && (