Skip to content

Commit 1512602

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
refactor(quickbooks): align with shared integration patterns
1 parent edec60b commit 1512602

63 files changed

Lines changed: 125 additions & 712 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/sim/app/api/auth/oauth/utils.test.ts

Lines changed: 0 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,6 @@
77
import { redisConfigMockFns } from '@sim/testing'
88
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
99

10-
const { capturedLeaderLockOptions } = vi.hoisted(() => ({
11-
capturedLeaderLockOptions: [] as Array<Record<string, unknown>>,
12-
}))
13-
vi.mock('@/lib/concurrency/leader-lock', async (importOriginal) => {
14-
const actual = await importOriginal<typeof import('@/lib/concurrency/leader-lock')>()
15-
return {
16-
...actual,
17-
withLeaderLock: vi.fn((options) => {
18-
capturedLeaderLockOptions.push(options as unknown as Record<string, unknown>)
19-
return actual.withLeaderLock(options)
20-
}),
21-
}
22-
})
23-
2410
vi.mock('@/lib/oauth/oauth', () => ({
2511
refreshOAuthToken: vi.fn(),
2612
OAUTH_PROVIDERS: {},
@@ -84,7 +70,6 @@ function mockUpdateChain() {
8470
describe('OAuth Utils', () => {
8571
beforeEach(() => {
8672
vi.clearAllMocks()
87-
capturedLeaderLockOptions.length = 0
8873
__resetCoalesceLocallyForTests()
8974
redisConfigMockFns.mockGetRedisClient.mockReturnValue(null)
9075
redisConfigMockFns.mockAcquireLock.mockResolvedValue(true)
@@ -435,62 +420,6 @@ describe('OAuth Utils', () => {
435420
})
436421
})
437422

438-
describe('QuickBooks refresh locking', () => {
439-
it('keeps the lock and follower wait alive beyond the provider timeout', async () => {
440-
const credential = {
441-
id: 'quickbooks-row',
442-
accessToken: 'expired-token',
443-
refreshToken: 'rotating-refresh-token',
444-
accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000),
445-
providerId: 'quickbooks',
446-
}
447-
mockRefreshOAuthToken.mockResolvedValueOnce({
448-
ok: true,
449-
accessToken: 'new-token',
450-
expiresIn: 3600,
451-
refreshToken: 'new-rotating-refresh-token',
452-
})
453-
mockUpdateChain()
454-
455-
const result = await refreshTokenIfNeeded('request-id', credential, credential.id)
456-
457-
expect(result).toEqual({ accessToken: 'new-token', refreshed: true })
458-
expect(redisConfigMockFns.mockAcquireLock).toHaveBeenCalledWith(
459-
'oauth:refresh:quickbooks-row',
460-
expect.any(String),
461-
30
462-
)
463-
expect(capturedLeaderLockOptions[0]).toMatchObject({ ttlSec: 30, maxWaitMs: 30_000 })
464-
})
465-
466-
it('keeps the default refresh-lock budget for other providers', async () => {
467-
const credential = {
468-
id: 'google-row',
469-
accessToken: 'expired-token',
470-
refreshToken: 'refresh-token',
471-
accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000),
472-
providerId: 'google',
473-
}
474-
mockRefreshOAuthToken.mockResolvedValueOnce({
475-
ok: true,
476-
accessToken: 'new-token',
477-
expiresIn: 3600,
478-
refreshToken: 'new-refresh-token',
479-
})
480-
mockUpdateChain()
481-
482-
await refreshTokenIfNeeded('request-id', credential, credential.id)
483-
484-
expect(redisConfigMockFns.mockAcquireLock).toHaveBeenCalledWith(
485-
'oauth:refresh:google-row',
486-
expect.any(String),
487-
10
488-
)
489-
expect(capturedLeaderLockOptions[0]).not.toHaveProperty('ttlSec')
490-
expect(capturedLeaderLockOptions[0]).not.toHaveProperty('maxWaitMs')
491-
})
492-
})
493-
494423
describe('resolveServiceAccountToken', () => {
495424
it('throws loudly for an unknown provider (never silently attempts Google)', async () => {
496425
await expect(resolveServiceAccountToken('cred-1', 'mystery-provider')).rejects.toThrow(

apps/sim/app/api/auth/oauth/utils.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -701,8 +701,6 @@ interface CoalescedRefreshOptions {
701701
*/
702702
const SLACK_LOCK_TTL_SEC = 30
703703
const SLACK_FOLLOWER_MAX_WAIT_MS = SLACK_LOCK_TTL_SEC * 1000
704-
const QUICKBOOKS_LOCK_TTL_SEC = 30
705-
const QUICKBOOKS_FOLLOWER_MAX_WAIT_MS = QUICKBOOKS_LOCK_TTL_SEC * 1000
706704

707705
async function performCoalescedRefresh({
708706
accountId,
@@ -718,7 +716,6 @@ async function performCoalescedRefresh({
718716
* dead-flagged, and written per installation rather than per row.
719717
*/
720718
const slackTeamId = isSlackProvider(providerId) ? extractSlackTeamId(providerAccountId) : null
721-
const isQuickBooks = providerId === 'quickbooks'
722719
const scopeKey = slackTeamId ? `slack:${slackTeamId}` : accountId
723720

724721
const logContext = {
@@ -747,14 +744,7 @@ async function performCoalescedRefresh({
747744
// so their wait and the lock TTL must outlast the 15s provider timeout —
748745
// the 3s/10s defaults would fail followers early and let a second leader
749746
// start a concurrent rotation mid-refresh.
750-
...(slackTeamId
751-
? { maxWaitMs: SLACK_FOLLOWER_MAX_WAIT_MS, ttlSec: SLACK_LOCK_TTL_SEC }
752-
: isQuickBooks
753-
? {
754-
maxWaitMs: QUICKBOOKS_FOLLOWER_MAX_WAIT_MS,
755-
ttlSec: QUICKBOOKS_LOCK_TTL_SEC,
756-
}
757-
: {}),
747+
...(slackTeamId ? { maxWaitMs: SLACK_FOLLOWER_MAX_WAIT_MS, ttlSec: SLACK_LOCK_TTL_SEC } : {}),
758748
onLeader: async () => {
759749
try {
760750
let refreshTokenToUse = refreshToken

apps/sim/app/api/tools/quickbooks/download-document/route.ts

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ import {
1919
readResponseToBufferWithLimit,
2020
} from '@/lib/core/utils/stream-limits'
2121
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
22-
import { storeToolOutputFile } from '@/lib/uploads/store-tool-output-file'
22+
import { uploadCopilotFile } from '@/lib/uploads/contexts/copilot'
23+
import { uploadExecutionFile } from '@/lib/uploads/contexts/execution'
2324
import { buildQuickBooksCompanyUrl, buildQuickBooksHeaders } from '@/tools/quickbooks/client'
2425
import {
2526
getQuickBooksDocumentError,
@@ -203,18 +204,29 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
203204
: await downloadQuickBooksTransactionPdf(body, request.signal)
204205

205206
request.signal.throwIfAborted()
207+
const executionContext =
208+
body.workspaceId && body.workflowId && body.executionId
209+
? {
210+
workspaceId: body.workspaceId,
211+
workflowId: body.workflowId,
212+
executionId: body.executionId,
213+
}
214+
: null
206215
const storedFile = userFileSchema.parse(
207-
await storeToolOutputFile({
208-
buffer: downloaded.buffer,
209-
fileName: downloaded.fileName,
210-
contentType: downloaded.mimeType,
211-
userId: authResult.userId,
212-
context: {
213-
workspaceId: body.workspaceId,
214-
workflowId: body.workflowId,
215-
executionId: body.executionId,
216-
},
217-
})
216+
executionContext
217+
? await uploadExecutionFile(
218+
executionContext,
219+
downloaded.buffer,
220+
downloaded.fileName,
221+
downloaded.mimeType,
222+
authResult.userId
223+
)
224+
: await uploadCopilotFile({
225+
buffer: downloaded.buffer,
226+
fileName: downloaded.fileName,
227+
contentType: downloaded.mimeType,
228+
userId: authResult.userId,
229+
})
218230
)
219231

220232
const shared = {

apps/sim/lib/auth/connectors/providers.ts

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { syntheticConnectorEmail } from '@/lib/auth/connector-email'
88
import { env } from '@/lib/core/config/env'
99
import { inspectConfiguredOAuthClient } from '@/lib/core/config/env-capabilities.server'
1010
import {
11-
DEFAULT_MAX_ERROR_BODY_BYTES,
1211
readResponseJsonWithLimit,
1312
readResponseTextWithLimit,
1413
} from '@/lib/core/utils/stream-limits'
@@ -22,7 +21,6 @@ import {
2221
QUICKBOOKS_TOKEN_URL,
2322
} from '@/lib/oauth/quickbooks'
2423
import { getCanonicalScopesForProvider } from '@/lib/oauth/utils'
25-
import { QUICKBOOKS_OAUTH_REQUEST_TIMEOUT_MS } from '@/tools/quickbooks/client'
2624
import { deriveZohoDeskBaseFromApiDomain } from '@/tools/zoho_desk/host-allowlist'
2725

2826
/**
@@ -2377,57 +2375,11 @@ export function buildConnectorProviders(): GenericOAuthConfig[] {
23772375
responseType: 'code',
23782376
accessType: 'offline',
23792377
prompt: 'consent',
2378+
authentication: 'basic',
23802379
redirectURI: `${getBaseUrl()}/api/auth/oauth2/callback/quickbooks`,
23812380
authorizationUrlParams: {
23822381
claims: JSON.stringify(QUICKBOOKS_OIDC_CLAIMS),
23832382
},
2384-
getToken: async ({ code, redirectURI }) => {
2385-
const clientId = env.QUICKBOOKS_CLIENT_ID
2386-
const clientSecret = env.QUICKBOOKS_CLIENT_SECRET
2387-
if (!clientId || !clientSecret) {
2388-
throw new Error('QuickBooks OAuth client credentials are not configured')
2389-
}
2390-
2391-
const response = await fetch(QUICKBOOKS_TOKEN_URL, {
2392-
method: 'POST',
2393-
headers: {
2394-
Authorization: `Basic ${Buffer.from(`${clientId}:${clientSecret}`).toString('base64')}`,
2395-
'Content-Type': 'application/x-www-form-urlencoded',
2396-
},
2397-
body: new URLSearchParams({
2398-
code,
2399-
grant_type: 'authorization_code',
2400-
redirect_uri: redirectURI,
2401-
}),
2402-
signal: AbortSignal.timeout(QUICKBOOKS_OAUTH_REQUEST_TIMEOUT_MS),
2403-
})
2404-
if (!response.ok) {
2405-
await readResponseTextWithLimit(response, {
2406-
maxBytes: DEFAULT_MAX_ERROR_BODY_BYTES,
2407-
label: 'QuickBooks OAuth token error response',
2408-
}).catch(() => {})
2409-
throw new Error(`QuickBooks OAuth token exchange failed with HTTP ${response.status}`)
2410-
}
2411-
2412-
const data = await readResponseJsonWithLimit<Record<string, unknown>>(response, {
2413-
maxBytes: DEFAULT_MAX_ERROR_BODY_BYTES,
2414-
label: 'QuickBooks OAuth token response',
2415-
})
2416-
if (!data || typeof data !== 'object' || Array.isArray(data)) {
2417-
throw new Error('QuickBooks OAuth token exchange returned an invalid response')
2418-
}
2419-
2420-
const tokens = getOAuth2Tokens(data)
2421-
if (!tokens.accessToken || !tokens.refreshToken) {
2422-
throw new Error(
2423-
'QuickBooks OAuth token response did not include access and refresh tokens'
2424-
)
2425-
}
2426-
if (typeof data.scope === 'string') {
2427-
tokens.scopes = data.scope.split(/\s+/).filter(Boolean)
2428-
}
2429-
return tokens
2430-
},
24312383
getUserInfo: async (tokens) => {
24322384
if (!tokens.accessToken) {
24332385
throw new Error('QuickBooks OAuth did not issue an access token')

apps/sim/lib/core/security/input-validation.server.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -966,7 +966,6 @@ export async function secureFetchWithPinnedIP(
966966
options: SecureFetchOptions & { allowHttp?: boolean } = {},
967967
redirectCount = 0
968968
): Promise<SecureFetchResponse> {
969-
options.signal?.throwIfAborted()
970969
const maxRedirects = options.maxRedirects ?? DEFAULT_MAX_REDIRECTS
971970
const requestedMaxResponseBytes = options.maxResponseBytes
972971
const maxResponseBytes =
@@ -1015,7 +1014,6 @@ export async function secureFetchWithPinnedIP(
10151014

10161015
validateUrlWithDNS(redirectUrl, 'redirectUrl', { allowHttp: options.allowHttp })
10171016
.then((validation) => {
1018-
options.signal?.throwIfAborted()
10191017
if (!validation.isValid) {
10201018
settledReject(new Error(`Redirect blocked: ${validation.error}`))
10211019
return

apps/sim/lib/core/security/secure-fetch-response-cap.server.test.ts

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,15 @@ vi.mock('@/lib/core/config/env-flags', () => ({
1616
getProxyUrl: () => undefined,
1717
}))
1818

19-
import { resolveHostAddresses } from '@sim/security/dns'
2019
import {
2120
DEFAULT_MAX_RESPONSE_BYTES,
2221
secureFetchWithPinnedIP,
2322
} from '@/lib/core/security/input-validation.server'
2423

2524
const servers: http.Server[] = []
26-
const mockResolveHostAddresses = vi.mocked(resolveHostAddresses)
2725

2826
afterEach(() => {
2927
for (const server of servers.splice(0)) server.close()
30-
vi.clearAllMocks()
3128
})
3229

3330
/** Starts a throwaway loopback server and returns its origin. */
@@ -39,49 +36,6 @@ async function startServer(handler: http.RequestListener): Promise<string> {
3936
}
4037

4138
describe('secureFetchWithPinnedIP response cap', () => {
42-
it('does not open a request when the signal is already aborted', async () => {
43-
let requests = 0
44-
const origin = await startServer((_req, res) => {
45-
requests += 1
46-
res.end('unexpected')
47-
})
48-
const controller = new AbortController()
49-
controller.abort(new Error('cancelled'))
50-
51-
await expect(
52-
secureFetchWithPinnedIP(origin, '127.0.0.1', {
53-
allowHttp: true,
54-
signal: controller.signal,
55-
})
56-
).rejects.toThrow('cancelled')
57-
expect(requests).toBe(0)
58-
})
59-
60-
it('does not follow a redirect when cancellation arrives during redirect DNS validation', async () => {
61-
let targetRequests = 0
62-
const targetOrigin = await startServer((_req, res) => {
63-
targetRequests += 1
64-
res.end('unexpected')
65-
})
66-
const redirectOrigin = await startServer((_req, res) => {
67-
res.writeHead(302, { Location: targetOrigin.replace('127.0.0.1', 'localhost') })
68-
res.end()
69-
})
70-
const controller = new AbortController()
71-
mockResolveHostAddresses.mockImplementationOnce(async () => {
72-
controller.abort(new Error('cancelled during redirect DNS'))
73-
return { addresses: ['127.0.0.1'], preferred: '127.0.0.1' }
74-
})
75-
76-
await expect(
77-
secureFetchWithPinnedIP(redirectOrigin, '127.0.0.1', {
78-
allowHttp: true,
79-
signal: controller.signal,
80-
})
81-
).rejects.toThrow('cancelled during redirect DNS')
82-
expect(targetRequests).toBe(0)
83-
})
84-
8539
it('rejects a body that exceeds an explicit cap instead of buffering it', async () => {
8640
const origin = await startServer((_req, res) => {
8741
res.writeHead(200, { 'Content-Type': 'application/octet-stream' })

0 commit comments

Comments
 (0)