From 328a37b735732ab0945a4c02b4b0ddfe8d6a2b9a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 1 Oct 2026 14:27:39 -0700 Subject: [PATCH 1/3] fix(slack): verify Search permissions before changing active grants --- .../integrations/live-member-integrations.tsx | 3 + .../live-search-settings.test.tsx | 2 +- .../integrations/live-search-settings.tsx | 21 +- .../components/slack-managed-users-modal.tsx | 50 +++- .../application/slack-managed-users.ts | 1 + apps/sim/lib/credential-groups/service.ts | 27 +- .../slack-managed-user-scopes.ts | 5 + .../credential-groups/slack-managed-users.ts | 60 +++- .../search-mcp-setup.integration.ts | 274 +++++++++++++++--- 9 files changed, 355 insertions(+), 88 deletions(-) diff --git a/apps/sim/app/o/[organizationId]/integrations/live-member-integrations.tsx b/apps/sim/app/o/[organizationId]/integrations/live-member-integrations.tsx index fc8a37a4ea5..887df98a989 100644 --- a/apps/sim/app/o/[organizationId]/integrations/live-member-integrations.tsx +++ b/apps/sim/app/o/[organizationId]/integrations/live-member-integrations.tsx @@ -1,6 +1,7 @@ 'use client' import { Chip, toast } from '@sim/emcn' +import { hasSlackSearchUserScopes } from '@/lib/credential-groups/slack-managed-user-scopes' import { LIVE_SEARCH_SCOPE_FIELDS } from '@/lib/sim-search/live/policy-schema' import { liveSearchProviderForCredential } from '@/lib/sim-search/live/provider-catalog' import { @@ -133,6 +134,8 @@ export function LiveMemberIntegrations({ organizationId, search }: LiveMemberInt Boolean(option || server) && approved && (!option || option.configurationStatus === 'ready') && + (provider !== 'slack' || + (option?.provider === 'slack' && hasSlackSearchUserScopes(option.requiredScopes))) && ((provider !== 'hubspot' && provider !== 'zoom') || data.availableMcpConnectors.includes(provider)) const scope = diff --git a/apps/sim/app/o/[organizationId]/settings/components/integrations/live-search-settings.test.tsx b/apps/sim/app/o/[organizationId]/settings/components/integrations/live-search-settings.test.tsx index 6b1c664d791..bb31f049282 100644 --- a/apps/sim/app/o/[organizationId]/settings/components/integrations/live-search-settings.test.tsx +++ b/apps/sim/app/o/[organizationId]/settings/components/integrations/live-search-settings.test.tsx @@ -261,7 +261,7 @@ describe('live search administration', () => { it('opens Slack setup in Sources instead of its redirected service-account page', async () => { mocks.policies.mockReturnValue({ data: [{ connectorType: 'slack', approved: true }] }) await render() - await act(async () => button('Slack app')!.click()) + await act(async () => button('Verify permissions')!.click()) expect(mockPush).not.toHaveBeenCalled() expect(container.querySelector('a[href*="providers/slack"]')).toBeNull() await act(async () => diff --git a/apps/sim/app/o/[organizationId]/settings/components/integrations/live-search-settings.tsx b/apps/sim/app/o/[organizationId]/settings/components/integrations/live-search-settings.tsx index 2170b304e25..838533ce9d6 100644 --- a/apps/sim/app/o/[organizationId]/settings/components/integrations/live-search-settings.tsx +++ b/apps/sim/app/o/[organizationId]/settings/components/integrations/live-search-settings.tsx @@ -7,6 +7,7 @@ import { useRouter } from 'next/navigation' import { useQueryState } from 'nuqs' import { SettingsPanel } from '@/components/settings/settings-panel' import type { SearchIntegrationApproval } from '@/lib/api/contracts/knowledge/search-integrations' +import { hasSlackSearchUserScopes } from '@/lib/credential-groups/slack-managed-user-scopes' import { organizationRoutes } from '@/lib/navigation/paths' import { defaultLiveSearchPolicy, @@ -163,6 +164,14 @@ export function LiveSearchSettings() { const memberProvider = liveSearchMemberAccountProvider(type) const mcpProvider = liveSearchMcpConnector(type) const group = accounts.data?.credentialGroup + const slackOption = group?.options.find((option) => option.provider === 'slack') + const needsSlackSetup = + type === 'slack' && + accounts.data && + (group?.status !== 'active' || + slackOption?.status !== 'active' || + slackOption.configurationStatus !== 'ready' || + !hasSlackSearchUserScopes(slackOption.requiredScopes)) const needsMemberSetup = integration.available !== false && accounts.data && @@ -193,7 +202,13 @@ export function LiveSearchSettings() { iconVariant='custom' icon={} title={meta.name} - description={integration.available === false ? 'Currently unavailable' : scope} + description={ + integration.available === false + ? 'Currently unavailable' + : needsSlackSetup + ? 'Member accounts ยท Verify Search permissions' + : scope + } trailing={
{serviceAccount && ( @@ -206,7 +221,9 @@ export function LiveSearchSettings() { )} {type === 'slack' && ( - void setConnectedAccounts('slack')}>Slack app + void setConnectedAccounts('slack')}> + {needsSlackSetup ? 'Verify permissions' : 'Slack app'} + )} {needsMemberSetup && ( app.appId === appId) ?? (availableApps.length === 1 && !appId ? availableApps[0] : undefined) const sharedAppInstalled = organizationSetup && selectedApp?.appKind === 'shared' + const searchPolicies = useSearchIntegrations(organizationId ?? '', { + enabled: open && sharedAppInstalled, + }) + const searchApproved = searchPolicies.data?.some( + (policy) => policy.connectorType === 'slack' && policy.approved + ) const accounts = useOrganizationAccounts(open && sharedAppInstalled ? organizationId : undefined) const memberGroup = accounts.data?.credentialGroup - const memberOption = memberGroup?.options.find( - (option) => option.provider === 'slack' && option.status === 'active' - ) + const memberOption = memberGroup?.options.find((option) => option.provider === 'slack') const sharedAppCanAuthorize = Boolean( sharedAppInstalled && apps.isSuccess && @@ -123,11 +129,22 @@ export function SlackManagedUsersModal({ accounts.isSuccess && !accounts.isFetching && !accounts.error && - memberGroup?.id === credentialGroupId + searchPolicies.isSuccess && + !searchPolicies.isFetching && + !searchPolicies.error && + memberGroup?.id === credentialGroupId && + memberOption?.status === 'active' ) - const sharedAppReady = sharedAppCanAuthorize && memberOption?.configurationStatus === 'ready' + const searchPermissionsMissing = + searchApproved && !hasSlackSearchUserScopes(memberOption?.requiredScopes) + const sharedAppReady = + sharedAppCanAuthorize && + memberOption?.configurationStatus === 'ready' && + !searchPermissionsMissing const sharedAppNeedsUpdate = - sharedAppCanAuthorize && memberOption?.configurationStatus === 'needs_update' + sharedAppCanAuthorize && + (memberOption?.configurationStatus === 'needs_update' || + (memberOption?.configurationStatus === 'ready' && searchPermissionsMissing)) const [clientId, setClientId] = useState('') const [clientSecret, setClientSecret] = useState('') const [pending, setPending] = useState(false) @@ -389,8 +406,19 @@ export function SlackManagedUsersModal({ const needsApp = organizationSetup && apps.isSuccess && availableApps.length === 0 const checkingSetup = apps.isPending || - (sharedAppInstalled && (apps.isFetching || accounts.isPending || accounts.isFetching)) - const failedSetup = apps.error ? apps : sharedAppInstalled && accounts.error ? accounts : null + (sharedAppInstalled && + (apps.isFetching || + accounts.isPending || + accounts.isFetching || + searchPolicies.isPending || + searchPolicies.isFetching)) + const failedSetup = apps.error + ? apps + : sharedAppInstalled && searchPolicies.error + ? searchPolicies + : sharedAppInstalled && accounts.error + ? accounts + : null const title = organizationSetup ? 'Set up Slack app' : 'Set up Slack' const primaryLabel = isLoading ? 'Loading...' @@ -405,7 +433,7 @@ export function SlackManagedUsersModal({ (!organizationSetup && !selectedBot) || pending || (organizationSetup - ? apps.isPending || Boolean(apps.error) || !selectedApp || !requiredScopes.length + ? checkingSetup || Boolean(failedSetup) || !selectedApp || !requiredScopes.length : !clientId.trim() || !clientSecret.trim()) return ( @@ -472,9 +500,9 @@ export function SlackManagedUsersModal({

{sharedAppInstalled ? sharedAppNeedsUpdate - ? 'Member access is outdated. Update it so members can reconnect their Slack accounts.' + ? 'Verify member permissions. Members will need to reconnect if their app or permissions change.' : 'The Sim Search installation needs attention. Manage the app to finish setup.' - : 'Verify member authorization for the installed app. Members can then search the Slack conversations they can access.'} + : 'Verify member permissions. Members will need to reconnect if their app or permissions change.'}

{selectedApp && ( setAppSetupOpen(true)} disabled={pending}> diff --git a/apps/sim/lib/credential-groups/application/slack-managed-users.ts b/apps/sim/lib/credential-groups/application/slack-managed-users.ts index 07e9cb69184..908b2f2965e 100644 --- a/apps/sim/lib/credential-groups/application/slack-managed-users.ts +++ b/apps/sim/lib/credential-groups/application/slack-managed-users.ts @@ -169,6 +169,7 @@ export const completeSlackCredentialGroupConfiguration: OperationUseCase< attempt.expectedAppId !== pending.expectedAppId || attempt.expectedTeamId !== pending.expectedTeamId || attempt.appRevision !== pending.appRevision || + attempt.searchApprovalUpdatedAt !== pending.searchApprovalUpdatedAt || attempt.clientId !== pending.clientId || attempt.redirectUri !== pending.redirectUri || credentialGroupScopePolicyVersion(attempt.requiredScopes) !== diff --git a/apps/sim/lib/credential-groups/service.ts b/apps/sim/lib/credential-groups/service.ts index 72b09db19f1..4dc75a59fe0 100644 --- a/apps/sim/lib/credential-groups/service.ts +++ b/apps/sim/lib/credential-groups/service.ts @@ -401,7 +401,7 @@ export async function ensureWorkspaceAccountsGroup( } } -/** Adds a provider or extends its required consent during an explicit administrator action. */ +/** Adds a provider without changing the verified consent policy of existing connections. */ export async function addOrganizationAccountProvider( organizationId: string, userId: string, @@ -430,31 +430,6 @@ export async function addOrganizationAccountProvider( 'validation', `Enable ${option.label} in Connected accounts first` ) - if (option.requiredScopes) { - const previousScopes = - current.provider === 'slack' - ? resolveSlackManagedUserScopes(current.requiredScopes) - : current.requiredScopes - const requiredScopes = [...new Set([...previousScopes, ...option.requiredScopes])] - const scopeVersion = credentialGroupScopePolicyVersion(requiredScopes) - if (!scopesEqual(requiredScopes, previousScopes) || scopeVersion !== current.scopeVersion) { - const [updated] = await executor - .update(credentialGroup) - .set({ - options: existing.options.map((entry) => - entry.id === current.id ? { ...entry, requiredScopes, scopeVersion } : entry - ), - updatedAt: new Date(), - }) - .where( - and(eq(credentialGroup.id, group.id), resourceScopeCondition(credentialGroup, scope)) - ) - .returning({ id: credentialGroup.id }) - if (!updated) throw new Error('Connected accounts policy update returned no row') - await invalidateOptionGrants(executor, group.id, [current.id]) - return { groupId: group.id, changed: true } - } - } return { groupId: group.id, changed: group.created } } if ( diff --git a/apps/sim/lib/credential-groups/slack-managed-user-scopes.ts b/apps/sim/lib/credential-groups/slack-managed-user-scopes.ts index 1193daefb55..dad1c2bb1e9 100644 --- a/apps/sim/lib/credential-groups/slack-managed-user-scopes.ts +++ b/apps/sim/lib/credential-groups/slack-managed-user-scopes.ts @@ -59,6 +59,11 @@ export const SLACK_SEARCH_USER_SCOPES = [ ...SLACK_RTS_USER_SCOPES, ] as const +/** Search readiness is separate from a connection's existing workflow permissions. */ +export function hasSlackSearchUserScopes(scopes: readonly string[] | undefined): boolean { + return SLACK_SEARCH_USER_SCOPES.every((scope) => scopes?.includes(scope)) +} + /** Existing workflow options retain their scope policy; every user grant must attest identity. */ export function resolveSlackManagedUserScopes(requiredScopes?: readonly string[]): string[] { return [ diff --git a/apps/sim/lib/credential-groups/slack-managed-users.ts b/apps/sim/lib/credential-groups/slack-managed-users.ts index e7a12675a1e..9535a18a08c 100644 --- a/apps/sim/lib/credential-groups/slack-managed-users.ts +++ b/apps/sim/lib/credential-groups/slack-managed-users.ts @@ -4,6 +4,7 @@ import { credential, credentialGroup, credentialGroupEnrollment, + organizationSearchIntegration, slackApp, slackSearchInstallation, } from '@sim/db/schema' @@ -38,7 +39,7 @@ import { getSharedSlackSearchAppConfiguration } from '@/lib/slack-search/shared- const logger = createLogger('SlackManagedUsers') const SLACK_MANAGED_USERS_ATTEMPT_TTL_MS = 10 * 60 * 1000 -const SLACK_MANAGED_USERS_ATTEMPT_VERSION = 4 as const +const SLACK_MANAGED_USERS_ATTEMPT_VERSION = 5 as const const MAX_SLACK_RESPONSE_BYTES = 64 * 1024 const CONSUME_SCRIPT = ` local value = redis.call('GET', KEYS[1]) @@ -61,6 +62,7 @@ interface SlackCustomBotSecret { type StoredSlackManagedUsersAttempt = { appRevision?: string + searchApprovalUpdatedAt?: number version: typeof SLACK_MANAGED_USERS_ATTEMPT_VERSION workspaceId?: string organizationId?: string @@ -82,6 +84,7 @@ type StoredSlackManagedUsersAttempt = { export interface SlackManagedUsersAttempt { appRevision?: string + searchApprovalUpdatedAt?: number workspaceId?: string organizationId?: string userId: string @@ -161,6 +164,10 @@ function isStoredAttempt(value: unknown): value is StoredSlackManagedUsersAttemp typeof candidate.credentialGroupId === 'string' && typeof candidate.credentialGroupUpdatedAt === 'number' && (candidate.appRevision === undefined || typeof candidate.appRevision === 'string') && + (candidate.searchApprovalUpdatedAt === undefined || + (candidate.organizationId !== undefined && + typeof candidate.searchApprovalUpdatedAt === 'number' && + Number.isFinite(candidate.searchApprovalUpdatedAt))) && (candidate.organizationId !== undefined ? candidate.slackBotCredentialId === undefined && candidate.slackBotCredentialUpdatedAt === undefined @@ -497,6 +504,7 @@ export async function createSlackManagedUsersAttempt(params: { let clientId = params.clientId let clientSecret = params.clientSecret let appRevision: string | undefined + let searchApprovalUpdatedAt: number | undefined if (scope.kind === 'organization') { if (params.slackBotCredentialId || !params.appId || params.clientId || params.clientSecret) throw new SlackManagedUsersError( @@ -548,6 +556,23 @@ export async function createSlackManagedUsersAttempt(params: { ? SLACK_SEARCH_USER_SCOPES : existingOption.requiredScopes ) + const [searchApproval] = await db + .select({ + approved: organizationSearchIntegration.approved, + updatedAt: organizationSearchIntegration.updatedAt, + }) + .from(organizationSearchIntegration) + .where( + and( + eq(organizationSearchIntegration.organizationId, scope.organizationId), + eq(organizationSearchIntegration.connectorType, 'slack') + ) + ) + .limit(1) + if (searchApproval?.approved) { + requiredScopes = [...new Set([...requiredScopes, ...SLACK_SEARCH_USER_SCOPES])] + searchApprovalUpdatedAt = searchApproval.updatedAt.getTime() + } } else { if (!params.slackBotCredentialId) throw new SlackManagedUsersError('Select a custom Slack bot.', 'invalid_response') @@ -565,6 +590,11 @@ export async function createSlackManagedUsersAttempt(params: { } if (!clientId || !clientSecret) throw new SlackManagedUsersError('Slack client credentials are required.', 'invalid_client') + if (requiredScopes.length > 100) + throw new SlackManagedUsersError( + 'The combined Slack request has too many permissions.', + 'invalid_response' + ) const redis = requireRedis() const state = generateId() const redirectUri = getSlackManagedUsersRedirectUri() @@ -583,6 +613,7 @@ export async function createSlackManagedUsersAttempt(params: { expectedTeamId: identity.teamId, clientId, ...(appRevision ? { appRevision } : {}), + ...(searchApprovalUpdatedAt !== undefined ? { searchApprovalUpdatedAt } : {}), ...(sharedApp && scope.kind === 'organization' ? { credentialSource: 'environment' as const, organizationId: scope.organizationId } : { encryptedClientSecret: (await encryptSecret(clientSecret)).encrypted }), @@ -660,6 +691,9 @@ async function parseSlackManagedUsersAttempt( expectedTeamId: parsed.expectedTeamId, clientId: parsed.clientId, ...(parsed.appRevision ? { appRevision: parsed.appRevision } : {}), + ...(parsed.searchApprovalUpdatedAt !== undefined + ? { searchApprovalUpdatedAt: parsed.searchApprovalUpdatedAt } + : {}), clientSecret, redirectUri: parsed.redirectUri, requiredScopes: parsed.requiredScopes, @@ -779,6 +813,30 @@ export async function exchangeAndConfigureSlackManagedUsers(params: { 'invalid_state' ) } + if (params.attempt.organizationId && params.attempt.searchApprovalUpdatedAt !== undefined) { + const [approval] = await tx + .select({ + approved: organizationSearchIntegration.approved, + updatedAt: organizationSearchIntegration.updatedAt, + }) + .from(organizationSearchIntegration) + .where( + and( + eq(organizationSearchIntegration.organizationId, params.attempt.organizationId), + eq(organizationSearchIntegration.connectorType, 'slack') + ) + ) + .limit(1) + .for('share') + if ( + !approval?.approved || + approval.updatedAt.getTime() !== params.attempt.searchApprovalUpdatedAt + ) + throw new SlackManagedUsersError( + 'Search approval changed during authorization. Start again.', + 'invalid_state' + ) + } if (params.attempt.workspaceId) { if (!params.attempt.slackBotCredentialId) throw new SlackManagedUsersError('Workspace Slack bot is missing.', 'invalid_state') diff --git a/apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts b/apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts index fc9bc7f2756..caf2148ceb6 100644 --- a/apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts +++ b/apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts @@ -10,8 +10,11 @@ import { organization, organizationSearchIntegration, resourcePolicy, + slackApp, + slackSearchInstallation, user, } from '@sim/db/schema' +import { readTestRedisUrl } from '@sim/db/testing/test-infrastructure' import * as dns from '@sim/security/dns' import { sha256Hex } from '@sim/security/hash' import { createSessionPrincipal } from '@sim/testing/factories/principal.factory' @@ -23,16 +26,29 @@ import { eq, inArray, sql } from 'drizzle-orm' import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' import { listSearchIntegrationsContract } from '@/lib/api/contracts/knowledge/search-integrations' 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 { credentialGroupScopePolicyVersion } from '@/lib/credential-groups/provider-adapter' import { + decryptCredentialGroupProviderConfiguration, emptyCredentialGroupProviderConfiguration, encryptCredentialGroupProviderConfiguration, } from '@/lib/credential-groups/provider-configuration' import { getCredentialGroup } from '@/lib/credential-groups/service' -import { SLACK_MANAGED_USER_SCOPES } from '@/lib/credential-groups/slack-managed-user-scopes' +import { + SLACK_MANAGED_USER_SCOPES, + SLACK_SEARCH_USER_SCOPES, +} from '@/lib/credential-groups/slack-managed-user-scopes' import { createOrganizationAccountsGroup } from '@/lib/credential-groups/workspace-accounts' import { deleteConnectionCredential } from '@/lib/credentials/deletion' +import { + encryptManagedOAuthTokenSet, + resolveManagedOAuthToken, +} from '@/lib/credentials/managed-oauth' import { acquireAdvisoryXactLock, tryAcquireAdvisoryXactLock } from '@/lib/db/advisory-locks' import { approveSearchIntegration, @@ -43,14 +59,14 @@ import { type GitHubInstallationBinding, } from '@/lib/oauth/github-installation-types' import { defaultLiveSearchPolicy } from '@/lib/sim-search/live/policy-schema' -import { SLACK_RTS_USER_SCOPES } from '@/lib/sim-search/live/scopes' /** - * Real authorization, transactions, constraints and persistence; only DNS is a fixture. + * Real authorization, PostgreSQL, Redis and token resolution; DNS and Slack HTTP use fixtures. * Run with TEST_DATABASE_URL naming a disposable database and pass * --outputFile.json="$SEARCH_MCP_SETUP_REPORT_PATH" for a caller-selected JSON report. */ describe('atomic organization live Search MCP setup', () => { + let restoreSlackHttp: (() => void) | undefined let ids: { organization: string; owner: string; member: string; outsider: string } beforeAll(() => { @@ -68,6 +84,7 @@ describe('atomic organization live Search MCP setup', () => { beforeEach(async () => { Object.assign(env, { ZOOM_SEARCH: false, + REDIS_URL: readTestRedisUrl(), }) ids = { organization: generateId(), @@ -98,10 +115,13 @@ describe('atomic organization live Search MCP setup', () => { }) afterEach(async () => { + restoreSlackHttp?.() + restoreSlackHttp = undefined await db.delete(organization).where(eq(organization.id, ids.organization)) await db.delete(user).where(inArray(user.id, [ids.owner, ids.member, ids.outsider])) }) afterAll(async () => { + await closeRedisConnection() await db.$client.end() }) @@ -146,7 +166,7 @@ describe('atomic organization live Search MCP setup', () => { id: generateId(), provider: 'slack', label: 'Slack', - authorizationAppId: 'slack:fixture-app:fixture-team', + authorizationAppId: 'slack:A_FIXTURE:T_FIXTURE', requiredScopes: [...scopes], scopeVersion: credentialGroupScopePolicyVersion([...scopes]), required: false, @@ -164,8 +184,8 @@ describe('atomic organization live Search MCP setup', () => { slack: { clientId: 'fixture-client', clientSecret: 'fixture-secret', - appId: 'fixture-app', - teamId: 'fixture-team', + appId: 'A_FIXTURE', + teamId: 'T_FIXTURE', scopes: [...scopes], verifiedAt: new Date().toISOString(), }, @@ -183,7 +203,7 @@ describe('atomic organization live Search MCP setup', () => { invitationExpiresAt: new Date(Date.now() + 60_000), invitedAt: new Date(), }) - const encrypted = (await encryptSecret('{"access_token":"fixture-token"}')).encrypted + const encrypted = await encryptManagedOAuthTokenSet({ accessToken: 'fixture-token' }) await db.insert(credential).values( [option, other].map((entry) => ({ id: generateId(), @@ -206,51 +226,196 @@ describe('atomic organization live Search MCP setup', () => { return { groupId: group.id, optionId: option.id, otherOptionId: other.id } } + async function seedSlackAuthorization(scopes: readonly string[] = SLACK_MANAGED_USER_SCOPES) { + const seeded = await seedWorkflowSlack(scopes) + const secret = (await encryptSecret('fixture-secret')).encrypted + await db.insert(slackApp).values({ + id: 'A_FIXTURE', + organizationId: ids.organization, + kind: 'custom', + clientId: 'fixture-client', + encryptedClientSecret: secret, + encryptedSigningSecret: secret, + 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: secret, + }) + await db.insert(slackSearchInstallation).values({ + id: generateId(), + organizationId: ids.organization, + credentialId: botId, + appId: 'A_FIXTURE', + slackAppId: 'A_FIXTURE', + teamId: 'T_FIXTURE', + teamName: 'Fixture team', + botUserId: 'B_FIXTURE', + credentialVersion: generateId(), + revision: generateId(), + }) + const before = await snapshot() + const connection = before.credentials.find( + (entry) => entry.credentialGroupOptionId === seeded.optionId + )! + const principal = createSessionPrincipal({ userId: ids.owner, sessionId: generateId() }) + return { + ...seeded, + before, + resolveToken: () => + resolveManagedOAuthToken({ + credentialId: connection.id, + organizationId: ids.organization, + expectedProviderId: 'slack', + requiredScopes: ['chat:write'], + }), + start: () => + startSlackCredentialGroupConfiguration.execute({ + principal, + input: { + organizationId: ids.organization, + credentialGroupId: seeded.groupId, + appId: 'A_FIXTURE', + teamId: 'T_FIXTURE', + }, + }), + complete: (state: string, providerError?: string) => + completeSlackCredentialGroupConfiguration.execute({ + principal, + input: { state, ...(providerError ? { providerError } : { code: 'fixture-code' }) }, + }), + } + } + + function provideSlackConsent(scopes: readonly 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: 'A_FIXTURE', + team: { id: 'T_FIXTURE', name: 'Fixture team' }, + authed_user: { + id: 'U_FIXTURE', + access_token: 'fixture-verification-token', + token_type: 'user', + scope: scopes.join(','), + }, + }) + case 'https://slack.com/api/auth.test': + return Response.json({ ok: true, team_id: 'T_FIXTURE', user_id: 'U_FIXTURE' }) + case 'https://slack.com/api/users.info': + return Response.json({ + ok: true, + user: { id: 'U_FIXTURE', profile: { email: 'member@fixture.test' } }, + }) + case 'https://slack.com/api/auth.revoke': + return Response.json({ ok: true, revoked: true }) + default: + throw new Error(`Unexpected OAuth fixture request: ${url}`) + } + }) + restoreSlackHttp = () => spy.mockRestore() + } + it.each([ { name: 'workflow policy', scopes: SLACK_MANAGED_USER_SCOPES }, { name: 'custom policy', scopes: ['chat:write', 'users:read', 'users:read.email'] }, - ])( - 'upgrades an existing Slack $name only through explicit Search approval', - async ({ scopes }) => { - const seeded = await seedWorkflowSlack(scopes) - const before = await snapshot() - await approve('slack') - const state = await snapshot() - const upgraded = state.groups[0].options.find((option) => option.id === seeded.optionId)! - expect(upgraded.requiredScopes).toEqual( - expect.arrayContaining([...scopes, ...SLACK_RTS_USER_SCOPES]) - ) - expect(upgraded.scopeVersion).not.toBe(before.groups[0].options[0].scopeVersion) - expect(state.groups[0].options.find((option) => option.id === seeded.otherOptionId)).toEqual( - before.groups[0].options[1] - ) - expect(state.policies).toEqual(before.policies) - expect(state.groups[0].encryptedProviderConfiguration).toBe( - before.groups[0].encryptedProviderConfiguration - ) - expect( - state.credentials.find((entry) => entry.credentialGroupOptionId === seeded.optionId) - ?.managedOauthStatus - ).toBe('needs_reauth') - expect( - state.credentials.find((entry) => entry.credentialGroupOptionId === seeded.otherOptionId) - ).toEqual( - before.credentials.find((entry) => entry.credentialGroupOptionId === seeded.otherOptionId) + ])('preserves a Slack $name until Search consent is verified', async ({ scopes }) => { + const setup = await seedSlackAuthorization(scopes) + await expect(setup.resolveToken()).resolves.toMatchObject({ accessToken: 'fixture-token' }) + const originalAttempt = await setup.start() + expect( + new URL(originalAttempt.authorizationUrl).searchParams.get('user_scope')!.split(',') + ).not.toContain('search:read.public') + await setup.complete(originalAttempt.state, 'access_denied') + await approve('slack') + await expect(approve('slack')).resolves.toMatchObject({ memberAccounts: { changed: false } }) + const pending = await snapshot() + expect(pending.groups).toEqual(setup.before.groups) + expect(pending.credentials).toEqual(setup.before.credentials) + expect(pending.policies).toEqual(setup.before.policies) + await expect(setup.resolveToken()).resolves.toMatchObject({ accessToken: 'fixture-token' }) + + const desiredScopes = [...new Set([...scopes, ...SLACK_SEARCH_USER_SCOPES])] + const cancelled = await setup.start() + expect(new URL(cancelled.authorizationUrl).searchParams.get('user_scope')!.split(',')).toEqual( + expect.arrayContaining(desiredScopes) + ) + await expect(setup.complete(cancelled.state, 'access_denied')).resolves.toEqual({ + ok: false, + reason: 'provider_error', + }) + await expect(setup.resolveToken()).resolves.toMatchObject({ accessToken: 'fixture-token' }) + expect((await snapshot()).groups).toEqual(setup.before.groups) + + const incomplete = await setup.start() + provideSlackConsent(scopes) + await expect(setup.complete(incomplete.state)).rejects.toThrow('did not grant every permission') + expect((await snapshot()).groups).toEqual(setup.before.groups) + await expect(setup.resolveToken()).resolves.toMatchObject({ accessToken: 'fixture-token' }) + + const verified = await setup.start() + provideSlackConsent(desiredScopes) + await expect(setup.complete(verified.state)).resolves.toMatchObject({ + ok: true, + reason: 'authorized', + }) + const state = await snapshot() + const upgraded = state.groups[0].options.find((option) => option.id === setup.optionId)! + expect(upgraded.requiredScopes).toEqual(expect.arrayContaining(desiredScopes)) + expect(upgraded.scopeVersion).not.toBe(setup.before.groups[0].options[0].scopeVersion) + const configuration = await decryptCredentialGroupProviderConfiguration( + state.groups[0].encryptedProviderConfiguration + ) + expect(configuration.slack?.scopes).toEqual(expect.arrayContaining(desiredScopes)) + expect( + state.credentials.find((entry) => entry.credentialGroupOptionId === setup.optionId) + ?.managedOauthStatus + ).toBe('needs_reauth') + expect( + state.credentials.find((entry) => entry.credentialGroupOptionId === setup.otherOptionId) + ).toEqual( + setup.before.credentials.find( + (entry) => entry.credentialGroupOptionId === setup.otherOptionId ) - expect( - await getCredentialGroup( - { kind: 'organization', organizationId: ids.organization }, - seeded.groupId - ) - ).toMatchObject({ - options: expect.arrayContaining([ - expect.objectContaining({ id: seeded.optionId, configurationStatus: 'needs_update' }), - ]), + ) + expect(state.groups[0].options.find((option) => option.id === setup.otherOptionId)).toEqual( + setup.before.groups[0].options[1] + ) + await expect( + getCredentialGroup({ kind: 'organization', organizationId: ids.organization }, setup.groupId) + ).resolves.toMatchObject({ + options: expect.arrayContaining([ + expect.objectContaining({ id: setup.optionId, configurationStatus: 'ready' }), + ]), + }) + await expect(setup.complete(verified.state)).rejects.toThrow('invalid or expired') + }) + + it.each([false, true])( + 'rejects a pending Search upgrade after approval changes (reapproved: %s)', + async (reapproved) => { + const setup = await seedSlackAuthorization() + await approve('slack') + const pending = await setup.start() + await approveSearchIntegration.execute({ + principal: createSessionPrincipal({ userId: ids.owner, sessionId: generateId() }), + input: { organizationId: ids.organization, connectorType: 'slack', approved: false }, }) - await expect(approve('slack')).resolves.toMatchObject({ memberAccounts: { changed: false } }) - const repeated = await snapshot() - expect(repeated.groups).toEqual(state.groups) - expect(repeated.credentials).toEqual(state.credentials) + if (reapproved) await approve('slack') + provideSlackConsent([...SLACK_MANAGED_USER_SCOPES, ...SLACK_SEARCH_USER_SCOPES]) + await expect(setup.complete(pending.state)).rejects.toThrow('Search approval changed') + expect((await snapshot()).groups).toEqual(setup.before.groups) + await expect(setup.resolveToken()).resolves.toMatchObject({ accessToken: 'fixture-token' }) } ) @@ -463,6 +628,21 @@ describe('atomic organization live Search MCP setup', () => { .set({ memberSyncStatus: 'idle', sourceConfig: {} }) .where(eq(knowledgeConnector.id, source.connectorId)) expect(await integrationStatus('github')).toMatchObject({ configuredServiceSource: false }) + + }) + + it('rejects an oversized combined permission request without changing existing connections', async () => { + const scopes = [ + 'chat:write', + 'users:read', + 'users:read.email', + ...Array.from({ length: 97 }, (_, index) => `custom:${index}`), + ] + const setup = await seedSlackAuthorization(scopes) + await approve('slack') + await expect(setup.start()).rejects.toThrow('too many permissions') + expect((await snapshot()).groups).toEqual(setup.before.groups) + await expect(setup.resolveToken()).resolves.toMatchObject({ accessToken: 'fixture-token' }) }) it('keeps disabled Zoom approvals visible and removable without permitting reapproval', async () => { From c765a57cce5761b1a57a78aafb81ae9e08754e1d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 1 Oct 2026 14:40:42 -0700 Subject: [PATCH 2/3] fix(slack): bind authorization to every Search approval state --- .../application/slack-managed-users.ts | 3 +- .../credential-groups/slack-managed-users.ts | 48 +++++++++++-------- .../search-mcp-setup.integration.ts | 19 ++++++++ 3 files changed, 50 insertions(+), 20 deletions(-) diff --git a/apps/sim/lib/credential-groups/application/slack-managed-users.ts b/apps/sim/lib/credential-groups/application/slack-managed-users.ts index 908b2f2965e..567ffa8058b 100644 --- a/apps/sim/lib/credential-groups/application/slack-managed-users.ts +++ b/apps/sim/lib/credential-groups/application/slack-managed-users.ts @@ -169,7 +169,8 @@ export const completeSlackCredentialGroupConfiguration: OperationUseCase< attempt.expectedAppId !== pending.expectedAppId || attempt.expectedTeamId !== pending.expectedTeamId || attempt.appRevision !== pending.appRevision || - attempt.searchApprovalUpdatedAt !== pending.searchApprovalUpdatedAt || + attempt.searchApproval?.approved !== pending.searchApproval?.approved || + attempt.searchApproval?.updatedAt !== pending.searchApproval?.updatedAt || attempt.clientId !== pending.clientId || attempt.redirectUri !== pending.redirectUri || credentialGroupScopePolicyVersion(attempt.requiredScopes) !== diff --git a/apps/sim/lib/credential-groups/slack-managed-users.ts b/apps/sim/lib/credential-groups/slack-managed-users.ts index 9535a18a08c..fdc6f3adcb4 100644 --- a/apps/sim/lib/credential-groups/slack-managed-users.ts +++ b/apps/sim/lib/credential-groups/slack-managed-users.ts @@ -12,6 +12,7 @@ import { createLogger } from '@sim/logger' import { sha256Hex } from '@sim/security/hash' import { getErrorMessage } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' +import { isRecordLike } from '@sim/utils/object' import { and, eq, inArray, isNull, or } from 'drizzle-orm' import { getRedisClient } from '@/lib/core/config/redis' import { resourceScopeFields, resourceScopeFromOwner } from '@/lib/core/resource-scope' @@ -39,7 +40,7 @@ import { getSharedSlackSearchAppConfiguration } from '@/lib/slack-search/shared- const logger = createLogger('SlackManagedUsers') const SLACK_MANAGED_USERS_ATTEMPT_TTL_MS = 10 * 60 * 1000 -const SLACK_MANAGED_USERS_ATTEMPT_VERSION = 5 as const +const SLACK_MANAGED_USERS_ATTEMPT_VERSION = 6 as const const MAX_SLACK_RESPONSE_BYTES = 64 * 1024 const CONSUME_SCRIPT = ` local value = redis.call('GET', KEYS[1]) @@ -60,9 +61,14 @@ interface SlackCustomBotSecret { metadata?: Record } +interface SlackSearchApprovalSnapshot { + approved: boolean + updatedAt: number | null +} + type StoredSlackManagedUsersAttempt = { appRevision?: string - searchApprovalUpdatedAt?: number + searchApproval?: SlackSearchApprovalSnapshot version: typeof SLACK_MANAGED_USERS_ATTEMPT_VERSION workspaceId?: string organizationId?: string @@ -84,7 +90,7 @@ type StoredSlackManagedUsersAttempt = { export interface SlackManagedUsersAttempt { appRevision?: string - searchApprovalUpdatedAt?: number + searchApproval?: SlackSearchApprovalSnapshot workspaceId?: string organizationId?: string userId: string @@ -152,6 +158,7 @@ function attemptKey(state: string): string { function isStoredAttempt(value: unknown): value is StoredSlackManagedUsersAttempt { if (!value || typeof value !== 'object') return false const candidate = value as Record + const approval = candidate.searchApproval return ( candidate.version === SLACK_MANAGED_USERS_ATTEMPT_VERSION && ((typeof candidate.workspaceId === 'string' && @@ -164,10 +171,12 @@ function isStoredAttempt(value: unknown): value is StoredSlackManagedUsersAttemp typeof candidate.credentialGroupId === 'string' && typeof candidate.credentialGroupUpdatedAt === 'number' && (candidate.appRevision === undefined || typeof candidate.appRevision === 'string') && - (candidate.searchApprovalUpdatedAt === undefined || - (candidate.organizationId !== undefined && - typeof candidate.searchApprovalUpdatedAt === 'number' && - Number.isFinite(candidate.searchApprovalUpdatedAt))) && + (candidate.organizationId !== undefined + ? isRecordLike(approval) && + typeof approval.approved === 'boolean' && + (approval.updatedAt === null || + (typeof approval.updatedAt === 'number' && Number.isFinite(approval.updatedAt))) + : approval === undefined) && (candidate.organizationId !== undefined ? candidate.slackBotCredentialId === undefined && candidate.slackBotCredentialUpdatedAt === undefined @@ -504,7 +513,7 @@ export async function createSlackManagedUsersAttempt(params: { let clientId = params.clientId let clientSecret = params.clientSecret let appRevision: string | undefined - let searchApprovalUpdatedAt: number | undefined + let searchApproval: SlackSearchApprovalSnapshot | undefined if (scope.kind === 'organization') { if (params.slackBotCredentialId || !params.appId || params.clientId || params.clientSecret) throw new SlackManagedUsersError( @@ -556,7 +565,7 @@ export async function createSlackManagedUsersAttempt(params: { ? SLACK_SEARCH_USER_SCOPES : existingOption.requiredScopes ) - const [searchApproval] = await db + const [approval] = await db .select({ approved: organizationSearchIntegration.approved, updatedAt: organizationSearchIntegration.updatedAt, @@ -569,10 +578,12 @@ export async function createSlackManagedUsersAttempt(params: { ) ) .limit(1) - if (searchApproval?.approved) { - requiredScopes = [...new Set([...requiredScopes, ...SLACK_SEARCH_USER_SCOPES])] - searchApprovalUpdatedAt = searchApproval.updatedAt.getTime() + searchApproval = { + approved: approval?.approved ?? false, + updatedAt: approval?.updatedAt.getTime() ?? null, } + if (searchApproval.approved) + requiredScopes = [...new Set([...requiredScopes, ...SLACK_SEARCH_USER_SCOPES])] } else { if (!params.slackBotCredentialId) throw new SlackManagedUsersError('Select a custom Slack bot.', 'invalid_response') @@ -613,7 +624,7 @@ export async function createSlackManagedUsersAttempt(params: { expectedTeamId: identity.teamId, clientId, ...(appRevision ? { appRevision } : {}), - ...(searchApprovalUpdatedAt !== undefined ? { searchApprovalUpdatedAt } : {}), + ...(searchApproval ? { searchApproval } : {}), ...(sharedApp && scope.kind === 'organization' ? { credentialSource: 'environment' as const, organizationId: scope.organizationId } : { encryptedClientSecret: (await encryptSecret(clientSecret)).encrypted }), @@ -691,9 +702,7 @@ async function parseSlackManagedUsersAttempt( expectedTeamId: parsed.expectedTeamId, clientId: parsed.clientId, ...(parsed.appRevision ? { appRevision: parsed.appRevision } : {}), - ...(parsed.searchApprovalUpdatedAt !== undefined - ? { searchApprovalUpdatedAt: parsed.searchApprovalUpdatedAt } - : {}), + ...(parsed.searchApproval ? { searchApproval: parsed.searchApproval } : {}), clientSecret, redirectUri: parsed.redirectUri, requiredScopes: parsed.requiredScopes, @@ -813,7 +822,7 @@ export async function exchangeAndConfigureSlackManagedUsers(params: { 'invalid_state' ) } - if (params.attempt.organizationId && params.attempt.searchApprovalUpdatedAt !== undefined) { + if (params.attempt.organizationId) { const [approval] = await tx .select({ approved: organizationSearchIntegration.approved, @@ -829,8 +838,9 @@ export async function exchangeAndConfigureSlackManagedUsers(params: { .limit(1) .for('share') if ( - !approval?.approved || - approval.updatedAt.getTime() !== params.attempt.searchApprovalUpdatedAt + !params.attempt.searchApproval || + (approval?.approved ?? false) !== params.attempt.searchApproval.approved || + (approval?.updatedAt.getTime() ?? null) !== params.attempt.searchApproval.updatedAt ) throw new SlackManagedUsersError( 'Search approval changed during authorization. Start again.', diff --git a/apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts b/apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts index caf2148ceb6..4fdddc61f6d 100644 --- a/apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts +++ b/apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts @@ -401,6 +401,25 @@ describe('atomic organization live Search MCP setup', () => { await expect(setup.complete(verified.state)).rejects.toThrow('invalid or expired') }) + it.each(['missing', 'disabled'] as const)( + 'rejects workflow-only authorization when Search becomes approved (previous approval: %s)', + async (previousApproval) => { + const setup = await seedSlackAuthorization() + if (previousApproval === 'disabled') + await db.insert(organizationSearchIntegration).values({ + organizationId: ids.organization, + connectorType: 'slack', + approved: false, + }) + const pending = await setup.start() + await approve('slack') + provideSlackConsent(SLACK_MANAGED_USER_SCOPES) + await expect(setup.complete(pending.state)).rejects.toThrow('Search approval changed') + expect((await snapshot()).groups).toEqual(setup.before.groups) + await expect(setup.resolveToken()).resolves.toMatchObject({ accessToken: 'fixture-token' }) + } + ) + it.each([false, true])( 'rejects a pending Search upgrade after approval changes (reapproved: %s)', async (reapproved) => { From f66dd3fee47c1ef779402e97ca8a81330615c1d8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 1 Oct 2026 14:58:19 -0700 Subject: [PATCH 3/3] chore(tests): require Redis for Slack authorization integration --- .../__integration__/search-mcp-setup.integration.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts b/apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts index 4fdddc61f6d..8d63e9d74d4 100644 --- a/apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts +++ b/apps/sim/lib/knowledge/__integration__/search-mcp-setup.integration.ts @@ -62,14 +62,20 @@ import { defaultLiveSearchPolicy } from '@/lib/sim-search/live/policy-schema' /** * Real authorization, PostgreSQL, Redis and token resolution; DNS and Slack HTTP use fixtures. - * Run with TEST_DATABASE_URL naming a disposable database and pass - * --outputFile.json="$SEARCH_MCP_SETUP_REPORT_PATH" for a caller-selected JSON report. + * Run `bun run test:integration lib/knowledge/__integration__/search-mcp-setup.integration.ts` + * with INTEGRATION_REPORT_PATH for a caller-selected JSON report. Direct Vitest runs require + * TEST_DATABASE_URL and TEST_REDIS_URL naming disposable local services. */ describe('atomic organization live Search MCP setup', () => { + const redisUrl = readTestRedisUrl() let restoreSlackHttp: (() => void) | undefined let ids: { organization: string; owner: string; member: string; outsider: string } beforeAll(() => { + if (!redisUrl) + throw new Error( + 'Set TEST_REDIS_URL to a disposable local Redis or use bun run test:integration' + ) vi.spyOn(dns, 'resolveHostAddresses').mockImplementation(async (hostname) => { if ( !['api.fireflies.ai', 'mcp.granola.ai', 'mcp.notion.com', 'mcp.lucid.app'].includes( @@ -84,7 +90,7 @@ describe('atomic organization live Search MCP setup', () => { beforeEach(async () => { Object.assign(env, { ZOOM_SEARCH: false, - REDIS_URL: readTestRedisUrl(), + REDIS_URL: redisUrl, }) ids = { organization: generateId(), @@ -647,7 +653,6 @@ describe('atomic organization live Search MCP setup', () => { .set({ memberSyncStatus: 'idle', sourceConfig: {} }) .where(eq(knowledgeConnector.id, source.connectorId)) expect(await integrationStatus('github')).toMatchObject({ configuredServiceSource: false }) - }) it('rejects an oversized combined permission request without changing existing connections', async () => {