fix(credentials): keep the refused grant's tokens on the identity's existing rows

Author: waleedlatif1

apps/sim/lib/auth/auth.ts

@@ -147,8 +147,8 @@ import { CredentialAccountConflictError } from '@/lib/credentials/draft-hooks'
 import {
   captureOAuthCredentialDraftBinding,
   consumeOAuthCredentialDraftBinding,
-  findCredentialReconnectConflictRedirect,
   processCredentialDraft,
+  refuseConflictingCredentialReconnect,
 } from '@/lib/credentials/draft-processor'
 import { sendEmail } from '@/lib/messaging/email/mailer'
 import { getFromEmailAddress, getPersonalEmailFrom } from '@/lib/messaging/email/utils'
@@ -297,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<void> {
+  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
@@ -503,6 +537,7 @@ 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
@@ -516,26 +551,7 @@ export const auth = betterAuth({
               })
               throw error
             }
-
-            /**
-             * 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.
-             */
-            if (draftId) {
-              const conflictRedirectUrl = await findCredentialReconnectConflictRedirect({
-                draftId,
-                userId: account.userId,
-                providerId: account.providerId,
-                externalAccountId: account.accountId,
-                oauthState: await getOAuthState(),
-              })
-              if (conflictRedirectUrl) {
-                consumeOAuthCredentialDraftBinding(context)
-                throw new APIError('FOUND', undefined, { location: conflictRedirectUrl })
-              }
-            }
+            reconnectDraftId = draftId
           }
 
           if (account.accessToken && isSalesforceOAuthProviderId(account.providerId)) {
@@ -562,6 +578,27 @@ 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) => {
@@ -618,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

apps/sim/lib/credentials/__integration__/oauth-reconnect-account-conflict.integration.ts

@@ -6,6 +6,7 @@
 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'
@@ -54,13 +55,6 @@ function cookieHeader(response: Response, previous = '') {
   return [...cookies].map(([name, value]) => `${name}=${value}`).join('; ')
 }
 
-function jsonResponse(body: unknown) {
-  return new Response(JSON.stringify(body), {
-    status: 200,
-    headers: { 'content-type': 'application/json' },
-  })
-}
-
 describe('reconnecting a credential onto an account another credential uses', () => {
   let auth: typeof import('@/lib/auth/auth').auth
   let sessionCookie: string
@@ -240,7 +234,7 @@ describe('reconnecting a credential onto an account another credential uses', ()
       .where(and(eq(account.userId, userId), eq(account.providerId, PROVIDER_ID)))
   }
 
-  it('sends the user back to the reconnect page with the reason and changes nothing', async () => {
+  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)
@@ -258,7 +252,10 @@ describe('reconnecting a credential onto an account another credential uses', ()
     expect(rows.map((row) => row.id).sort()).toEqual(
       [workAccountRowId, personalAccountRowId].sort()
     )
-    expect(rows.find((row) => row.id === personalAccountRowId)?.accessToken).toBe('personal-access')
+    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)

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

@@ -1,7 +1,8 @@
 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 { 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 {
@@ -15,6 +16,7 @@ import {
 } from '@/lib/credentials/draft-hooks'
 import { completeOrganizationCredentialDraft } from '@/lib/credentials/organization-draft'
 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'
@@ -113,10 +115,18 @@ export function consumeOAuthCredentialDraftBinding(
 
 interface CredentialReconnectConflictParams {
   draftId: string
-  userId: string
-  providerId: string
-  /** The provider identity Better Auth is about to store, still carrying its random suffix. */
-  externalAccountId: 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
+    scope?: string | null
+  }
   oauthState: OAuthStateWithCallbackUrl | null | undefined
 }
 
@@ -130,14 +140,20 @@ interface CredentialReconnectConflictParams {
  * 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 and account exactly as it was.
+ * `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, the same tokens those credentials would have
+ * inherited from the new row, and a provider that rotates on reauthorization
+ * cannot strand them on a dead chain.
  */
-export async function findCredentialReconnectConflictRedirect(
+export async function refuseConflictingCredentialReconnect(
   params: CredentialReconnectConflictParams
 ): Promise<string | null> {
-  const { draftId, userId, providerId, externalAccountId, oauthState } = params
-  const stableIdentity = stableExternalAccountId(externalAccountId)
+  const { draftId, account, oauthState } = params
+  const { userId, providerId } = account
+  const stableIdentity = stableExternalAccountId(account.accountId)
   if (!stableIdentity) return null
 
   const [draft] = await db
@@ -181,6 +197,22 @@ export async function findCredentialReconnectConflictRedirect(
   })
   if (!conflicting) return null
 
+  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,
+        scope: account.scope ?? undefined,
+      }),
+      updatedAt: new Date(),
+    })
+    .where(inArray(schema.account.id, identityAccountIds))
+  await Promise.all(identityAccountIds.map((id) => clearOAuthRefreshFailure(id)))
+
   await db
     .delete(schema.pendingCredentialDraft)
     .where(eq(schema.pendingCredentialDraft.id, draftId))