fix(credentials): address reconnect review findings

Author: j15z

apps/sim/app/api/auth/oauth2/authorize/route.test.ts

@@ -58,6 +58,7 @@ function oauthCredentialActor(overrides: Record<string, unknown> = {}) {
       workspaceId: WORKSPACE_ID,
       type: 'oauth',
       providerId: 'google-email',
+      displayName: 'Work Gmail',
       ...((overrides.credential as Record<string, unknown>) ?? {}),
     },
     member: null,
@@ -189,6 +190,39 @@ describe('OAuth2 authorize route', () => {
       )
     })
 
+    it("uses the credential's actual display name for the reconnect draft (audit accuracy)", async () => {
+      mockGetCredentialActorContext.mockResolvedValue(
+        oauthCredentialActor({ credential: { displayName: 'Renamed By User' } })
+      )
+
+      await GET(
+        authorizeRequest({
+          providerId: 'google-email',
+          workspaceId: WORKSPACE_ID,
+          credentialId: CREDENTIAL_ID,
+        })
+      )
+
+      expect(dbChainMockFns.values).toHaveBeenCalledWith(
+        expect.objectContaining({ displayName: 'Renamed By User' })
+      )
+    })
+
+    it('rejects reconnect for custom-flow providers (trello/shopify) and writes no draft', async () => {
+      for (const providerId of ['trello', 'shopify']) {
+        const response = await GET(
+          authorizeRequest({ providerId, workspaceId: WORKSPACE_ID, credentialId: CREDENTIAL_ID })
+        )
+
+        expect(response.headers.get('location')).toBe(
+          `${BASE_URL}/workspace?error=credential_reconnect_unsupported`
+        )
+      }
+      expect(mockGetCredentialActorContext).not.toHaveBeenCalled()
+      expect(dbChainMockFns.values).not.toHaveBeenCalled()
+      expect(mockOAuth2LinkAccount).not.toHaveBeenCalled()
+    })
+
     it('rejects when the caller is not a credential admin and writes no draft', async () => {
       mockGetCredentialActorContext.mockResolvedValue(oauthCredentialActor({ isAdmin: false }))
 

apps/sim/app/api/auth/oauth2/authorize/route.ts

@@ -33,37 +33,42 @@ async function createConnectDraft(params: {
   workspaceId: string
   providerId: string
   credentialId?: string
+  /** Reconnect only: the credential's actual name, so audit records stay accurate. */
+  displayName?: string
 }): Promise<void> {
   const { userId, workspaceId, providerId, credentialId } = params
 
-  const service = getAllOAuthServices().find((s) => s.providerId === providerId)
-  const serviceName = service?.name ?? providerId
+  let displayName = params.displayName
+  if (!displayName) {
+    const service = getAllOAuthServices().find((s) => s.providerId === providerId)
+    const serviceName = service?.name ?? providerId
+
+    let userName: string | null = null
+    try {
+      const [row] = await db.select({ name: user.name }).from(user).where(eq(user.id, userId))
+      userName = row?.name ?? null
+    } catch {
+      // Fall back to the "My {Service}" default
+    }
 
-  let userName: string | null = null
-  try {
-    const [row] = await db.select({ name: user.name }).from(user).where(eq(user.id, userId))
-    userName = row?.name ?? null
-  } catch {
-    // Fall back to the "My {Service}" default
-  }
+    // Auto-number against existing workspace credentials so repeat connects for
+    // the same provider stay distinguishable — same behavior as the connect
+    // modal, which computes this client-side. Best effort: on failure the name
+    // simply skips deduplication.
+    let takenNames: ReadonlySet<string> = new Set<string>()
+    try {
+      const rows = await db
+        .select({ displayName: credential.displayName })
+        .from(credential)
+        .where(and(eq(credential.workspaceId, workspaceId), eq(credential.type, 'oauth')))
+      takenNames = new Set(rows.map((row) => row.displayName.toLowerCase()))
+    } catch {
+      // Best effort — proceed without collision numbering
+    }
 
-  // Auto-number against existing workspace credentials so repeat connects for
-  // the same provider stay distinguishable — same behavior as the connect
-  // modal, which computes this client-side. Best effort: on failure the name
-  // simply skips deduplication.
-  let takenNames: ReadonlySet<string> = new Set<string>()
-  try {
-    const rows = await db
-      .select({ displayName: credential.displayName })
-      .from(credential)
-      .where(and(eq(credential.workspaceId, workspaceId), eq(credential.type, 'oauth')))
-    takenNames = new Set(rows.map((row) => row.displayName.toLowerCase()))
-  } catch {
-    // Best effort — proceed without collision numbering
+    displayName = defaultCredentialDisplayName(userName, serviceName, takenNames)
   }
 
-  const displayName = defaultCredentialDisplayName(userName, serviceName, takenNames)
-
   const now = new Date()
   const expiresAt = new Date(now.getTime() + DRAFT_TTL_MS)
   await db
@@ -141,7 +146,22 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
       return NextResponse.redirect(`${baseUrl}/workspace?error=workspace_access_denied`)
     }
 
+    let reconnectDisplayName: string | undefined
     if (credentialId) {
+      // Trello and Shopify authorize through their own custom flows that bypass
+      // this endpoint, so a reconnect draft written here would linger unconsumed
+      // and could later be picked up by their token-store callbacks, silently
+      // rebinding the credential. Mirror the copilot tool and reject reconnect.
+      if (providerId === 'trello' || providerId === 'shopify') {
+        logger.warn('Reconnect not supported for custom-flow provider', {
+          userId,
+          workspaceId,
+          providerId,
+          credentialId,
+        })
+        return NextResponse.redirect(`${baseUrl}/workspace?error=credential_reconnect_unsupported`)
+      }
+
       // Reconnect: the OAuth callback will rebind this credential to the fresh
       // account, so require the same credential-admin access as the draft POST
       // route — workspace write alone must not be enough to swap someone's tokens.
@@ -172,12 +192,19 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
         })
         return NextResponse.redirect(`${baseUrl}/workspace?error=credential_provider_mismatch`)
       }
+      reconnectDisplayName = actor.credential.displayName
     }
 
     // Create the draft before initiating the link so it is guaranteed to exist
     // (and freshly clocked) when the OAuth callback's `account.create.after`
     // hook runs. If this throws, we never start the OAuth flow.
-    await createConnectDraft({ userId, workspaceId, providerId, credentialId })
+    await createConnectDraft({
+      userId,
+      workspaceId,
+      providerId,
+      credentialId,
+      displayName: reconnectDisplayName,
+    })
 
     const linkResponse = await auth.api.oAuth2LinkAccount({
       body: { providerId, callbackURL },

apps/sim/lib/copilot/tools/handlers/access.ts

@@ -1,6 +1,6 @@
 import { authorizeWorkflowByWorkspacePermission } from '@sim/platform-authz/workflow'
 import type { getWorkflowById } from '@/lib/workflows/utils'
-import { checkWorkspaceAccess } from '@/lib/workspaces/permissions/utils'
+import { checkWorkspaceAccess, type WorkspaceAccess } from '@/lib/workspaces/permissions/utils'
 import { listAccessibleWorkspaceRowsForUser } from '@/lib/workspaces/utils'
 
 type WorkflowRecord = NonNullable<Awaited<ReturnType<typeof getWorkflowById>>>
@@ -47,22 +47,23 @@ export async function ensureWorkspaceAccess(
   workspaceId: string,
   userId: string,
   level: 'read' | 'write' | 'admin' = 'read'
-): Promise<void> {
+): Promise<WorkspaceAccess> {
   const access = await checkWorkspaceAccess(workspaceId, userId)
   if (!access.exists || !access.hasAccess) {
     throw new Error(`Workspace ${workspaceId} not found`)
   }
 
-  if (level === 'read') return
+  if (level === 'read') return access
 
   if (level === 'admin') {
     if (!access.canAdmin) {
       throw new Error('Admin access required for this workspace')
     }
-    return
+    return access
   }
 
   if (!access.canWrite) {
     throw new Error('Write or admin access required for this workspace')
   }
+  return access
 }

apps/sim/lib/copilot/tools/handlers/oauth.test.ts

@@ -39,6 +39,14 @@ const context = {
   chatId: 'chat-1',
 } as unknown as ExecutionContext
 
+const WORKSPACE_ACCESS = {
+  exists: true,
+  hasAccess: true,
+  canWrite: true,
+  canAdmin: false,
+  workspace: { id: WORKSPACE_ID },
+}
+
 function oauthCredentialActor(overrides: Record<string, unknown> = {}) {
   return {
     credential: {
@@ -60,7 +68,7 @@ describe('executeOAuthGetAuthLink', () => {
   beforeEach(() => {
     vi.clearAllMocks()
     process.env.NEXT_PUBLIC_APP_URL = BASE_URL
-    mockEnsureWorkspaceAccess.mockResolvedValue(undefined)
+    mockEnsureWorkspaceAccess.mockResolvedValue(WORKSPACE_ACCESS)
   })
 
   describe('connect (no credentialId)', () => {
@@ -93,6 +101,19 @@ describe('executeOAuthGetAuthLink', () => {
       expect(output.message).toContain(CREDENTIAL_ID)
     })
 
+    it('reuses the already-resolved workspace access for the credential lookup', async () => {
+      mockGetCredentialActorContext.mockResolvedValue(oauthCredentialActor())
+
+      await executeOAuthGetAuthLink(
+        { providerName: 'google-email', credentialId: CREDENTIAL_ID },
+        context
+      )
+
+      expect(mockGetCredentialActorContext).toHaveBeenCalledWith(CREDENTIAL_ID, USER_ID, {
+        workspaceAccess: WORKSPACE_ACCESS,
+      })
+    })
+
     it('fails with an agent-visible error for a nonexistent credential', async () => {
       mockGetCredentialActorContext.mockResolvedValue({
         credential: null,

apps/sim/lib/copilot/tools/handlers/oauth.ts

@@ -4,6 +4,7 @@ import { ensureWorkspaceAccess } from '@/lib/copilot/tools/handlers/access'
 import { getBaseUrl } from '@/lib/core/utils/urls'
 import { getCredentialActorContext } from '@/lib/credentials/access'
 import { getAllOAuthServices } from '@/lib/oauth/utils'
+import type { WorkspaceAccess } from '@/lib/workspaces/permissions/utils'
 
 export async function executeOAuthGetAuthLink(
   rawParams: Record<string, unknown>,
@@ -17,14 +18,18 @@ export async function executeOAuthGetAuthLink(
     if (!context.workspaceId || !context.userId) {
       throw new Error('workspaceId and userId are required to generate an OAuth link')
     }
-    await ensureWorkspaceAccess(context.workspaceId, context.userId, 'write')
+    const workspaceAccess = await ensureWorkspaceAccess(
+      context.workspaceId,
+      context.userId,
+      'write'
+    )
     const result = await generateOAuthLink(
       context.workspaceId,
       context.workflowId,
       context.chatId,
       providerName,
       baseUrl,
-      credentialId ? { credentialId, userId: context.userId } : undefined
+      credentialId ? { credentialId, userId: context.userId, workspaceAccess } : undefined
     )
     const action = credentialId ? 'reconnect' : 'connect'
     return {
@@ -92,7 +97,7 @@ async function generateOAuthLink(
   chatId: string | undefined,
   providerName: string,
   baseUrl: string,
-  reconnect?: { credentialId: string; userId: string }
+  reconnect?: { credentialId: string; userId: string; workspaceAccess: WorkspaceAccess }
 ): Promise<{ url: string; providerId: string; serviceName: string }> {
   if (!workspaceId) {
     throw new Error('workspaceId is required to generate an OAuth link')
@@ -127,7 +132,9 @@ async function generateOAuthLink(
           `integrations page and press Reconnect on the credential there.`
       )
     }
-    const actor = await getCredentialActorContext(reconnect.credentialId, reconnect.userId)
+    const actor = await getCredentialActorContext(reconnect.credentialId, reconnect.userId, {
+      workspaceAccess: reconnect.workspaceAccess,
+    })
     if (!actor.credential || actor.credential.workspaceId !== workspaceId) {
       throw new Error(
         `Credential "${reconnect.credentialId}" was not found in this workspace. Read ` +