diff --git a/apps/sim/hooks/use-oauth-return.test.ts b/apps/sim/hooks/use-oauth-return.test.ts index 96e2467b202..96ce49b340f 100644 --- a/apps/sim/hooks/use-oauth-return.test.ts +++ b/apps/sim/hooks/use-oauth-return.test.ts @@ -121,6 +121,28 @@ describe('resolveOAuthCallbackError', () => { text: 'The "New Gmail" connection didn’t finish. Try again.', }) }) + + it('names the credential that already uses the account a reconnect chose', () => { + const reason = 'That Google account is already connected in this workspace as "Personal".' + const url = new URL('https://sim.ai/workspace/workspace-1/integrations') + url.searchParams.set('error', 'credential_account_conflict') + url.searchParams.set('error_description', reason) + + expect(resolveOAuthCallbackError(url.toString(), context)).toEqual({ + kind: 'error', + text: reason, + }) + }) + + it('keeps a provider-supplied error description out of the message', () => { + const url = new URL('https://sim.ai/workspace/workspace-1/integrations') + url.searchParams.set('error', 'access_denied') + url.searchParams.set('error_description', 'Visit example.test to restore access') + + expect(resolveOAuthCallbackError(url.toString(), context)?.text).toBe( + 'The "New Gmail" connection didn’t finish. Try again.' + ) + }) }) describe('buildKnowledgeBaseOAuthReturnUrl', () => { diff --git a/apps/sim/hooks/use-oauth-return.ts b/apps/sim/hooks/use-oauth-return.ts index 160b944ddfa..5153128d40f 100644 --- a/apps/sim/hooks/use-oauth-return.ts +++ b/apps/sim/hooks/use-oauth-return.ts @@ -17,6 +17,7 @@ import { type OAuthReturnContext, readOAuthReturnContext, } from '@/lib/credentials/client-state' +import { OAUTH_CREDENTIAL_ACCOUNT_CONFLICT_ERROR } from '@/lib/credentials/draft-constants' import { hasOAuthCredentialChanged, OAUTH_CHAT_ATTEMPT_MAX_AGE_MS, @@ -124,13 +125,26 @@ export function resolveOAuthCallbackError( callbackUrl: string, ctx: Pick ): OAuthResultMessage | null { - if (!new URL(callbackUrl).searchParams.has('error')) return null + const params = new URL(callbackUrl).searchParams + if (!params.has('error')) return null return { kind: 'error', - text: `The "${ctx.displayName}" connection didn’t finish. Try again.`, + text: + accountConflictDescription(params) ?? + `The "${ctx.displayName}" connection didn’t finish. Try again.`, } } +/** + * The server-written reason a reconnect was refused because the chosen account + * already backs another credential. Other `error_description` values come from + * the provider and stay hidden behind the generic message. + */ +function accountConflictDescription(params: URLSearchParams): string | null { + if (params.get('error') !== OAUTH_CREDENTIAL_ACCOUNT_CONFLICT_ERROR) return null + return params.get('error_description') || null +} + function consumeOAuthCallbackError(ctx: OAuthReturnContext): OAuthResultMessage | null { const result = resolveOAuthCallbackError(window.location.href, ctx) if (!result) return null @@ -221,9 +235,13 @@ async function verifyOAuthChatAttempt(queryClient: QueryClient, attemptId: strin const attempt = readOAuthChatAttempt(attemptId) if (!attempt || Date.now() - attempt.requestedAt > OAUTH_CHAT_ATTEMPT_MAX_AGE_MS) return - if (new URL(window.location.href).searchParams.has('error')) { + const callbackParams = new URL(window.location.href).searchParams + if (callbackParams.has('error')) { setOAuthChatAttemptStatus(attempt.id, 'failed') - toast.error(`The ${attempt.displayName} connection didn’t finish. Try again.`) + toast.error( + accountConflictDescription(callbackParams) ?? + `The ${attempt.displayName} connection didn’t finish. Try again.` + ) return } diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index b3fd7be1f5f..16c3e44b278 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -40,6 +40,7 @@ import { reportFreebuffConversion } from '@/lib/analytics/freebuff.server' import { getAccessControlConfig, isEmailBlockedByAccessControl } from '@/lib/auth/access-control' import { holdAdminRemovalStops, stopAdminRemovedRuns } from '@/lib/auth/admin-removal-stops' import { createAnonymousSession, ensureAnonymousUserExists } from '@/lib/auth/anonymous' +import { stableExternalAccountId } from '@/lib/auth/connectors/account-id' import { buildConnectorProviders } from '@/lib/auth/connectors/providers' import { applyRegistrationGate, @@ -142,10 +143,12 @@ import { validateCallbackUrl } from '@/lib/core/security/input-validation' import { PlatformEvents } from '@/lib/core/telemetry' import { trustedProxies } from '@/lib/core/utils/request' import { getBaseUrl, isLocalhostUrl, parseOriginList } from '@/lib/core/utils/urls' +import { CredentialAccountConflictError } from '@/lib/credentials/draft-hooks' import { captureOAuthCredentialDraftBinding, consumeOAuthCredentialDraftBinding, processCredentialDraft, + refuseConflictingCredentialReconnect, } from '@/lib/credentials/draft-processor' import { sendEmail } from '@/lib/messaging/email/mailer' import { getFromEmailAddress, getPersonalEmailFrom } from '@/lib/messaging/email/utils' @@ -294,6 +297,40 @@ async function fetchSalesforceInstanceUrl( } } +/** + * A fresh Slack connect re-issues the installation's rotating token chain, + * invalidating the copies held by sibling account rows for the same team (Slack + * bot tokens are per-installation, not per-grant). Propagates the new chain so + * every sibling is valid again, and clears the installation's dead flag. A + * failure is logged, never thrown: the connect itself already succeeded. + */ +async function propagateFreshSlackConnect(account: { + userId: string + providerId: string + accountId: string + accessToken?: string | null + refreshToken?: string | null + accessTokenExpiresAt?: Date | null +}): Promise { + if (account.providerId !== 'slack' || !account.accessToken) return + try { + const teamId = extractSlackTeamId(account.accountId) + if (!teamId) return + // Clear the dead flag before fanning out: the connect itself proves the + // installation has live tokens, and a fan-out failure must not leave the + // hour-long flag blocking refreshes. + await clearOAuthRefreshDeadFlag(`slack:${teamId}`) + await fanOutSlackTokenChain(teamId, { + accessToken: account.accessToken, + refreshToken: account.refreshToken ?? null, + accessTokenExpiresAt: account.accessTokenExpiresAt ?? null, + }) + logger.info('Propagated Slack installation token chain', { userId: account.userId, teamId }) + } catch (error) { + logger.error('Failed to propagate Slack token chain', { userId: account.userId, error }) + } +} + export const auth = betterAuth({ baseURL: getBaseUrl(), // Where Better Auth sends OAuth callbacks that fail before the flow state is @@ -500,10 +537,12 @@ export const auth = betterAuth({ create: { before: async (account, context) => { const modifiedAccount = { ...account } + let reconnectDraftId: string | undefined if (context?.path.startsWith('/oauth2/callback/')) { + let draftId: string | undefined try { - await captureOAuthCredentialDraftBinding(context, () => getOAuthState()) + draftId = await captureOAuthCredentialDraftBinding(context, () => getOAuthState()) } catch (error) { logger.error('[account.create.before] Failed to read OAuth credential draft state', { userId: account.userId, @@ -512,6 +551,7 @@ export const auth = betterAuth({ }) throw error } + reconnectDraftId = draftId } if (account.accessToken && isSalesforceOAuthProviderId(account.providerId)) { @@ -538,25 +578,42 @@ export const auth = betterAuth({ } } + /** + * The after-hook runs once the callback has already answered, so a + * reconnect onto an account another credential in the workspace uses + * has to be refused here, before the row exists, to send the user + * back with a reason instead of a 500. The fresh grant still lands + * on the identity's existing rows: the provider may already have + * rotated their tokens away (Slack installations do). + */ + if (context && reconnectDraftId) { + const conflictRedirectUrl = await refuseConflictingCredentialReconnect({ + draftId: reconnectDraftId, + account: modifiedAccount, + oauthState: await getOAuthState(), + }) + if (conflictRedirectUrl) { + consumeOAuthCredentialDraftBinding(context) + await propagateFreshSlackConnect(modifiedAccount) + throw new APIError('FOUND', undefined, { location: conflictRedirectUrl }) + } + } + return { data: modifiedAccount } }, after: async (account, context) => { /** * Migrate credentials from stale account rows to the newly created one. * - * Each `getUserInfo` in `lib/auth/connectors/providers.ts` appends a - * random UUID to the stable external ID so that Better Auth never - * blocks cross-user connections — keep the two in step. This means - * re-connecting the same external identity creates a new row. We detect - * the stale siblings here by comparing the stable prefix (everything - * before the trailing UUID), migrate any credential FKs to the new row, - * then delete the stale rows. + * Re-connecting the same external identity creates a new row (see + * `stableExternalAccountId`). We detect the stale siblings here by + * comparing the stable prefix, migrate any credential FKs to the new + * row, then delete the stale rows. */ try { - const UUID_SUFFIX_RE = /-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/ - const stablePrefix = account.accountId.replace(UUID_SUFFIX_RE, '') + const stablePrefix = stableExternalAccountId(account.accountId) - if (stablePrefix && stablePrefix !== account.accountId) { + if (stablePrefix) { const siblings = await db .select({ id: schema.account.id, accountId: schema.account.accountId }) .from(schema.account) @@ -569,7 +626,7 @@ export const auth = betterAuth({ ) const staleRows = siblings.filter( - (row) => row.accountId.replace(UUID_SUFFIX_RE, '') === stablePrefix + (row) => stableExternalAccountId(row.accountId) === stablePrefix ) if (staleRows.length > 0) { @@ -598,40 +655,7 @@ export const auth = betterAuth({ }) } - /** - * A fresh Slack connect re-issues the installation's rotating token - * chain, invalidating the copies held by sibling account rows for the - * same team (Slack bot tokens are per-installation, not per-grant). - * Propagate the new chain so every sibling is valid again, and clear - * the installation's dead flag. - */ - if (account.providerId === 'slack' && account.accessToken) { - try { - const teamId = extractSlackTeamId(account.accountId) - if (teamId) { - // Clear the dead flag before fanning out: the connect itself - // proves the installation has live tokens, and a fan-out - // failure must not leave the hour-long flag blocking refreshes. - await clearOAuthRefreshDeadFlag(`slack:${teamId}`) - await fanOutSlackTokenChain(teamId, { - accessToken: account.accessToken, - refreshToken: account.refreshToken ?? null, - accessTokenExpiresAt: account.accessTokenExpiresAt ?? null, - }) - logger.info('[account.create.after] Propagated Slack installation token chain', { - userId: account.userId, - teamId, - newAccountId: account.id, - }) - } - } catch (error) { - logger.error('[account.create.after] Failed to propagate Slack token chain', { - userId: account.userId, - accountId: account.id, - error, - }) - } - } + await propagateFreshSlackConnect(account) const isOAuth2Callback = context?.path.startsWith('/oauth2/callback/') === true const credentialDraftBinding = context @@ -653,12 +677,21 @@ export const auth = betterAuth({ accountId: account.id, }) } catch (error) { - logger.error('[account.create.after] Failed to process credential draft', { - userId: account.userId, - providerId: account.providerId, - error, - }) - if (credentialDraftBinding.draftId) throw error + if (error instanceof CredentialAccountConflictError) { + logger.warn('[account.create.after] Refused a conflicting credential reconnect', { + userId: account.userId, + providerId: account.providerId, + credentialId: error.credentialId, + conflictingCredentialId: error.conflictingCredentialId, + }) + } else { + logger.error('[account.create.after] Failed to process credential draft', { + userId: account.userId, + providerId: account.providerId, + error, + }) + if (credentialDraftBinding.draftId) throw error + } } } diff --git a/apps/sim/lib/auth/connectors/account-id.ts b/apps/sim/lib/auth/connectors/account-id.ts new file mode 100644 index 00000000000..3e38fef7857 --- /dev/null +++ b/apps/sim/lib/auth/connectors/account-id.ts @@ -0,0 +1,15 @@ +const RANDOM_UUID_SUFFIX = /-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/ + +/** + * Returns the provider's stable identity inside a connector account id. + * + * Each `getUserInfo` in `lib/auth/connectors/providers.ts` appends a random + * UUID to the stable external id so Better Auth never blocks cross-user + * connections, which means every connect of the same identity creates a new + * account row. Rows whose ids share this prefix are the same external identity. + * Returns `null` for an id that carries no such suffix. + */ +export function stableExternalAccountId(accountId: string): string | null { + const stable = accountId.replace(RANDOM_UUID_SUFFIX, '') + return stable && stable !== accountId ? stable : null +} diff --git a/apps/sim/lib/credentials/__integration__/oauth-reconnect-account-conflict.integration.ts b/apps/sim/lib/credentials/__integration__/oauth-reconnect-account-conflict.integration.ts new file mode 100644 index 00000000000..dd40a2dd99d --- /dev/null +++ b/apps/sim/lib/credentials/__integration__/oauth-reconnect-account-conflict.integration.ts @@ -0,0 +1,282 @@ +/** + * Reconnecting a credential through Better Auth's real OAuth link callback onto a provider + * identity that already backs another credential in the same workspace. + */ + +import { db } from '@sim/db' +import { account, credential, pendingCredentialDraft, user, workspace } from '@sim/db/schema' +import { deleteWorkspaceFixture, insertWorkspaceFixture } from '@sim/db/testing/workspace-fixtures' +import { jsonResponse } from '@sim/testing/helpers/http' +import { emailMailerMock } from '@sim/testing/mocks/email-mailer.mock' +import { emailTemplatesMock } from '@sim/testing/mocks/email-templates.mock' +import { posthogServerMock } from '@sim/testing/mocks/posthog-server.mock' +import { generateId } from '@sim/utils/id' +import { hashPassword } from 'better-auth/crypto' +import { and, eq } from 'drizzle-orm' +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +vi.hoisted(() => { + process.env.GOOGLE_CLIENT_ID = 'reconnect-conflict-integration-client' + process.env.GOOGLE_CLIENT_SECRET = 'reconnect-conflict-integration-secret' +}) + +vi.mock('@/lib/messaging/email/mailer', () => emailMailerMock) +vi.mock('@/components/emails', () => emailTemplatesMock) +vi.mock('@/lib/posthog/server', () => posthogServerMock) + +const ORIGIN = 'http://localhost:3000' +const PROVIDER_ID = 'google-drive' +const DISCOVERY_URL = 'https://accounts.google.com/.well-known/openid-configuration' +const TOKEN_URL = 'https://oauth2.googleapis.com/token' +const USERINFO_URL = 'https://openidconnect.googleapis.com/v1/userinfo' +const fixturePassword = 'Reconnect-conflict-fixture-password-36!' + +const userId = generateId() +const email = `${userId}@reconnect-conflict.test` +const workspaceId = generateId() +const workCredentialId = generateId() +const personalCredentialId = generateId() +const workAccountRowId = generateId() +const personalAccountRowId = generateId() +const workSubject = `work-subject-${generateId()}` +const personalSubject = `personal-subject-${generateId()}` +const originalFetch = globalThis.fetch + +/** The subject the fixture userinfo endpoint answers with on the next callback. */ +let chosenSubject = personalSubject + +function cookieHeader(response: Response, previous = '') { + const cookies = new Map() + for (const value of [...previous.split('; '), ...response.headers.getSetCookie()]) { + const pair = value.split(';')[0] + const separator = pair.indexOf('=') + if (separator > 0) cookies.set(pair.slice(0, separator), pair.slice(separator + 1)) + } + return [...cookies].map(([name, value]) => `${name}=${value}`).join('; ') +} + +describe('reconnecting a credential onto an account another credential uses', () => { + let auth: typeof import('@/lib/auth/auth').auth + let sessionCookie: string + + beforeAll(async () => { + ;({ auth } = await import('@/lib/auth/auth')) + const now = new Date() + await db.insert(user).values({ + id: userId, + name: 'Reconnect conflict fixture', + email, + emailVerified: true, + createdAt: now, + updatedAt: now, + }) + await db.insert(account).values({ + id: generateId(), + accountId: userId, + userId, + providerId: 'credential', + password: await hashPassword(fixturePassword), + createdAt: now, + updatedAt: now, + }) + await insertWorkspaceFixture(db, { + id: workspaceId, + name: 'Reconnect conflict fixture', + ownerId: userId, + billedAccountUserId: userId, + }) + + vi.stubGlobal('fetch', async (input: string | URL | Request, init?: RequestInit) => { + const target = typeof input === 'string' || input instanceof URL ? String(input) : input.url + if (target === DISCOVERY_URL) { + return jsonResponse({ + issuer: 'https://accounts.google.com', + authorization_endpoint: 'https://accounts.google.com/o/oauth2/v2/auth', + token_endpoint: TOKEN_URL, + userinfo_endpoint: USERINFO_URL, + }) + } + if (target === TOKEN_URL) { + return jsonResponse({ + access_token: `fresh-access-${chosenSubject}`, + refresh_token: `fresh-refresh-${chosenSubject}`, + expires_in: 3600, + token_type: 'Bearer', + }) + } + if (target === USERINFO_URL) { + return jsonResponse({ sub: chosenSubject, email, name: 'Reconnect conflict fixture' }) + } + return originalFetch(input, init) + }) + + const signIn = await auth.handler( + new Request(`${ORIGIN}/api/auth/sign-in/email`, { + method: 'POST', + headers: { 'content-type': 'application/json', origin: ORIGIN }, + body: JSON.stringify({ email, password: fixturePassword }), + }) + ) + expect(signIn.status).toBe(200) + sessionCookie = cookieHeader(signIn) + }, 90_000) + + beforeEach(async () => { + await db.delete(credential).where(eq(credential.workspaceId, workspaceId)) + await db + .delete(account) + .where(and(eq(account.userId, userId), eq(account.providerId, PROVIDER_ID))) + const past = new Date(Date.now() - 60_000) + await db.insert(account).values([ + { + id: workAccountRowId, + userId, + providerId: PROVIDER_ID, + accountId: `${workSubject}-${generateId()}`, + accessToken: 'work-access', + refreshToken: 'work-refresh', + createdAt: past, + updatedAt: past, + }, + { + id: personalAccountRowId, + userId, + providerId: PROVIDER_ID, + accountId: `${personalSubject}-${generateId()}`, + accessToken: 'personal-access', + refreshToken: 'personal-refresh', + createdAt: past, + updatedAt: past, + }, + ]) + await db.insert(credential).values([ + { + id: workCredentialId, + type: 'oauth', + workspaceId, + accountId: workAccountRowId, + providerId: PROVIDER_ID, + displayName: 'Work Drive', + createdBy: userId, + updatedAt: past, + }, + { + id: personalCredentialId, + type: 'oauth', + workspaceId, + accountId: personalAccountRowId, + providerId: PROVIDER_ID, + displayName: 'Personal Drive', + createdBy: userId, + updatedAt: past, + }, + ]) + }) + + afterAll(async () => { + globalThis.fetch = originalFetch + vi.unstubAllGlobals() + await deleteWorkspaceFixture(db, eq(workspace.id, workspaceId)) + await db.delete(user).where(eq(user.id, userId)) + }) + + /** Starts a reconnect of the work credential and returns the provider callback's response. */ + async function reconnectWorkCredentialAs(subject: string) { + chosenSubject = subject + const draftId = generateId() + await db.insert(pendingCredentialDraft).values({ + id: draftId, + userId, + workspaceId, + providerId: PROVIDER_ID, + displayName: 'Work Drive', + credentialId: workCredentialId, + expiresAt: new Date(Date.now() + 10 * 60_000), + }) + const startedFrom = `${ORIGIN}/workspace/${workspaceId}/integrations/connected/${workCredentialId}` + + const link = await auth.handler( + new Request(`${ORIGIN}/api/auth/oauth2/link`, { + method: 'POST', + headers: { 'content-type': 'application/json', origin: ORIGIN, cookie: sessionCookie }, + body: JSON.stringify({ + providerId: PROVIDER_ID, + callbackURL: `${startedFrom}?credentialDraftId=${draftId}`, + }), + }) + ) + expect(link.status).toBe(200) + const { url } = (await link.json()) as { url: string } + const state = new URL(url).searchParams.get('state') + if (!state) throw new Error('OAuth link did not return a state') + + const callback = await auth.handler( + new Request( + `${ORIGIN}/api/auth/oauth2/callback/${PROVIDER_ID}?code=fixture-code&state=${encodeURIComponent(state)}`, + { headers: { cookie: cookieHeader(link, sessionCookie) } } + ) + ) + return { callback, draftId, startedFrom } + } + + async function credentialAccount(credentialId: string) { + const [row] = await db + .select({ accountId: credential.accountId }) + .from(credential) + .where(eq(credential.id, credentialId)) + return row?.accountId + } + + async function providerAccountRows() { + return db + .select({ id: account.id, accessToken: account.accessToken }) + .from(account) + .where(and(eq(account.userId, userId), eq(account.providerId, PROVIDER_ID))) + } + + it('sends the user back with the reason, keeping each credential on its account', async () => { + const { callback, draftId, startedFrom } = await reconnectWorkCredentialAs(personalSubject) + + expect(callback.status).toBe(302) + const location = new URL(callback.headers.get('location') ?? '') + expect(`${location.origin}${location.pathname}`).toBe(startedFrom) + expect(location.searchParams.get('error')).toBe('credential_account_conflict') + expect(location.searchParams.get('error_description')).toBe( + 'That Google account is already connected in this workspace as "Personal Drive". Use that connection, or reconnect with the account that belongs to this one.' + ) + expect(location.searchParams.has('credentialDraftId')).toBe(false) + + expect(await credentialAccount(workCredentialId)).toBe(workAccountRowId) + expect(await credentialAccount(personalCredentialId)).toBe(personalAccountRowId) + const rows = await providerAccountRows() + expect(rows.map((row) => row.id).sort()).toEqual( + [workAccountRowId, personalAccountRowId].sort() + ) + expect(rows.find((row) => row.id === personalAccountRowId)?.accessToken).toBe( + `fresh-access-${personalSubject}` + ) + expect(rows.find((row) => row.id === workAccountRowId)?.accessToken).toBe('work-access') + const drafts = await db + .select({ id: pendingCredentialDraft.id }) + .from(pendingCredentialDraft) + .where(eq(pendingCredentialDraft.id, draftId)) + expect(drafts).toEqual([]) + }) + + it('still reconnects the credential when the user picks the identity it already had', async () => { + const { callback, startedFrom } = await reconnectWorkCredentialAs(workSubject) + + expect(callback.status).toBe(302) + const location = new URL(callback.headers.get('location') ?? '') + expect(`${location.origin}${location.pathname}`).toBe(startedFrom) + expect(location.searchParams.has('error')).toBe(false) + + const reconnectedAccountId = await credentialAccount(workCredentialId) + expect(reconnectedAccountId).not.toBe(workAccountRowId) + expect(await credentialAccount(personalCredentialId)).toBe(personalAccountRowId) + const rows = await providerAccountRows() + expect(rows.map((row) => row.id).sort()).toEqual( + [reconnectedAccountId, personalAccountRowId].sort() + ) + }) +}) diff --git a/apps/sim/lib/credentials/draft-constants.ts b/apps/sim/lib/credentials/draft-constants.ts index 26a244bf02c..b07a7ec85d6 100644 --- a/apps/sim/lib/credentials/draft-constants.ts +++ b/apps/sim/lib/credentials/draft-constants.ts @@ -1,3 +1,9 @@ export const CREDENTIAL_DRAFT_TTL_MS = 15 * 60 * 1000 export const CREDENTIAL_DRAFT_TTL_SECONDS = CREDENTIAL_DRAFT_TTL_MS / 1000 export const OAUTH_CREDENTIAL_DRAFT_CALLBACK_PARAM = 'credentialDraftId' +/** + * OAuth callback `error` code for a reconnect whose chosen account already backs + * another credential in the same workspace. Its `error_description` carries the + * message to show. + */ +export const OAUTH_CREDENTIAL_ACCOUNT_CONFLICT_ERROR = 'credential_account_conflict' diff --git a/apps/sim/lib/credentials/draft-hooks.ts b/apps/sim/lib/credentials/draft-hooks.ts index 98311f5f365..63c47924cd1 100644 --- a/apps/sim/lib/credentials/draft-hooks.ts +++ b/apps/sim/lib/credentials/draft-hooks.ts @@ -4,7 +4,7 @@ import * as schema from '@sim/db/schema' import { createLogger } from '@sim/logger' import { getPostgresConstraintName, getPostgresErrorCode } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' -import { and, eq, sql } from 'drizzle-orm' +import { and, eq, inArray, ne } from 'drizzle-orm' import { deleteOrphanedOAuthAccount } from '@/lib/credentials/deletion' import { resumeConnectorsAfterCredentialReconnect } from '@/lib/knowledge/connectors/credential-recovery' import { clearOAuthRefreshFailure } from '@/lib/oauth/credential-service' @@ -12,6 +12,47 @@ import { captureServerEvent } from '@/lib/posthog/server' const logger = createLogger('CredentialDraftHooks') +/** + * A reconnect chose an account that already backs another credential in the + * same workspace. Two credentials in one workspace never share an account, so + * the reconnect is refused and the credential keeps the account it had. + */ +export class CredentialAccountConflictError extends Error { + constructor( + readonly credentialId: string, + readonly conflictingCredentialId: string + ) { + super( + `Cannot reconnect credential ${credentialId}: its new account is already used by credential ${conflictingCredentialId}` + ) + this.name = 'CredentialAccountConflictError' + } +} + +/** + * Finds a credential other than `credentialId` in the workspace that uses one + * of `accountIds`, the credential a reconnect onto those accounts would collide with. + */ +export async function findConflictingWorkspaceCredential(params: { + credentialId: string + workspaceId: string + accountIds: string[] +}): Promise<{ id: string; displayName: string } | null> { + if (params.accountIds.length === 0) return null + const [conflicting] = await db + .select({ id: schema.credential.id, displayName: schema.credential.displayName }) + .from(schema.credential) + .where( + and( + eq(schema.credential.workspaceId, params.workspaceId), + inArray(schema.credential.accountId, params.accountIds), + ne(schema.credential.id, params.credentialId) + ) + ) + .limit(1) + return conflicting ?? null +} + /** * Creates a new credential from a pending draft (normal OAuth connect flow). */ @@ -175,22 +216,13 @@ export async function handleReconnectCredential(params: { const accountChanged = oldAccountId !== newAccountId if (accountChanged) { - const [conflicting] = await db - .select({ id: schema.credential.id }) - .from(schema.credential) - .where( - and( - eq(schema.credential.workspaceId, workspaceId), - eq(schema.credential.accountId, newAccountId), - sql`${schema.credential.id} != ${draft.credentialId}` - ) - ) - .limit(1) - + const conflicting = await findConflictingWorkspaceCredential({ + credentialId: draft.credentialId, + workspaceId, + accountIds: [newAccountId], + }) if (conflicting) { - throw new Error( - `Cannot reconnect credential ${draft.credentialId}: account ${newAccountId} is already used by credential ${conflicting.id}` - ) + throw new CredentialAccountConflictError(draft.credentialId, conflicting.id) } } diff --git a/apps/sim/lib/credentials/draft-processor.ts b/apps/sim/lib/credentials/draft-processor.ts index 1ec7526736d..e49b829ccc0 100644 --- a/apps/sim/lib/credentials/draft-processor.ts +++ b/apps/sim/lib/credentials/draft-processor.ts @@ -1,18 +1,32 @@ import { db } from '@sim/db' import * as schema from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { and, desc, eq, sql } from 'drizzle-orm' -import { OAUTH_CREDENTIAL_DRAFT_CALLBACK_PARAM } from '@/lib/credentials/draft-constants' +import { filterUndefined } from '@sim/utils/object' +import { and, desc, eq, inArray, sql } from 'drizzle-orm' +import { stableExternalAccountId } from '@/lib/auth/connectors/account-id' +import { getBaseUrl } from '@/lib/core/utils/urls' import { + OAUTH_CREDENTIAL_ACCOUNT_CONFLICT_ERROR, + OAUTH_CREDENTIAL_DRAFT_CALLBACK_PARAM, +} from '@/lib/credentials/draft-constants' +import { + findConflictingWorkspaceCredential, handleCreateCredentialFromDraft, handleReconnectCredential, } from '@/lib/credentials/draft-hooks' import { completeOrganizationCredentialDraft } from '@/lib/credentials/organization-draft' +import { resumeConnectorsAfterCredentialReconnect } from '@/lib/knowledge/connectors/credential-recovery' +import { APP_ENTRY_PATH } from '@/lib/navigation/paths' +import { clearOAuthRefreshFailure } from '@/lib/oauth/credential-service' +import { OAUTH_PROVIDERS } from '@/lib/oauth/oauth' +import type { OAuthProvider } from '@/lib/oauth/types' +import { parseProvider } from '@/lib/oauth/utils' const logger = createLogger('CredentialDraftProcessor') interface OAuthStateWithCallbackUrl { callbackURL?: unknown + errorURL?: unknown } export type OAuthCredentialDraftBinding = @@ -77,14 +91,18 @@ export async function loadOAuthCredentialDraftBinding( } } -/** Captures request-scoped OAuth state before Better Auth defers its account after-hook. */ +/** + * Captures request-scoped OAuth state before Better Auth defers its account + * after-hook, returning the draft id the state carries. + */ export async function captureOAuthCredentialDraftBinding( context: object, loadOAuthState: () => Promise -): Promise { +): Promise { const binding = await loadOAuthCredentialDraftBinding(loadOAuthState) if (binding.status === 'unavailable') throw binding.error oauthCredentialDraftBindings.set(context, binding) + return binding.draftId } /** Consumes the binding captured for the same Better Auth account-hook context. */ @@ -96,6 +114,139 @@ export function consumeOAuthCredentialDraftBinding( return binding } +interface CredentialReconnectConflictParams { + draftId: string + /** The account row Better Auth is about to insert, its `accountId` still carrying the random suffix. */ + account: { + userId: string + providerId: string + accountId: string + accessToken?: string | null + refreshToken?: string | null + idToken?: string | null + accessTokenExpiresAt?: Date | null + refreshTokenExpiresAt?: Date | null + } + oauthState: OAuthStateWithCallbackUrl | null | undefined +} + +/** + * Decides, before Better Auth inserts the account row, whether completing this + * reconnect draft would collide with another credential in its workspace. + * + * A new row for an external identity inherits every credential on that + * identity's older rows (the stale-sibling migration in `account.create.after`), + * so a reconnect onto an identity that already backs another credential in the + * workspace would leave two credentials on one account. When it would, the + * draft is discarded and this returns where to send the user: the page the + * reconnect started from, carrying the reason as the `error` and + * `error_description` the OAuth return handlers already read. + * + * Returning before the insert leaves every credential bound to the account it + * had. The grant itself is still real, so its tokens replace the ones on the + * identity's existing rows and their connectors resume, as they would have on + * the new row, and a provider that rotates on reauthorization cannot strand + * them on a dead chain. Each row keeps its stored scope, which can carry state + * a fresh grant does not repeat (a Salesforce instance host). + */ +export async function refuseConflictingCredentialReconnect( + params: CredentialReconnectConflictParams +): Promise { + const { draftId, account, oauthState } = params + const { userId, providerId } = account + const stableIdentity = stableExternalAccountId(account.accountId) + if (!stableIdentity) return null + + const [draft] = await db + .select({ + credentialId: schema.pendingCredentialDraft.credentialId, + workspaceId: schema.pendingCredentialDraft.workspaceId, + }) + .from(schema.pendingCredentialDraft) + .where( + and( + eq(schema.pendingCredentialDraft.id, draftId), + eq(schema.pendingCredentialDraft.userId, userId), + eq(schema.pendingCredentialDraft.providerId, providerId), + sql`${schema.pendingCredentialDraft.expiresAt} > NOW()` + ) + ) + .limit(1) + if (!draft?.credentialId || !draft.workspaceId) return null + + const [reconnecting] = await db + .select({ accountId: schema.credential.accountId }) + .from(schema.credential) + .where(eq(schema.credential.id, draft.credentialId)) + .limit(1) + if (!reconnecting) return null + + const identityAccountIds = ( + await db + .select({ id: schema.account.id, accountId: schema.account.accountId }) + .from(schema.account) + .where(and(eq(schema.account.userId, userId), eq(schema.account.providerId, providerId))) + ) + .filter((row) => stableExternalAccountId(row.accountId) === stableIdentity) + .map((row) => row.id) + if (reconnecting.accountId && identityAccountIds.includes(reconnecting.accountId)) return null + + const conflicting = await findConflictingWorkspaceCredential({ + credentialId: draft.credentialId, + workspaceId: draft.workspaceId, + accountIds: identityAccountIds, + }) + if (!conflicting) return null + + const now = new Date() + await db + .update(schema.account) + .set({ + ...filterUndefined({ + accessToken: account.accessToken ?? undefined, + refreshToken: account.refreshToken ?? undefined, + idToken: account.idToken ?? undefined, + accessTokenExpiresAt: account.accessTokenExpiresAt ?? undefined, + refreshTokenExpiresAt: account.refreshTokenExpiresAt ?? undefined, + }), + updatedAt: now, + }) + .where(inArray(schema.account.id, identityAccountIds)) + for (const id of identityAccountIds) { + await clearOAuthRefreshFailure(id) + await resumeConnectorsAfterCredentialReconnect(id, now) + } + + await db + .delete(schema.pendingCredentialDraft) + .where(eq(schema.pendingCredentialDraft.id, draftId)) + + logger.warn('Refused a credential reconnect onto an account another credential uses', { + draftId, + userId, + providerId, + credentialId: draft.credentialId, + conflictingCredentialId: conflicting.id, + }) + + const returnUrl = [oauthState?.callbackURL, oauthState?.errorURL].find( + (url): url is string => typeof url === 'string' && url.length > 0 + ) + const redirectUrl = new URL(returnUrl ?? APP_ENTRY_PATH, getBaseUrl()) + redirectUrl.searchParams.delete(OAUTH_CREDENTIAL_DRAFT_CALLBACK_PARAM) + redirectUrl.searchParams.set('error', OAUTH_CREDENTIAL_ACCOUNT_CONFLICT_ERROR) + redirectUrl.searchParams.set( + 'error_description', + `That ${providerDisplayName(providerId)} account is already connected in this workspace as "${conflicting.displayName}". Use that connection, or reconnect with the account that belongs to this one.` + ) + return redirectUrl.toString() +} + +function providerDisplayName(providerId: string): string { + const { baseProvider } = parseProvider(providerId as OAuthProvider) + return OAUTH_PROVIDERS[baseProvider]?.name ?? 'provider' +} + interface ProcessCredentialDraftParams { draftId?: string userId: string