Skip to content

Commit c77dd6d

Browse files
committed
fix(webhooks): authorize both requested and stored credentials on upsert
1 parent e631110 commit c77dd6d

2 files changed

Lines changed: 53 additions & 12 deletions

File tree

apps/sim/app/api/webhooks/route.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,4 +607,43 @@ describe('POST /api/webhooks credential references', () => {
607607
expect(response.status).toBe(403)
608608
expect(dbChainMockFns.set).not.toHaveBeenCalled()
609609
})
610+
611+
/**
612+
* Clearing `credentialId` does not stop the save from acting with the stored
613+
* credential: a recreate still cleans up the previous subscription with it.
614+
*/
615+
it.each([null, ''])(
616+
'still authorizes the stored credential when a re-save sends credentialId %j',
617+
async (credentialId) => {
618+
mocks.authorizeCredentialUseForAuth.mockResolvedValue({
619+
ok: false,
620+
error: 'You do not have access to this credential.',
621+
})
622+
mocks.shouldRecreateExternalWebhookSubscription.mockReturnValue(true)
623+
queueUpdatePathRows(true, { credentialId: 'stored-credential' })
624+
625+
const response = await POST(upsertRequest({ credentialId }))
626+
627+
expect(response.status).toBe(403)
628+
expect(mocks.authorizeCredentialUseForAuth).toHaveBeenCalledWith(expect.anything(), {
629+
credentialId: 'stored-credential',
630+
workflowId: 'workflow-1',
631+
})
632+
expect(mocks.createExternalWebhookSubscription).not.toHaveBeenCalled()
633+
expect(dbChainMockFns.set).not.toHaveBeenCalled()
634+
}
635+
)
636+
637+
it('authorizes both credentials when a re-save replaces the stored one', async () => {
638+
mocks.authorizeCredentialUseForAuth.mockResolvedValue({ ok: true, workspaceId: 'workspace-1' })
639+
queueUpdatePathRows(true, { credentialId: 'stored-credential' })
640+
641+
const response = await POST(upsertRequest({ credentialId: 'new-credential' }))
642+
643+
expect(response.status).toBe(200)
644+
expect(mocks.authorizeCredentialUseForAuth.mock.calls.map(([, params]) => params)).toEqual([
645+
{ credentialId: 'new-credential', workflowId: 'workflow-1' },
646+
{ credentialId: 'stored-credential', workflowId: 'workflow-1' },
647+
])
648+
})
610649
})

apps/sim/app/api/webhooks/route.ts

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -420,31 +420,33 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
420420

421421
/**
422422
* Subscription handlers, pollers, and subscription cleanup look `credentialId`
423-
* up by id alone and mint tokens as its owner. A save acts with the requested
424-
* credential or, when the request omits it, the stored one, so that credential
425-
* must be usable by the actor in the workflow's workspace before anything is
426-
* subscribed, cleaned up, or saved.
423+
* up by id alone and mint tokens as its owner. A save can act with both the
424+
* requested credential and the stored one — the stored one is merged back when
425+
* the request omits it, or used to clean up the previous subscription — so
426+
* each must be usable by the actor in the workflow's workspace before anything
427+
* is subscribed, cleaned up, or saved.
427428
*/
428-
const effectiveCredentialId =
429-
'credentialId' in originalProviderConfig
430-
? originalProviderConfig.credentialId
431-
: existingWebhook?.providerConfig?.credentialId
432-
if (effectiveCredentialId != null && effectiveCredentialId !== '') {
433-
if (typeof effectiveCredentialId !== 'string') {
429+
const credentialIds = new Set(
430+
[originalProviderConfig.credentialId, existingWebhook?.providerConfig?.credentialId].filter(
431+
(id) => id != null && id !== ''
432+
)
433+
)
434+
for (const credentialId of credentialIds) {
435+
if (typeof credentialId !== 'string') {
434436
return NextResponse.json(
435437
{ error: 'providerConfig.credentialId must be a literal credential id' },
436438
{ status: 400 }
437439
)
438440
}
439441
const credentialAccess = await authorizeCredentialUseForAuth(
440442
{ success: true, userId, authType: AuthType.SESSION },
441-
{ credentialId: effectiveCredentialId, workflowId }
443+
{ credentialId, workflowId }
442444
)
443445
if (!credentialAccess.ok) {
444446
logger.warn(`[${requestId}] Webhook credential reference denied`, {
445447
userId,
446448
workflowId,
447-
credentialId: effectiveCredentialId,
449+
credentialId,
448450
reason: credentialAccess.error,
449451
})
450452
return NextResponse.json({ error: credentialAccess.error }, { status: 403 })

0 commit comments

Comments
 (0)