Skip to content

Commit ef61470

Browse files
committed
fix(quickbooks): harden OAuth returns and webhook lookup
1 parent 62beebf commit ef61470

7 files changed

Lines changed: 79 additions & 8 deletions

File tree

‎apps/sim/app/api/auth/oauth/token/route.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -825,9 +825,12 @@ describe('OAuth Token API Routes', () => {
825825
})
826826

827827
const response = await GET(
828-
new NextRequest(
828+
createMockRequest(
829+
'GET',
830+
undefined,
831+
{},
829832
'http://localhost:3000/api/auth/oauth/token?credentialId=credential-id'
830-
) as any
833+
)
831834
)
832835
const data = await response.json()
833836

‎apps/sim/app/api/auth/oauth2/callback/quickbooks/route.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,28 @@ describe('QuickBooks OAuth callback', () => {
100100
)
101101
})
102102

103+
it('returns completion failures to the signed initiating surface', async () => {
104+
mockCompleteQuickBooksConnection.mockRejectedValueOnce(new Error('Token exchange failed'))
105+
const state = createQuickBooksOAuthState({
106+
userId: 'user-1',
107+
draftId: 'draft-1',
108+
returnUrl: 'https://sim.test/desktop/connect/complete?state=handoff',
109+
})
110+
const response = await GET(
111+
callbackRequest(
112+
new URLSearchParams({
113+
code: 'authorization-code',
114+
state,
115+
realmId: '1234567890',
116+
})
117+
)
118+
)
119+
120+
expect(response.headers.get('location')).toBe(
121+
'https://sim.test/desktop/connect/complete?state=handoff&error=quickbooks_callback_error'
122+
)
123+
})
124+
103125
it('rejects signed cross-origin return destinations', async () => {
104126
const state = createQuickBooksOAuthState({
105127
userId: 'user-1',

‎apps/sim/app/api/auth/oauth2/callback/quickbooks/route.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export const dynamic = 'force-dynamic'
1717
export const GET = withRouteHandler(async (request: NextRequest) => {
1818
const baseUrl = getBaseUrl()
1919
const fallbackUrl = `${baseUrl}/workspace`
20+
let validatedReturnUrl: URL | null = null
2021

2122
try {
2223
const session = await getSession()
@@ -33,14 +34,14 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
3334
if (!isSameOrigin(state.returnUrl)) {
3435
throw new Error('QuickBooks OAuth state contains an invalid return URL')
3536
}
36-
const returnUrl = new URL(state.returnUrl)
37+
validatedReturnUrl = new URL(state.returnUrl)
3738
if (parsed.data.query.error) {
3839
logger.warn('QuickBooks OAuth authorization was not completed', {
3940
error: parsed.data.query.error,
4041
hasDescription: Boolean(parsed.data.query.error_description),
4142
})
42-
returnUrl.searchParams.set('error', 'quickbooks_access_denied')
43-
return NextResponse.redirect(returnUrl)
43+
validatedReturnUrl.searchParams.set('error', 'quickbooks_access_denied')
44+
return NextResponse.redirect(validatedReturnUrl)
4445
}
4546
if (!parsed.data.query.code || !parsed.data.query.realmId) {
4647
throw new Error('QuickBooks callback is missing its authorization code or company identity')
@@ -62,10 +63,12 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
6263
request,
6364
})
6465

65-
returnUrl.searchParams.set('quickbooks_connected', 'true')
66-
return NextResponse.redirect(returnUrl)
66+
validatedReturnUrl.searchParams.set('quickbooks_connected', 'true')
67+
return NextResponse.redirect(validatedReturnUrl)
6768
} catch (error) {
6869
logger.error('QuickBooks OAuth callback failed', { error })
69-
return NextResponse.redirect(`${fallbackUrl}?error=quickbooks_callback_error`)
70+
const errorUrl = validatedReturnUrl ?? new URL(fallbackUrl)
71+
errorUrl.searchParams.set('error', 'quickbooks_callback_error')
72+
return NextResponse.redirect(errorUrl)
7073
}
7174
})

‎apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.test.tsx‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ describe('ConnectOAuthModal reauthorization', () => {
206206
mocks.onConnect.mockResolvedValue(undefined)
207207
mocks.getServiceConfigByProviderId.mockReturnValue(null)
208208
mocks.workspaceCredentials.mockReturnValue({ data: [], isPending: false })
209+
window.history.replaceState({}, '', '/')
209210
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
210211
container = document.createElement('div')
211212
document.body.appendChild(container)
@@ -300,6 +301,23 @@ describe('ConnectOAuthModal reauthorization', () => {
300301
})
301302
})
302303

304+
it('does not carry a prior OAuth result into a new provider callback URL', async () => {
305+
window.history.replaceState(
306+
{},
307+
'',
308+
'/?keep=1&error=stale&error_description=stale-detail&quickbooks_connected=true'
309+
)
310+
renderReauthorizeModal()
311+
312+
await clickConnect()
313+
314+
expect(mocks.connectOAuthService).toHaveBeenCalledWith({
315+
providerId: 'slack',
316+
callbackURL: 'http://localhost:3000/?keep=1',
317+
draftId: undefined,
318+
})
319+
})
320+
303321
it('keeps an onConnect override ahead of credential-bound reauthorization', async () => {
304322
renderReauthorizeModal({
305323
reconnectTarget: {

‎apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,9 @@ export function ConnectOAuthModal(props: ConnectOAuthModalProps) {
439439
}
440440

441441
const callbackURL = new URL(window.location.href)
442+
callbackURL.searchParams.delete('error')
443+
callbackURL.searchParams.delete('error_description')
444+
callbackURL.searchParams.delete('quickbooks_connected')
442445
if (connectorType) {
443446
callbackURL.searchParams.set(ADD_CONNECTOR_SEARCH_PARAM, connectorType)
444447
}

‎apps/sim/lib/webhooks/quickbooks-credentials.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,22 @@ describe('QuickBooks webhook credential lookup', () => {
7979
])
8080
})
8181

82+
it('fails closed instead of loading an unbounded number of app accounts', async () => {
83+
queueTableRows(
84+
account,
85+
Array.from({ length: 1001 }, (_, index) => ({
86+
accountId: createQuickBooksAccountId(String(index + 1), `subject-${index}`, CLIENT_CONFIG),
87+
oauthConfig: 'encrypted-config',
88+
}))
89+
)
90+
91+
await expect(getQuickBooksWebhookVerifierTokensByAppKey(APP_KEY)).rejects.toThrow(
92+
'QuickBooks webhook app account limit exceeded'
93+
)
94+
expect(dbChainMockFns.limit).toHaveBeenCalledWith(1001)
95+
expect(mockDecryptSecret).not.toHaveBeenCalled()
96+
})
97+
8298
it('binds a deployed credential to the same app and company identity', async () => {
8399
queueTableRows(account, [
84100
{

‎apps/sim/lib/webhooks/quickbooks-credentials.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
} from '@/lib/oauth/quickbooks-client-config'
1717

1818
const QUICKBOOKS_ACCOUNT_PREFIX = 'quickbooks:v2:'
19+
const MAX_QUICKBOOKS_WEBHOOK_APP_ACCOUNTS = 1000
1920

2021
export interface QuickBooksWebhookCredentialContext {
2122
clientConfig: QuickBooksOAuthClientConfig
@@ -75,6 +76,11 @@ export async function getQuickBooksWebhookVerifierTokensByAppKey(
7576
like(account.accountId, buildQuickBooksWebhookAccountIdPattern(normalizedAppKey))
7677
)
7778
)
79+
.limit(MAX_QUICKBOOKS_WEBHOOK_APP_ACCOUNTS + 1)
80+
81+
if (rows.length > MAX_QUICKBOOKS_WEBHOOK_APP_ACCOUNTS) {
82+
throw new Error('QuickBooks webhook app account limit exceeded')
83+
}
7884

7985
const verifierTokens = new Set<string>()
8086
for (const row of rows) {

0 commit comments

Comments
 (0)