Skip to content

Commit 4a89897

Browse files
committed
fix(credentials): send a conflicting OAuth reconnect back with a reason instead of a 500
1 parent 5a98a62 commit 4a89897

8 files changed

Lines changed: 563 additions & 41 deletions

File tree

‎apps/sim/hooks/use-oauth-return.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,28 @@ describe('resolveOAuthCallbackError', () => {
121121
text: 'The "New Gmail" connection didn’t finish. Try again.',
122122
})
123123
})
124+
125+
it('names the credential that already uses the account a reconnect chose', () => {
126+
const reason = 'That Google account is already connected in this workspace as "Personal".'
127+
const url = new URL('https://sim.ai/workspace/workspace-1/integrations')
128+
url.searchParams.set('error', 'credential_account_conflict')
129+
url.searchParams.set('error_description', reason)
130+
131+
expect(resolveOAuthCallbackError(url.toString(), context)).toEqual({
132+
kind: 'error',
133+
text: reason,
134+
})
135+
})
136+
137+
it('keeps a provider-supplied error description out of the message', () => {
138+
const url = new URL('https://sim.ai/workspace/workspace-1/integrations')
139+
url.searchParams.set('error', 'access_denied')
140+
url.searchParams.set('error_description', 'Visit example.test to restore access')
141+
142+
expect(resolveOAuthCallbackError(url.toString(), context)?.text).toBe(
143+
'The "New Gmail" connection didn’t finish. Try again.'
144+
)
145+
})
124146
})
125147

126148
describe('buildKnowledgeBaseOAuthReturnUrl', () => {

‎apps/sim/hooks/use-oauth-return.ts‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
type OAuthReturnContext,
1818
readOAuthReturnContext,
1919
} from '@/lib/credentials/client-state'
20+
import { OAUTH_CREDENTIAL_ACCOUNT_CONFLICT_ERROR } from '@/lib/credentials/draft-constants'
2021
import {
2122
hasOAuthCredentialChanged,
2223
OAUTH_CHAT_ATTEMPT_MAX_AGE_MS,
@@ -124,13 +125,26 @@ export function resolveOAuthCallbackError(
124125
callbackUrl: string,
125126
ctx: Pick<OAuthReturnContext, 'displayName'>
126127
): OAuthResultMessage | null {
127-
if (!new URL(callbackUrl).searchParams.has('error')) return null
128+
const params = new URL(callbackUrl).searchParams
129+
if (!params.has('error')) return null
128130
return {
129131
kind: 'error',
130-
text: `The "${ctx.displayName}" connection didn’t finish. Try again.`,
132+
text:
133+
accountConflictDescription(params) ??
134+
`The "${ctx.displayName}" connection didn’t finish. Try again.`,
131135
}
132136
}
133137

138+
/**
139+
* The server-written reason a reconnect was refused because the chosen account
140+
* already backs another credential. Other `error_description` values come from
141+
* the provider and stay hidden behind the generic message.
142+
*/
143+
function accountConflictDescription(params: URLSearchParams): string | null {
144+
if (params.get('error') !== OAUTH_CREDENTIAL_ACCOUNT_CONFLICT_ERROR) return null
145+
return params.get('error_description') || null
146+
}
147+
134148
function consumeOAuthCallbackError(ctx: OAuthReturnContext): OAuthResultMessage | null {
135149
const result = resolveOAuthCallbackError(window.location.href, ctx)
136150
if (!result) return null
@@ -221,9 +235,13 @@ async function verifyOAuthChatAttempt(queryClient: QueryClient, attemptId: strin
221235
const attempt = readOAuthChatAttempt(attemptId)
222236
if (!attempt || Date.now() - attempt.requestedAt > OAUTH_CHAT_ATTEMPT_MAX_AGE_MS) return
223237

224-
if (new URL(window.location.href).searchParams.has('error')) {
238+
const callbackParams = new URL(window.location.href).searchParams
239+
if (callbackParams.has('error')) {
225240
setOAuthChatAttemptStatus(attempt.id, 'failed')
226-
toast.error(`The ${attempt.displayName} connection didn’t finish. Try again.`)
241+
toast.error(
242+
accountConflictDescription(callbackParams) ??
243+
`The ${attempt.displayName} connection didn’t finish. Try again.`
244+
)
227245
return
228246
}
229247

‎apps/sim/lib/auth/auth.ts‎

Lines changed: 47 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import { reportFreebuffConversion } from '@/lib/analytics/freebuff.server'
4040
import { getAccessControlConfig, isEmailBlockedByAccessControl } from '@/lib/auth/access-control'
4141
import { holdAdminRemovalStops, stopAdminRemovedRuns } from '@/lib/auth/admin-removal-stops'
4242
import { createAnonymousSession, ensureAnonymousUserExists } from '@/lib/auth/anonymous'
43+
import { stableExternalAccountId } from '@/lib/auth/connectors/account-id'
4344
import { buildConnectorProviders } from '@/lib/auth/connectors/providers'
4445
import {
4546
applyRegistrationGate,
@@ -142,9 +143,11 @@ import { validateCallbackUrl } from '@/lib/core/security/input-validation'
142143
import { PlatformEvents } from '@/lib/core/telemetry'
143144
import { trustedProxies } from '@/lib/core/utils/request'
144145
import { getBaseUrl, isLocalhostUrl, parseOriginList } from '@/lib/core/utils/urls'
146+
import { CredentialAccountConflictError } from '@/lib/credentials/draft-hooks'
145147
import {
146148
captureOAuthCredentialDraftBinding,
147149
consumeOAuthCredentialDraftBinding,
150+
findCredentialReconnectConflictRedirect,
148151
processCredentialDraft,
149152
} from '@/lib/credentials/draft-processor'
150153
import { sendEmail } from '@/lib/messaging/email/mailer'
@@ -502,8 +505,9 @@ export const auth = betterAuth({
502505
const modifiedAccount = { ...account }
503506

504507
if (context?.path.startsWith('/oauth2/callback/')) {
508+
let draftId: string | undefined
505509
try {
506-
await captureOAuthCredentialDraftBinding(context, () => getOAuthState())
510+
draftId = await captureOAuthCredentialDraftBinding(context, () => getOAuthState())
507511
} catch (error) {
508512
logger.error('[account.create.before] Failed to read OAuth credential draft state', {
509513
userId: account.userId,
@@ -512,6 +516,26 @@ export const auth = betterAuth({
512516
})
513517
throw error
514518
}
519+
520+
/**
521+
* The after-hook runs once the callback has already answered, so a
522+
* reconnect onto an account another credential in the workspace
523+
* uses has to be refused here, before the row exists, to send the
524+
* user back with a reason instead of a 500.
525+
*/
526+
if (draftId) {
527+
const conflictRedirectUrl = await findCredentialReconnectConflictRedirect({
528+
draftId,
529+
userId: account.userId,
530+
providerId: account.providerId,
531+
externalAccountId: account.accountId,
532+
oauthState: await getOAuthState(),
533+
})
534+
if (conflictRedirectUrl) {
535+
consumeOAuthCredentialDraftBinding(context)
536+
throw new APIError('FOUND', undefined, { location: conflictRedirectUrl })
537+
}
538+
}
515539
}
516540

517541
if (account.accessToken && isSalesforceOAuthProviderId(account.providerId)) {
@@ -544,19 +568,15 @@ export const auth = betterAuth({
544568
/**
545569
* Migrate credentials from stale account rows to the newly created one.
546570
*
547-
* Each `getUserInfo` in `lib/auth/connectors/providers.ts` appends a
548-
* random UUID to the stable external ID so that Better Auth never
549-
* blocks cross-user connections — keep the two in step. This means
550-
* re-connecting the same external identity creates a new row. We detect
551-
* the stale siblings here by comparing the stable prefix (everything
552-
* before the trailing UUID), migrate any credential FKs to the new row,
553-
* then delete the stale rows.
571+
* Re-connecting the same external identity creates a new row (see
572+
* `stableExternalAccountId`). We detect the stale siblings here by
573+
* comparing the stable prefix, migrate any credential FKs to the new
574+
* row, then delete the stale rows.
554575
*/
555576
try {
556-
const UUID_SUFFIX_RE = /-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/
557-
const stablePrefix = account.accountId.replace(UUID_SUFFIX_RE, '')
577+
const stablePrefix = stableExternalAccountId(account.accountId)
558578

559-
if (stablePrefix && stablePrefix !== account.accountId) {
579+
if (stablePrefix) {
560580
const siblings = await db
561581
.select({ id: schema.account.id, accountId: schema.account.accountId })
562582
.from(schema.account)
@@ -569,7 +589,7 @@ export const auth = betterAuth({
569589
)
570590

571591
const staleRows = siblings.filter(
572-
(row) => row.accountId.replace(UUID_SUFFIX_RE, '') === stablePrefix
592+
(row) => stableExternalAccountId(row.accountId) === stablePrefix
573593
)
574594

575595
if (staleRows.length > 0) {
@@ -653,12 +673,21 @@ export const auth = betterAuth({
653673
accountId: account.id,
654674
})
655675
} catch (error) {
656-
logger.error('[account.create.after] Failed to process credential draft', {
657-
userId: account.userId,
658-
providerId: account.providerId,
659-
error,
660-
})
661-
if (credentialDraftBinding.draftId) throw error
676+
if (error instanceof CredentialAccountConflictError) {
677+
logger.warn('[account.create.after] Refused a conflicting credential reconnect', {
678+
userId: account.userId,
679+
providerId: account.providerId,
680+
credentialId: error.credentialId,
681+
conflictingCredentialId: error.conflictingCredentialId,
682+
})
683+
} else {
684+
logger.error('[account.create.after] Failed to process credential draft', {
685+
userId: account.userId,
686+
providerId: account.providerId,
687+
error,
688+
})
689+
if (credentialDraftBinding.draftId) throw error
690+
}
662691
}
663692
}
664693

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
const RANDOM_UUID_SUFFIX = /-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/
2+
3+
/**
4+
* Returns the provider's stable identity inside a connector account id.
5+
*
6+
* Each `getUserInfo` in `lib/auth/connectors/providers.ts` appends a random
7+
* UUID to the stable external id so Better Auth never blocks cross-user
8+
* connections, which means every connect of the same identity creates a new
9+
* account row. Rows whose ids share this prefix are the same external identity.
10+
* Returns `null` for an id that carries no such suffix.
11+
*/
12+
export function stableExternalAccountId(accountId: string): string | null {
13+
const stable = accountId.replace(RANDOM_UUID_SUFFIX, '')
14+
return stable && stable !== accountId ? stable : null
15+
}

0 commit comments

Comments
 (0)