Skip to content

Commit a20e84f

Browse files
fix(slack): reject stale uninstall before revoking member grants
1 parent 8e34f89 commit a20e84f

2 files changed

Lines changed: 31 additions & 5 deletions

File tree

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

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,18 +144,44 @@ describe('Slack access revocation', () => {
144144
await revokeSlackSearchAccess.execute({ principal, input })
145145
expect(dbChainMockFns.update).not.toHaveBeenCalled()
146146
})
147-
it('does not revoke a replacement installation because of a delayed uninstall', async () => {
147+
it.each([
148+
{ type: 'app_uninstalled' as const },
149+
{ type: 'tokens_revoked' as const, tokens: { bot: ['UBOT'], oauth: ['U1'] } },
150+
])('ignores stale installation-wide revocations before any writes: %j', async (event) => {
148151
resetDbChainMock()
149152
queueTableRows(slackSearchInstallation, [
150153
{
151154
id: 'i1',
152155
organizationId: 'org',
153156
appId: 'A1',
154157
teamId: 'T1',
155-
updatedAt: new Date(Date.now() + 1000),
158+
botUserId: 'UBOT',
159+
updatedAt: new Date(input.event_time * 1000 + 1000),
156160
},
157161
])
158-
await revokeSlackSearchAccess.execute({ principal, input })
159-
expect(dbChainMockFns.set.mock.calls.some(([value]) => 'enabled' in value)).toBe(false)
162+
await revokeSlackSearchAccess.execute({ principal, input: { ...input, event } })
163+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
164+
})
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+
])
160186
})
161187
})

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ export const revokeSlackSearchAccess: OperationUseCase<
7474
uninstall ||
7575
(input.event.type === 'tokens_revoked' &&
7676
(input.event.tokens.bot ?? []).includes(installation.botUserId))
77+
if (revokeBot && installation.updatedAt > occurredAt) return
7778
let revokedMemberIds: string[] = []
7879
if (uninstall || revokedUsers.length) {
7980
const revokeCredentials = tx
@@ -102,7 +103,6 @@ export const revokeSlackSearchAccess: OperationUseCase<
102103
}
103104
}
104105
if (revokeBot) {
105-
if (installation.updatedAt > occurredAt) return
106106
await tx
107107
.update(slackSearchInstallation)
108108
.set({

0 commit comments

Comments
 (0)