Skip to content

Commit cbab86c

Browse files
committed
fix(search): close Slack and desktop source-connect review gaps
- Forward the abort signal on the browser Slack Search OAuth start. - Report a signed-out managed Slack callback as signin_required so the completion page and desktop handoff show sign-in recovery. - Cap desktop source-connect bodies at 64 KiB at the parser.
1 parent 55d877b commit cbab86c

6 files changed

Lines changed: 122 additions & 2 deletions

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { authMockFns } from '@sim/testing'
2+
import { NextRequest } from 'next/server'
3+
import { describe, expect, it } from 'vitest'
4+
import { GET } from '@/app/api/credential-groups/slack-managed-users/callback/route'
5+
6+
describe('GET /api/credential-groups/slack-managed-users/callback', () => {
7+
it('sends a signed-out browser to the sign-in recovery screen', async () => {
8+
authMockFns.mockGetSession.mockResolvedValueOnce(null)
9+
const response = await GET(
10+
new NextRequest(
11+
'http://localhost/api/credential-groups/slack-managed-users/callback?state=s1&code=c1'
12+
),
13+
{}
14+
)
15+
expect(response.status).toBe(303)
16+
const location = new URL(response.headers.get('location') ?? '')
17+
expect(location.pathname).toBe('/credential-groups/slack-complete')
18+
expect(location.searchParams.get('reason')).toBe('signin_required')
19+
})
20+
})

‎apps/sim/app/api/credential-groups/slack-managed-users/callback/route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
4141
ok: false,
4242
message: 'Sign in to Sim to complete this Slack setup.',
4343
state: rawState,
44-
reason: 'unauthenticated',
44+
reason: 'signin_required',
4545
})
4646
}
4747
const parsed = await parseRequest(slackCredentialGroupConfigurationCallbackContract, request, {})
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { authMockFns } from '@sim/testing'
2+
import { NextRequest } from 'next/server'
3+
import { beforeEach, describe, expect, it } from 'vitest'
4+
import { POST } from '@/app/api/desktop/source-connect/route'
5+
6+
describe('POST /api/desktop/source-connect', () => {
7+
beforeEach(() => {
8+
authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'u1' }, session: { id: 's1' } })
9+
})
10+
11+
it('rejects a body far beyond the ticket limit at the parser', async () => {
12+
const body = JSON.stringify({
13+
requestId: 'a'.repeat(32),
14+
request: { kind: 'reconnect-account', credentialId: 'c'.repeat(128) },
15+
padding: 'x'.repeat(256 * 1024),
16+
})
17+
const response = await POST(
18+
new NextRequest('http://localhost/api/desktop/source-connect', {
19+
method: 'POST',
20+
headers: { 'content-type': 'application/json' },
21+
body,
22+
}),
23+
{}
24+
)
25+
expect(response.status).toBe(413)
26+
})
27+
})

‎apps/sim/app/api/desktop/source-connect/route.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export const POST = defineInternalJsonRoute({
1313
operation: createDesktopSourceRequest.operation,
1414
rateLimit: internalRateLimits.user({ bucketName: 'desktop-source-connect' }),
1515
errorPolicy: internalOrchestrationErrorPolicy,
16+
parseOptions: { maxBodyBytes: 64 * 1024 },
1617
mapInput: ({ body }) => ({ requestId: body.requestId, payload: JSON.stringify(body.request) }),
1718
useCase: createDesktopSourceRequest,
1819
staticResponseHeaders: { 'Cache-Control': 'no-store' },
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/** @vitest-environment jsdom */
2+
3+
import { act } from 'react'
4+
import {
5+
apiClientRequestMock,
6+
apiClientRequestMockFns,
7+
} from '@sim/testing/mocks/api-client-request.mock'
8+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
9+
import { createRoot, type Root } from 'react-dom/client'
10+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
11+
12+
vi.mock('@/lib/api/client/request', () => apiClientRequestMock)
13+
14+
import { useStartSlackSearchOAuth } from '@/hooks/queries/slack-search'
15+
16+
const mockRequestJson = apiClientRequestMockFns.mockRequestJson
17+
18+
describe('useStartSlackSearchOAuth', () => {
19+
let root: Root
20+
let client: QueryClient
21+
let result: ReturnType<typeof useStartSlackSearchOAuth>
22+
23+
function Probe() {
24+
result = useStartSlackSearchOAuth()
25+
return null
26+
}
27+
28+
beforeEach(async () => {
29+
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true)
30+
mockRequestJson.mockReset()
31+
client = new QueryClient()
32+
root = createRoot(document.createElement('div'))
33+
await act(async () =>
34+
root.render(
35+
<QueryClientProvider client={client}>
36+
<Probe />
37+
</QueryClientProvider>
38+
)
39+
)
40+
})
41+
42+
afterEach(async () => {
43+
await act(async () => root.unmount())
44+
client.clear()
45+
})
46+
47+
it('cancels an in-flight browser OAuth start when its signal aborts', async () => {
48+
mockRequestJson.mockImplementation(
49+
(_contract: unknown, input: { signal?: AbortSignal }) =>
50+
new Promise((_resolve, reject) =>
51+
input.signal?.addEventListener('abort', () => reject(input.signal?.reason))
52+
)
53+
)
54+
const controller = new AbortController()
55+
let outcome: 'pending' | 'rejected' = 'pending'
56+
await act(async () => {
57+
void result
58+
.mutateAsync({
59+
organizationId: 'org-1',
60+
name: 'Sim Search',
61+
description: 'Search Slack',
62+
mode: 'shared',
63+
signal: controller.signal,
64+
})
65+
.catch(() => {
66+
outcome = 'rejected'
67+
})
68+
})
69+
await act(async () => controller.abort())
70+
expect(outcome).toBe('rejected')
71+
})
72+
})

‎apps/sim/hooks/queries/slack-search.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export function useStartSlackSearchOAuth() {
4848
await connectDesktopSource({ kind: 'slack-search', body }, signal)
4949
return null
5050
}
51-
return requestJson(startSlackSearchOAuthContract, { body })
51+
return requestJson(startSlackSearchOAuthContract, { body, signal })
5252
},
5353
onSettled: (_data, _error, input) =>
5454
Promise.all([

0 commit comments

Comments
 (0)