Skip to content

Commit e268ddd

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(imap): resolve mailbox selector contexts server-side
1 parent 0486f57 commit e268ddd

7 files changed

Lines changed: 393 additions & 52 deletions

File tree

apps/sim/app/api/tools/imap/mailboxes/route.ts

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,25 @@ import { createLogger } from '@sim/logger'
22
import { getErrorMessage } from '@sim/utils/errors'
33
import { ImapFlow } from 'imapflow'
44
import { type NextRequest, NextResponse } from 'next/server'
5-
import { imapMailboxesContract } from '@/lib/api/contracts/tools/imap'
5+
import {
6+
imapMailboxesContract,
7+
resolvedImapMailboxesBodySchema,
8+
} from '@/lib/api/contracts/tools/imap'
69
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
7-
import { getSession } from '@/lib/auth'
810
import { validateDatabaseHost } from '@/lib/core/security/input-validation.server'
911
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
12+
import {
13+
authenticateSelectorRequest,
14+
resolveAuthorizedSelectorContext,
15+
} from '@/lib/selectors/server/resolve-authorized-context'
1016

1117
const logger = createLogger('ImapMailboxesAPI')
1218

1319
export const POST = withRouteHandler(async (request: NextRequest) => {
14-
const session = await getSession()
15-
if (!session?.user?.id) {
16-
return NextResponse.json({ success: false, message: 'Unauthorized' }, { status: 401 })
20+
const authentication = await authenticateSelectorRequest(request)
21+
if (!authentication.ok) {
22+
return NextResponse.json({ error: authentication.error }, { status: authentication.status })
1723
}
18-
1924
const parsed = await parseRequest(
2025
imapMailboxesContract,
2126
request,
@@ -40,10 +45,29 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4045
}
4146
)
4247
if (!parsed.success) return parsed.response
43-
const { host, port, secure, username, password } = parsed.data.body
48+
const { workflowId, ...context } = parsed.data.body
49+
50+
const resolution = await resolveAuthorizedSelectorContext(authentication.principal, {
51+
workflowId,
52+
context,
53+
})
54+
if (!resolution.ok) {
55+
return NextResponse.json(
56+
{ success: false, message: resolution.error },
57+
{ status: resolution.status }
58+
)
59+
}
60+
const validated = resolvedImapMailboxesBodySchema.safeParse(resolution.context)
61+
if (!validated.success) {
62+
return NextResponse.json(
63+
{ success: false, message: 'Invalid IMAP selector configuration' },
64+
{ status: 400 }
65+
)
66+
}
67+
const { host, port, secure, username, password } = validated.data
4468

4569
try {
46-
const hostValidation = await validateDatabaseHost(host, 'host')
70+
const hostValidation = await validateDatabaseHost(host, 'host', { logFailureDetails: false })
4771
if (!hostValidation.isValid) {
4872
return NextResponse.json({ success: false, message: hostValidation.error }, { status: 400 })
4973
}
@@ -94,17 +118,18 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
94118
throw error
95119
}
96120
} catch (error) {
97-
const errorMessage = getErrorMessage(error, 'Unknown error')
98-
logger.error('Error fetching IMAP mailboxes:', errorMessage)
99-
100-
let userMessage = 'Failed to connect to IMAP server. Please check your connection settings.'
101-
if (
102-
errorMessage.includes('AUTHENTICATIONFAILED') ||
103-
errorMessage.includes('Invalid credentials')
104-
) {
105-
userMessage = 'Invalid username or password. For Gmail, use an App Password.'
106-
}
107-
108-
return NextResponse.json({ success: false, message: userMessage }, { status: 500 })
121+
logger.error('Error fetching IMAP mailboxes')
122+
const errorMessage = getErrorMessage(error)
123+
const userMessage =
124+
errorMessage.includes('AUTHENTICATIONFAILED') || errorMessage.includes('Invalid credentials')
125+
? 'Invalid username or password. For Gmail, use an App Password.'
126+
: 'Failed to connect to IMAP server. Please check your connection settings.'
127+
return NextResponse.json(
128+
{
129+
success: false,
130+
message: userMessage,
131+
},
132+
{ status: 500 }
133+
)
109134
}
110135
})
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { createMockRequest } from '@sim/testing'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const mocks = vi.hoisted(() => ({
8+
authenticate: vi.fn(),
9+
resolveContext: vi.fn(),
10+
validateDatabaseHost: vi.fn(),
11+
imapConstruct: vi.fn(),
12+
imapConnect: vi.fn(),
13+
imapList: vi.fn(),
14+
imapLogout: vi.fn(),
15+
}))
16+
17+
vi.mock('@/lib/selectors/server/resolve-authorized-context', () => ({
18+
authenticateSelectorRequest: mocks.authenticate,
19+
resolveAuthorizedSelectorContext: mocks.resolveContext,
20+
}))
21+
22+
vi.mock('@/lib/core/security/input-validation.server', () => ({
23+
validateDatabaseHost: mocks.validateDatabaseHost,
24+
}))
25+
26+
vi.mock('imapflow', () => ({
27+
ImapFlow: class MockImapFlow {
28+
constructor(config: unknown) {
29+
mocks.imapConstruct(config)
30+
}
31+
32+
connect = mocks.imapConnect
33+
list = mocks.imapList
34+
logout = mocks.imapLogout
35+
},
36+
}))
37+
38+
import { POST as listMailboxes } from '@/app/api/tools/imap/mailboxes/route'
39+
40+
function request(body: unknown) {
41+
return createMockRequest('POST', body, {}, 'http://localhost:3000/api/tools/imap/mailboxes')
42+
}
43+
44+
const wireBody = {
45+
workflowId: 'workflow-1',
46+
host: '{{IMAP_HOST}}',
47+
port: '{{IMAP_PORT}}',
48+
secure: '{{IMAP_TLS}}',
49+
username: '{{IMAP_USERNAME}}',
50+
password: '{{IMAP_PASSWORD}}',
51+
}
52+
53+
describe('server-resolved IMAP mailbox selector', () => {
54+
beforeEach(() => {
55+
vi.clearAllMocks()
56+
mocks.authenticate.mockResolvedValue({
57+
ok: true,
58+
principal: { kind: 'session', userId: 'viewer-1', sessionId: 'session-1' },
59+
})
60+
mocks.resolveContext.mockResolvedValue({
61+
ok: true,
62+
context: {
63+
host: 'imap.example.com',
64+
port: '993',
65+
secure: 'true',
66+
username: 'mailbox@example.com',
67+
password: 'resolved-password',
68+
},
69+
requesterUserId: 'viewer-1',
70+
workspaceId: 'workspace-1',
71+
})
72+
mocks.validateDatabaseHost.mockResolvedValue({
73+
isValid: true,
74+
resolvedIP: '203.0.113.10',
75+
})
76+
mocks.imapConnect.mockResolvedValue(undefined)
77+
mocks.imapList.mockResolvedValue([
78+
{ path: 'Archive', name: 'Archive', delimiter: '/' },
79+
{ path: 'INBOX', name: 'INBOX', delimiter: '/' },
80+
])
81+
mocks.imapLogout.mockResolvedValue(undefined)
82+
})
83+
84+
it('authenticates before parsing malformed requests', async () => {
85+
mocks.authenticate.mockResolvedValue({ ok: false, status: 401, error: 'Unauthorized' })
86+
87+
const response = await listMailboxes(request({}))
88+
89+
expect(response.status).toBe(401)
90+
expect(mocks.resolveContext).not.toHaveBeenCalled()
91+
})
92+
93+
it('passes every raw connection field to the resolver and maps sorted mailboxes', async () => {
94+
const response = await listMailboxes(request(wireBody))
95+
96+
expect(await response.json()).toEqual({
97+
success: true,
98+
mailboxes: [
99+
{ path: 'INBOX', name: 'INBOX', delimiter: '/' },
100+
{ path: 'Archive', name: 'Archive', delimiter: '/' },
101+
],
102+
})
103+
expect(mocks.resolveContext).toHaveBeenCalledWith(expect.anything(), {
104+
workflowId: 'workflow-1',
105+
context: {
106+
host: '{{IMAP_HOST}}',
107+
port: '{{IMAP_PORT}}',
108+
secure: '{{IMAP_TLS}}',
109+
username: '{{IMAP_USERNAME}}',
110+
password: '{{IMAP_PASSWORD}}',
111+
},
112+
})
113+
expect(mocks.validateDatabaseHost).toHaveBeenCalledWith('imap.example.com', 'host', {
114+
logFailureDetails: false,
115+
})
116+
expect(mocks.imapConstruct).toHaveBeenCalledWith(
117+
expect.objectContaining({
118+
host: '203.0.113.10',
119+
servername: 'imap.example.com',
120+
port: 993,
121+
secure: true,
122+
auth: { user: 'mailbox@example.com', pass: 'resolved-password' },
123+
})
124+
)
125+
expect(mocks.imapLogout).toHaveBeenCalledOnce()
126+
})
127+
128+
it('short-circuits inaccessible references before DNS or provider access', async () => {
129+
mocks.resolveContext.mockResolvedValue({
130+
ok: false,
131+
status: 400,
132+
error: 'Unable to resolve selector configuration',
133+
})
134+
135+
const response = await listMailboxes(request(wireBody))
136+
137+
expect(response.status).toBe(400)
138+
expect(await response.json()).toEqual({
139+
success: false,
140+
message: 'Unable to resolve selector configuration',
141+
})
142+
expect(mocks.validateDatabaseHost).not.toHaveBeenCalled()
143+
expect(mocks.imapConstruct).not.toHaveBeenCalled()
144+
})
145+
146+
it('rejects invalid resolved port/TLS values before DNS or provider access', async () => {
147+
mocks.resolveContext.mockResolvedValue({
148+
ok: true,
149+
context: {
150+
host: 'imap.example.com',
151+
port: 'invalid',
152+
secure: 'invalid',
153+
username: 'mailbox@example.com',
154+
password: 'resolved-password',
155+
},
156+
requesterUserId: 'viewer-1',
157+
workspaceId: 'workspace-1',
158+
})
159+
160+
const response = await listMailboxes(request(wireBody))
161+
162+
expect(response.status).toBe(400)
163+
expect(mocks.validateDatabaseHost).not.toHaveBeenCalled()
164+
expect(mocks.imapConstruct).not.toHaveBeenCalled()
165+
})
166+
167+
it('sanitizes provider connection failures and logs out best-effort', async () => {
168+
mocks.imapConnect.mockRejectedValue(
169+
new Error('connection failed with resolved-password and imap.example.com')
170+
)
171+
172+
const response = await listMailboxes(request(wireBody))
173+
const body = await response.json()
174+
175+
expect(response.status).toBe(500)
176+
expect(body).toEqual({
177+
success: false,
178+
message: 'Failed to connect to IMAP server. Please check your connection settings.',
179+
})
180+
expect(mocks.imapLogout).toHaveBeenCalledOnce()
181+
})
182+
})

apps/sim/hooks/selectors/providers/imap/selectors.ts

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,34 +9,23 @@ export const imapSelectors = {
99
* not a stored credential the server can resolve by id — the user types the connection in,
1010
* so the parameters travel on the context.
1111
*
12-
* **The password is deliberately absent from the query key.** A query key identifies a
13-
* resource; a credential authorizes access to it. `oauthCredential` is safe in a key because
14-
* it is only an id, but a typed password is a secret, and React Query keys are held in cache
15-
* and surfaced by devtools. Host, port, TLS and username already identify the mailbox list
16-
* uniquely — the password only proves the caller may read it, and it rides the request body
17-
* exactly as it did before.
18-
*
19-
* The consequence is intentional: correcting a wrong password re-runs the request (the
20-
* previous attempt failed and cached nothing), while changing ONLY the password on an
21-
* otherwise identical connection reuses the cached list, which is the same list.
12+
* **Connection fields are deliberately absent from the query key.** React Query keys are
13+
* retained in browser memory and surfaced by devtools, so server-resolved dependencies are
14+
* represented by the shared opaque revision instead of their literal or referenced values.
2215
*/
2316
'imap.mailboxes': {
2417
key: 'imap.mailboxes',
2518
contracts: [imapMailboxesContract],
19+
serverResolvedContextFields: ['host', 'port', 'secure', 'username', 'password'],
2620
staleTime: SELECTOR_STALE,
27-
getQueryKey: ({ context }: SelectorQueryArgs) => [
28-
'selectors',
29-
'imap.mailboxes',
30-
context.host ?? 'none',
31-
context.port ?? 'default',
32-
context.secure ?? 'default',
33-
context.username ?? 'none',
34-
],
35-
enabled: ({ context }) => Boolean(context.host && context.username && context.password),
21+
getQueryKey: () => ['selectors', 'imap.mailboxes'],
22+
enabled: ({ context }) =>
23+
Boolean(context.host && context.username && context.password && context.workflowId),
3624
fetchList: async ({ context, signal }: SelectorQueryArgs) => {
3725
if (!context.host || !context.username || !context.password) return []
3826
const data = await requestJson(imapMailboxesContract, {
3927
body: {
28+
workflowId: context.workflowId!,
4029
host: context.host,
4130
port: context.port,
4231
secure: context.secure,
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const mocks = vi.hoisted(() => ({ requestJson: vi.fn() }))
7+
8+
vi.mock('@/lib/api/client/request', () => ({ requestJson: mocks.requestJson }))
9+
10+
import { imapSelectors } from '@/hooks/selectors/providers/imap/selectors'
11+
12+
describe('IMAP server-resolved selector context', () => {
13+
beforeEach(() => vi.clearAllMocks())
14+
15+
it('opts every connection field into server resolution', () => {
16+
expect(imapSelectors['imap.mailboxes']?.serverResolvedContextFields).toEqual([
17+
'host',
18+
'port',
19+
'secure',
20+
'username',
21+
'password',
22+
])
23+
})
24+
25+
it('forwards raw references and maps mailbox options', async () => {
26+
const definition = imapSelectors['imap.mailboxes']!
27+
const context = {
28+
workflowId: 'workflow-1',
29+
host: '{{IMAP_HOST}}',
30+
port: '{{IMAP_PORT}}',
31+
secure: '{{IMAP_TLS}}',
32+
username: '{{IMAP_USERNAME}}',
33+
password: '{{IMAP_PASSWORD}}',
34+
}
35+
mocks.requestJson.mockResolvedValue({
36+
success: true,
37+
mailboxes: [{ path: 'INBOX', name: 'Inbox', delimiter: '/' }],
38+
})
39+
40+
expect(await definition.fetchList!({ key: 'imap.mailboxes', context })).toEqual([
41+
{ id: 'INBOX', label: 'Inbox' },
42+
])
43+
expect(mocks.requestJson.mock.calls[0][1].body).toEqual(context)
44+
})
45+
46+
it('requires a workflow and keeps connection plaintext out of base query keys', () => {
47+
const definition = imapSelectors['imap.mailboxes']!
48+
const context = {
49+
workspaceId: 'workspace-1',
50+
workflowId: 'workflow-1',
51+
host: 'private-imap.example.com',
52+
port: '993',
53+
secure: 'true',
54+
username: 'private-user@example.com',
55+
password: 'imap-literal-secret',
56+
}
57+
const key = definition.getQueryKey!({ key: 'imap.mailboxes', context })
58+
59+
expect(definition.enabled?.({ key: 'imap.mailboxes', context })).toBe(true)
60+
expect(
61+
definition.enabled?.({
62+
key: 'imap.mailboxes',
63+
context: { ...context, workflowId: undefined },
64+
})
65+
).toBe(false)
66+
for (const secret of [
67+
'private-imap.example.com',
68+
'private-user@example.com',
69+
'imap-literal-secret',
70+
]) {
71+
expect(JSON.stringify(key)).not.toContain(secret)
72+
}
73+
})
74+
})

0 commit comments

Comments
 (0)