Skip to content

Commit ad22bd1

Browse files
committed
fix(webhooks): authorize the stored credential only when the save uses it
1 parent c77dd6d commit ad22bd1

2 files changed

Lines changed: 45 additions & 19 deletions

File tree

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -634,8 +634,30 @@ describe('POST /api/webhooks credential references', () => {
634634
}
635635
)
636636

637-
it('authorizes both credentials when a re-save replaces the stored one', async () => {
637+
/** Rotation without recreation never touches the old credential, so it needs no access to it. */
638+
it('rotates the credential without access to the stored one when nothing is recreated', async () => {
639+
mocks.authorizeCredentialUseForAuth.mockImplementation(async (_auth, { credentialId }) =>
640+
credentialId === 'new-credential'
641+
? { ok: true, workspaceId: 'workspace-1' }
642+
: { ok: false, error: 'You do not have access to this credential.' }
643+
)
644+
queueUpdatePathRows(true, { credentialId: 'stored-credential' })
645+
646+
const response = await POST(upsertRequest({ credentialId: 'new-credential' }))
647+
648+
expect(response.status).toBe(200)
649+
expect(mocks.authorizeCredentialUseForAuth.mock.calls.map(([, params]) => params)).toEqual([
650+
{ credentialId: 'new-credential', workflowId: 'workflow-1' },
651+
])
652+
expect(dbChainMockFns.set).toHaveBeenCalledWith(
653+
expect.objectContaining({ providerConfig: { credentialId: 'new-credential' } })
654+
)
655+
})
656+
657+
/** Recreation cleans up the previous subscription with the stored credential. */
658+
it('authorizes both credentials when a rotation recreates the subscription', async () => {
638659
mocks.authorizeCredentialUseForAuth.mockResolvedValue({ ok: true, workspaceId: 'workspace-1' })
660+
mocks.shouldRecreateExternalWebhookSubscription.mockReturnValue(true)
639661
queueUpdatePathRows(true, { credentialId: 'stored-credential' })
640662

641663
const response = await POST(upsertRequest({ credentialId: 'new-credential' }))

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

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -418,18 +418,32 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
418418
existingWebhook = existingRows[0] || null
419419
}
420420

421+
const shouldRecreateSubscription =
422+
existingWebhook &&
423+
shouldRecreateExternalWebhookSubscription({
424+
previousProvider: existingWebhook.provider as string,
425+
nextProvider: provider,
426+
previousConfig: ((existingWebhook.providerConfig as Record<string, unknown>) ||
427+
{}) as Record<string, unknown>,
428+
nextConfig: resolvedProviderConfig,
429+
})
430+
421431
/**
422432
* Subscription handlers, pollers, and subscription cleanup look `credentialId`
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.
433+
* up by id alone and mint tokens as its owner, so every credential this save
434+
* acts with must be usable by the actor in the workflow's workspace before
435+
* anything is subscribed, cleaned up, or saved. That is the requested
436+
* credential, plus the stored one when the save uses it: merged back because
437+
* the request omits `credentialId`, or used to clean up the previous
438+
* subscription on recreation.
428439
*/
440+
const usesStoredCredential =
441+
existingWebhook && (shouldRecreateSubscription || !('credentialId' in originalProviderConfig))
429442
const credentialIds = new Set(
430-
[originalProviderConfig.credentialId, existingWebhook?.providerConfig?.credentialId].filter(
431-
(id) => id != null && id !== ''
432-
)
443+
[
444+
originalProviderConfig.credentialId,
445+
usesStoredCredential ? existingWebhook.providerConfig?.credentialId : undefined,
446+
].filter((id) => id != null && id !== '')
433447
)
434448
for (const credentialId of credentialIds) {
435449
if (typeof credentialId !== 'string') {
@@ -492,16 +506,6 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
492506
}
493507
}
494508

495-
const shouldRecreateSubscription =
496-
existingWebhook &&
497-
shouldRecreateExternalWebhookSubscription({
498-
previousProvider: existingWebhook.provider as string,
499-
nextProvider: provider,
500-
previousConfig: ((existingWebhook.providerConfig as Record<string, unknown>) ||
501-
{}) as Record<string, unknown>,
502-
nextConfig: resolvedProviderConfig,
503-
})
504-
505509
if (!existingWebhook || shouldRecreateSubscription) {
506510
try {
507511
const result = await createExternalWebhookSubscription(

0 commit comments

Comments
 (0)