From daabd06f6ecb65f6f9118dc67740e3326c9f93fd Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 13 Oct 2026 11:50:19 -0700 Subject: [PATCH 1/3] fix(slack): connect Slack member accounts without the Sim Search bot --- .../components/slack-managed-users-modal.tsx | 97 +++-- apps/sim/hooks/queries/credential-groups.ts | 2 - .../api/contracts/organization-accounts.ts | 6 +- ...lack-shared-member-accounts.integration.ts | 364 ++++++++++++++++++ .../provider-configuration.ts | 20 +- .../credential-groups/slack-managed-users.ts | 140 +++++-- apps/sim/lib/desktop/source-browser.ts | 2 +- .../application/search-integrations.ts | 25 +- .../slack-search/lifecycle.test.ts | 62 ++- .../application/slack-search/lifecycle.ts | 39 +- .../application/slack-search/setup.ts | 16 +- .../sim/lib/slack-search/app-configuration.ts | 12 + 12 files changed, 649 insertions(+), 136 deletions(-) create mode 100644 apps/sim/lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts diff --git a/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx b/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx index 5f0909d6d5a..a97eb0a4f9d 100644 --- a/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx +++ b/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx @@ -109,15 +109,27 @@ export function SlackManagedUsersModal({ availableApps.find((app) => app.appId === appId) ?? (availableApps.length === 1 && !appId ? availableApps[0] : undefined) const sharedAppInstalled = organizationSetup && selectedApp?.appKind === 'shared' + const noInstalledApp = organizationSetup && apps.isSuccess && availableApps.length === 0 + /** The shared app connects member accounts without a Search bot installation. */ + const sharedAppWithoutBot = noInstalledApp && Boolean(apps.data?.sharedAppAvailable) + const sharedSetup = sharedAppInstalled || sharedAppWithoutBot const searchPolicies = useSearchIntegrations(organizationId ?? '', { - enabled: open && sharedAppInstalled, + enabled: open && sharedSetup, }) const searchApproved = searchPolicies.data?.some( (policy) => policy.connectorType === 'slack' && policy.approved ) - const accounts = useOrganizationAccounts(open && sharedAppInstalled ? organizationId : undefined) + const accounts = useOrganizationAccounts(open && sharedSetup ? organizationId : undefined) const memberGroup = accounts.data?.credentialGroup const memberOption = memberGroup?.options.find((option) => option.provider === 'slack') + const memberSetupSettled = + accounts.isSuccess && + !accounts.isFetching && + !accounts.error && + searchPolicies.isSuccess && + !searchPolicies.isFetching && + !searchPolicies.error && + memberGroup?.id === credentialGroupId const sharedAppCanAuthorize = Boolean( sharedAppInstalled && apps.isSuccess && @@ -126,19 +138,18 @@ export function SlackManagedUsersModal({ apps.data?.sharedAppAvailable && selectedApp.enabled && !selectedApp.needsValidation && - accounts.isSuccess && - !accounts.isFetching && - !accounts.error && - searchPolicies.isSuccess && - !searchPolicies.isFetching && - !searchPolicies.error && - memberGroup?.id === credentialGroupId && + memberSetupSettled && memberOption?.status === 'active' ) + const memberOnlyCanAuthorize = + sharedAppWithoutBot && + !apps.isFetching && + memberSetupSettled && + (!memberOption || memberOption.status === 'active') const searchPermissionsMissing = searchApproved && !hasSlackSearchUserScopes(memberOption?.requiredScopes) const sharedAppReady = - sharedAppCanAuthorize && + (sharedAppCanAuthorize || memberOnlyCanAuthorize) && memberOption?.configurationStatus === 'ready' && !searchPermissionsMissing const sharedAppNeedsUpdate = @@ -176,7 +187,7 @@ export function SlackManagedUsersModal({ : 'search') const requiredScopes = scope.kind === 'organization' - ? selectedApp + ? selectedApp || sharedAppWithoutBot ? [...SLACK_SEARCH_USER_SCOPES] : [] : access === null @@ -300,15 +311,24 @@ export function SlackManagedUsersModal({ if ( pending || (sharedAppInstalled && !sharedAppNeedsUpdate) || + (sharedAppWithoutBot && !memberOnlyCanAuthorize) || (!organizationSetup && !selectedBot) ) return - if ( - organizationSetup - ? !selectedApp || !requiredScopes.length - : !clientId.trim() || !clientSecret.trim() - ) + if (organizationSetup ? !requiredScopes.length : !clientId.trim() || !clientSecret.trim()) return + const body = { + ...(organizationSetup + ? selectedApp + ? { appId: selectedApp.appId, teamId: selectedApp.teamId } + : {} + : { + slackBotCredentialId: selectedBot?.id, + clientId: clientId.trim(), + clientSecret: clientSecret.trim(), + }), + requiredScopes, + } if (isDesktopApp()) { const controller = new AbortController() @@ -320,16 +340,7 @@ export function SlackManagedUsersModal({ kind: 'slack-managed-users', owner: resourceScopeFields(scope), credentialGroupId, - body: { - ...(organizationSetup - ? { appId: selectedApp?.appId, teamId: selectedApp?.teamId } - : { - slackBotCredentialId: selectedBot?.id, - clientId: clientId.trim(), - clientSecret: clientSecret.trim(), - }), - requiredScopes, - }, + body, }, controller.signal ) @@ -365,16 +376,7 @@ export function SlackManagedUsersModal({ const result = await startAuthorization.mutateAsync({ ...resourceScopeFields(scope), credentialGroupId, - body: { - ...(organizationSetup - ? { appId: selectedApp?.appId, teamId: selectedApp?.teamId } - : { - slackBotCredentialId: selectedBot?.id, - clientId: clientId.trim(), - clientSecret: clientSecret.trim(), - }), - requiredScopes, - }, + body, }) if (popup.current !== opened) return expectedState.current = result.state @@ -403,10 +405,10 @@ export function SlackManagedUsersModal({ if (sharedAppReady && !appSetupOpen) return null const noBots = !organizationSetup && !isLoading && bots.length === 0 - const needsApp = organizationSetup && apps.isSuccess && availableApps.length === 0 + const needsApp = noInstalledApp && !sharedAppWithoutBot const checkingSetup = apps.isPending || - (sharedAppInstalled && + (sharedSetup && (apps.isFetching || accounts.isPending || accounts.isFetching || @@ -414,9 +416,9 @@ export function SlackManagedUsersModal({ searchPolicies.isFetching)) const failedSetup = apps.error ? apps - : sharedAppInstalled && searchPolicies.error + : sharedSetup && searchPolicies.error ? searchPolicies - : sharedAppInstalled && accounts.error + : sharedSetup && accounts.error ? accounts : null const title = organizationSetup ? 'Set up Slack app' : 'Set up Slack' @@ -424,7 +426,7 @@ export function SlackManagedUsersModal({ ? 'Loading...' : pending ? 'Waiting for Slack...' - : sharedAppNeedsUpdate + : sharedAppNeedsUpdate || (sharedAppWithoutBot && memberOption) ? 'Update member access' : 'Verify and add' const primaryDisabled = @@ -433,7 +435,10 @@ export function SlackManagedUsersModal({ (!organizationSetup && !selectedBot) || pending || (organizationSetup - ? checkingSetup || Boolean(failedSetup) || !selectedApp || !requiredScopes.length + ? checkingSetup || + Boolean(failedSetup) || + (sharedAppWithoutBot && !memberOnlyCanAuthorize) || + !requiredScopes.length : !clientId.trim() || !clientSecret.trim()) return ( @@ -465,6 +470,14 @@ export function SlackManagedUsersModal({ Retry + ) : sharedAppWithoutBot ? ( + +

+ {memberOption + ? 'Verify member permissions. Members will need to reconnect if their app or permissions change.' + : 'Verify member permissions in your Slack workspace.'} +

+ ) : needsApp ? (

diff --git a/apps/sim/hooks/queries/credential-groups.ts b/apps/sim/hooks/queries/credential-groups.ts index 98ade022106..ed0c4d79312 100644 --- a/apps/sim/hooks/queries/credential-groups.ts +++ b/apps/sim/hooks/queries/credential-groups.ts @@ -42,8 +42,6 @@ export function useStartSlackCredentialGroupConfiguration() { )) => { const scope = resourceScopeFromOwner({ workspaceId, organizationId }) if (scope.kind === 'organization') { - if (!body.appId || !body.teamId) - throw new Error('Slack App ID and workspace ID are required') return requestJson(startOrganizationSlackConfigurationContract, { params: { id: scope.organizationId, groupId: credentialGroupId }, body: { diff --git a/apps/sim/lib/api/contracts/organization-accounts.ts b/apps/sim/lib/api/contracts/organization-accounts.ts index e97824502f5..2086b48e9e4 100644 --- a/apps/sim/lib/api/contracts/organization-accounts.ts +++ b/apps/sim/lib/api/contracts/organization-accounts.ts @@ -130,9 +130,13 @@ export const startOrganizationSlackConfigurationContract = defineRouteContract({ method: 'POST', path: '/api/organizations/[id]/connected-accounts/[groupId]/slack-managed-users', params: organizationAccountsParamsSchema.extend({ groupId: z.string().min(1).max(128) }), + /** Omitting `appId` selects the shared Slack app for an organization with no installed bot. */ body: startSlackCredentialGroupConfigurationBodySchema .omit({ slackBotCredentialId: true, clientId: true, clientSecret: true }) - .required({ appId: true, teamId: true }), + .refine((body) => body.appId !== undefined || body.teamId === undefined, { + message: 'Select the Slack app for this workspace', + path: ['appId'], + }), response: startSlackCredentialGroupConfigurationContract.response, }) export type OrganizationAccountsSettings = z.output< 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 new file mode 100644 index 00000000000..d97c0df7639 --- /dev/null +++ b/apps/sim/lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts @@ -0,0 +1,364 @@ +/** + * Real authorization, PostgreSQL, Redis, encryption and policy resolution for Slack member + * accounts on the shared app with no Search bot; Slack HTTP uses fixtures. The shared app is a + * hosted rollout, so `isHosted` is the one deployment fixture, paired with an enterprise plan. + * Run `bun run test:integration lib/credential-groups/__integration__/slack-shared-member-accounts.integration.ts` + * with INTEGRATION_REPORT_PATH for a caller-selected JSON report. + */ + +import { db } from '@sim/db' +import { + credential, + credentialGroup, + credentialGroupEnrollment, + member, + organization, + slackApp, + slackSearchInstallation, + subscription, + user, +} from '@sim/db/schema' +import { readTestRedisUrl } from '@sim/db/testing/test-infrastructure' +import { sha256Hex } from '@sim/security/hash' +import { createSessionPrincipal } from '@sim/testing/factories/principal.factory' +import { generateId } from '@sim/utils/id' +import { eq, inArray } from 'drizzle-orm' +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' +import { env } from '@/lib/core/config/env' +import { closeRedisConnection } from '@/lib/core/config/redis' +import { encryptSecret } from '@/lib/core/security/encryption' +import { + completeSlackCredentialGroupConfiguration, + startSlackCredentialGroupConfiguration, +} from '@/lib/credential-groups/application/slack-managed-users' +import { decryptCredentialGroupProviderConfiguration } from '@/lib/credential-groups/provider-configuration' +import { getCredentialGroup } from '@/lib/credential-groups/service' +import { SLACK_SEARCH_USER_SCOPES } from '@/lib/credential-groups/slack-managed-user-scopes' +import { slackCredentialGroupProviderAdapter } from '@/lib/credential-groups/slack-provider' +import { createOrganizationAccountsGroup } from '@/lib/credential-groups/workspace-accounts' +import { encryptManagedOAuthTokenSet } from '@/lib/credentials/managed-oauth' +import { approveSearchIntegration } from '@/lib/knowledge/application/search-integrations' +import { revokeSlackSearchAccess } from '@/lib/knowledge/application/slack-search/lifecycle' +import { defaultLiveSearchPolicy } from '@/lib/sim-search/live/policy-schema' +import { getSharedSlackSearchAppConfiguration } from '@/lib/slack-search/shared-app-env' + +vi.mock('@/lib/core/config/env-flags', async (importOriginal) => ({ + ...(await importOriginal()), + isHosted: true, +})) + +const SHARED_APP_ID = 'ASHAREDFIXTURE' +const MEMBER_TEAM = 'TMEMBERS' +const OTHER_TEAM = 'TOTHER' + +describe('Slack member accounts on the shared app without a Search bot', () => { + const redisUrl = readTestRedisUrl() + let ids: { organization: string; otherOrganization: string; owner: string } + let restoreSlackHttp: (() => void) | undefined + + beforeAll(() => { + if (!redisUrl) + throw new Error( + 'Set TEST_REDIS_URL to a disposable local Redis or use bun run test:integration' + ) + }) + + beforeEach(async () => { + Object.assign(env, { + REDIS_URL: redisUrl, + SLACK_SEARCH_APP_ID: SHARED_APP_ID, + SLACK_SEARCH_CLIENT_ID: 'fixture-shared-client', + SLACK_SEARCH_CLIENT_SECRET: 'fixture-shared-secret', + SLACK_SEARCH_SIGNING_SECRET: 'fixture-shared-signing', + SLACK_SEARCH_SHARED_APP: true, + }) + ids = { organization: generateId(), otherOrganization: generateId(), owner: generateId() } + await db.insert(user).values({ + id: ids.owner, + name: 'Slack member fixture', + email: `${ids.owner}@fixture.test`, + emailVerified: true, + createdAt: new Date(), + updatedAt: new Date(), + }) + await db.insert(organization).values( + [ids.organization, ids.otherOrganization].map((id) => ({ + id, + name: 'Slack member fixture', + slug: id, + })) + ) + await db.insert(member).values({ + id: generateId(), + organizationId: ids.organization, + userId: ids.owner, + role: 'owner', + }) + await db.insert(subscription).values({ + id: generateId(), + plan: 'enterprise', + referenceId: ids.organization, + status: 'active', + metadata: { workspaces: 'unlimited' }, + }) + }) + + afterEach(async () => { + restoreSlackHttp?.() + restoreSlackHttp = undefined + await db.delete(subscription).where(eq(subscription.referenceId, ids.organization)) + await db + .delete(organization) + .where(inArray(organization.id, [ids.organization, ids.otherOrganization])) + await db.delete(user).where(eq(user.id, ids.owner)) + await db.delete(slackApp).where(eq(slackApp.id, SHARED_APP_ID)) + }) + afterAll(async () => { + await closeRedisConnection() + await db.$client.end() + }) + + const principal = () => createSessionPrincipal({ userId: ids.owner, sessionId: generateId() }) + + const approveSlack = () => + approveSearchIntegration.execute({ + principal: principal(), + input: { + organizationId: ids.organization, + connectorType: 'slack', + approved: true, + policy: defaultLiveSearchPolicy(), + }, + }) + + const startSetup = (credentialGroupId: string) => + startSlackCredentialGroupConfiguration.execute({ + principal: principal(), + input: { organizationId: ids.organization, credentialGroupId }, + }) + + const completeSetup = (state: string) => + completeSlackCredentialGroupConfiguration.execute({ + principal: principal(), + input: { state, code: 'fixture-code' }, + }) + + async function accountsGroup() { + const [group] = await db + .select() + .from(credentialGroup) + .where(eq(credentialGroup.organizationId, ids.organization)) + return group + } + + function provideSlackConsent(teamId: string) { + restoreSlackHttp?.() + const spy = vi.spyOn(globalThis, 'fetch').mockImplementation(async (input) => { + const url = String(input) + switch (url) { + case 'https://slack.com/api/oauth.v2.access': + return Response.json({ + ok: true, + app_id: SHARED_APP_ID, + team: { id: teamId, name: 'Fixture team' }, + authed_user: { + id: 'UADMIN', + access_token: 'fixture-verification-token', + token_type: 'user', + scope: SLACK_SEARCH_USER_SCOPES.join(','), + }, + }) + case 'https://slack.com/api/auth.test': + return Response.json({ ok: true, team_id: teamId, user_id: 'UADMIN' }) + case 'https://slack.com/api/users.info': + return Response.json({ + ok: true, + user: { id: 'UADMIN', profile: { email: 'admin@fixture.test' } }, + }) + case 'https://slack.com/api/auth.revoke': + return Response.json({ ok: true, revoked: true }) + default: + throw new Error(`Unexpected Slack fixture request: ${url}`) + } + }) + restoreSlackHttp = () => spy.mockRestore() + } + + 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 }) + const groupId = approval.memberAccounts?.groupId + if (!groupId) throw new Error('Approval did not provision organization accounts') + expect((await accountsGroup()).options.some((option) => option.provider === 'slack')).toBe( + false + ) + + const first = new URL((await startSetup(groupId)).authorizationUrl) + expect(first.searchParams.get('client_id')).toBe('fixture-shared-client') + expect(first.searchParams.has('team')).toBe(false) + expect(first.searchParams.get('user_scope')?.split(',')).toEqual( + expect.arrayContaining([...SLACK_SEARCH_USER_SCOPES]) + ) + provideSlackConsent(MEMBER_TEAM) + await expect(completeSetup(first.searchParams.get('state') ?? '')).resolves.toMatchObject({ + ok: true, + result: { appId: SHARED_APP_ID, teamId: MEMBER_TEAM }, + }) + + const group = await accountsGroup() + const option = group.options.find((entry) => entry.provider === 'slack') + expect(option).toMatchObject({ + status: 'active', + authorizationAppId: `slack:${SHARED_APP_ID}:${MEMBER_TEAM}`, + }) + expect( + (await decryptCredentialGroupProviderConfiguration(group.encryptedProviderConfiguration)) + .slack + ).toMatchObject({ source: 'slack_app', appId: SHARED_APP_ID, teamId: MEMBER_TEAM }) + await expect( + getCredentialGroup({ kind: 'organization', organizationId: ids.organization }, groupId) + ).resolves.toMatchObject({ + options: expect.arrayContaining([ + expect.objectContaining({ provider: 'slack', configurationStatus: 'ready' }), + ]), + }) + await expect( + slackCredentialGroupProviderAdapter.getPolicy(undefined, { + organizationId: ids.organization, + credentialGroupId: groupId, + credentialGroupOptionId: option?.id, + }) + ).resolves.toMatchObject({ authorizationAppId: `slack:${SHARED_APP_ID}:${MEMBER_TEAM}` }) + await expect(approveSlack()).resolves.toMatchObject({ memberAccounts: { changed: false } }) + + const second = new URL((await startSetup(groupId)).authorizationUrl) + expect(second.searchParams.get('team')).toBe(MEMBER_TEAM) + provideSlackConsent(OTHER_TEAM) + await expect(completeSetup(second.searchParams.get('state') ?? '')).rejects.toThrow( + 'different app or workspace' + ) + expect(await accountsGroup()).toEqual(group) + }) + + it('refuses shared-app member setup when the organization already has a Search bot', async () => { + const groupId = (await approveSlack()).memberAccounts?.groupId + if (!groupId) throw new Error('Approval did not provision organization accounts') + await db + .insert(slackApp) + .values({ id: SHARED_APP_ID, kind: 'shared', organizationId: null, revision: generateId() }) + const botId = generateId() + await db.insert(credential).values({ + id: botId, + organizationId: ids.organization, + type: 'service_account', + providerId: 'slack', + displayName: 'Fixture Slack bot', + createdBy: ids.owner, + encryptedServiceAccountKey: (await encryptSecret('fixture-bot')).encrypted, + }) + await db.insert(slackSearchInstallation).values({ + id: generateId(), + organizationId: ids.organization, + credentialId: botId, + appId: SHARED_APP_ID, + slackAppId: SHARED_APP_ID, + teamId: MEMBER_TEAM, + teamName: 'Fixture team', + botUserId: 'UBOT', + credentialVersion: generateId(), + revision: generateId(), + }) + await expect(startSetup(groupId)).rejects.toThrow('installed Slack app') + }) + + it('revokes shared-app member grants in every organization when a workspace has no bot', async () => { + const groupId = (await approveSlack()).memberAccounts?.groupId + if (!groupId) throw new Error('Approval did not provision organization accounts') + const { id: otherGroupId } = await db.transaction((tx) => + createOrganizationAccountsGroup(tx, ids.otherOrganization, ids.owner) + ) + const enrollment = generateId() + const otherEnrollment = generateId() + await db.insert(credentialGroupEnrollment).values( + [ + { id: enrollment, credentialGroupId: groupId }, + { id: otherEnrollment, credentialGroupId: otherGroupId }, + ].map((binding) => ({ + ...binding, + userId: ids.owner, + email: `${ids.owner}@fixture.test`, + status: 'completed' as const, + invitationTokenHash: sha256Hex(generateId()), + invitationExpiresAt: new Date(Date.now() + 60_000), + invitedAt: new Date(), + })) + ) + const grantedAt = new Date(Date.now() - 60_000) + const encryptedOauthTokenSet = await encryptManagedOAuthTokenSet({ accessToken: 'xoxp-member' }) + const grants = [ + { subject: 'UREVOKED', team: MEMBER_TEAM, organizationId: ids.organization, enrollment }, + { subject: 'UKEPT', team: MEMBER_TEAM, organizationId: ids.organization, enrollment }, + { subject: 'UREVOKED', team: OTHER_TEAM, organizationId: ids.organization, enrollment }, + { + subject: 'UREVOKED', + team: MEMBER_TEAM, + organizationId: ids.otherOrganization, + enrollment: otherEnrollment, + }, + ].map(({ subject, team, organizationId, enrollment: credentialGroupEnrollmentId }) => ({ + id: generateId(), + organizationId, + type: 'managed_oauth' as const, + providerId: 'slack', + displayName: subject, + createdBy: ids.owner, + authorizationAppId: `slack:${SHARED_APP_ID}:${team}`, + providerSubjectId: subject, + credentialGroupEnrollmentId, + credentialGroupOptionId: generateId(), + managedOauthScopeVersion: 1, + managedOauthStatus: 'active' as const, + grantedScopes: [...SLACK_SEARCH_USER_SCOPES], + encryptedOauthTokenSet, + grantedAt, + })) + await db.insert(credential).values(grants) + const shared = getSharedSlackSearchAppConfiguration() + if (!shared) throw new Error('Shared Slack app fixture is not configured') + + await revokeSlackSearchAccess.execute({ + principal: { + kind: 'slack_app', + appId: SHARED_APP_ID, + appRevision: shared.revision, + receivedAt: new Date(), + }, + input: { + type: 'event_callback', + api_app_id: SHARED_APP_ID, + team_id: MEMBER_TEAM, + event_id: 'EvFixture', + event_time: Math.floor(Date.now() / 1000), + event: { type: 'tokens_revoked', tokens: { oauth: ['UREVOKED'] } }, + }, + }) + + const rows = await db + .select({ id: credential.id, status: credential.managedOauthStatus }) + .from(credential) + .where( + inArray( + credential.id, + grants.map((grant) => grant.id) + ) + ) + expect(new Map(rows.map((row) => [row.id, row.status]))).toEqual( + new Map([ + [grants[0].id, 'needs_reauth'], + [grants[1].id, 'active'], + [grants[2].id, 'active'], + [grants[3].id, 'needs_reauth'], + ]) + ) + }) +}) diff --git a/apps/sim/lib/credential-groups/provider-configuration.ts b/apps/sim/lib/credential-groups/provider-configuration.ts index 8b50a524a20..6ddcce5ce02 100644 --- a/apps/sim/lib/credential-groups/provider-configuration.ts +++ b/apps/sim/lib/credential-groups/provider-configuration.ts @@ -1,5 +1,5 @@ import { db } from '@sim/db' -import { credentialGroup, slackApp, slackSearchInstallation } from '@sim/db/schema' +import { credentialGroup, slackApp } from '@sim/db/schema' import { getErrorMessage } from '@sim/utils/errors' import { and, eq, isNull, or, sql } from 'drizzle-orm' import { resourceScopeFromOwner } from '@/lib/core/resource-scope' @@ -158,22 +158,8 @@ async function resolveSlackConfiguration( ) .limit(1) if (!app) throw new Error('Organization Slack app configuration is missing') - if (app.kind === 'shared') { - await requireSlackSearchAppAvailable(app.id, params.organizationId) - const [installation] = await (params.executor ?? db) - .select({ id: slackSearchInstallation.id }) - .from(slackSearchInstallation) - .where( - and( - eq(slackSearchInstallation.slackAppId, app.id), - eq(slackSearchInstallation.organizationId, params.organizationId), - eq(slackSearchInstallation.teamId, configuration.teamId), - eq(slackSearchInstallation.enabled, true) - ) - ) - .limit(1) - if (!installation) throw new Error('The shared Slack installation is disabled or removed') - } + /** 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) 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 afb120db6f7..8a9fc573c2b 100644 --- a/apps/sim/lib/credential-groups/slack-managed-users.ts +++ b/apps/sim/lib/credential-groups/slack-managed-users.ts @@ -38,8 +38,14 @@ import { lockOrganizationSearchApproval, } from '@/lib/knowledge/search/integration-policy' import { SLACK_CUSTOM_BOT_PROVIDER_ID, SLACK_CUSTOM_BOT_SECRET_TYPE } from '@/lib/oauth/types' -import { resolveSlackAppCredentials } from '@/lib/slack-search/app-configuration' -import { requireSlackSearchAppAvailable } from '@/lib/slack-search/shared-app' +import { + ensureSharedSlackAppRow, + resolveSlackAppCredentials, +} from '@/lib/slack-search/app-configuration' +import { + readSharedSlackSearchApp, + requireSlackSearchAppAvailable, +} from '@/lib/slack-search/shared-app' import { getSharedSlackSearchAppConfiguration } from '@/lib/slack-search/shared-app-env' const logger = createLogger('SlackManagedUsers') @@ -82,7 +88,8 @@ type StoredSlackManagedUsersAttempt = { slackBotCredentialId?: string slackBotCredentialUpdatedAt?: number expectedAppId: string - expectedTeamId: string + /** `null` only for a shared-app member setup with no workspace yet: Slack's grant chooses it. */ + expectedTeamId: string | null clientId: string redirectUri: string requiredScopes: string[] @@ -103,7 +110,7 @@ export interface SlackManagedUsersAttempt { slackBotCredentialId?: string slackBotCredentialUpdatedAt?: number expectedAppId: string - expectedTeamId: string + expectedTeamId: string | null clientId: string clientSecret: string redirectUri: string @@ -187,7 +194,8 @@ function isStoredAttempt(value: unknown): value is StoredSlackManagedUsersAttemp : typeof candidate.slackBotCredentialId === 'string' && typeof candidate.slackBotCredentialUpdatedAt === 'number') && typeof candidate.expectedAppId === 'string' && - typeof candidate.expectedTeamId === 'string' && + (typeof candidate.expectedTeamId === 'string' || + (candidate.expectedTeamId === null && candidate.credentialSource === 'environment')) && typeof candidate.clientId === 'string' && (candidate.credentialSource === 'environment' ? typeof candidate.organizationId === 'string' && @@ -478,6 +486,77 @@ export function getSlackManagedUsersRedirectUri(): string { return `${getBaseUrl()}${SLACK_MANAGED_USER_CONFIGURATION_CALLBACK_PATH}` } +/** An organization app the Search bot installed; its installation fixes the Slack workspace. */ +async function resolveInstalledOrganizationSlackApp( + organizationId: string, + appId: string, + teamId: string | undefined +) { + const [configured] = await db + .select({ app: slackApp, teamId: slackSearchInstallation.teamId }) + .from(slackApp) + .innerJoin( + slackSearchInstallation, + and( + eq(slackSearchInstallation.slackAppId, slackApp.id), + eq(slackSearchInstallation.organizationId, organizationId) + ) + ) + .where( + and( + eq(slackApp.id, appId), + or( + and(eq(slackApp.organizationId, organizationId), eq(slackApp.kind, 'custom')), + and(eq(slackApp.kind, 'shared'), isNull(slackApp.organizationId)) + ) + ) + ) + .limit(1) + if (!configured || (teamId && teamId !== configured.teamId)) + throw new SlackManagedUsersError( + 'Set up this organization’s Slack app first.', + 'invalid_response' + ) + await requireSlackSearchAppAvailable(configured.app.id, organizationId) + const app = await resolveSlackAppCredentials(configured.app) + return { ...app, teamId: configured.teamId } +} + +/** + * Member sign-in through the shared app needs no Search bot. An existing member setup keeps its + * workspace; a first setup leaves it to the admin's Slack grant, which attests the team. + */ +async function resolveSharedSlackMemberApp( + organizationId: string, + encryptedProviderConfiguration: string | null +) { + const [app, [installation], { slack: configured }] = await Promise.all([ + readSharedSlackSearchApp(organizationId), + db + .select({ id: slackSearchInstallation.id }) + .from(slackSearchInstallation) + .where(eq(slackSearchInstallation.organizationId, organizationId)) + .limit(1), + decryptCredentialGroupProviderConfiguration(encryptedProviderConfiguration), + ]) + if (!app) + throw new SlackManagedUsersError( + 'Set up this organization’s Slack app first.', + 'invalid_response' + ) + if (installation) + throw new SlackManagedUsersError( + 'Select the organization’s installed Slack app.', + 'invalid_response' + ) + if (configured && configured.appId !== app.id) + throw new SlackManagedUsersError( + 'Remove the existing Slack setup before connecting the shared Slack app.', + 'invalid_response' + ) + return { ...app, teamId: configured?.teamId ?? null } +} + export async function createSlackManagedUsersAttempt(params: { workspaceId?: string organizationId?: string @@ -496,6 +575,7 @@ export async function createSlackManagedUsersAttempt(params: { id: credentialGroup.id, updatedAt: credentialGroup.updatedAt, options: credentialGroup.options, + encryptedProviderConfiguration: credentialGroup.encryptedProviderConfiguration, }) .from(credentialGroup) .where( @@ -513,45 +593,33 @@ export async function createSlackManagedUsersAttempt(params: { (existingOption ? undefined : SLACK_SEARCH_USER_SCOPES) ) let bot: Awaited> = null - let identity: { appId: string; teamId: string } + let identity: { appId: string; teamId: string | null } let clientId = params.clientId let clientSecret = params.clientSecret let appRevision: string | undefined let searchApproval: SlackSearchApprovalSnapshot | undefined if (scope.kind === 'organization') { - if (params.slackBotCredentialId || !params.appId || params.clientId || params.clientSecret) + if ( + params.slackBotCredentialId || + params.clientId || + params.clientSecret || + (!params.appId && params.teamId) + ) throw new SlackManagedUsersError( 'Select the organization’s Slack app. Configure its credentials in Slack app setup.', 'invalid_response' ) - const [configured] = await db - .select({ app: slackApp, teamId: slackSearchInstallation.teamId }) - .from(slackApp) - .innerJoin( - slackSearchInstallation, - and( - eq(slackSearchInstallation.slackAppId, slackApp.id), - eq(slackSearchInstallation.organizationId, scope.organizationId) + const app = params.appId + ? await resolveInstalledOrganizationSlackApp( + scope.organizationId, + params.appId, + params.teamId ) - ) - .where( - and( - eq(slackApp.id, params.appId), - or( - and(eq(slackApp.organizationId, scope.organizationId), eq(slackApp.kind, 'custom')), - and(eq(slackApp.kind, 'shared'), isNull(slackApp.organizationId)) - ) + : await resolveSharedSlackMemberApp( + scope.organizationId, + group.encryptedProviderConfiguration ) - ) - .limit(1) - if (!configured || (params.teamId && params.teamId !== configured.teamId)) - throw new SlackManagedUsersError( - 'Set up this organization’s Slack app first.', - 'invalid_response' - ) - await requireSlackSearchAppAvailable(configured.app.id, scope.organizationId) - const app = await resolveSlackAppCredentials(configured.app) - identity = { appId: configured.app.id, teamId: configured.teamId } + identity = { appId: app.id, teamId: app.teamId } clientId = app.clientId clientSecret = app.clientSecret appRevision = app.revision @@ -653,7 +721,7 @@ export async function createSlackManagedUsersAttempt(params: { authorizationUrl.searchParams.set('user_scope', requiredScopes.join(',')) authorizationUrl.searchParams.set('redirect_uri', redirectUri) authorizationUrl.searchParams.set('state', state) - authorizationUrl.searchParams.set('team', identity.teamId) + if (identity.teamId) authorizationUrl.searchParams.set('team', identity.teamId) return { state, authorizationUrl: authorizationUrl.toString() } } @@ -749,7 +817,7 @@ export async function exchangeAndConfigureSlackManagedUsers(params: { try { if ( grant.appId !== params.attempt.expectedAppId || - grant.teamId !== params.attempt.expectedTeamId + (params.attempt.expectedTeamId !== null && grant.teamId !== params.attempt.expectedTeamId) ) { throw new SlackManagedUsersError( 'Slack returned a different app or workspace than the configured App ID and workspace ID.', @@ -777,6 +845,8 @@ export async function exchangeAndConfigureSlackManagedUsers(params: { return db.transaction(async (tx) => { if (params.attempt.organizationId) { await lockOrganizationSearchApproval(tx, params.attempt.organizationId) + const sharedApp = getSharedSlackSearchAppConfiguration(params.attempt.expectedAppId) + if (sharedApp) await ensureSharedSlackAppRow(tx, sharedApp) const [app] = await tx .select() .from(slackApp) diff --git a/apps/sim/lib/desktop/source-browser.ts b/apps/sim/lib/desktop/source-browser.ts index ffe71f0c111..ba30049c243 100644 --- a/apps/sim/lib/desktop/source-browser.ts +++ b/apps/sim/lib/desktop/source-browser.ts @@ -122,7 +122,7 @@ async function startRequest( const result = owner.organizationId ? await requestJson(startOrganizationSlackConfigurationContract, { params: { id: owner.organizationId, groupId: credentialGroupId }, - body: { appId: body.appId!, teamId: body.teamId!, requiredScopes: body.requiredScopes }, + body: { appId: body.appId, teamId: body.teamId, requiredScopes: body.requiredScopes }, }) : await requestJson(startSlackCredentialGroupConfigurationContract, { params: { id: owner.workspaceId!, groupId: credentialGroupId }, diff --git a/apps/sim/lib/knowledge/application/search-integrations.ts b/apps/sim/lib/knowledge/application/search-integrations.ts index dccdeb4e90f..d7975378261 100644 --- a/apps/sim/lib/knowledge/application/search-integrations.ts +++ b/apps/sim/lib/knowledge/application/search-integrations.ts @@ -12,8 +12,10 @@ import { and, eq, exists, inArray, isNotNull, isNull, ne, or, sql } from 'drizzl import { OrchestrationError } from '@/lib/core/orchestration/types' import { CredentialGroupProviderConfigurationError } from '@/lib/credential-groups/provider-adapter' import { isScopedCredentialGroupsAvailable } from '@/lib/credential-groups/scoped-availability' -import { addOrganizationAccountProvider } from '@/lib/credential-groups/service' -import { SLACK_SEARCH_USER_SCOPES } from '@/lib/credential-groups/slack-managed-user-scopes' +import { + addOrganizationAccountProvider, + ensureWorkspaceAccountsGroup, +} from '@/lib/credential-groups/service' import { resolveKnowledgeAccessAvailability } from '@/lib/knowledge/access/availability' import { defineAuthorizedKnowledgeUseCase } from '@/lib/knowledge/application/authorized-knowledge-use-case' import { resolveKnowledgeOwnerContext } from '@/lib/knowledge/application/contexts' @@ -271,17 +273,20 @@ export const approveSearchIntegration = defineAuthorizedKnowledgeUseCase({ let memberAccounts: { groupId: string; changed: boolean } | undefined const changed = await db.transaction(async (tx) => { await lockOrganizationSearchApproval(tx, context.organizationId!) - if (memberProvider) + /** Slack sign-in belongs to Slack setup (bot install or admin verification), not approval. */ + if (memberProvider === 'slack') { + const group = await ensureWorkspaceAccountsGroup( + { kind: 'organization', organizationId: context.organizationId! }, + requirePrincipalSubjectUserId(principal), + undefined, + tx + ) + memberAccounts = { groupId: group.id, changed: group.created } + } else if (memberProvider) memberAccounts = await addOrganizationAccountProvider( context.organizationId!, requirePrincipalSubjectUserId(principal), - { - provider: memberProvider, - label: source[1].name, - ...(memberProvider === 'slack' - ? { requiredScopes: [...SLACK_SEARCH_USER_SCOPES] } - : {}), - }, + { provider: memberProvider, label: source[1].name }, tx ).catch((error: unknown) => { if (error instanceof CredentialGroupProviderConfigurationError) diff --git a/apps/sim/lib/knowledge/application/slack-search/lifecycle.test.ts b/apps/sim/lib/knowledge/application/slack-search/lifecycle.test.ts index dd4d4c16fa0..5d2b4e06a13 100644 --- a/apps/sim/lib/knowledge/application/slack-search/lifecycle.test.ts +++ b/apps/sim/lib/knowledge/application/slack-search/lifecycle.test.ts @@ -66,10 +66,13 @@ describe('Slack access revocation', () => { }, ], }) + const memberGrants = dbChainMockFns.where.mock.calls[1][0] + expect(memberGrants.conditions).not.toContainEqual( + expect.objectContaining({ left: credential.organizationId }) + ) expect(dbChainMockFns.where).toHaveBeenNthCalledWith(2, { type: 'and', conditions: expect.arrayContaining([ - { type: 'eq', left: credential.organizationId, right: 'org' }, { type: 'eq', left: credential.authorizationAppId, right: 'slack:A1:T1' }, { type: 'inArray', column: credential.providerSubjectId, values: ['U1', 'U2'] }, { @@ -86,6 +89,63 @@ describe('Slack access revocation', () => { ]), }) }) + it('keeps a custom app revocation inside the installation organization', async () => { + m.app.mockResolvedValue({ + app: { id: 'A1', kind: 'custom', revision: 'r1', organizationId: 'org' }, + }) + dbChainMockFns.returning.mockResolvedValueOnce([]) + await revokeSlackSearchAccess.execute({ + principal, + input: { ...input, event: { type: 'tokens_revoked', tokens: { oauth: ['U1'] } } }, + }) + expect(dbChainMockFns.where).toHaveBeenNthCalledWith(2, { + type: 'and', + conditions: expect.arrayContaining([ + { type: 'eq', left: credential.organizationId, right: 'org' }, + { type: 'eq', left: credential.authorizationAppId, right: 'slack:A1:T1' }, + ]), + }) + }) + it.each([ + { type: 'app_uninstalled' as const }, + { type: 'tokens_revoked' as const, tokens: { oauth: ['U1'] } }, + ])('revokes shared-app member grants in a workspace with no Search bot: %j', async (event) => { + resetDbChainMock() + queueTableRows(slackSearchInstallation, []) + await revokeSlackSearchAccess.execute({ principal, input: { ...input, event } }) + expect(dbChainMockFns.update.mock.calls.map(([table]) => table)).toEqual([credential]) + expect(dbChainMockFns.set).toHaveBeenCalledWith( + expect.objectContaining({ managedOauthStatus: 'needs_reauth' }) + ) + const memberGrants = dbChainMockFns.where.mock.calls[1][0] + expect(memberGrants.conditions).toEqual( + expect.arrayContaining([ + { type: 'eq', left: credential.type, right: 'managed_oauth' }, + { type: 'eq', left: credential.authorizationAppId, right: 'slack:A1:T1' }, + ...(event.type === 'tokens_revoked' + ? [{ type: 'inArray', column: credential.providerSubjectId, values: ['U1'] }] + : []), + ]) + ) + expect(memberGrants.conditions).not.toContainEqual( + expect.objectContaining({ left: credential.organizationId }) + ) + }) + it.each([ + { kind: 'custom', event: { type: 'app_uninstalled' as const } }, + { kind: 'shared', event: { type: 'tokens_revoked' as const, tokens: { bot: ['UBOT'] } } }, + ])( + 'leaves grants untouched with no installation for a custom app or a bot-only revocation: $kind', + async ({ kind, event }) => { + resetDbChainMock() + m.app.mockResolvedValue({ + app: { id: 'A1', kind, revision: 'r1', organizationId: kind === 'custom' ? 'org' : null }, + }) + queueTableRows(slackSearchInstallation, []) + await revokeSlackSearchAccess.execute({ principal, input: { ...input, event } }) + expect(dbChainMockFns.update).not.toHaveBeenCalled() + } + ) it.each([{ bot: ['UBOT'] }, { bot: ['UBOT'], oauth: ['U1'] }])( 'cancels all installation work when its bot is revoked: %j', async (tokens) => { diff --git a/apps/sim/lib/knowledge/application/slack-search/lifecycle.ts b/apps/sim/lib/knowledge/application/slack-search/lifecycle.ts index b712a4cb6f7..a7f012ddd1c 100644 --- a/apps/sim/lib/knowledge/application/slack-search/lifecycle.ts +++ b/apps/sim/lib/knowledge/application/slack-search/lifecycle.ts @@ -52,6 +52,19 @@ export const revokeSlackSearchAccess: OperationUseCase< throw new OrchestrationError('forbidden', 'Slack app configuration changed') const occurredAt = new Date(input.event_time * 1000) if (occurredAt.getTime() > Date.now() + 60_000) throw new Error('Invalid Slack revocation time') + /** A shared-app grant may belong to several organizations; its app and team name it exactly. */ + const sharedApp = app.app.kind === 'shared' + const uninstall = input.event.type === 'app_uninstalled' + const revokedUsers = + input.event.type === 'tokens_revoked' ? (input.event.tokens.oauth ?? []) : [] + const memberGrants = (organizationId?: string) => + and( + ...(organizationId ? [eq(credential.organizationId, organizationId)] : []), + eq(credential.type, 'managed_oauth'), + eq(credential.authorizationAppId, `slack:${principal.appId}:${input.team_id}`), + or(isNull(credential.grantedAt), lte(credential.grantedAt, occurredAt)), + ...(uninstall ? [] : [inArray(credential.providerSubjectId, revokedUsers)]) + ) await db.transaction(async (tx) => { const [installation] = await tx .select() @@ -64,14 +77,19 @@ export const revokeSlackSearchAccess: OperationUseCase< ) .for('update') .limit(1) - if (!installation) return + /** Member sign-in through the shared app needs no Search bot, so it has no installation. */ + if (!installation) { + if (sharedApp && (uninstall || revokedUsers.length)) + await tx + .update(credential) + .set({ managedOauthStatus: 'needs_reauth', updatedAt: new Date() }) + .where(memberGrants()) + return + } if (app.app.kind === 'custom' && app.app.organizationId !== installation.organizationId) throw new Error('Slack installation ownership is inconsistent') - const uninstall = input.event.type === 'app_uninstalled' const staleInstallation = installation.updatedAt > occurredAt if (uninstall && staleInstallation) return - const revokedUsers = - input.event.type === 'tokens_revoked' ? (input.event.tokens.oauth ?? []) : [] const revokeBot = !staleInstallation && (uninstall || @@ -82,18 +100,7 @@ export const revokeSlackSearchAccess: OperationUseCase< const revokeCredentials = tx .update(credential) .set({ managedOauthStatus: 'needs_reauth', updatedAt: new Date() }) - .where( - and( - eq(credential.organizationId, installation.organizationId), - eq(credential.type, 'managed_oauth'), - eq( - credential.authorizationAppId, - `slack:${installation.appId}:${installation.teamId}` - ), - or(isNull(credential.grantedAt), lte(credential.grantedAt, occurredAt)), - ...(uninstall ? [] : [inArray(credential.providerSubjectId, revokedUsers)]) - ) - ) + .where(memberGrants(sharedApp ? undefined : installation.organizationId)) if (revokeBot) await revokeCredentials else { const revokedCredentials = await revokeCredentials.returning({ diff --git a/apps/sim/lib/knowledge/application/slack-search/setup.ts b/apps/sim/lib/knowledge/application/slack-search/setup.ts index cc6a1dac795..a73da66a2b0 100644 --- a/apps/sim/lib/knowledge/application/slack-search/setup.ts +++ b/apps/sim/lib/knowledge/application/slack-search/setup.ts @@ -31,7 +31,10 @@ import { defineAuthorizedKnowledgeUseCase } from '@/lib/knowledge/application/au import { resolveKnowledgeOrganizationContext } from '@/lib/knowledge/application/contexts' import { knowledgeOperations } from '@/lib/knowledge/application/operations' import { SLACK_CUSTOM_BOT_PROVIDER_ID, SLACK_CUSTOM_BOT_SECRET_TYPE } from '@/lib/oauth/types' -import { slackBotCredentialVersion } from '@/lib/slack-search/app-configuration' +import { + ensureSharedSlackAppRow, + slackBotCredentialVersion, +} from '@/lib/slack-search/app-configuration' import { SLACK_SEARCH_SCOPES, SLACK_SHARED_SEARCH_BOT_SCOPES } from '@/lib/slack-search/constants' import { createSlackSearchManifest, SLACK_SEARCH_CALLBACK_PATH } from '@/lib/slack-search/manifest' import { @@ -418,16 +421,7 @@ async function saveInstallation( } const appRevision = attempt.sharedApp?.revision ?? generateId() if (attempt.sharedApp) { - /** The row supplies foreign-key identity only; shared secrets remain in the environment. */ - await tx - .insert(slackApp) - .values({ - id: identity.appId, - kind: 'shared', - organizationId: null, - revision: appRevision, - }) - .onConflictDoNothing() + await ensureSharedSlackAppRow(tx, { id: identity.appId, revision: appRevision }) } else { const appValues = { id: identity.appId, diff --git a/apps/sim/lib/slack-search/app-configuration.ts b/apps/sim/lib/slack-search/app-configuration.ts index 83171eb9ab0..f5433289573 100644 --- a/apps/sim/lib/slack-search/app-configuration.ts +++ b/apps/sim/lib/slack-search/app-configuration.ts @@ -3,6 +3,7 @@ import { slackApp } from '@sim/db/schema' import { sha256Hex } from '@sim/security/hash' import { eq } from 'drizzle-orm' import { decryptSecret } from '@/lib/core/security/encryption' +import type { DbOrTx } from '@/lib/db/types' import { getSharedSlackSearchAppConfiguration } from '@/lib/slack-search/shared-app-env' /** Shared credentials always come from the deployment, even for previously registered apps. */ @@ -31,6 +32,17 @@ export async function resolveSlackAppCredentials(app: typeof slackApp.$inferSele } } +/** The row supplies foreign-key identity only; shared secrets remain in the environment. */ +export async function ensureSharedSlackAppRow( + executor: DbOrTx, + app: { id: string; revision: string } +): Promise { + await executor + .insert(slackApp) + .values({ id: app.id, kind: 'shared', organizationId: null, revision: app.revision }) + .onConflictDoNothing() +} + /** Authentication lookup only: an app ID selects a key, never grants organization access. */ export async function loadSlackAppConfiguration(appId: string) { const shared = getSharedSlackSearchAppConfiguration(appId) From ecdc10b2aa4fdfd4afbcccb147ad1efea8401f22 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 13 Oct 2026 11:59:59 -0700 Subject: [PATCH 2/3] fix(slack): revoke shared grants on stale uninstall, pair app and workspace ids, audit text --- .../api/contracts/organization-accounts.ts | 4 +- .../provider-configuration.test.ts | 17 +++----- .../application/search-integrations.ts | 11 +++-- .../slack-search/lifecycle.test.ts | 42 ++++++++++++++++--- .../application/slack-search/lifecycle.ts | 26 +++++++----- scripts/check-explicit-any.baseline.json | 2 +- 6 files changed, 69 insertions(+), 33 deletions(-) diff --git a/apps/sim/lib/api/contracts/organization-accounts.ts b/apps/sim/lib/api/contracts/organization-accounts.ts index 2086b48e9e4..97ad6919540 100644 --- a/apps/sim/lib/api/contracts/organization-accounts.ts +++ b/apps/sim/lib/api/contracts/organization-accounts.ts @@ -133,8 +133,8 @@ export const startOrganizationSlackConfigurationContract = defineRouteContract({ /** Omitting `appId` selects the shared Slack app for an organization with no installed bot. */ body: startSlackCredentialGroupConfigurationBodySchema .omit({ slackBotCredentialId: true, clientId: true, clientSecret: true }) - .refine((body) => body.appId !== undefined || body.teamId === undefined, { - message: 'Select the Slack app for this workspace', + .refine((body) => (body.appId === undefined) === (body.teamId === undefined), { + message: 'Select the Slack app and its workspace together', path: ['appId'], }), response: startSlackCredentialGroupConfigurationContract.response, diff --git a/apps/sim/lib/credential-groups/provider-configuration.test.ts b/apps/sim/lib/credential-groups/provider-configuration.test.ts index 0caa96b79e6..35db9b26254 100644 --- a/apps/sim/lib/credential-groups/provider-configuration.test.ts +++ b/apps/sim/lib/credential-groups/provider-configuration.test.ts @@ -79,13 +79,9 @@ describe('organization Slack app references', () => { clientSecret: 'current-secret', }) }) - it.each([ - [true, true], - [false, true], - [true, false], - ])( - 'resolves shared credentials only for an active installation and enabled organization (active=%s, enabled=%s)', - async (active, enabled) => { + it.each([true, false])( + 'resolves shared credentials without a Search bot only for an enabled organization (enabled=%s)', + async (enabled) => { setEnv({ SLACK_SEARCH_APP_ID: 'A1' }) shared.flag.mockImplementation( async (_flag, context) => enabled && context?.orgId === 'org-1' @@ -107,20 +103,19 @@ describe('organization Slack app references', () => { encryptedSigningSecret: null, }, ]) - .mockResolvedValueOnce(active ? [{ id: 'installation' }] : []) const result = getSlackCredentialGroupConfiguration({ organizationId: 'org-1', credentialGroupId: 'group-1', }) - if (!enabled) await expect(result).rejects.toThrow('unavailable') - else if (active) + if (enabled) await expect(result).resolves.toMatchObject({ clientId: 'environment-client', clientSecret: 'environment-secret', appId: 'A1', teamId: 'T1', }) - else await expect(result).rejects.toThrow('disabled or removed') + else await expect(result).rejects.toThrow('unavailable') + expect(dbChainMockFns.limit).toHaveBeenCalledTimes(2) expect(shared.flag).toHaveBeenCalledWith('slack-search-shared-app', { orgId: 'org-1' }) } ) diff --git a/apps/sim/lib/knowledge/application/search-integrations.ts b/apps/sim/lib/knowledge/application/search-integrations.ts index d7975378261..6347cabd1fe 100644 --- a/apps/sim/lib/knowledge/application/search-integrations.ts +++ b/apps/sim/lib/knowledge/application/search-integrations.ts @@ -179,8 +179,8 @@ export const approveSearchIntegration = defineAuthorizedKnowledgeUseCase({ resolveContext: ({ input }: { input: ApproveSearchIntegrationInput }) => resolveKnowledgeOwnerContext({ organizationId: input.organizationId }), async execute({ input, context, principal }) { - if (!context.organizationId) - throw new OrchestrationError('validation', 'Organization is required') + const { organizationId } = context + if (!organizationId) throw new OrchestrationError('validation', 'Organization is required') const source = LIVE_SEARCH_SOURCE_TYPES.find(([type]) => type === input.connectorType) if (!source) { throw new OrchestrationError('validation', 'This integration is not supported by Sim Search') @@ -276,7 +276,7 @@ export const approveSearchIntegration = defineAuthorizedKnowledgeUseCase({ /** Slack sign-in belongs to Slack setup (bot install or admin verification), not approval. */ if (memberProvider === 'slack') { const group = await ensureWorkspaceAccountsGroup( - { kind: 'organization', organizationId: context.organizationId! }, + { kind: 'organization', organizationId }, requirePrincipalSubjectUserId(principal), undefined, tx @@ -333,7 +333,10 @@ export const approveSearchIntegration = defineAuthorizedKnowledgeUseCase({ action: AuditAction.CREDENTIAL_GROUP_UPDATED, resourceType: AuditResourceType.CREDENTIAL_GROUP, resourceId: result.memberAccounts.groupId, - description: `Configured ${result.connectorType} member sign-in for Sim Search`, + description: + result.connectorType === 'slack' + ? 'Created connected accounts for Slack Sim Search' + : `Configured ${result.connectorType} member sign-in for Sim Search`, metadata: { connectorType: result.connectorType }, }, ] diff --git a/apps/sim/lib/knowledge/application/slack-search/lifecycle.test.ts b/apps/sim/lib/knowledge/application/slack-search/lifecycle.test.ts index 5d2b4e06a13..4d788323980 100644 --- a/apps/sim/lib/knowledge/application/slack-search/lifecycle.test.ts +++ b/apps/sim/lib/knowledge/application/slack-search/lifecycle.test.ts @@ -181,10 +181,7 @@ describe('Slack access revocation', () => { expect(m.app).not.toHaveBeenCalled() } ) - it.each([ - { type: 'app_uninstalled' as const }, - { type: 'tokens_revoked' as const, tokens: { bot: ['UBOT'] } }, - ])('ignores stale installation-wide revocations before any writes: %j', async (event) => { + const queueReinstalledBot = () => { resetDbChainMock() queueTableRows(slackSearchInstallation, [ { @@ -196,7 +193,42 @@ describe('Slack access revocation', () => { updatedAt: new Date(input.event_time * 1000 + 1000), }, ]) - await revokeSlackSearchAccess.execute({ principal, input: { ...input, event } }) + } + it('ignores a stale bot revocation before any writes', async () => { + queueReinstalledBot() + await revokeSlackSearchAccess.execute({ + principal, + input: { ...input, event: { type: 'tokens_revoked', tokens: { bot: ['UBOT'] } } }, + }) + expect(dbChainMockFns.update).not.toHaveBeenCalled() + }) + it('keeps a reinstalled bot but revokes shared-app grants issued before a stale uninstall', async () => { + queueReinstalledBot() + await revokeSlackSearchAccess.execute({ principal, input }) + expect(dbChainMockFns.update.mock.calls.map(([table]) => table)).toEqual([credential]) + const memberGrants = dbChainMockFns.where.mock.calls[1][0] + expect(memberGrants.conditions).toEqual( + expect.arrayContaining([ + { type: 'eq', left: credential.authorizationAppId, right: 'slack:A1:T1' }, + { + type: 'or', + conditions: [ + { type: 'isNull', column: credential.grantedAt }, + { type: 'lte', left: credential.grantedAt, right: new Date(input.event_time * 1000) }, + ], + }, + ]) + ) + expect(memberGrants.conditions).not.toContainEqual( + expect.objectContaining({ left: credential.organizationId }) + ) + }) + it('ignores a stale custom-app uninstall before any writes', async () => { + m.app.mockResolvedValue({ + app: { id: 'A1', kind: 'custom', revision: 'r1', organizationId: 'org' }, + }) + queueReinstalledBot() + await revokeSlackSearchAccess.execute({ principal, input }) expect(dbChainMockFns.update).not.toHaveBeenCalled() }) it.each([{ oauth: ['U1'] }, { oauth: ['U1'], bot: ['UBOT'] }])( diff --git a/apps/sim/lib/knowledge/application/slack-search/lifecycle.ts b/apps/sim/lib/knowledge/application/slack-search/lifecycle.ts index a7f012ddd1c..52e67f18e3e 100644 --- a/apps/sim/lib/knowledge/application/slack-search/lifecycle.ts +++ b/apps/sim/lib/knowledge/application/slack-search/lifecycle.ts @@ -5,6 +5,7 @@ import { and, eq, inArray, isNull, lte, or, sql } from 'drizzle-orm' import { z } from 'zod' import type { OperationUseCase } from '@/lib/core/application/operation' import { OrchestrationError } from '@/lib/core/orchestration/types' +import type { DbTransaction } from '@/lib/db/types' import { loadSlackAppConfiguration } from '@/lib/slack-search/app-configuration' const id = z.string().min(1).max(200) @@ -65,6 +66,11 @@ export const revokeSlackSearchAccess: OperationUseCase< or(isNull(credential.grantedAt), lte(credential.grantedAt, occurredAt)), ...(uninstall ? [] : [inArray(credential.providerSubjectId, revokedUsers)]) ) + const revokeMemberGrants = (tx: DbTransaction, organizationId?: string) => + tx + .update(credential) + .set({ managedOauthStatus: 'needs_reauth', updatedAt: new Date() }) + .where(memberGrants(organizationId)) await db.transaction(async (tx) => { const [installation] = await tx .select() @@ -79,17 +85,17 @@ export const revokeSlackSearchAccess: OperationUseCase< .limit(1) /** Member sign-in through the shared app needs no Search bot, so it has no installation. */ if (!installation) { - if (sharedApp && (uninstall || revokedUsers.length)) - await tx - .update(credential) - .set({ managedOauthStatus: 'needs_reauth', updatedAt: new Date() }) - .where(memberGrants()) + if (sharedApp && (uninstall || revokedUsers.length)) await revokeMemberGrants(tx) return } if (app.app.kind === 'custom' && app.app.organizationId !== installation.organizationId) throw new Error('Slack installation ownership is inconsistent') const staleInstallation = installation.updatedAt > occurredAt - if (uninstall && staleInstallation) return + if (uninstall && staleInstallation) { + /** A reinstall keeps its bot, but shared-app grants issued before the uninstall are gone. */ + if (sharedApp) await revokeMemberGrants(tx) + return + } const revokeBot = !staleInstallation && (uninstall || @@ -97,10 +103,10 @@ export const revokeSlackSearchAccess: OperationUseCase< (input.event.tokens.bot ?? []).includes(installation.botUserId))) let revokedMemberIds: string[] = [] if (uninstall || revokedUsers.length) { - const revokeCredentials = tx - .update(credential) - .set({ managedOauthStatus: 'needs_reauth', updatedAt: new Date() }) - .where(memberGrants(sharedApp ? undefined : installation.organizationId)) + const revokeCredentials = revokeMemberGrants( + tx, + sharedApp ? undefined : installation.organizationId + ) if (revokeBot) await revokeCredentials else { const revokedCredentials = await revokeCredentials.returning({ diff --git a/scripts/check-explicit-any.baseline.json b/scripts/check-explicit-any.baseline.json index ca5f3557445..678c51faf62 100644 --- a/scripts/check-explicit-any.baseline.json +++ b/scripts/check-explicit-any.baseline.json @@ -1422,7 +1422,7 @@ "apps/sim/lib/data-drains/sources/copilot-runs.ts": 2, "apps/sim/lib/data-drains/sources/job-logs.ts": 2, "apps/sim/lib/data-drains/sources/workflow-logs.ts": 2, - "apps/sim/lib/desktop/source-browser.ts": 3, + "apps/sim/lib/desktop/source-browser.ts": 1, "apps/sim/lib/desktop/source-connect.ts": 1, "apps/sim/lib/desktop/source-request.integration.ts": 1, "apps/sim/lib/embeddings/client.ts": 4, From 4e347bbf9ef9dc83da030a588677c64cca9bb25f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 13 Oct 2026 12:22:33 -0700 Subject: [PATCH 3/3] fix(slack): cancel stale-uninstall turns for this installation's revoked members --- .../slack-search/lifecycle.test.ts | 35 +++++++++++++++++-- .../application/slack-search/lifecycle.ts | 15 ++++---- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/apps/sim/lib/knowledge/application/slack-search/lifecycle.test.ts b/apps/sim/lib/knowledge/application/slack-search/lifecycle.test.ts index 4d788323980..95c4499833e 100644 --- a/apps/sim/lib/knowledge/application/slack-search/lifecycle.test.ts +++ b/apps/sim/lib/knowledge/application/slack-search/lifecycle.test.ts @@ -39,7 +39,9 @@ beforeEach(() => { }) describe('Slack access revocation', () => { it('cancels only affected members without rotating the shared installation revision', async () => { - dbChainMockFns.returning.mockResolvedValueOnce([{ providerSubjectId: 'U1' }]) + dbChainMockFns.returning.mockResolvedValueOnce([ + { organizationId: 'org', providerSubjectId: 'U1' }, + ]) await revokeSlackSearchAccess.execute({ principal, input: { ...input, event: { type: 'tokens_revoked', tokens: { oauth: ['U1', 'U2'] } } }, @@ -223,6 +225,33 @@ describe('Slack access revocation', () => { expect.objectContaining({ left: credential.organizationId }) ) }) + it('cancels stale-uninstall turns only for members revoked in the installation organization', async () => { + queueReinstalledBot() + dbChainMockFns.returning.mockResolvedValueOnce([ + { organizationId: 'org', providerSubjectId: 'U1' }, + { organizationId: 'other-org', providerSubjectId: 'U2' }, + ]) + await revokeSlackSearchAccess.execute({ principal, input }) + expect(dbChainMockFns.update.mock.calls.map(([table]) => table)).toEqual([ + credential, + slackSearchTurn, + ]) + expect(dbChainMockFns.where).toHaveBeenLastCalledWith({ + type: 'and', + conditions: [ + { type: 'eq', left: slackSearchTurn.installationId, right: 'i1' }, + { type: 'inArray', column: slackSearchTurn.status, values: ['pending', 'running'] }, + { + type: 'inArray', + column: expect.objectContaining({ + strings: ['', " #>> '{message,userId}'"], + values: [slackSearchTurn.payload], + }), + values: ['U1'], + }, + ], + }) + }) it('ignores a stale custom-app uninstall before any writes', async () => { m.app.mockResolvedValue({ app: { id: 'A1', kind: 'custom', revision: 'r1', organizationId: 'org' }, @@ -245,7 +274,9 @@ describe('Slack access revocation', () => { updatedAt: new Date(input.event_time * 1000 + 1000), }, ]) - dbChainMockFns.returning.mockResolvedValueOnce([{ providerSubjectId: 'U1' }]) + dbChainMockFns.returning.mockResolvedValueOnce([ + { organizationId: 'org', providerSubjectId: 'U1' }, + ]) await revokeSlackSearchAccess.execute({ principal, input: { ...input, event: { type: 'tokens_revoked', tokens } }, diff --git a/apps/sim/lib/knowledge/application/slack-search/lifecycle.ts b/apps/sim/lib/knowledge/application/slack-search/lifecycle.ts index 52e67f18e3e..26e5738d605 100644 --- a/apps/sim/lib/knowledge/application/slack-search/lifecycle.ts +++ b/apps/sim/lib/knowledge/application/slack-search/lifecycle.ts @@ -91,11 +91,8 @@ export const revokeSlackSearchAccess: OperationUseCase< if (app.app.kind === 'custom' && app.app.organizationId !== installation.organizationId) throw new Error('Slack installation ownership is inconsistent') const staleInstallation = installation.updatedAt > occurredAt - if (uninstall && staleInstallation) { - /** A reinstall keeps its bot, but shared-app grants issued before the uninstall are gone. */ - if (sharedApp) await revokeMemberGrants(tx) - return - } + /** A reinstall keeps its bot, but shared-app grants issued before the uninstall are gone. */ + if (uninstall && staleInstallation && !sharedApp) return const revokeBot = !staleInstallation && (uninstall || @@ -110,10 +107,14 @@ export const revokeSlackSearchAccess: OperationUseCase< if (revokeBot) await revokeCredentials else { const revokedCredentials = await revokeCredentials.returning({ + organizationId: credential.organizationId, providerSubjectId: credential.providerSubjectId, }) - revokedMemberIds = revokedCredentials.flatMap(({ providerSubjectId }) => - providerSubjectId ? [providerSubjectId] : [] + /** Only this installation's members have turns on its bot. */ + revokedMemberIds = revokedCredentials.flatMap(({ organizationId, providerSubjectId }) => + providerSubjectId && organizationId === installation.organizationId + ? [providerSubjectId] + : [] ) } }