fix(slack): revoke shared grants on stale uninstall, pair app and workspace ids, audit text

Author: waleedlatif1

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,

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' })
     }
   )

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 },
                 },
               ]

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'] }])(

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,28 +85,28 @@ 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 ||
           (input.event.type === 'tokens_revoked' &&
             (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({

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,