Skip to content

Commit 3c572d6

Browse files
fix(slack): process member revocations independently of bot events
1 parent a20e84f commit 3c572d6

2 files changed

Lines changed: 46 additions & 27 deletions

File tree

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

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ describe('Slack access revocation', () => {
146146
})
147147
it.each([
148148
{ type: 'app_uninstalled' as const },
149-
{ type: 'tokens_revoked' as const, tokens: { bot: ['UBOT'], oauth: ['U1'] } },
149+
{ type: 'tokens_revoked' as const, tokens: { bot: ['UBOT'] } },
150150
])('ignores stale installation-wide revocations before any writes: %j', async (event) => {
151151
resetDbChainMock()
152152
queueTableRows(slackSearchInstallation, [
@@ -162,26 +162,43 @@ describe('Slack access revocation', () => {
162162
await revokeSlackSearchAccess.execute({ principal, input: { ...input, event } })
163163
expect(dbChainMockFns.update).not.toHaveBeenCalled()
164164
})
165-
it('still revokes matching personal grants when only the installation is newer', async () => {
166-
resetDbChainMock()
167-
queueTableRows(slackSearchInstallation, [
168-
{
169-
id: 'i1',
170-
organizationId: 'org',
171-
appId: 'A1',
172-
teamId: 'T1',
173-
botUserId: 'UBOT',
174-
updatedAt: new Date(input.event_time * 1000 + 1000),
175-
},
176-
])
177-
dbChainMockFns.returning.mockResolvedValueOnce([{ providerSubjectId: 'U1' }])
178-
await revokeSlackSearchAccess.execute({
179-
principal,
180-
input: { ...input, event: { type: 'tokens_revoked', tokens: { oauth: ['U1'] } } },
181-
})
182-
expect(dbChainMockFns.update.mock.calls.map(([table]) => table)).toEqual([
183-
credential,
184-
slackSearchTurn,
185-
])
186-
})
165+
it.each([{ oauth: ['U1'] }, { oauth: ['U1'], bot: ['UBOT'] }])(
166+
'revokes matching member grants independently of a newer bot installation: %j',
167+
async (tokens) => {
168+
resetDbChainMock()
169+
queueTableRows(slackSearchInstallation, [
170+
{
171+
id: 'i1',
172+
organizationId: 'org',
173+
appId: 'A1',
174+
teamId: 'T1',
175+
botUserId: 'UBOT',
176+
updatedAt: new Date(input.event_time * 1000 + 1000),
177+
},
178+
])
179+
dbChainMockFns.returning.mockResolvedValueOnce([{ providerSubjectId: 'U1' }])
180+
await revokeSlackSearchAccess.execute({
181+
principal,
182+
input: { ...input, event: { type: 'tokens_revoked', tokens } },
183+
})
184+
expect(dbChainMockFns.update.mock.calls.map(([table]) => table)).toEqual([
185+
credential,
186+
slackSearchTurn,
187+
])
188+
expect(dbChainMockFns.where).toHaveBeenLastCalledWith(
189+
expect.objectContaining({
190+
conditions: expect.arrayContaining([
191+
{
192+
type: 'inArray',
193+
column: expect.objectContaining({
194+
strings: ['', " #>> '{message,userId}'"],
195+
values: [slackSearchTurn.payload],
196+
}),
197+
values: ['U1'],
198+
},
199+
]),
200+
})
201+
)
202+
}
203+
)
187204
})

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,15 @@ export const revokeSlackSearchAccess: OperationUseCase<
6868
if (app.app.kind === 'custom' && app.app.organizationId !== installation.organizationId)
6969
throw new Error('Slack installation ownership is inconsistent')
7070
const uninstall = input.event.type === 'app_uninstalled'
71+
const staleInstallation = installation.updatedAt > occurredAt
72+
if (uninstall && staleInstallation) return
7173
const revokedUsers =
7274
input.event.type === 'tokens_revoked' ? (input.event.tokens.oauth ?? []) : []
7375
const revokeBot =
74-
uninstall ||
75-
(input.event.type === 'tokens_revoked' &&
76-
(input.event.tokens.bot ?? []).includes(installation.botUserId))
77-
if (revokeBot && installation.updatedAt > occurredAt) return
76+
!staleInstallation &&
77+
(uninstall ||
78+
(input.event.type === 'tokens_revoked' &&
79+
(input.event.tokens.bot ?? []).includes(installation.botUserId)))
7880
let revokedMemberIds: string[] = []
7981
if (uninstall || revokedUsers.length) {
8082
const revokeCredentials = tx

0 commit comments

Comments
 (0)