Skip to content

Commit d035b0e

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(slack): preserve referenced selector credentials
1 parent f068f5f commit d035b0e

4 files changed

Lines changed: 160 additions & 76 deletions

File tree

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ import { ensureCredential, SELECTOR_STALE } from '@/hooks/selectors/providers/sh
55
import type { SelectorDefinition, SelectorKey, SelectorQueryArgs } from '@/hooks/selectors/types'
66

77
function slackCredentialQueryKey(credential: string | undefined): string {
8-
if (!credential) return 'none'
9-
return credential.startsWith('xoxb-') ? 'direct-bot-token' : credential
8+
return credential ? 'credential-present' : 'none'
109
}
1110

1211
function slackCredentialHasServerSecret(credential: string | undefined): boolean {

apps/sim/hooks/selectors/providers/slack/server-resolved-context.test.ts

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -72,29 +72,30 @@ describe('Slack server-resolved selector context', () => {
7272
expect(mocks.requestJson.mock.calls[0][1].body).toEqual({ credential: 'credential-1' })
7373
})
7474

75-
it('separates direct-token cache entries without putting plaintext in keys', () => {
75+
it('separates arbitrary credentials without putting their text in keys', () => {
7676
const definition = slackSelectors['slack.channels']
77-
const firstKey = getScopedSelectorQueryKey(definition, {
78-
key: 'slack.channels',
79-
context: {
80-
workflowId: 'workflow-1',
81-
workspaceId: 'workspace-1',
82-
oauthCredential: 'xoxb-first-secret',
83-
selectorCacheScope: 'revision-1',
84-
},
85-
})
86-
const secondKey = getScopedSelectorQueryKey(definition, {
87-
key: 'slack.channels',
88-
context: {
89-
workflowId: 'workflow-1',
90-
workspaceId: 'workspace-1',
91-
oauthCredential: 'xoxb-second-secret',
92-
selectorCacheScope: 'revision-2',
93-
},
94-
})
77+
const credentials = [
78+
'credential-id',
79+
'xoxb-clean-secret',
80+
' xoxb-padded-secret',
81+
'"xoxb-quoted-secret"',
82+
'{{SLACK_SECRET_REFERENCE}}',
83+
]
84+
const keys = credentials.map((oauthCredential, index) =>
85+
getScopedSelectorQueryKey(definition, {
86+
key: 'slack.channels',
87+
context: {
88+
workflowId: 'workflow-1',
89+
workspaceId: 'workspace-1',
90+
oauthCredential,
91+
selectorCacheScope: `revision-${index}`,
92+
},
93+
})
94+
)
9595

96-
expect(firstKey).not.toEqual(secondKey)
97-
expect(JSON.stringify([firstKey, secondKey])).not.toContain('xoxb-first-secret')
98-
expect(JSON.stringify([firstKey, secondKey])).not.toContain('xoxb-second-secret')
96+
expect(new Set(keys.map((key) => JSON.stringify(key))).size).toBe(credentials.length)
97+
for (const credential of credentials) {
98+
expect(JSON.stringify(keys)).not.toContain(credential)
99+
}
99100
})
100101
})

apps/sim/lib/selectors/server/slack-credential.test.ts

Lines changed: 94 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -136,40 +136,108 @@ describe('resolveSlackSelectorCredential', () => {
136136
expect(mocks.refreshToken).not.toHaveBeenCalled()
137137
})
138138

139-
it('requires workflow scope for a direct bot token', async () => {
140-
const result = await resolveSlackSelectorCredential(principal, {
141-
credential: 'xoxb-literal-secret',
142-
requestId: 'request-1',
143-
})
139+
it.each(['xoxb-literal-secret', '{{SLACK_CREDENTIAL}}'])(
140+
'requires workflow scope before resolving %s',
141+
async (credential) => {
142+
const result = await resolveSlackSelectorCredential(principal, {
143+
credential,
144+
requestId: 'request-1',
145+
})
144146

145-
expect(result).toEqual({
146-
ok: false,
147-
status: 400,
148-
error: 'Unable to resolve selector configuration',
149-
})
150-
expect(mocks.resolveContext).not.toHaveBeenCalled()
151-
})
147+
expect(result).toEqual({
148+
ok: false,
149+
status: 400,
150+
error: 'Unable to resolve selector configuration',
151+
})
152+
expect(mocks.resolveContext).not.toHaveBeenCalled()
153+
}
154+
)
152155

153-
it('does not reinterpret a referenced non-bot value as an OAuth credential id', async () => {
154-
mocks.resolveContext.mockResolvedValue({
155-
ok: true,
156-
context: { credential: 'not-a-bot-token' },
157-
requesterUserId: 'viewer-1',
158-
workspaceId: 'workspace-1',
159-
})
156+
it.each([
157+
['OAuth', 'oauth', 'xoxp-oauth-token', false],
158+
['custom bot', 'service_account', 'xoxb-custom-bot-token', true],
159+
])(
160+
'authorizes a referenced %s credential after environment resolution',
161+
async (_label, credentialType, accessToken, isBotToken) => {
162+
mocks.resolveContext
163+
.mockResolvedValueOnce({
164+
ok: true,
165+
context: { credential: 'resolved-credential-id' },
166+
requesterUserId: 'viewer-1',
167+
workspaceId: 'workspace-1',
168+
})
169+
.mockResolvedValueOnce({
170+
ok: true,
171+
context: {},
172+
requesterUserId: 'viewer-1',
173+
workspaceId: 'workspace-1',
174+
credentialAccess: {
175+
ok: true,
176+
workspaceId: 'workspace-1',
177+
credentialOwnerUserId: 'owner-1',
178+
credentialType,
179+
},
180+
})
181+
mocks.refreshToken.mockResolvedValue(accessToken)
182+
183+
const result = await resolveSlackSelectorCredential(principal, {
184+
credential: '{{SLACK_CREDENTIAL_ID}}',
185+
workflowId: 'workflow-1',
186+
requestId: 'request-1',
187+
})
188+
189+
expect(mocks.resolveContext).toHaveBeenNthCalledWith(1, principal, {
190+
workflowId: 'workflow-1',
191+
context: { credential: '{{SLACK_CREDENTIAL_ID}}' },
192+
})
193+
expect(mocks.resolveContext).toHaveBeenNthCalledWith(2, principal, {
194+
workflowId: 'workflow-1',
195+
credentialId: 'resolved-credential-id',
196+
context: {},
197+
})
198+
expect(mocks.providerMatches).toHaveBeenCalledWith({
199+
credentialId: 'resolved-credential-id',
200+
credentialOwnerUserId: 'owner-1',
201+
serviceId: 'slack',
202+
})
203+
expect(mocks.refreshToken).toHaveBeenCalledWith(
204+
'resolved-credential-id',
205+
'owner-1',
206+
'request-1'
207+
)
208+
expect(result).toMatchObject({ ok: true, accessToken, isBotToken })
209+
}
210+
)
211+
212+
it('provider-binds a referenced credential before token refresh', async () => {
213+
mocks.resolveContext
214+
.mockResolvedValueOnce({
215+
ok: true,
216+
context: { credential: 'resolved-credential-id' },
217+
requesterUserId: 'viewer-1',
218+
workspaceId: 'workspace-1',
219+
})
220+
.mockResolvedValueOnce({
221+
ok: true,
222+
context: {},
223+
requesterUserId: 'viewer-1',
224+
workspaceId: 'workspace-1',
225+
credentialAccess: {
226+
ok: true,
227+
workspaceId: 'workspace-1',
228+
credentialOwnerUserId: 'owner-1',
229+
credentialType: 'oauth',
230+
},
231+
})
232+
mocks.providerMatches.mockResolvedValue(false)
160233

161234
const result = await resolveSlackSelectorCredential(principal, {
162-
credential: '{{SLACK_BOT_TOKEN}}',
235+
credential: '{{SLACK_CREDENTIAL_ID}}',
163236
workflowId: 'workflow-1',
164237
requestId: 'request-1',
165238
})
166239

167-
expect(result).toEqual({
168-
ok: false,
169-
status: 400,
170-
error: 'Unable to resolve selector configuration',
171-
})
172-
expect(mocks.providerMatches).not.toHaveBeenCalled()
240+
expect(result).toEqual({ ok: false, status: 400, error: 'Select a Slack credential.' })
173241
expect(mocks.refreshToken).not.toHaveBeenCalled()
174242
})
175243

apps/sim/lib/selectors/server/slack-credential.ts

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,35 +16,13 @@ export type SlackSelectorCredentialResult =
1616
}
1717
| { ok: false; status: number; error: string }
1818

19-
/** Resolves direct Slack bot-token references and credential ids behind one authorization path. */
20-
export async function resolveSlackSelectorCredential(
19+
async function resolveStoredSlackCredential(
2120
principal: Principal,
2221
input: { credential: string; workflowId?: string; requestId: string }
2322
): Promise<SlackSelectorCredentialResult> {
24-
let credential = input.credential
25-
const isReferencedBotToken = isEnvVarReference(credential)
26-
const isLiteralBotToken = credential.startsWith('xoxb-')
27-
28-
if (isReferencedBotToken || isLiteralBotToken) {
29-
if (!input.workflowId) {
30-
return { ok: false, status: 400, error: SAFE_RESOLUTION_ERROR }
31-
}
32-
const resolution = await resolveAuthorizedSelectorContext(principal, {
33-
workflowId: input.workflowId,
34-
context: { credential },
35-
})
36-
if (!resolution.ok) return resolution
37-
credential = resolution.context.credential
38-
39-
if (credential.startsWith('xoxb-')) {
40-
return { ok: true, accessToken: credential, isBotToken: true }
41-
}
42-
return { ok: false, status: 400, error: SAFE_RESOLUTION_ERROR }
43-
}
44-
4523
const resolution = await resolveAuthorizedSelectorContext(principal, {
4624
workflowId: input.workflowId,
47-
credentialId: credential,
25+
credentialId: input.credential,
4826
context: {},
4927
})
5028
if (!resolution.ok) return resolution
@@ -55,7 +33,7 @@ export async function resolveSlackSelectorCredential(
5533
}
5634

5735
const providerMatches = await selectorCredentialMatchesService({
58-
credentialId: credential,
36+
credentialId: input.credential,
5937
credentialOwnerUserId: credentialAccess.credentialOwnerUserId,
6038
serviceId: 'slack',
6139
})
@@ -64,7 +42,7 @@ export async function resolveSlackSelectorCredential(
6442
}
6543

6644
const accessToken = await refreshAccessTokenIfNeeded(
67-
credential,
45+
input.credential,
6846
credentialAccess.credentialOwnerUserId,
6947
input.requestId
7048
)
@@ -79,3 +57,41 @@ export async function resolveSlackSelectorCredential(
7957
credentialAccess,
8058
}
8159
}
60+
61+
/** Resolves Slack references before selecting the direct-token or stored-credential path. */
62+
export async function resolveSlackSelectorCredential(
63+
principal: Principal,
64+
input: { credential: string; workflowId?: string; requestId: string }
65+
): Promise<SlackSelectorCredentialResult> {
66+
let credential = input.credential
67+
const isReferencedCredential = isEnvVarReference(credential)
68+
const isLiteralBotToken = credential.startsWith('xoxb-')
69+
70+
if (isReferencedCredential || isLiteralBotToken) {
71+
if (!input.workflowId) {
72+
return { ok: false, status: 400, error: SAFE_RESOLUTION_ERROR }
73+
}
74+
const resolution = await resolveAuthorizedSelectorContext(principal, {
75+
workflowId: input.workflowId,
76+
context: { credential },
77+
})
78+
if (!resolution.ok) return resolution
79+
credential = resolution.context.credential
80+
81+
if (credential.startsWith('xoxb-')) {
82+
return { ok: true, accessToken: credential, isBotToken: true }
83+
}
84+
85+
return resolveStoredSlackCredential(principal, {
86+
credential,
87+
workflowId: input.workflowId,
88+
requestId: input.requestId,
89+
})
90+
}
91+
92+
return resolveStoredSlackCredential(principal, {
93+
credential,
94+
workflowId: input.workflowId,
95+
requestId: input.requestId,
96+
})
97+
}

0 commit comments

Comments
 (0)