Skip to content

Commit 75d3327

Browse files
fix(credentials): fail closed on oauth completion
1 parent cbe9b7b commit 75d3327

7 files changed

Lines changed: 59 additions & 46 deletions

File tree

apps/sim/app/api/auth/oauth2/callback/instagram/route.ts

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -299,18 +299,15 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
299299
),
300300
}))
301301

302-
if (persisted) {
303-
try {
304-
await processCredentialDraft({
305-
draftId,
306-
userId: session.user.id,
307-
providerId: 'instagram',
308-
accountId: persisted.id,
309-
})
310-
} catch (draftError) {
311-
logger.error('Failed to process credential draft for Instagram', { error: draftError })
312-
}
302+
if (!persisted) {
303+
throw new Error(`Instagram OAuth account ${igUserId} was not persisted`)
313304
}
305+
await processCredentialDraft({
306+
draftId,
307+
userId: session.user.id,
308+
providerId: 'instagram',
309+
accountId: persisted.id,
310+
})
314311

315312
const returnUrlCookie = request.cookies.get(INSTAGRAM_RETURN_URL_COOKIE)?.value
316313
const redirectUrl =

apps/sim/app/api/auth/oauth2/shopify/store/route.ts

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -119,18 +119,15 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
119119
),
120120
}))
121121

122-
if (persisted) {
123-
try {
124-
await processCredentialDraft({
125-
draftId,
126-
userId: session.user.id,
127-
providerId: 'shopify',
128-
accountId: persisted.id,
129-
})
130-
} catch (error) {
131-
logger.error('Failed to process credential draft for Shopify', { error })
132-
}
122+
if (!persisted) {
123+
throw new Error(`Shopify OAuth account ${stableAccountId} was not persisted`)
133124
}
125+
await processCredentialDraft({
126+
draftId,
127+
userId: session.user.id,
128+
providerId: 'shopify',
129+
accountId: persisted.id,
130+
})
134131

135132
const redirectUrl = returnUrl && isSameOrigin(returnUrl) ? returnUrl : `${baseUrl}/workspace`
136133
const finalUrl = new URL(redirectUrl)

apps/sim/app/api/auth/trello/store/route.ts

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -138,18 +138,15 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
138138
),
139139
}))
140140

141-
if (persisted) {
142-
try {
143-
await processCredentialDraft({
144-
draftId,
145-
userId: session.user.id,
146-
providerId: 'trello',
147-
accountId: persisted.id,
148-
})
149-
} catch (error) {
150-
logger.error('Failed to process credential draft for Trello', { error })
151-
}
141+
if (!persisted) {
142+
throw new Error(`Trello OAuth account ${trelloUser.id} was not persisted`)
152143
}
144+
await processCredentialDraft({
145+
draftId,
146+
userId: session.user.id,
147+
providerId: 'trello',
148+
accountId: persisted.id,
149+
})
153150

154151
return clearStateCookie(NextResponse.json({ success: true }))
155152
} catch (error) {

apps/sim/lib/auth/auth.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -524,18 +524,19 @@ export const auth = betterAuth({
524524
}
525525
}
526526

527+
let credentialDraftId: string | undefined
527528
try {
528529
const oauthState = await getOAuthState()
529530
const rawCallbackUrl = oauthState?.callbackURL
530531
if (rawCallbackUrl !== undefined && typeof rawCallbackUrl !== 'string') {
531532
throw new Error('OAuth state callback URL must be a string')
532533
}
533-
const draftId = rawCallbackUrl
534+
credentialDraftId = rawCallbackUrl
534535
? (new URL(rawCallbackUrl).searchParams.get(OAUTH_CREDENTIAL_DRAFT_CALLBACK_PARAM) ??
535536
undefined)
536537
: undefined
537538
await processCredentialDraft({
538-
draftId,
539+
draftId: credentialDraftId,
539540
userId: account.userId,
540541
providerId: account.providerId,
541542
accountId: account.id,
@@ -546,6 +547,7 @@ export const auth = betterAuth({
546547
providerId: account.providerId,
547548
error,
548549
})
550+
if (credentialDraftId) throw error
549551
}
550552

551553
try {

apps/sim/lib/credentials/draft-hooks.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,7 @@ export async function handleReconnectCredential(params: {
121121
.limit(1)
122122

123123
if (!existingCredential) {
124-
logger.warn('Credential not found for reconnect, skipping', {
125-
credentialId: draft.credentialId,
126-
})
127-
return
124+
throw new Error(`Cannot reconnect missing credential ${draft.credentialId}`)
128125
}
129126

130127
const oldAccountId = existingCredential.accountId
@@ -144,12 +141,9 @@ export async function handleReconnectCredential(params: {
144141
.limit(1)
145142

146143
if (conflicting) {
147-
logger.warn('New account already used by another credential, skipping reconnect', {
148-
credentialId: draft.credentialId,
149-
newAccountId,
150-
conflictingCredentialId: conflicting.id,
151-
})
152-
return
144+
throw new Error(
145+
`Cannot reconnect credential ${draft.credentialId}: account ${newAccountId} is already used by credential ${conflicting.id}`
146+
)
153147
}
154148
}
155149

apps/sim/lib/credentials/draft-processor.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,23 @@ describe('processCredentialDraft', () => {
8383
expect(mockHandleCreateCredentialFromDraft).not.toHaveBeenCalled()
8484
expect(mockHandleReconnectCredential).not.toHaveBeenCalled()
8585
})
86+
87+
it('fails when an exact draft is missing or expired', async () => {
88+
queueTableRows(schemaMock.pendingCredentialDraft, [])
89+
90+
await expect(
91+
processCredentialDraft({
92+
draftId: 'draft-missing',
93+
userId: 'user-1',
94+
providerId: 'google-email',
95+
accountId: 'account-1',
96+
})
97+
).rejects.toThrow(
98+
'Cannot process missing or expired OAuth credential draft draft-missing for user user-1'
99+
)
100+
101+
expect(mockHandleCreateCredentialFromDraft).not.toHaveBeenCalled()
102+
expect(mockHandleReconnectCredential).not.toHaveBeenCalled()
103+
expect(dbChainMockFns.delete).not.toHaveBeenCalled()
104+
})
86105
})

apps/sim/lib/credentials/draft-processor.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,14 @@ export async function processCredentialDraft(params: ProcessCredentialDraftParam
5050

5151
const [draft] = drafts
5252

53-
if (!draft) return
53+
if (!draft) {
54+
if (draftId) {
55+
throw new Error(
56+
`Cannot process missing or expired OAuth credential draft ${draftId} for user ${userId}`
57+
)
58+
}
59+
return
60+
}
5461

5562
const now = new Date()
5663

0 commit comments

Comments
 (0)