Skip to content

Commit 4460a0a

Browse files
feat(slack): support organization-scoped shared app rollout (#7833)
1 parent 580fc58 commit 4460a0a

13 files changed

Lines changed: 134 additions & 35 deletions

File tree

apps/sim/lib/core/config/feature-flags.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const { mockFetch, mockIsPlatformAdmin, envRef } = vi.hoisted(() => ({
1515
TABLE_ROW_TTL: undefined as boolean | undefined,
1616
CREDENTIAL_GROUPS: undefined as boolean | undefined,
1717
KNOWLEDGE_MEMBER_ACCESS: undefined as boolean | undefined,
18+
SLACK_SEARCH_SHARED_APP: undefined as boolean | undefined,
1819
},
1920
}))
2021

@@ -125,6 +126,40 @@ describe('isFeatureEnabled', () => {
125126
setEnvFlags({ isAppConfigEnabled: false })
126127
envRef.CREDENTIAL_GROUPS = undefined
127128
envRef.KNOWLEDGE_MEMBER_ACCESS = undefined
129+
envRef.SLACK_SEARCH_SHARED_APP = undefined
130+
})
131+
132+
describe('slack-search-shared-app flag', () => {
133+
it('enables only the allowlisted organization', async () => {
134+
withAppConfig({ 'slack-search-shared-app': { enabled: false, orgIds: ['review-org'] } })
135+
expect(await isFeatureEnabled('slack-search-shared-app', { orgId: 'review-org' })).toBe(true)
136+
expect(await isFeatureEnabled('slack-search-shared-app', { orgId: 'other-org' })).toBe(false)
137+
expect(await isFeatureEnabled('slack-search-shared-app')).toBe(false)
138+
expect(mockIsPlatformAdmin).not.toHaveBeenCalled()
139+
})
140+
141+
it('does not grant organization access from user or workspace targeting', async () => {
142+
withAppConfig({
143+
'slack-search-shared-app': {
144+
userIds: ['review-org'],
145+
workspaceIds: ['review-org'],
146+
adminEnabled: true,
147+
},
148+
})
149+
expect(await isFeatureEnabled('slack-search-shared-app', { orgId: 'review-org' })).toBe(false)
150+
expect(mockIsPlatformAdmin).not.toHaveBeenCalled()
151+
})
152+
153+
it('preserves the global AppConfig switch', async () => {
154+
withAppConfig({ 'slack-search-shared-app': { enabled: true } })
155+
expect(await isFeatureEnabled('slack-search-shared-app', { orgId: 'any-org' })).toBe(true)
156+
})
157+
158+
it('preserves the global fallback switch off AppConfig', async () => {
159+
expect(await isFeatureEnabled('slack-search-shared-app', { orgId: 'review-org' })).toBe(false)
160+
envRef.SLACK_SEARCH_SHARED_APP = true
161+
expect(await isFeatureEnabled('slack-search-shared-app', { orgId: 'review-org' })).toBe(true)
162+
})
128163
})
129164

130165
describe('knowledge-member-access flag', () => {

apps/sim/lib/core/config/feature-flags.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ interface FeatureFlagDefinition {
4848
const FEATURE_FLAGS = {
4949
'slack-search-shared-app': {
5050
description:
51-
'Enable the official shared Slack app for existing Search customers. Global on/off only.',
51+
'Enable the official shared Slack app for existing Search customers. Supports orgId ' +
52+
'targeting for setup, personal connections, and bot execution. Off-AppConfig falls back ' +
53+
'to SLACK_SEARCH_SHARED_APP.',
5254
fallback: 'SLACK_SEARCH_SHARED_APP',
5355
},
5456
'trigger-eu-region': {

apps/sim/lib/credential-groups/provider-configuration.test.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,17 @@ describe('organization Slack app references', () => {
7070
clientSecret: 'current-secret',
7171
})
7272
})
73-
it.each([true, false])(
74-
'resolves environment credentials only for an active shared installation (active=%s)',
75-
async (active) => {
73+
it.each([
74+
[true, true],
75+
[false, true],
76+
[true, false],
77+
])(
78+
'resolves shared credentials only for an active installation and enabled organization (active=%s, enabled=%s)',
79+
async (active, enabled) => {
7680
shared.env.SLACK_SEARCH_APP_ID = 'A1'
81+
shared.flag.mockImplementation(
82+
async (_flag, context) => enabled && context?.orgId === 'org-1'
83+
)
7784
dbChainMockFns.limit
7885
.mockResolvedValueOnce([
7986
{
@@ -96,14 +103,16 @@ describe('organization Slack app references', () => {
96103
organizationId: 'org-1',
97104
credentialGroupId: 'group-1',
98105
})
99-
if (active)
106+
if (!enabled) await expect(result).rejects.toThrow('unavailable')
107+
else if (active)
100108
await expect(result).resolves.toMatchObject({
101109
clientId: 'environment-client',
102110
clientSecret: 'environment-secret',
103111
appId: 'A1',
104112
teamId: 'T1',
105113
})
106114
else await expect(result).rejects.toThrow('disabled or removed')
115+
expect(shared.flag).toHaveBeenCalledWith('slack-search-shared-app', { orgId: 'org-1' })
107116
}
108117
)
109118
it('keeps using the custom app for personal sources after a different native app is installed', async () => {

apps/sim/lib/credential-groups/provider-configuration.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ async function resolveSlackConfiguration(
159159
.limit(1)
160160
if (!app) throw new Error('Organization Slack app configuration is missing')
161161
if (app.kind === 'shared') {
162-
await requireSlackSearchAppAvailable(app.id)
162+
await requireSlackSearchAppAvailable(app.id, params.organizationId)
163163
const [installation] = await (params.executor ?? db)
164164
.select({ id: slackSearchInstallation.id })
165165
.from(slackSearchInstallation)

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ describe('Slack managed-user authorization', () => {
146146
'keeps shared setup state secret-free and rechecks configuration on %s',
147147
async (outcome) => {
148148
shared.env.SLACK_SEARCH_APP_ID = 'ASHARED'
149+
shared.flag.mockImplementation(async (_flag, context) => context?.orgId === 'org-1')
149150
dbChainMockFns.limit
150151
.mockResolvedValueOnce([{ id: 'group-1', updatedAt: new Date(1), options: [] }])
151152
.mockResolvedValueOnce([
@@ -173,6 +174,7 @@ describe('Slack managed-user authorization', () => {
173174
expect(stored).toMatchObject({ credentialSource: 'environment', expectedAppId: 'ASHARED' })
174175
expect(stored).not.toHaveProperty('encryptedClientSecret')
175176
expect(JSON.stringify(stored)).not.toContain('environment-secret')
177+
expect(shared.flag).toHaveBeenCalledWith('slack-search-shared-app', { orgId: 'org-1' })
176178
if (outcome === 'rotation') {
177179
shared.env.SLACK_SEARCH_CLIENT_SECRET = 'rotated'
178180
await expect(consumeSlackManagedUsersAttempt(created.state)).rejects.toThrow('changed')
@@ -185,6 +187,7 @@ describe('Slack managed-user authorization', () => {
185187
clientSecret: 'environment-secret',
186188
organizationId: 'org-1',
187189
})
190+
expect(shared.flag).toHaveBeenLastCalledWith('slack-search-shared-app', { orgId: 'org-1' })
188191
await expect(consumeSlackManagedUsersAttempt(created.state)).resolves.toBeNull()
189192
}
190193
}

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ type StoredSlackManagedUsersAttempt = {
7373
requiredScopes: string[]
7474
createdAt: number
7575
} & (
76-
| { credentialSource: 'environment'; encryptedClientSecret?: never }
76+
| { credentialSource: 'environment'; organizationId: string; encryptedClientSecret?: never }
7777
| { credentialSource?: undefined; encryptedClientSecret: string }
7878
)
7979

@@ -525,7 +525,7 @@ export async function createSlackManagedUsersAttempt(params: {
525525
'Set up this organization’s Slack app first.',
526526
'invalid_response'
527527
)
528-
await requireSlackSearchAppAvailable(configured.app.id)
528+
await requireSlackSearchAppAvailable(configured.app.id, scope.organizationId)
529529
const app = await resolveSlackAppCredentials(configured.app)
530530
identity = { appId: configured.app.id, teamId: configured.teamId }
531531
clientId = app.clientId
@@ -569,8 +569,8 @@ export async function createSlackManagedUsersAttempt(params: {
569569
expectedTeamId: identity.teamId,
570570
clientId,
571571
...(appRevision ? { appRevision } : {}),
572-
...(sharedApp
573-
? { credentialSource: 'environment' as const }
572+
...(sharedApp && scope.kind === 'organization'
573+
? { credentialSource: 'environment' as const, organizationId: scope.organizationId }
574574
: { encryptedClientSecret: (await encryptSecret(clientSecret)).encrypted }),
575575
requiredScopes,
576576
redirectUri,
@@ -626,7 +626,7 @@ async function parseSlackManagedUsersAttempt(
626626
'The shared Slack app changed. Start again.',
627627
'invalid_state'
628628
)
629-
await requireSlackSearchAppAvailable(app.id)
629+
await requireSlackSearchAppAvailable(app.id, parsed.organizationId)
630630
clientSecret = app.clientSecret
631631
} else {
632632
clientSecret = (await decryptSecret(parsed.encryptedClientSecret)).decrypted
@@ -729,7 +729,8 @@ export async function exchangeAndConfigureSlackManagedUsers(params: {
729729
)
730730
.limit(1)
731731
.for('update')
732-
if (app?.kind === 'shared') await requireSlackSearchAppAvailable(app.id)
732+
if (app?.kind === 'shared')
733+
await requireSlackSearchAppAvailable(app.id, params.attempt.organizationId)
733734
const resolved = app ? await resolveSlackAppCredentials(app) : null
734735
if (
735736
!resolved ||

apps/sim/lib/knowledge/application/slack-search/authorization.test.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const mocks = vi.hoisted(() => ({
77
credential: vi.fn(),
88
availability: vi.fn(),
99
replacement: vi.fn(),
10+
appAvailable: vi.fn(),
1011
}))
1112
vi.mock('@/lib/knowledge/application/slack-search/repository', () => ({
1213
findSlackSearchInstallation: mocks.installation,
@@ -16,7 +17,7 @@ vi.mock('@/lib/knowledge/access/availability', () => ({
1617
requireOrganizationSearchAvailable: mocks.availability,
1718
}))
1819
vi.mock('@/lib/slack-search/shared-app', () => ({
19-
requireSlackSearchAppAvailable: vi.fn(),
20+
requireSlackSearchAppAvailable: mocks.appAvailable,
2021
findSharedSlackSearchInstallation: mocks.replacement,
2122
}))
2223

@@ -49,6 +50,7 @@ beforeEach(() => {
4950
mocks.installation.mockResolvedValue(installation)
5051
mocks.credential.mockResolvedValue({ version: 'version1', botToken: 'secret' })
5152
mocks.availability.mockResolvedValue(undefined)
53+
mocks.appAvailable.mockResolvedValue(undefined)
5254
mocks.replacement.mockResolvedValue(null)
5355
})
5456

@@ -154,6 +156,22 @@ describe('Slack Search installation authorization', () => {
154156
installation,
155157
})
156158
expect(mocks.credential).toHaveBeenCalledWith('cred1', 'org1')
159+
expect(mocks.appAvailable).toHaveBeenCalledWith('A1', 'org1')
160+
})
161+
it('rejects the next lifecycle check when the organization loses shared app access', async () => {
162+
await expect(authorizeSlackSearchInstallation(principal)).resolves.toMatchObject({
163+
installation,
164+
})
165+
mocks.credential.mockClear()
166+
mocks.appAvailable.mockRejectedValueOnce(new Error('unavailable'))
167+
await expect(
168+
authorizeSlackSearchInstallation(principal, {
169+
installationId: installation.id,
170+
revision: installation.revision,
171+
})
172+
).rejects.toThrow('unavailable')
173+
expect(mocks.appAvailable).toHaveBeenLastCalledWith('A1', 'org1')
174+
expect(mocks.credential).not.toHaveBeenCalled()
157175
})
158176
it.each([null, { ...installation, enabled: false }])(
159177
'does no work for removed or disabled installations',

apps/sim/lib/knowledge/application/slack-search/authorization.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ async function authorizeSlackSearchBinding(
5151
throw new OrchestrationError('forbidden', 'Slack Search binding is no longer valid')
5252
}
5353
await requireOrganizationSearchAvailable(installation.organizationId)
54-
await requireSlackSearchAppAvailable(installation.appId)
54+
await requireSlackSearchAppAvailable(installation.appId, installation.organizationId)
5555
const secret = await loadSlackSearchCredential(
5656
installation.credentialId,
5757
installation.organizationId

apps/sim/lib/knowledge/application/slack-search/installations.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export const listSlackSearchInstallations = defineAuthorizedKnowledgeUseCase({
7777
)
7878
)
7979
.limit(101),
80-
readSharedSlackSearchApp(),
80+
readSharedSlackSearchApp(context.organizationId),
8181
])
8282
if (installations.length > 100 || bots.length > 100)
8383
throw new OrchestrationError(
@@ -158,7 +158,7 @@ export const configureSlackSearchInstallation = defineAuthorizedKnowledgeUseCase
158158
? await tx.select().from(slackApp).where(eq(slackApp.id, current.slackAppId)).limit(1)
159159
: []
160160
if (input.enabled && current.slackAppId)
161-
await requireSlackSearchAppAvailable(current.slackAppId)
161+
await requireSlackSearchAppAvailable(current.slackAppId, context.organizationId)
162162
if (current.slackAppId && !app) throw new Error('Slack app configuration is missing')
163163
const appRevision =
164164
app && secret ? (await resolveSlackAppCredentials(app)).revision : undefined

apps/sim/lib/knowledge/application/slack-search/setup.test.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,9 +300,10 @@ describe('Search OAuth installation', () => {
300300
})
301301
})
302302

303-
it('rejects a shared-app callback if the global configuration was disabled or rotated', async () => {
303+
it('rejects a shared-app callback if organization access was disabled or configuration rotated', async () => {
304304
m.consume.mockResolvedValue({ ...attempt, sharedApp: { id: 'ASHARED', revision: 'app-rev' } })
305305
await expect(complete()).rejects.toThrow('configuration changed')
306+
expect(m.shared).toHaveBeenCalledWith('org1')
306307
expect(m.exchange).not.toHaveBeenCalled()
307308
m.shared.mockResolvedValue({ id: 'ASHARED', revision: 'new-rev' })
308309
await expect(complete()).rejects.toThrow()
@@ -327,6 +328,24 @@ describe('shared app completion', () => {
327328
})
328329
})
329330

331+
it('only offers and starts shared OAuth for the enabled organization', async () => {
332+
m.shared.mockImplementation(async (orgId) => (orgId === 'org1' ? sharedApp : null))
333+
const details = { name: 'Sim Search', description: 'Search with sources' }
334+
await expect(
335+
prepareSlackSearchSetup.execute({ principal, input: { ...details, organizationId: 'org1' } })
336+
).resolves.toHaveProperty('sharedAppId', 'A1')
337+
await expect(
338+
prepareSlackSearchSetup.execute({ principal, input: { ...details, organizationId: 'org2' } })
339+
).resolves.toHaveProperty('sharedAppId', null)
340+
await expect(
341+
startSlackSearchSetup.execute({
342+
principal,
343+
input: { ...details, organizationId: 'org2', mode: 'shared' },
344+
})
345+
).rejects.toThrow('unavailable')
346+
expect(m.store).not.toHaveBeenCalled()
347+
})
348+
330349
it('starts shared OAuth without storing deployment secrets in the attempt', async () => {
331350
const result = await startSlackSearchSetup.execute({
332351
principal,
@@ -338,6 +357,7 @@ describe('shared app completion', () => {
338357
},
339358
})
340359
expect(new URL(result.authorizationUrl).searchParams.get('client_id')).toBe('client')
360+
expect(m.shared).toHaveBeenCalledWith('org1')
341361
const stored = m.store.mock.calls[0][0]
342362
expect(stored.sharedApp).toEqual({ id: 'A1', revision: 'shared-revision' })
343363
expect(stored).not.toHaveProperty('encryptedClientSecret')
@@ -468,6 +488,7 @@ describe('shared app completion', () => {
468488
queueTransitionRows()
469489
m.shared.mockResolvedValueOnce(sharedApp).mockResolvedValueOnce(null)
470490
await expect(complete()).rejects.toThrow('configuration changed')
491+
expect(m.shared.mock.calls).toEqual([['org1'], ['org1']])
471492
expect(m.update).not.toHaveBeenCalled()
472493
expect(m.values).not.toHaveBeenCalled()
473494
})

0 commit comments

Comments
 (0)