diff --git a/apps/sim/lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts b/apps/sim/lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts index de45d377b5c..4f978575962 100644 --- a/apps/sim/lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts +++ b/apps/sim/lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts @@ -47,11 +47,14 @@ import { encryptManagedOAuthTokenSet } from '@/lib/credentials/managed-oauth' import type { DbOrTx, DbTransaction } from '@/lib/db/types' import { approveSearchIntegration } from '@/lib/knowledge/application/search-integrations' import { + configureSlackSearchInstallation, listSlackSearchInstallations, removeSlackSearchInstallation, } from '@/lib/knowledge/application/slack-search/installations' import { revokeSlackSearchAccess } from '@/lib/knowledge/application/slack-search/lifecycle' +import { SLACK_CUSTOM_BOT_PROVIDER_ID, SLACK_CUSTOM_BOT_SECRET_TYPE } from '@/lib/oauth/types' import { defaultLiveSearchPolicy } from '@/lib/sim-search/live/policy-schema' +import { SLACK_SHARED_SEARCH_BOT_SCOPES } from '@/lib/slack-search/constants' import { getSharedSlackSearchAppConfiguration } from '@/lib/slack-search/shared-app-env' vi.mock('@/lib/core/config/env-flags', async (importOriginal) => ({ @@ -156,8 +159,8 @@ describe('Slack member accounts on the shared app without a Search bot', () => { input: { state, code: 'fixture-code' }, }) - async function installBot(appId: string, teamId: string, executor: DbOrTx = db) { - const secret = (await encryptSecret('fixture-bot')).encrypted + async function upsertSlackApp(appId: string, executor: DbOrTx = db) { + const secret = (await encryptSecret('fixture-app')).encrypted await executor .insert(slackApp) .values( @@ -174,6 +177,11 @@ describe('Slack member accounts on the shared app without a Search bot', () => { } ) .onConflictDoNothing() + } + + async function installBot(appId: string, teamId: string, executor: DbOrTx = db) { + await upsertSlackApp(appId, executor) + const secret = (await encryptSecret('fixture-bot')).encrypted const botId = generateId() await executor.insert(credential).values({ id: botId, @@ -281,6 +289,99 @@ describe('Slack member accounts on the shared app without a Search bot', () => { restoreSlackHttp = () => spy.mockRestore() } + /** Answers bot verification as a bot of `appId` installed in `teamId`. */ + function provideSlackBot(appId: string, teamId: string) { + restoreSlackHttp?.() + const spy = vi.spyOn(globalThis, 'fetch').mockImplementation(async (input) => { + const url = new URL(String(input)) + switch (`${url.origin}${url.pathname}`) { + case 'https://slack.com/api/auth.test': + return Response.json( + { ok: true, team_id: teamId, team: 'Fixture team', user_id: 'UBOT', bot_id: 'BBOT' }, + { headers: { 'x-oauth-scopes': SLACK_SHARED_SEARCH_BOT_SCOPES.join(',') } } + ) + case 'https://slack.com/api/bots.info': + return Response.json({ ok: true, bot: { id: 'BBOT', app_id: appId } }) + default: + throw new Error(`Unexpected Slack fixture request: ${url}`) + } + }) + restoreSlackHttp = () => spy.mockRestore() + } + + async function connectMembersOnSharedApp() { + const groupId = (await approveSlack()).memberAccounts?.groupId + if (!groupId) throw new Error('Approval did not provision organization accounts') + const pending = new URL((await startSetup(groupId)).authorizationUrl) + provideSlackConsent(MEMBER_TEAM) + await completeSetup(pending.searchParams.get('state') ?? '') + } + + /** A bot credential left without a Search installation, as after removing its binding. */ + async function insertLeftoverBot(appId: string, teamId: string) { + await upsertSlackApp(appId) + const { encrypted } = await encryptSecret( + JSON.stringify({ + type: SLACK_CUSTOM_BOT_SECRET_TYPE, + botToken: 'xoxb-fixture', + teamId, + botUserId: 'UBOT', + teamName: 'Fixture team', + }) + ) + const credentialId = generateId() + await db.insert(credential).values({ + id: credentialId, + organizationId: ids.organization, + type: 'service_account', + providerId: SLACK_CUSTOM_BOT_PROVIDER_ID, + displayName: 'Leftover Slack bot', + createdBy: ids.owner, + encryptedServiceAccountKey: encrypted, + slackAppId: appId, + }) + return credentialId + } + + const enableBot = (credentialId: string) => + configureSlackSearchInstallation.execute({ + principal: principal(), + input: { organizationId: ids.organization, credentialId, enabled: true }, + }) + + const installations = () => + db + .select() + .from(slackSearchInstallation) + .where(eq(slackSearchInstallation.organizationId, ids.organization)) + + it('refuses to bind a leftover bot to a different app or workspace than members', async () => { + await connectMembersOnSharedApp() + const credentialId = await insertLeftoverBot(CUSTOM_APP_ID, OTHER_TEAM) + provideSlackBot(CUSTOM_APP_ID, OTHER_TEAM) + await expect(enableBot(credentialId)).rejects.toMatchObject({ + code: 'conflict', + message: 'Install the same Slack app and workspace connected by your members', + }) + expect(await installations()).toEqual([]) + }) + + it('binds a leftover shared-app bot in the members workspace', async () => { + await connectMembersOnSharedApp() + const credentialId = await insertLeftoverBot(SHARED_APP_ID, MEMBER_TEAM) + provideSlackBot(SHARED_APP_ID, MEMBER_TEAM) + const created = await enableBot(credentialId) + expect(await installations()).toEqual([ + expect.objectContaining({ + id: created.id, + credentialId, + appId: SHARED_APP_ID, + teamId: MEMBER_TEAM, + enabled: true, + }), + ]) + }) + it('approves Slack, verifies the workspace from the admin grant, and resolves member sign-in', async () => { const approval = await approveSlack() expect(approval).toMatchObject({ approved: true }) diff --git a/apps/sim/lib/credential-groups/organization-slack-app.ts b/apps/sim/lib/credential-groups/organization-slack-app.ts index b9c0124af29..a66cf54fc73 100644 --- a/apps/sim/lib/credential-groups/organization-slack-app.ts +++ b/apps/sim/lib/credential-groups/organization-slack-app.ts @@ -5,6 +5,7 @@ import { OrchestrationError } from '@/lib/core/orchestration/types' import { decryptCredentialGroupProviderConfiguration, encryptCredentialGroupProviderConfiguration, + type SlackCredentialGroupConfiguration, } from '@/lib/credential-groups/provider-configuration' import { acquireAdvisoryXactLock } from '@/lib/db/advisory-locks' import type { DbOrTx, DbTransaction } from '@/lib/db/types' @@ -49,6 +50,57 @@ export async function loadOrganizationSlackMemberApps( return configurations.filter((row) => row.configuration.slack !== undefined) } +/** + * Resolves the one Slack app and workspace the organization's members connect through, with the + * union of their scopes. Fails when member configurations reference more than one app. + */ +export async function resolveOrganizationSlackMemberApp( + organizationId: string, + executor: DbOrTx = db +) { + const rows = await loadOrganizationSlackMemberApps(organizationId, executor) + const configurations = rows.flatMap((row) => + row.configuration.slack ? [row.configuration.slack] : [] + ) + const first = configurations[0] + if ( + first && + configurations.some( + (configuration) => + configuration.appId !== first.appId || configuration.teamId !== first.teamId + ) + ) + throw new OrchestrationError( + 'conflict', + 'Organization Slack member accounts reference multiple apps. Consolidate them before configuring Search.' + ) + return { + app: first ? { appId: first.appId, teamId: first.teamId } : null, + scopes: [...new Set(configurations.flatMap((configuration) => configuration.scopes))], + } +} + +type SlackAppIdentity = Pick + +/** + * Refuses a Search bot outside the members' Slack workspace, or on a different app unless the + * bot runs on the shared Sim Search app. + */ +export function requireSearchBotMatchesMemberApp( + memberApp: SlackAppIdentity | null | undefined, + bot: SlackAppIdentity, + sharedBot: boolean +) { + if ( + memberApp && + ((!sharedBot && memberApp.appId !== bot.appId) || memberApp.teamId !== bot.teamId) + ) + throw new OrchestrationError( + 'conflict', + 'Install the same Slack app and workspace connected by your members' + ) +} + /** Replaces legacy duplicated secrets without changing member grants or their scope policies. */ export async function adoptOrganizationSlackMemberApp( executor: DbOrTx, diff --git a/apps/sim/lib/credential-groups/provider-configuration.ts b/apps/sim/lib/credential-groups/provider-configuration.ts index 6ddcce5ce02..62e71efe89c 100644 --- a/apps/sim/lib/credential-groups/provider-configuration.ts +++ b/apps/sim/lib/credential-groups/provider-configuration.ts @@ -159,7 +159,8 @@ async function resolveSlackConfiguration( .limit(1) if (!app) throw new Error('Organization Slack app configuration is missing') /** Member grants are verified against the app and team directly; the Search bot is optional. */ - if (app.kind === 'shared') await requireSlackSearchAppAvailable(app.id, params.organizationId) + if (app.kind === 'shared') + await requireSlackSearchAppAvailable(app.id, params.organizationId, params.executor) const resolved = await resolveSlackAppCredentials(app) return { appId: app.id, diff --git a/apps/sim/lib/credential-groups/slack-managed-users.ts b/apps/sim/lib/credential-groups/slack-managed-users.ts index b9649f75429..4aefdb93f45 100644 --- a/apps/sim/lib/credential-groups/slack-managed-users.ts +++ b/apps/sim/lib/credential-groups/slack-managed-users.ts @@ -918,7 +918,7 @@ export async function exchangeAndConfigureSlackManagedUsers(params: { 'This Slack app belongs to another installation owner' ) if (app?.kind === 'shared') - await requireSlackSearchAppAvailable(app.id, params.attempt.organizationId) + await requireSlackSearchAppAvailable(app.id, params.attempt.organizationId, tx) const resolved = app ? await resolveSlackAppCredentials(app) : null if ( !resolved || diff --git a/apps/sim/lib/knowledge/application/slack-search/installations.ts b/apps/sim/lib/knowledge/application/slack-search/installations.ts index 6c1c64cbe13..5348d3c545e 100644 --- a/apps/sim/lib/knowledge/application/slack-search/installations.ts +++ b/apps/sim/lib/knowledge/application/slack-search/installations.ts @@ -7,6 +7,8 @@ import { OrchestrationError } from '@/lib/core/orchestration/types' import { loadOrganizationSlackMemberApps, lockOrganizationSlackApp, + requireSearchBotMatchesMemberApp, + resolveOrganizationSlackMemberApp, } from '@/lib/credential-groups/organization-slack-app' import { acquireAdvisoryXactLock } from '@/lib/db/advisory-locks' import { @@ -173,7 +175,7 @@ export const configureSlackSearchInstallation = defineAuthorizedKnowledgeUseCase ? await tx.select().from(slackApp).where(eq(slackApp.id, current.slackAppId)).limit(1) : [] if (input.enabled && current.slackAppId) - await requireSlackSearchAppAvailable(current.slackAppId, context.organizationId) + await requireSlackSearchAppAvailable(current.slackAppId, context.organizationId, tx) if (current.slackAppId && !app) throw new Error('Slack app configuration is missing') const appRevision = app && secret ? (await resolveSlackAppCredentials(app)).revision : undefined @@ -239,6 +241,8 @@ export const configureSlackSearchInstallation = defineAuthorizedKnowledgeUseCase } if (!identity || !secret) throw new OrchestrationError('not_found', 'Slack Search installation not found') + const member = await resolveOrganizationSlackMemberApp(context.organizationId, tx) + requireSearchBotMatchesMemberApp(member.app, identity, app?.kind === 'shared') const [created] = await tx .insert(slackSearchInstallation) .values({ diff --git a/apps/sim/lib/knowledge/application/slack-search/setup.test.ts b/apps/sim/lib/knowledge/application/slack-search/setup.test.ts index 3fa9f732176..5fee1ad38cc 100644 --- a/apps/sim/lib/knowledge/application/slack-search/setup.test.ts +++ b/apps/sim/lib/knowledge/application/slack-search/setup.test.ts @@ -24,7 +24,7 @@ const hoisted = vi.hoisted(() => ({ shared: vi.fn(), revoke: vi.fn(), validateGrant: vi.fn(), - memberApps: vi.fn(), + memberApp: vi.fn(), adoptMemberApp: vi.fn(), insert: vi.fn(), update: vi.fn(), @@ -45,8 +45,11 @@ vi.mock('@/lib/internal/slack/oauth', () => ({ validateSlackBotAuthorization: hoisted.validateGrant, })) vi.mock('@/lib/credential-groups/service', () => credentialGroupsServiceMock) -vi.mock('@/lib/credential-groups/organization-slack-app', () => ({ - loadOrganizationSlackMemberApps: hoisted.memberApps, +vi.mock('@/lib/credential-groups/organization-slack-app', async (importOriginal) => ({ + requireSearchBotMatchesMemberApp: ( + await importOriginal() + ).requireSearchBotMatchesMemberApp, + resolveOrganizationSlackMemberApp: hoisted.memberApp, adoptOrganizationSlackMemberApp: hoisted.adoptMemberApp, lockOrganizationSlackApp: vi.fn(), })) @@ -100,7 +103,7 @@ beforeEach(() => { m.revoke.mockResolvedValue(undefined) m.validateGrant.mockReset() m.ensureGroup.mockResolvedValue({ id: 'accounts' }) - m.memberApps.mockReset().mockResolvedValue([]) + m.memberApp.mockReset().mockResolvedValue({ app: null, scopes: [] }) urlsMockFns.mockGetBaseUrl.mockReturnValue('https://sim.test') m.membership.mockResolvedValue([{ role: 'admin' }]) m.rows.mockReset().mockResolvedValue([]) @@ -311,9 +314,7 @@ describe('shared app completion', () => { } beforeEach(() => { - m.memberApps.mockResolvedValue([ - { configuration: { slack: { ...memberApp, scopes: ['im:history'] } } }, - ]) + m.memberApp.mockResolvedValue({ app: memberApp, scopes: ['im:history'] }) m.consume.mockResolvedValue({ ...attempt, sharedApp: { id: sharedApp.id, revision: sharedApp.revision }, @@ -428,17 +429,7 @@ describe('connect an app already installed through a Slack manifest', () => { }) it('rejects a token from a different app than member indexing', async () => { - m.memberApps.mockResolvedValue([ - { - configuration: { - slack: { - appId: 'AOTHER', - teamId: 'T1', - scopes: [], - }, - }, - }, - ]) + m.memberApp.mockResolvedValue({ app: { appId: 'AOTHER', teamId: 'T1' }, scopes: [] }) await expect(connect()).rejects.toThrow('same Slack app and workspace') expect(db.transaction).not.toHaveBeenCalled() }) diff --git a/apps/sim/lib/knowledge/application/slack-search/setup.ts b/apps/sim/lib/knowledge/application/slack-search/setup.ts index 3f77566eaae..92d6c84f94a 100644 --- a/apps/sim/lib/knowledge/application/slack-search/setup.ts +++ b/apps/sim/lib/knowledge/application/slack-search/setup.ts @@ -10,12 +10,12 @@ import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption' import { getBaseUrl } from '@/lib/core/utils/urls' import { adoptOrganizationSlackMemberApp, - loadOrganizationSlackMemberApps, lockOrganizationSlackApp, + requireSearchBotMatchesMemberApp, + resolveOrganizationSlackMemberApp, } from '@/lib/credential-groups/organization-slack-app' import { configureSharedSlackMemberApp } from '@/lib/credential-groups/shared-slack-app' import { acquireAdvisoryXactLock } from '@/lib/db/advisory-locks' -import type { DbOrTx } from '@/lib/db/types' import { buildSlackAppCreationUrl } from '@/lib/integrations/slack-manifest' import { exchangeSlackBotAuthorization, @@ -63,36 +63,13 @@ interface CompleteInput { error?: string } -async function existingMemberApp(organizationId: string, executor: DbOrTx = db) { - const rows = await loadOrganizationSlackMemberApps(organizationId, executor) - const configurations = rows.flatMap((row) => - row.configuration.slack ? [row.configuration.slack] : [] - ) - const first = configurations[0] - if ( - first && - configurations.some( - (configuration) => - configuration.appId !== first.appId || configuration.teamId !== first.teamId - ) - ) - throw new OrchestrationError( - 'conflict', - 'Organization Slack member accounts reference multiple apps. Consolidate them before configuring Search.' - ) - return { - app: first ? { appId: first.appId, teamId: first.teamId } : null, - scopes: [...new Set(configurations.flatMap((configuration) => configuration.scopes))], - } -} - export const prepareSlackSearchSetup = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.prepareSlackInstallation, resolveContext: ({ input }: { input: PrepareInput }) => resolveKnowledgeOrganizationContext(input), async execute({ input, context }) { await requireOrganizationSearchAvailable(context.organizationId) - const member = await existingMemberApp(context.organizationId) + const member = await resolveOrganizationSlackMemberApp(context.organizationId) const manifest = createSlackSearchManifest( input.name, input.description, @@ -114,7 +91,7 @@ async function prepareInstallationAttempt( input: StartInput ): Promise { const origin = getBaseUrl() - const member = await existingMemberApp(input.organizationId) + const member = await resolveOrganizationSlackMemberApp(input.organizationId) const [installation] = input.installationId ? await db .select() @@ -279,15 +256,7 @@ async function saveInstallation( attempt.customInstallation.appId === identity.appId) ) throw new OrchestrationError('conflict', 'Install Sim Search in the same Slack workspace') - if ( - attempt.memberApp && - ((!attempt.sharedApp && attempt.memberApp.appId !== identity.appId) || - attempt.memberApp.teamId !== identity.teamId) - ) - throw new OrchestrationError( - 'conflict', - 'Install the same Slack app and workspace connected by your members' - ) + requireSearchBotMatchesMemberApp(attempt.memberApp, identity, Boolean(attempt.sharedApp)) const { encrypted: encryptedToken } = await encryptSecret( JSON.stringify({ type: SLACK_CUSTOM_BOT_SECRET_TYPE, @@ -440,7 +409,7 @@ async function saveInstallation( .values(appValues) .onConflictDoUpdate({ target: slackApp.id, set: appValues }) } - const member = await existingMemberApp(attempt.organizationId, tx) + const member = await resolveOrganizationSlackMemberApp(attempt.organizationId, tx) if ( member.app?.appId !== attempt.memberApp?.appId || member.app?.teamId !== attempt.memberApp?.teamId diff --git a/apps/sim/lib/slack-search/shared-app.ts b/apps/sim/lib/slack-search/shared-app.ts index 9c7ba042415..5a36842d825 100644 --- a/apps/sim/lib/slack-search/shared-app.ts +++ b/apps/sim/lib/slack-search/shared-app.ts @@ -4,6 +4,7 @@ import { and, eq } from 'drizzle-orm' import { isHosted } from '@/lib/core/config/env-flags' import { isFeatureEnabled } from '@/lib/core/config/feature-flags' import { OrchestrationError } from '@/lib/core/orchestration/types' +import type { DbOrTx } from '@/lib/db/types' import { getSharedSlackSearchAppConfiguration } from '@/lib/slack-search/shared-app-env' /** Called only inside authorized installation/member operations; never returns secrets to a surface. */ @@ -14,14 +15,18 @@ export async function readSharedSlackSearchApp(organizationId: string) { } /** Existing custom bots remain independent of the shared-app rollout. */ -export async function requireSlackSearchAppAvailable(appId: string, organizationId: string) { +export async function requireSlackSearchAppAvailable( + appId: string, + organizationId: string, + executor: DbOrTx = db +) { const shared = getSharedSlackSearchAppConfiguration(appId) if (shared?.id === appId) { if (!(await readSharedSlackSearchApp(organizationId))) throw new OrchestrationError('forbidden', 'The shared Slack Search app is unavailable') return } - const [app] = await db + const [app] = await executor .select({ kind: slackApp.kind }) .from(slackApp) .where(eq(slackApp.id, appId))