fix(quickbooks): harden OAuth company binding

Author: Bill Leoutsakos

apps/docs/content/docs/en/integrations/quickbooks.mdx

@@ -32,7 +32,7 @@ Get information about the connected QuickBooks Online company
 | Parameter | Type | Description |
 | --------- | ---- | ----------- |
 | `company` | json | Verified QuickBooks CompanyInfo object, including Id, CompanyName, LegalName, addresses, contact details, NameValue settings, and MetaData when populated |
-|   ↳ `Id` | string | Connected QuickBooks company ID |
+|   ↳ `Id` | string | QuickBooks CompanyInfo entity ID \(commonly "1"\); this is not the OAuth realmId |
 |   ↳ `SyncToken` | string | CompanyInfo sync token |
 |   ↳ `CompanyName` | string | Company display name |
 |   ↳ `LegalName` | string | Company legal name |

apps/sim/app/api/auth/[...all]/route.test.ts

@@ -30,6 +30,7 @@ vi.mock('@/lib/auth/anonymous', () => ({
   createAnonymousSession: handlerMocks.createAnonymousSession,
 }))
 
+import { getQuickBooksCallbackRealm } from '@/lib/oauth/quickbooks'
 import { GET, POST } from '@/app/api/auth/[...all]/route'
 
 afterAll(resetEnvFlagsMock)
@@ -132,3 +133,67 @@ describe('auth catch-all route organization mutations', () => {
     expect(json).toEqual({ data: { ok: true } })
   })
 })
+
+describe('auth catch-all route QuickBooks callback', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    setEnvFlags({ isAuthDisabled: false })
+  })
+
+  it('binds the callback realm only while Better Auth processes the OAuth response', async () => {
+    const { NextResponse } = await import('next/server')
+    handlerMocks.betterAuthGET.mockImplementationOnce(async () => {
+      await Promise.resolve()
+      expect(getQuickBooksCallbackRealm()).toBe('123456789')
+      return new NextResponse(null, { status: 302 })
+    })
+
+    const req = createMockRequest(
+      'GET',
+      undefined,
+      {},
+      'http://localhost:3000/api/auth/oauth2/callback/quickbooks?code=test&state=test&realmId=123456789'
+    )
+
+    const res = await GET(req as any)
+
+    expect(res.status).toBe(302)
+    expect(handlerMocks.betterAuthGET).toHaveBeenCalledTimes(1)
+    expect(() => getQuickBooksCallbackRealm()).toThrow(/did not include a company identity/)
+  })
+
+  it('delegates a denied callback without requiring a realm', async () => {
+    const { NextResponse } = await import('next/server')
+    handlerMocks.betterAuthGET.mockImplementationOnce(async () => {
+      expect(() => getQuickBooksCallbackRealm()).toThrow(/did not include a company identity/)
+      return new NextResponse(null, { status: 302 })
+    })
+
+    const req = createMockRequest(
+      'GET',
+      undefined,
+      {},
+      'http://localhost:3000/api/auth/oauth2/callback/quickbooks?error=access_denied&state=test'
+    )
+
+    const res = await GET(req as any)
+
+    expect(res.status).toBe(302)
+    expect(handlerMocks.betterAuthGET).toHaveBeenCalledTimes(1)
+  })
+
+  it.each([
+    ['missing', 'http://localhost:3000/api/auth/oauth2/callback/quickbooks?code=test&state=test'],
+    [
+      'invalid',
+      'http://localhost:3000/api/auth/oauth2/callback/quickbooks?code=test&state=test&realmId=not-a-company',
+    ],
+  ])('rejects a %s callback realm before Better Auth exchanges the code', async (_, url) => {
+    const req = createMockRequest('GET', undefined, {}, url)
+
+    const res = await GET(req as any)
+
+    expect(res.status).toBe(400)
+    expect(handlerMocks.betterAuthGET).not.toHaveBeenCalled()
+  })
+})

apps/sim/app/api/auth/[...all]/route.ts

@@ -4,6 +4,7 @@ import { auth } from '@/lib/auth'
 import { createAnonymousSession, ensureAnonymousUserExists } from '@/lib/auth/anonymous'
 import { isAuthDisabled } from '@/lib/core/config/env-flags'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
+import { normalizeQuickBooksRealmId, withQuickBooksCallbackRealm } from '@/lib/oauth/quickbooks'
 
 export const dynamic = 'force-dynamic'
 
@@ -27,6 +28,32 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
     return NextResponse.json(createAnonymousSession())
   }
 
+  if (path === 'oauth2/callback/quickbooks') {
+    const authorizationCode = request.nextUrl.searchParams.get('code')
+    if (!authorizationCode) {
+      return betterAuthGET(request)
+    }
+
+    const realmId = request.nextUrl.searchParams.get('realmId')
+    if (!realmId) {
+      return NextResponse.json(
+        { error: 'QuickBooks callback did not include a company identity.' },
+        { status: 400 }
+      )
+    }
+
+    try {
+      normalizeQuickBooksRealmId(realmId)
+    } catch {
+      return NextResponse.json(
+        { error: 'QuickBooks callback included an invalid company identity.' },
+        { status: 400 }
+      )
+    }
+
+    return withQuickBooksCallbackRealm(realmId, () => betterAuthGET(request))
+  }
+
   return betterAuthGET(request)
 })
 

apps/sim/lib/auth/auth.ts

@@ -105,6 +105,7 @@ import {
 } from '@/lib/oauth/microsoft'
 import {
   fetchQuickBooksConnectionProfile,
+  getQuickBooksCallbackRealm,
   QUICKBOOKS_AUTHORIZATION_URL,
   QUICKBOOKS_OIDC_CLAIMS,
   QUICKBOOKS_TOKEN_URL,
@@ -114,6 +115,7 @@ import { clearDeadFlag } from '@/lib/oauth/terminal-errors'
 import { getCanonicalScopesForProvider } from '@/lib/oauth/utils'
 import { joinInstanceOrganization } from '@/lib/organizations/instance-org'
 import { captureServerEvent, getPostHogClient } from '@/lib/posthog/server'
+import { QUICKBOOKS_OAUTH_REQUEST_TIMEOUT_MS } from '@/lib/quickbooks/client'
 import { disableUserResources } from '@/lib/workflows/lifecycle'
 import { SSO_TRUSTED_PROVIDERS } from '@/ee/sso/constants'
 
@@ -3240,6 +3242,7 @@ export const auth = betterAuth({
                 grant_type: 'authorization_code',
                 redirect_uri: redirectURI,
               }),
+              signal: AbortSignal.timeout(QUICKBOOKS_OAUTH_REQUEST_TIMEOUT_MS),
             })
             const data = await readResponseJsonWithLimit<Record<string, unknown>>(response, {
               maxBytes: DEFAULT_MAX_ERROR_BODY_BYTES,
@@ -3266,7 +3269,10 @@ export const auth = betterAuth({
               throw new Error('QuickBooks OAuth did not issue an access token')
             }
 
-            const profile = await fetchQuickBooksConnectionProfile(tokens.accessToken)
+            const profile = await fetchQuickBooksConnectionProfile(
+              tokens.accessToken,
+              getQuickBooksCallbackRealm()
+            )
 
             const now = new Date()
             return {

apps/sim/lib/core/config/env.ts

@@ -417,7 +417,7 @@ export const env = createEnv({
     PIPEDRIVE_CLIENT_SECRET:               z.string().optional(),                  // Pipedrive OAuth client secret
     QUICKBOOKS_CLIENT_ID:                  z.string().optional(),                  // QuickBooks Online OAuth client ID
     QUICKBOOKS_CLIENT_SECRET:              z.string().optional(),                  // QuickBooks Online OAuth client secret
-    QUICKBOOKS_ENV:                        z.enum(['sandbox', 'production']).optional(), // QuickBooks Online API environment (defaults to sandbox)
+    QUICKBOOKS_ENV:                        z.enum(['sandbox', 'production']).optional(), // QuickBooks Online API environment (must be configured explicitly)
     LINEAR_CLIENT_ID:                      z.string().optional(),                  // Linear OAuth client ID
     LINEAR_CLIENT_SECRET:                  z.string().optional(),                  // Linear OAuth client secret
     CLICKUP_CLIENT_ID:                     z.string().optional(),                  // ClickUp OAuth client ID

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

@@ -0,0 +1,113 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const mocks = vi.hoisted(() => ({
+  limit: vi.fn(),
+  deleteWhere: vi.fn(),
+  handleCreate: vi.fn(),
+  handleReconnect: vi.fn(),
+  loggerError: vi.fn(),
+  loggerInfo: vi.fn(),
+}))
+
+vi.mock('@sim/db', () => ({
+  db: {
+    select: vi.fn(() => ({
+      from: vi.fn(() => ({
+        where: vi.fn(() => ({
+          limit: mocks.limit,
+        })),
+      })),
+    })),
+    delete: vi.fn(() => ({
+      where: mocks.deleteWhere,
+    })),
+  },
+}))
+
+vi.mock('@sim/logger', () => ({
+  createLogger: () => ({
+    error: mocks.loggerError,
+    info: mocks.loggerInfo,
+  }),
+}))
+
+vi.mock('@/lib/credentials/draft-hooks', () => ({
+  handleCreateCredentialFromDraft: mocks.handleCreate,
+  handleReconnectCredential: mocks.handleReconnect,
+}))
+
+import { processCredentialDraft } from '@/lib/credentials/draft-processor'
+
+const draft = {
+  id: 'draft-1',
+  userId: 'user-1',
+  workspaceId: 'workspace-1',
+  providerId: 'quickbooks',
+  displayName: 'QuickBooks',
+  description: null,
+  credentialId: null,
+  expiresAt: new Date(Date.now() + 60_000),
+  createdAt: new Date(),
+}
+
+describe('processCredentialDraft', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('does nothing when no matching draft exists', async () => {
+    mocks.limit.mockResolvedValueOnce([])
+
+    await processCredentialDraft({
+      userId: 'user-1',
+      providerId: 'quickbooks',
+      accountId: 'account-1',
+    })
+
+    expect(mocks.limit).toHaveBeenCalledWith(2)
+    expect(mocks.handleCreate).not.toHaveBeenCalled()
+    expect(mocks.handleReconnect).not.toHaveBeenCalled()
+    expect(mocks.deleteWhere).not.toHaveBeenCalled()
+  })
+
+  it('creates a credential when exactly one matching draft exists', async () => {
+    mocks.limit.mockResolvedValueOnce([draft])
+
+    await processCredentialDraft({
+      userId: 'user-1',
+      providerId: 'quickbooks',
+      accountId: 'account-1',
+    })
+
+    expect(mocks.handleCreate).toHaveBeenCalledWith(
+      expect.objectContaining({
+        draft,
+        accountId: 'account-1',
+        providerId: 'quickbooks',
+        userId: 'user-1',
+      })
+    )
+    expect(mocks.deleteWhere).toHaveBeenCalledTimes(1)
+  })
+
+  it('fails closed instead of binding an account when multiple workspaces have drafts', async () => {
+    mocks.limit.mockResolvedValueOnce([
+      draft,
+      { ...draft, id: 'draft-2', workspaceId: 'workspace-2' },
+    ])
+
+    await processCredentialDraft({
+      userId: 'user-1',
+      providerId: 'quickbooks',
+      accountId: 'account-1',
+    })
+
+    expect(mocks.handleCreate).not.toHaveBeenCalled()
+    expect(mocks.handleReconnect).not.toHaveBeenCalled()
+    expect(mocks.deleteWhere).not.toHaveBeenCalled()
+    expect(mocks.loggerError).toHaveBeenCalledWith(
+      'Refusing to process ambiguous credential drafts',
+      expect.objectContaining({ userId: 'user-1', providerId: 'quickbooks', draftCount: 2 })
+    )
+  })
+})

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

@@ -18,12 +18,14 @@ interface ProcessCredentialDraftParams {
 /**
  * Looks up a pending credential draft for the given user/provider and processes it.
  * Creates a new credential or reconnects an existing one depending on the draft state.
+ * If more than one workspace has a matching draft, fail closed: an uncorrelated
+ * OAuth callback cannot safely determine which workspace initiated the connection.
  * Used by Better Auth's `account.create.after` hook and custom OAuth flows (Shopify, Trello).
  */
 export async function processCredentialDraft(params: ProcessCredentialDraftParams): Promise<void> {
   const { userId, providerId, accountId } = params
 
-  const [draft] = await db
+  const drafts = await db
     .select()
     .from(schema.pendingCredentialDraft)
     .where(
@@ -33,9 +35,18 @@ export async function processCredentialDraft(params: ProcessCredentialDraftParam
         sql`${schema.pendingCredentialDraft.expiresAt} > NOW()`
       )
     )
-    .limit(1)
+    .limit(2)
 
+  const [draft] = drafts
   if (!draft) return
+  if (drafts.length > 1) {
+    logger.error('Refusing to process ambiguous credential drafts', {
+      userId,
+      providerId,
+      draftCount: drafts.length,
+    })
+    return
+  }
 
   const now = new Date()