Skip to content

Commit 445ef62

Browse files
BillLeoutsakosvl346Bill Leoutsakos
andauthored
fix(oauth): bind update access to selected credential (#6999)
* fix(oauth): bind update access to selected credential * fix(oauth): guard unresolved connector credentials * fix(oauth): clear stale connector return context * fix(oauth): fail closed when reconnect target disappears * fix(oauth): wait for reconnect credential lookup * fix(oauth): refresh resolved connector credential --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local>
1 parent 472532e commit 445ef62

7 files changed

Lines changed: 585 additions & 46 deletions

File tree

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act, type ReactNode } from 'react'
5+
import { createRoot, type Root } from 'react-dom/client'
6+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
const mocks = vi.hoisted(() => ({
9+
createDraft: vi.fn(),
10+
connectOAuthService: vi.fn(),
11+
onConnect: vi.fn(),
12+
}))
13+
14+
vi.mock('@sim/emcn', () => ({
15+
Badge: ({ children }: { children?: ReactNode }) => <span>{children}</span>,
16+
ChipModal: ({ open, children }: { open: boolean; children?: ReactNode }) =>
17+
open ? <div>{children}</div> : null,
18+
ChipModalBody: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
19+
ChipModalError: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
20+
ChipModalField: ({ title, children }: { title: string; children?: ReactNode }) => (
21+
<section>
22+
<span>{title}</span>
23+
{children}
24+
</section>
25+
),
26+
ChipModalFooter: ({
27+
primaryAction,
28+
}: {
29+
primaryAction: { label: string; onClick: () => void; disabled: boolean }
30+
}) => (
31+
<button
32+
type='button'
33+
data-testid='connect'
34+
onClick={primaryAction.onClick}
35+
disabled={primaryAction.disabled}
36+
>
37+
{primaryAction.label}
38+
</button>
39+
),
40+
ChipModalHeader: ({ children }: { children?: ReactNode }) => <header>{children}</header>,
41+
InfoCard: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
42+
InfoCardItem: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
43+
InfoCardList: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
44+
}))
45+
46+
vi.mock('@/lib/auth/auth-client', () => ({
47+
useSession: () => ({ data: { user: { name: 'Test User' } } }),
48+
}))
49+
50+
vi.mock('@/lib/credentials/client-state', () => ({
51+
ADD_CONNECTOR_SEARCH_PARAM: 'addConnector',
52+
writeOAuthReturnContext: vi.fn(),
53+
}))
54+
55+
vi.mock('@/lib/credentials/display-name', () => ({
56+
defaultCredentialDisplayName: () => 'Test credential',
57+
}))
58+
59+
vi.mock('@/lib/oauth', () => ({
60+
getProviderIdFromServiceId: (serviceId: string) => serviceId,
61+
OAUTH_PROVIDERS: {
62+
slack: {
63+
name: 'Slack',
64+
icon: null,
65+
services: {},
66+
},
67+
},
68+
parseProvider: (provider: string) => ({ baseProvider: provider }),
69+
}))
70+
71+
vi.mock('@/lib/oauth/utils', () => ({
72+
getScopeDescription: (scope: string) => scope,
73+
getServiceConfigByProviderId: () => null,
74+
}))
75+
76+
vi.mock('@/blocks/brand-icon', () => ({
77+
withBrandIcon: () => null,
78+
}))
79+
80+
vi.mock('@/hooks/queries/credentials', () => ({
81+
useCreateCredentialDraft: () => ({
82+
mutateAsync: mocks.createDraft,
83+
isPending: false,
84+
}),
85+
useWorkspaceCredentials: () => ({
86+
data: [],
87+
isPending: false,
88+
}),
89+
}))
90+
91+
vi.mock('@/hooks/queries/oauth/oauth-connections', () => ({
92+
useConnectOAuthService: () => ({
93+
mutateAsync: mocks.connectOAuthService,
94+
isPending: false,
95+
}),
96+
}))
97+
98+
import { ConnectOAuthModal } from '@/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal'
99+
100+
let container: HTMLDivElement
101+
let root: Root
102+
103+
function renderReauthorizeModal({
104+
reconnectTarget,
105+
onConnect,
106+
}: {
107+
reconnectTarget?: {
108+
workspaceId: string
109+
credentialId: string
110+
displayName: string
111+
}
112+
onConnect?: () => Promise<void> | void
113+
} = {}) {
114+
act(() => {
115+
root.render(
116+
<ConnectOAuthModal
117+
mode='reauthorize'
118+
open={true}
119+
onOpenChange={vi.fn()}
120+
providerId='slack'
121+
toolName='Slack'
122+
reconnectTarget={reconnectTarget}
123+
onConnect={onConnect}
124+
/>
125+
)
126+
})
127+
}
128+
129+
async function clickConnect() {
130+
const button = container.querySelector<HTMLButtonElement>('[data-testid="connect"]')
131+
expect(button).not.toBeNull()
132+
await act(async () => {
133+
button?.click()
134+
})
135+
}
136+
137+
describe('ConnectOAuthModal reauthorization', () => {
138+
beforeEach(() => {
139+
vi.clearAllMocks()
140+
mocks.createDraft.mockResolvedValue({ success: true, draftId: 'draft-exact' })
141+
mocks.connectOAuthService.mockResolvedValue({ success: true })
142+
mocks.onConnect.mockResolvedValue(undefined)
143+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
144+
container = document.createElement('div')
145+
document.body.appendChild(container)
146+
root = createRoot(container)
147+
})
148+
149+
afterEach(() => {
150+
act(() => root.unmount())
151+
container.remove()
152+
})
153+
154+
it('binds the selected credential draft to the OAuth launch', async () => {
155+
renderReauthorizeModal({
156+
reconnectTarget: {
157+
workspaceId: 'workspace-1',
158+
credentialId: 'credential-slack',
159+
displayName: 'Team Slack',
160+
},
161+
})
162+
163+
await clickConnect()
164+
165+
expect(mocks.createDraft).toHaveBeenCalledWith({
166+
workspaceId: 'workspace-1',
167+
providerId: 'slack',
168+
credentialId: 'credential-slack',
169+
displayName: 'Team Slack',
170+
})
171+
expect(mocks.connectOAuthService).toHaveBeenCalledWith({
172+
providerId: 'slack',
173+
callbackURL: window.location.href,
174+
draftId: 'draft-exact',
175+
})
176+
expect(mocks.createDraft.mock.invocationCallOrder[0]).toBeLessThan(
177+
mocks.connectOAuthService.mock.invocationCallOrder[0]
178+
)
179+
})
180+
181+
it('does not launch OAuth when the reconnect draft cannot be created', async () => {
182+
mocks.createDraft.mockRejectedValue(new Error('Draft creation failed'))
183+
renderReauthorizeModal({
184+
reconnectTarget: {
185+
workspaceId: 'workspace-1',
186+
credentialId: 'credential-slack',
187+
displayName: 'Team Slack',
188+
},
189+
})
190+
191+
await clickConnect()
192+
193+
expect(mocks.connectOAuthService).not.toHaveBeenCalled()
194+
expect(container).toHaveTextContent('Draft creation failed')
195+
})
196+
197+
it('preserves provider-only reauthorization without creating a draft', async () => {
198+
renderReauthorizeModal()
199+
200+
await clickConnect()
201+
202+
expect(mocks.createDraft).not.toHaveBeenCalled()
203+
expect(mocks.connectOAuthService).toHaveBeenCalledWith({
204+
providerId: 'slack',
205+
callbackURL: window.location.href,
206+
draftId: undefined,
207+
})
208+
})
209+
210+
it('keeps an onConnect override ahead of credential-bound reauthorization', async () => {
211+
renderReauthorizeModal({
212+
reconnectTarget: {
213+
workspaceId: 'workspace-1',
214+
credentialId: 'credential-slack',
215+
displayName: 'Team Slack',
216+
},
217+
onConnect: mocks.onConnect,
218+
})
219+
220+
await clickConnect()
221+
222+
expect(mocks.onConnect).toHaveBeenCalledOnce()
223+
expect(mocks.createDraft).not.toHaveBeenCalled()
224+
expect(mocks.connectOAuthService).not.toHaveBeenCalled()
225+
})
226+
})

apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,11 @@ interface ConnectOAuthModalReauthorizeProps extends ConnectOAuthModalBaseProps {
112112
toolName: string
113113
requiredScopes?: readonly string[]
114114
newScopes?: readonly string[]
115+
reconnectTarget?: {
116+
workspaceId: string
117+
credentialId: string
118+
displayName: string
119+
}
115120
onConnect?: () => Promise<void> | void
116121
}
117122

@@ -316,6 +321,16 @@ export function ConnectOAuthModal(props: ConnectOAuthModalProps) {
316321
handleClose()
317322
return
318323
} else {
324+
if (props.reconnectTarget) {
325+
const draft = await createDraft.mutateAsync({
326+
workspaceId: props.reconnectTarget.workspaceId,
327+
providerId,
328+
credentialId: props.reconnectTarget.credentialId,
329+
displayName: props.reconnectTarget.displayName,
330+
})
331+
draftId = draft.draftId
332+
}
333+
319334
logger.info('Reauthorizing OAuth2', {
320335
providerId,
321336
requiredScopes,
@@ -341,7 +356,8 @@ export function ConnectOAuthModal(props: ConnectOAuthModalProps) {
341356
}
342357
}
343358

344-
const isPending = (isConnect && createDraft.isPending) || connectOAuthService.isPending
359+
const createsDraft = isConnect || (!isConnect && Boolean(props.reconnectTarget))
360+
const isPending = (createsDraft && createDraft.isPending) || connectOAuthService.isPending
345361
const isDisabled = isConnect
346362
? !displayName.trim() || isPending || Boolean(existingCredential)
347363
: isPending

0 commit comments

Comments
 (0)