Skip to content

Commit 1dc6f18

Browse files
authored
fix(slack): serialize member Slack setup with bot installs and explain blocked setup (#8986)
* fix(slack): serialize member Slack setup with bot installs and explain blocked setup * fix(slack): snapshot the installation set for member setup, lock order, desktop fixture * fix(slack): snapshot installations before choosing the member setup app
1 parent 3170714 commit 1dc6f18

14 files changed

Lines changed: 283 additions & 38 deletions

File tree

‎apps/desktop/e2e/source-connect.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,7 @@ test('source authorization returns to its desktop screen and refreshes live', as
336336
if (path === '/api/knowledge/slack') {
337337
json({
338338
sharedAppAvailable: true,
339+
memberAppKind: installed ? 'shared' : null,
339340
bots: [],
340341
installations: installed
341342
? [

‎apps/sim/app/api/knowledge/slack/route.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ export const GET = defineInternalJsonRoute({
2222
errorPolicy: internalOrchestrationErrorPolicy,
2323
mapInput: ({ query }) => query,
2424
useCase: listSlackSearchInstallations,
25-
present: ({ installations, bots, sharedAppAvailable }) => ({
25+
present: ({ installations, bots, sharedAppAvailable, memberAppKind }) => ({
2626
sharedAppAvailable,
27+
memberAppKind,
2728
bots,
2829
installations: installations.map((row) => ({
2930
...row,

‎apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
import { useSearchIntegrations } from '@/hooks/queries/search-integrations'
3737
import { useSlackSearchInstallations } from '@/hooks/queries/slack-search'
3838
import { credentialGroupKeys } from '@/hooks/queries/utils/credential-group-queries'
39+
import { slackSearchKeys } from '@/hooks/queries/utils/slack-search-keys'
3940

4041
const CHANNEL_NAME = 'slack-managed-users'
4142
const AUTHORIZATION_TIMEOUT_MS = 10 * 60 * 1000
@@ -71,6 +72,15 @@ export function getSlackManagedUsersFailureNotification(reason?: string): {
7172
: { message: 'Slack app verification failed. Please try again.', variant: 'error' }
7273
}
7374

75+
/** `optionActive` is undefined until the organization has a Slack sign-in option. */
76+
function memberAccountsMessage(previousAppSetup: boolean, optionActive: boolean | undefined) {
77+
if (previousAppSetup)
78+
return 'Remove app setup on the Slack source to clear the previous Slack app first.'
79+
if (optionActive === undefined) return 'Verify member permissions in your Slack workspace.'
80+
if (!optionActive) return 'Enable Slack in Credential Groups to verify member permissions.'
81+
return 'Verify member permissions. Members will need to reconnect if their app or permissions change.'
82+
}
83+
7484
function isSlackManagedUsersMessage(value: unknown): value is SlackManagedUsersMessage {
7585
if (!value || typeof value !== 'object') return false
7686
const message = value as Record<string, unknown>
@@ -112,6 +122,7 @@ export function SlackManagedUsersModal({
112122
const noInstalledApp = organizationSetup && apps.isSuccess && availableApps.length === 0
113123
/** The shared app connects member accounts without a Search bot installation. */
114124
const sharedAppWithoutBot = noInstalledApp && Boolean(apps.data?.sharedAppAvailable)
125+
const previousAppSetup = sharedAppWithoutBot && apps.data?.memberAppKind === 'custom'
115126
const sharedSetup = sharedAppInstalled || sharedAppWithoutBot
116127
const searchPolicies = useSearchIntegrations(organizationId ?? '', {
117128
enabled: open && sharedSetup,
@@ -143,6 +154,7 @@ export function SlackManagedUsersModal({
143154
)
144155
const memberOnlyCanAuthorize =
145156
sharedAppWithoutBot &&
157+
!previousAppSetup &&
146158
!apps.isFetching &&
147159
memberSetupSettled &&
148160
(!memberOption || memberOption.status === 'active')
@@ -245,6 +257,7 @@ export function SlackManagedUsersModal({
245257
void queryClient.invalidateQueries({
246258
queryKey: organizationAccountsKeys.detail(scope.organizationId),
247259
})
260+
void queryClient.invalidateQueries({ queryKey: slackSearchKeys.list(scope.organizationId) })
248261
} else {
249262
void queryClient.invalidateQueries({
250263
queryKey: credentialGroupKeys.workspace(scope.workspaceId),
@@ -346,9 +359,14 @@ export function SlackManagedUsersModal({
346359
)
347360
controller.signal.throwIfAborted()
348361
if (scope.kind === 'organization')
349-
await queryClient.invalidateQueries({
350-
queryKey: organizationAccountsKeys.detail(scope.organizationId),
351-
})
362+
await Promise.all([
363+
queryClient.invalidateQueries({
364+
queryKey: organizationAccountsKeys.detail(scope.organizationId),
365+
}),
366+
queryClient.invalidateQueries({
367+
queryKey: slackSearchKeys.list(scope.organizationId),
368+
}),
369+
])
352370
else await queryClient.invalidateQueries({ queryKey: credentialGroupKeys.all })
353371
controller.signal.throwIfAborted()
354372
toast.success('Slack configured')
@@ -473,9 +491,10 @@ export function SlackManagedUsersModal({
473491
) : sharedAppWithoutBot ? (
474492
<ChipModalField type='custom' title='Member accounts'>
475493
<p className='text-[var(--text-secondary)] text-sm'>
476-
{memberOption
477-
? 'Verify member permissions. Members will need to reconnect if their app or permissions change.'
478-
: 'Verify member permissions in your Slack workspace.'}
494+
{memberAccountsMessage(
495+
previousAppSetup,
496+
memberOption && memberOption.status === 'active'
497+
)}
479498
</p>
480499
</ChipModalField>
481500
) : needsApp ? (

‎apps/sim/hooks/queries/organization-accounts.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ export function useUpdateOrganizationAccounts() {
241241
queryClient.invalidateQueries({
242242
queryKey: slackSearchKeys.organizationManifests(organizationId),
243243
}),
244+
queryClient.invalidateQueries({ queryKey: slackSearchKeys.list(organizationId) }),
244245
]),
245246
})
246247
}

‎apps/sim/lib/api/contracts/knowledge/slack.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ export const slackSearchInstallationSchema = z.object({
1818
})
1919
export const listSlackSearchResponseSchema = z.object({
2020
sharedAppAvailable: z.boolean(),
21+
/** The app the organization's Slack member sign-in uses, when configured. */
22+
memberAppKind: z.enum(['custom', 'shared']).nullable(),
2123
installations: z.array(slackSearchInstallationSchema).max(100),
2224
bots: z
2325
.array(z.object({ id: z.string().min(1).max(200), displayName: z.string().max(500) }))

‎apps/sim/lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts‎

Lines changed: 173 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ import {
2121
import { readTestRedisUrl } from '@sim/db/testing/test-infrastructure'
2222
import { sha256Hex } from '@sim/security/hash'
2323
import { createSessionPrincipal } from '@sim/testing/factories/principal.factory'
24+
import { createDeferred } from '@sim/testing/helpers/deferred'
2425
import { generateId } from '@sim/utils/id'
25-
import { eq, inArray } from 'drizzle-orm'
26+
import { eq, inArray, sql } from 'drizzle-orm'
2627
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
2728
import { env } from '@/lib/core/config/env'
2829
import { closeRedisConnection } from '@/lib/core/config/redis'
@@ -31,13 +32,21 @@ import {
3132
completeSlackCredentialGroupConfiguration,
3233
startSlackCredentialGroupConfiguration,
3334
} from '@/lib/credential-groups/application/slack-managed-users'
34-
import { decryptCredentialGroupProviderConfiguration } from '@/lib/credential-groups/provider-configuration'
35+
import { lockOrganizationSlackApp } from '@/lib/credential-groups/organization-slack-app'
36+
import { credentialGroupScopePolicyVersion } from '@/lib/credential-groups/provider-adapter'
37+
import {
38+
decryptCredentialGroupProviderConfiguration,
39+
emptyCredentialGroupProviderConfiguration,
40+
encryptCredentialGroupProviderConfiguration,
41+
} from '@/lib/credential-groups/provider-configuration'
3542
import { getCredentialGroup } from '@/lib/credential-groups/service'
3643
import { SLACK_SEARCH_USER_SCOPES } from '@/lib/credential-groups/slack-managed-user-scopes'
3744
import { slackCredentialGroupProviderAdapter } from '@/lib/credential-groups/slack-provider'
3845
import { createOrganizationAccountsGroup } from '@/lib/credential-groups/workspace-accounts'
3946
import { encryptManagedOAuthTokenSet } from '@/lib/credentials/managed-oauth'
47+
import type { DbOrTx } from '@/lib/db/types'
4048
import { approveSearchIntegration } from '@/lib/knowledge/application/search-integrations'
49+
import { listSlackSearchInstallations } from '@/lib/knowledge/application/slack-search/installations'
4150
import { revokeSlackSearchAccess } from '@/lib/knowledge/application/slack-search/lifecycle'
4251
import { defaultLiveSearchPolicy } from '@/lib/sim-search/live/policy-schema'
4352
import { getSharedSlackSearchAppConfiguration } from '@/lib/slack-search/shared-app-env'
@@ -48,6 +57,7 @@ vi.mock('@/lib/core/config/env-flags', async (importOriginal) => ({
4857
}))
4958

5059
const SHARED_APP_ID = 'ASHAREDFIXTURE'
60+
const CUSTOM_APP_ID = 'ACUSTOMFIXTURE'
5161
const MEMBER_TEAM = 'TMEMBERS'
5262
const OTHER_TEAM = 'TOTHER'
5363

@@ -131,10 +141,10 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
131141
},
132142
})
133143

134-
const startSetup = (credentialGroupId: string) =>
144+
const startSetup = (credentialGroupId: string, app?: { appId: string; teamId: string }) =>
135145
startSlackCredentialGroupConfiguration.execute({
136146
principal: principal(),
137-
input: { organizationId: ids.organization, credentialGroupId },
147+
input: { organizationId: ids.organization, credentialGroupId, ...app },
138148
})
139149

140150
const completeSetup = (state: string) =>
@@ -143,6 +153,48 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
143153
input: { state, code: 'fixture-code' },
144154
})
145155

156+
async function installBot(appId: string, teamId: string, executor: DbOrTx = db) {
157+
const secret = (await encryptSecret('fixture-bot')).encrypted
158+
await executor
159+
.insert(slackApp)
160+
.values(
161+
appId === SHARED_APP_ID
162+
? { id: appId, kind: 'shared', organizationId: null, revision: generateId() }
163+
: {
164+
id: appId,
165+
kind: 'custom',
166+
organizationId: ids.organization,
167+
clientId: 'fixture-custom-client',
168+
encryptedClientSecret: secret,
169+
encryptedSigningSecret: secret,
170+
revision: generateId(),
171+
}
172+
)
173+
.onConflictDoNothing()
174+
const botId = generateId()
175+
await executor.insert(credential).values({
176+
id: botId,
177+
organizationId: ids.organization,
178+
type: 'service_account',
179+
providerId: 'slack',
180+
displayName: 'Fixture Slack bot',
181+
createdBy: ids.owner,
182+
encryptedServiceAccountKey: secret,
183+
})
184+
await executor.insert(slackSearchInstallation).values({
185+
id: generateId(),
186+
organizationId: ids.organization,
187+
credentialId: botId,
188+
appId,
189+
slackAppId: appId,
190+
teamId,
191+
teamName: 'Fixture team',
192+
botUserId: 'UBOT',
193+
credentialVersion: generateId(),
194+
revision: generateId(),
195+
})
196+
}
197+
146198
async function accountsGroup() {
147199
const [group] = await db
148200
.select()
@@ -230,6 +282,16 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
230282
})
231283
).resolves.toMatchObject({ authorizationAppId: `slack:${SHARED_APP_ID}:${MEMBER_TEAM}` })
232284
await expect(approveSlack()).resolves.toMatchObject({ memberAccounts: { changed: false } })
285+
await expect(
286+
listSlackSearchInstallations.execute({
287+
principal: principal(),
288+
input: { organizationId: ids.organization },
289+
})
290+
).resolves.toMatchObject({
291+
sharedAppAvailable: true,
292+
memberAppKind: 'shared',
293+
installations: [],
294+
})
233295

234296
const second = new URL((await startSetup(groupId)).authorizationUrl)
235297
expect(second.searchParams.get('team')).toBe(MEMBER_TEAM)
@@ -243,34 +305,116 @@ describe('Slack member accounts on the shared app without a Search bot', () => {
243305
it('refuses shared-app member setup when the organization already has a Search bot', async () => {
244306
const groupId = (await approveSlack()).memberAccounts?.groupId
245307
if (!groupId) throw new Error('Approval did not provision organization accounts')
246-
await db
247-
.insert(slackApp)
248-
.values({ id: SHARED_APP_ID, kind: 'shared', organizationId: null, revision: generateId() })
249-
const botId = generateId()
250-
await db.insert(credential).values({
251-
id: botId,
252-
organizationId: ids.organization,
253-
type: 'service_account',
254-
providerId: 'slack',
255-
displayName: 'Fixture Slack bot',
256-
createdBy: ids.owner,
257-
encryptedServiceAccountKey: (await encryptSecret('fixture-bot')).encrypted,
258-
})
259-
await db.insert(slackSearchInstallation).values({
260-
id: generateId(),
261-
organizationId: ids.organization,
262-
credentialId: botId,
263-
appId: SHARED_APP_ID,
264-
slackAppId: SHARED_APP_ID,
265-
teamId: MEMBER_TEAM,
266-
teamName: 'Fixture team',
267-
botUserId: 'UBOT',
268-
credentialVersion: generateId(),
269-
revision: generateId(),
270-
})
308+
await installBot(SHARED_APP_ID, MEMBER_TEAM)
271309
await expect(startSetup(groupId)).rejects.toThrow('installed Slack app')
272310
})
273311

312+
it('refuses to finish member setup when a custom bot is installed during authorization', async () => {
313+
const groupId = (await approveSlack()).memberAccounts?.groupId
314+
if (!groupId) throw new Error('Approval did not provision organization accounts')
315+
const pending = new URL((await startSetup(groupId)).authorizationUrl)
316+
const before = await accountsGroup()
317+
await installBot(CUSTOM_APP_ID, OTHER_TEAM)
318+
provideSlackConsent(MEMBER_TEAM)
319+
await expect(completeSetup(pending.searchParams.get('state') ?? '')).rejects.toThrow(
320+
'installations changed during authorization'
321+
)
322+
expect(await accountsGroup()).toEqual(before)
323+
})
324+
325+
it('refuses member setup when another bot joins an installed one during authorization', async () => {
326+
const groupId = (await approveSlack()).memberAccounts?.groupId
327+
if (!groupId) throw new Error('Approval did not provision organization accounts')
328+
await installBot(SHARED_APP_ID, MEMBER_TEAM)
329+
const pending = new URL(
330+
(await startSetup(groupId, { appId: SHARED_APP_ID, teamId: MEMBER_TEAM })).authorizationUrl
331+
)
332+
expect(pending.searchParams.get('team')).toBe(MEMBER_TEAM)
333+
const before = await accountsGroup()
334+
await installBot(CUSTOM_APP_ID, OTHER_TEAM)
335+
provideSlackConsent(MEMBER_TEAM)
336+
await expect(completeSetup(pending.searchParams.get('state') ?? '')).rejects.toThrow(
337+
'installations changed during authorization'
338+
)
339+
expect(await accountsGroup()).toEqual(before)
340+
})
341+
342+
it('waits for an in-flight bot install before finishing member setup', async () => {
343+
const groupId = (await approveSlack()).memberAccounts?.groupId
344+
if (!groupId) throw new Error('Approval did not provision organization accounts')
345+
const pending = new URL((await startSetup(groupId)).authorizationUrl)
346+
const before = await accountsGroup()
347+
provideSlackConsent(MEMBER_TEAM)
348+
const locked = createDeferred<number>()
349+
const release = createDeferred<void>()
350+
const install = db.transaction(async (tx) => {
351+
await lockOrganizationSlackApp(tx, ids.organization)
352+
await installBot(CUSTOM_APP_ID, OTHER_TEAM, tx)
353+
const [connection] = await tx.execute<{ pid: number }>(sql`SELECT pg_backend_pid() AS pid`)
354+
locked.resolve(connection.pid)
355+
await release.promise
356+
})
357+
const installPid = await locked.promise
358+
const completion = completeSetup(pending.searchParams.get('state') ?? '')
359+
completion.catch(() => undefined)
360+
try {
361+
await vi.waitFor(
362+
async () => {
363+
const [waiting] = await db.execute<{ pid: number }>(
364+
sql`SELECT pid FROM pg_stat_activity WHERE ${installPid} = ANY(pg_blocking_pids(pid))`
365+
)
366+
expect(waiting).toBeDefined()
367+
},
368+
{ timeout: 5_000 }
369+
)
370+
} finally {
371+
release.resolve()
372+
await install
373+
}
374+
await expect(completion).rejects.toThrow('installations changed during authorization')
375+
expect(await accountsGroup()).toEqual(before)
376+
})
377+
378+
it('reports a custom-app member setup and keeps it off the shared-app path', async () => {
379+
const groupId = (await approveSlack()).memberAccounts?.groupId
380+
if (!groupId) throw new Error('Approval did not provision organization accounts')
381+
const scopes = [...SLACK_SEARCH_USER_SCOPES]
382+
await db
383+
.update(credentialGroup)
384+
.set({
385+
options: [
386+
{
387+
id: generateId(),
388+
provider: 'slack',
389+
label: 'Slack',
390+
authorizationAppId: `slack:${CUSTOM_APP_ID}:${OTHER_TEAM}`,
391+
requiredScopes: scopes,
392+
scopeVersion: credentialGroupScopePolicyVersion(scopes),
393+
required: false,
394+
status: 'active',
395+
},
396+
],
397+
encryptedProviderConfiguration: await encryptCredentialGroupProviderConfiguration({
398+
...emptyCredentialGroupProviderConfiguration(),
399+
slack: {
400+
source: 'slack_app',
401+
appId: CUSTOM_APP_ID,
402+
teamId: OTHER_TEAM,
403+
scopes,
404+
verifiedAt: new Date().toISOString(),
405+
},
406+
}),
407+
})
408+
.where(eq(credentialGroup.id, groupId))
409+
await expect(
410+
listSlackSearchInstallations.execute({
411+
principal: principal(),
412+
input: { organizationId: ids.organization },
413+
})
414+
).resolves.toMatchObject({ memberAppKind: 'custom', installations: [] })
415+
await expect(startSetup(groupId)).rejects.toThrow('Remove the existing Slack setup')
416+
})
417+
274418
it('revokes shared-app member grants in every organization when a workspace has no bot', async () => {
275419
const groupId = (await approveSlack()).memberAccounts?.groupId
276420
if (!groupId) throw new Error('Approval did not provision organization accounts')

‎apps/sim/lib/credential-groups/application/slack-managed-users.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ export const completeSlackCredentialGroupConfiguration: OperationUseCase<
171171
attempt.appRevision !== pending.appRevision ||
172172
attempt.searchApproval?.approved !== pending.searchApproval?.approved ||
173173
attempt.searchApproval?.updatedAt !== pending.searchApproval?.updatedAt ||
174+
attempt.installationsRevision !== pending.installationsRevision ||
174175
attempt.clientId !== pending.clientId ||
175176
attempt.redirectUri !== pending.redirectUri ||
176177
credentialGroupScopePolicyVersion(attempt.requiredScopes) !==

0 commit comments

Comments
 (0)