Skip to content

Commit 36fe7d0

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(oauth): guard unresolved connector credentials
1 parent 6a18417 commit 36fe7d0

2 files changed

Lines changed: 157 additions & 33 deletions

File tree

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.test.tsx

Lines changed: 126 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
11
/**
22
* @vitest-environment jsdom
33
*/
4-
import type { ReactNode, SVGProps } from 'react'
4+
import type { ButtonHTMLAttributes, ReactNode, SVGProps } from 'react'
55
import { act } from 'react'
66
import { createRoot, type Root } from 'react-dom/client'
77
import { afterEach, describe, expect, it, vi } from 'vitest'
88
import type { SyncLogData } from '@/lib/api/contracts/knowledge/connectors'
99
import { CONNECTOR_SYNC_STALE_LOCK_TTL_MS } from '@/lib/knowledge/connectors/sync-limits'
1010

11-
const { icon } = vi.hoisted(() => ({
11+
const { connectOAuthModalMock, icon, oauthCredentialsState } = vi.hoisted(() => ({
12+
connectOAuthModalMock: vi.fn(),
1213
icon: (name: string) => (props: SVGProps<SVGSVGElement>) => (
1314
<svg data-testid={`icon-${name}`} className={props.className} />
1415
),
16+
oauthCredentialsState: {
17+
current: [] as Array<{ id: string; name: string; provider: string }>,
18+
},
1519
}))
1620

1721
vi.mock('@sim/emcn/icons', () => ({
@@ -30,15 +34,28 @@ vi.mock('@sim/emcn/icons', () => ({
3034

3135
vi.mock('@sim/emcn', () => ({
3236
Badge: ({ children }: { children?: ReactNode }) => <span>{children}</span>,
33-
Button: ({ children }: { children?: ReactNode }) => <button type='button'>{children}</button>,
37+
Button: ({
38+
children,
39+
variant: _variant,
40+
size: _size,
41+
...props
42+
}: ButtonHTMLAttributes<HTMLButtonElement> & { variant?: string; size?: string }) => (
43+
<button type='button' {...props}>
44+
{children}
45+
</button>
46+
),
3447
Checkbox: () => <input type='checkbox' />,
3548
ChipConfirmModal: () => null,
3649
cn: (...classes: unknown[]) => classes.filter(Boolean).join(' '),
3750
DropdownMenu: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
3851
DropdownMenuContent: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
3952
DropdownMenuItem: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
4053
DropdownMenuTrigger: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
41-
Tooltip: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
54+
Tooltip: {
55+
Root: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
56+
Trigger: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
57+
Content: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
58+
},
4259
}))
4360

4461
vi.mock('@/lib/credentials/client-state', () => ({
@@ -47,33 +64,52 @@ vi.mock('@/lib/credentials/client-state', () => ({
4764
}))
4865
vi.mock('@/lib/oauth', () => ({
4966
getCanonicalScopesForProvider: vi.fn(() => []),
50-
getProviderIdFromServiceId: vi.fn(() => undefined),
67+
getProviderIdFromServiceId: vi.fn(() => 'slack'),
5168
}))
5269
vi.mock('@/lib/oauth/utils', () => ({ getMissingRequiredScopes: vi.fn(() => []) }))
5370
vi.mock('@/app/workspace/[workspaceId]/components/connect-oauth-modal', () => ({
54-
ConnectOAuthModal: () => null,
71+
ConnectOAuthModal: (props: unknown) => {
72+
connectOAuthModalMock(props)
73+
return null
74+
},
5575
}))
5676
vi.mock(
5777
'@/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/edit-connector-modal',
5878
() => ({ EditConnectorModal: () => null })
5979
)
6080
vi.mock('@/blocks', () => ({ getBlock: vi.fn(() => undefined) }))
6181
vi.mock('@/blocks/icon-color', () => ({ getTileIconColorClass: vi.fn(() => '') }))
62-
vi.mock('@/connectors/registry', () => ({ CONNECTOR_META_REGISTRY: {} }))
82+
vi.mock('@/connectors/registry', () => ({
83+
CONNECTOR_META_REGISTRY: {
84+
slack: {
85+
id: 'slack',
86+
name: 'Slack',
87+
auth: { mode: 'oauth', provider: 'slack', requiredScopes: ['channels:read'] },
88+
},
89+
},
90+
}))
6391
vi.mock('@/hooks/queries/kb/connectors', () => ({
92+
isConnectorSyncingOrPending: vi.fn(
93+
(connector: { status: string }) =>
94+
connector.status === 'pending' || connector.status === 'syncing'
95+
),
6496
useConnectorDetail: vi.fn(() => ({ data: undefined, isLoading: false })),
6597
useDeleteConnector: vi.fn(() => ({ mutate: vi.fn(), isPending: false })),
6698
useTriggerSync: vi.fn(() => ({ mutate: vi.fn() })),
6799
useUpdateConnector: vi.fn(() => ({ mutate: vi.fn() })),
68100
}))
69101
vi.mock('@/hooks/queries/oauth/oauth-credentials', () => ({
70-
useOAuthCredentials: vi.fn(() => ({ data: [] })),
102+
useOAuthCredentials: vi.fn(() => ({ data: oauthCredentialsState.current })),
71103
}))
72104
vi.mock('@/hooks/use-credential-refresh-triggers', () => ({
73105
useCredentialRefreshTriggers: vi.fn(),
74106
}))
75107

76-
import { SyncHistory } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section'
108+
import {
109+
ConnectorsSection,
110+
SyncHistory,
111+
} from '@/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section'
112+
import type { ConnectorData } from '@/hooks/queries/kb/connectors'
77113

78114
let root: Root | null = null
79115

@@ -102,6 +138,46 @@ function render(log: SyncLogData) {
102138
return container
103139
}
104140

141+
function makeConnector(overrides: Partial<ConnectorData> = {}): ConnectorData {
142+
return {
143+
id: 'connector-1',
144+
knowledgeBaseId: 'knowledge-1',
145+
connectorType: 'slack',
146+
credentialId: 'credential-1',
147+
sourceConfig: {},
148+
syncMode: null,
149+
syncIntervalMinutes: 60,
150+
status: 'disabled',
151+
lastSyncAt: null,
152+
lastSyncError: 'invalid_auth',
153+
lastSyncDocCount: null,
154+
nextSyncAt: null,
155+
consecutiveFailures: 3,
156+
createdAt: new Date().toISOString(),
157+
updatedAt: new Date().toISOString(),
158+
...overrides,
159+
}
160+
}
161+
162+
function renderSection(connector: ConnectorData) {
163+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
164+
const container = document.createElement('div')
165+
document.body.appendChild(container)
166+
root = createRoot(container)
167+
act(() =>
168+
root?.render(
169+
<ConnectorsSection
170+
workspaceId='workspace-1'
171+
knowledgeBaseId='knowledge-1'
172+
connectors={[connector]}
173+
isLoading={false}
174+
canEdit
175+
/>
176+
)
177+
)
178+
return container
179+
}
180+
105181
function icons(container: HTMLElement) {
106182
return Array.from(container.querySelectorAll('[data-testid^="icon-"]')).map((node) =>
107183
node.getAttribute('data-testid')
@@ -112,9 +188,50 @@ afterEach(() => {
112188
act(() => root?.unmount())
113189
root = null
114190
document.body.innerHTML = ''
191+
oauthCredentialsState.current = []
115192
vi.clearAllMocks()
116193
})
117194

195+
describe('Connector credential reauthorization', () => {
196+
it('fails closed when the connector credential cannot be resolved', () => {
197+
const container = renderSection(makeConnector())
198+
const reconnectButton = Array.from(container.querySelectorAll('button')).find(
199+
(button) => button.textContent === 'Reconnect'
200+
)
201+
202+
expect(reconnectButton?.disabled).toBe(true)
203+
204+
act(() => reconnectButton?.click())
205+
206+
expect(connectOAuthModalMock).not.toHaveBeenCalled()
207+
})
208+
209+
it('reauthorizes with the resolved credential provider and identity', () => {
210+
oauthCredentialsState.current = [
211+
{ id: 'credential-1', name: 'Workspace Slack', provider: 'slack-custom' },
212+
]
213+
const container = renderSection(makeConnector())
214+
const reconnectButton = Array.from(container.querySelectorAll('button')).find(
215+
(button) => button.textContent === 'Reconnect'
216+
)
217+
218+
expect(reconnectButton?.disabled).toBe(false)
219+
220+
act(() => reconnectButton?.click())
221+
222+
expect(connectOAuthModalMock).toHaveBeenCalledWith(
223+
expect.objectContaining({
224+
providerId: 'slack-custom',
225+
reconnectTarget: {
226+
workspaceId: 'workspace-1',
227+
credentialId: 'credential-1',
228+
displayName: 'Workspace Slack',
229+
},
230+
})
231+
)
232+
})
233+
})
234+
118235
describe('SyncHistory', () => {
119236
it('renders a fresh "started" row as in progress, not as a success', () => {
120237
const container = render(makeLog({ status: 'started' }))

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connectors-section.tsx

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -518,13 +518,15 @@ function ConnectorCard({
518518
{canEdit && serviceId && providerId && (
519519
<Button
520520
variant='primary'
521+
disabled={Boolean(connector.credentialId && !selectedCredential)}
521522
onClick={() => {
522523
if (connector.credentialId) {
524+
if (!selectedCredential) return
523525
writeOAuthReturnContext({
524526
origin: 'kb-connectors',
525527
knowledgeBaseId,
526528
displayName: connectorDef?.name ?? connector.connectorType,
527-
providerId: providerId!,
529+
providerId: selectedCredential.provider,
528530
preCount: credentials?.length ?? 0,
529531
workspaceId,
530532
reconnect: true,
@@ -555,11 +557,12 @@ function ConnectorCard({
555557
variant='primary'
556558
onClick={() => {
557559
if (connector.credentialId) {
560+
if (!selectedCredential) return
558561
writeOAuthReturnContext({
559562
origin: 'kb-connectors',
560563
knowledgeBaseId,
561564
displayName: connectorDef?.name ?? connector.connectorType,
562-
providerId: providerId!,
565+
providerId: selectedCredential.provider,
563566
preCount: credentials?.length ?? 0,
564567
workspaceId,
565568
reconnect: true,
@@ -603,28 +606,32 @@ function ConnectorCard({
603606
/>
604607
)}
605608

606-
{showOAuthModal && serviceId && providerId && connector.credentialId && (
607-
<ConnectOAuthModal
608-
mode='reauthorize'
609-
open={showOAuthModal}
610-
onOpenChange={(open) => {
611-
if (!open) {
612-
consumeOAuthReturnContext()
613-
setShowOAuthModal(false)
614-
}
615-
}}
616-
toolName={connectorDef?.name ?? connector.connectorType}
617-
requiredScopes={getCanonicalScopesForProvider(providerId)}
618-
newScopes={missingScopes}
619-
serviceId={serviceId}
620-
providerId={selectedCredential?.provider ?? providerId}
621-
reconnectTarget={{
622-
workspaceId,
623-
credentialId: connector.credentialId,
624-
displayName: selectedCredential?.name ?? connectorDef?.name ?? connector.connectorType,
625-
}}
626-
/>
627-
)}
609+
{showOAuthModal &&
610+
serviceId &&
611+
providerId &&
612+
connector.credentialId &&
613+
selectedCredential && (
614+
<ConnectOAuthModal
615+
mode='reauthorize'
616+
open={showOAuthModal}
617+
onOpenChange={(open) => {
618+
if (!open) {
619+
consumeOAuthReturnContext()
620+
setShowOAuthModal(false)
621+
}
622+
}}
623+
toolName={connectorDef?.name ?? connector.connectorType}
624+
requiredScopes={getCanonicalScopesForProvider(providerId)}
625+
newScopes={missingScopes}
626+
serviceId={serviceId}
627+
providerId={selectedCredential.provider}
628+
reconnectTarget={{
629+
workspaceId,
630+
credentialId: selectedCredential.id,
631+
displayName: selectedCredential.name,
632+
}}
633+
/>
634+
)}
628635
</div>
629636
)
630637
}

0 commit comments

Comments
 (0)