Skip to content

Commit 4e347bb

Browse files
committed
fix(slack): cancel stale-uninstall turns for this installation's revoked members
1 parent ecdc10b commit 4e347bb

2 files changed

Lines changed: 41 additions & 9 deletions

File tree

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

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ beforeEach(() => {
3939
})
4040
describe('Slack access revocation', () => {
4141
it('cancels only affected members without rotating the shared installation revision', async () => {
42-
dbChainMockFns.returning.mockResolvedValueOnce([{ providerSubjectId: 'U1' }])
42+
dbChainMockFns.returning.mockResolvedValueOnce([
43+
{ organizationId: 'org', providerSubjectId: 'U1' },
44+
])
4345
await revokeSlackSearchAccess.execute({
4446
principal,
4547
input: { ...input, event: { type: 'tokens_revoked', tokens: { oauth: ['U1', 'U2'] } } },
@@ -223,6 +225,33 @@ describe('Slack access revocation', () => {
223225
expect.objectContaining({ left: credential.organizationId })
224226
)
225227
})
228+
it('cancels stale-uninstall turns only for members revoked in the installation organization', async () => {
229+
queueReinstalledBot()
230+
dbChainMockFns.returning.mockResolvedValueOnce([
231+
{ organizationId: 'org', providerSubjectId: 'U1' },
232+
{ organizationId: 'other-org', providerSubjectId: 'U2' },
233+
])
234+
await revokeSlackSearchAccess.execute({ principal, input })
235+
expect(dbChainMockFns.update.mock.calls.map(([table]) => table)).toEqual([
236+
credential,
237+
slackSearchTurn,
238+
])
239+
expect(dbChainMockFns.where).toHaveBeenLastCalledWith({
240+
type: 'and',
241+
conditions: [
242+
{ type: 'eq', left: slackSearchTurn.installationId, right: 'i1' },
243+
{ type: 'inArray', column: slackSearchTurn.status, values: ['pending', 'running'] },
244+
{
245+
type: 'inArray',
246+
column: expect.objectContaining({
247+
strings: ['', " #>> '{message,userId}'"],
248+
values: [slackSearchTurn.payload],
249+
}),
250+
values: ['U1'],
251+
},
252+
],
253+
})
254+
})
226255
it('ignores a stale custom-app uninstall before any writes', async () => {
227256
m.app.mockResolvedValue({
228257
app: { id: 'A1', kind: 'custom', revision: 'r1', organizationId: 'org' },
@@ -245,7 +274,9 @@ describe('Slack access revocation', () => {
245274
updatedAt: new Date(input.event_time * 1000 + 1000),
246275
},
247276
])
248-
dbChainMockFns.returning.mockResolvedValueOnce([{ providerSubjectId: 'U1' }])
277+
dbChainMockFns.returning.mockResolvedValueOnce([
278+
{ organizationId: 'org', providerSubjectId: 'U1' },
279+
])
249280
await revokeSlackSearchAccess.execute({
250281
principal,
251282
input: { ...input, event: { type: 'tokens_revoked', tokens } },

‎apps/sim/lib/knowledge/application/slack-search/lifecycle.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,8 @@ export const revokeSlackSearchAccess: OperationUseCase<
9191
if (app.app.kind === 'custom' && app.app.organizationId !== installation.organizationId)
9292
throw new Error('Slack installation ownership is inconsistent')
9393
const staleInstallation = installation.updatedAt > occurredAt
94-
if (uninstall && staleInstallation) {
95-
/** A reinstall keeps its bot, but shared-app grants issued before the uninstall are gone. */
96-
if (sharedApp) await revokeMemberGrants(tx)
97-
return
98-
}
94+
/** A reinstall keeps its bot, but shared-app grants issued before the uninstall are gone. */
95+
if (uninstall && staleInstallation && !sharedApp) return
9996
const revokeBot =
10097
!staleInstallation &&
10198
(uninstall ||
@@ -110,10 +107,14 @@ export const revokeSlackSearchAccess: OperationUseCase<
110107
if (revokeBot) await revokeCredentials
111108
else {
112109
const revokedCredentials = await revokeCredentials.returning({
110+
organizationId: credential.organizationId,
113111
providerSubjectId: credential.providerSubjectId,
114112
})
115-
revokedMemberIds = revokedCredentials.flatMap(({ providerSubjectId }) =>
116-
providerSubjectId ? [providerSubjectId] : []
113+
/** Only this installation's members have turns on its bot. */
114+
revokedMemberIds = revokedCredentials.flatMap(({ organizationId, providerSubjectId }) =>
115+
providerSubjectId && organizationId === installation.organizationId
116+
? [providerSubjectId]
117+
: []
117118
)
118119
}
119120
}

0 commit comments

Comments
 (0)