Skip to content

Commit 3532741

Browse files
committed
fix(oauth): keep Slack connect fan-out unchanged and skip the cross-process check without Redis
1 parent 4531f34 commit 3532741

3 files changed

Lines changed: 36 additions & 34 deletions

File tree

‎apps/sim/lib/auth/auth.ts‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -560,15 +560,11 @@ export const auth = betterAuth({
560560
// proves the installation has live tokens, and a fan-out
561561
// failure must not leave the hour-long flag blocking refreshes.
562562
await clearOAuthRefreshDeadFlag(`slack:${teamId}`)
563-
await fanOutSlackTokenChain(
564-
teamId,
565-
{
566-
accessToken: account.accessToken,
567-
refreshToken: account.refreshToken ?? null,
568-
accessTokenExpiresAt: account.accessTokenExpiresAt ?? null,
569-
},
570-
{ freshlyIssued: true }
571-
)
563+
await fanOutSlackTokenChain(teamId, {
564+
accessToken: account.accessToken,
565+
refreshToken: account.refreshToken ?? null,
566+
accessTokenExpiresAt: account.accessTokenExpiresAt ?? null,
567+
})
572568
logger.info('[account.create.after] Propagated Slack installation token chain', {
573569
userId: account.userId,
574570
teamId,

‎apps/sim/lib/oauth/credential-revocation.integration.ts‎

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,15 @@ import { generateId } from '@sim/utils/id'
77
import { eq, inArray } from 'drizzle-orm'
88
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
99

10-
await vi.hoisted(async () => {
10+
const { redisUrl } = await vi.hoisted(async () => {
1111
const { readTestRedisUrl } = await import('@sim/db/testing/test-infrastructure')
1212
const redisUrl = readTestRedisUrl()
1313
if (redisUrl) process.env.REDIS_URL = redisUrl
1414
process.env.GOOGLE_CLIENT_ID = 'revocation-integration-client'
1515
process.env.GOOGLE_CLIENT_SECRET = 'revocation-integration-secret'
1616
process.env.SLACK_CLIENT_ID = 'revocation-integration-slack-client'
1717
process.env.SLACK_CLIENT_SECRET = 'revocation-integration-slack-secret'
18+
return { redisUrl }
1819
})
1920

2021
import { closeRedisConnection, getRedisClient } from '@/lib/core/config/redis'
@@ -357,29 +358,33 @@ describe('OAuth refresh-token revocation against PostgreSQL and Redis', () => {
357358
expect((await storedAccount()).refreshRevokedAt).toBeNull()
358359
})
359360

360-
it('answers a waiting refresh from the revocation another process recorded', async () => {
361-
await expect(resolveToken()).rejects.toBeInstanceOf(CredentialRevokedError)
362-
const { refreshRevokedAt, refreshRevokedCode, refreshRevokedTokenHash } = await storedAccount()
363-
await db.update(account).set(CLEARED_REFRESH_REVOCATION).where(eq(account.id, accountId))
364-
await expireRedisFlags()
365-
366-
const redis = getRedisClient()
367-
if (!redis) throw new Error('This check needs TEST_REDIS_URL')
368-
const lockKey = `oauth:refresh:${getOAuthRefreshCoordinationIdentity(accountId)}`
369-
await redis.set(lockKey, 'another-process', 'EX', 60)
370-
try {
371-
const staleRow = await storedAccount()
372-
const follower = refreshTokenIfNeeded('revocation-integration', staleRow, accountId)
373-
await db
374-
.update(account)
375-
.set({ refreshRevokedAt, refreshRevokedCode, refreshRevokedTokenHash })
376-
.where(eq(account.id, accountId))
377-
await expect(follower).rejects.toBeInstanceOf(CredentialRevokedError)
378-
} finally {
379-
await redis.del(lockKey)
361+
it.skipIf(!redisUrl)(
362+
'answers a waiting refresh from the revocation another process recorded',
363+
async () => {
364+
await expect(resolveToken()).rejects.toBeInstanceOf(CredentialRevokedError)
365+
const { refreshRevokedAt, refreshRevokedCode, refreshRevokedTokenHash } =
366+
await storedAccount()
367+
await db.update(account).set(CLEARED_REFRESH_REVOCATION).where(eq(account.id, accountId))
368+
await expireRedisFlags()
369+
370+
const redis = getRedisClient()
371+
if (!redis) throw new Error('This check needs TEST_REDIS_URL')
372+
const lockKey = `oauth:refresh:${getOAuthRefreshCoordinationIdentity(accountId)}`
373+
await redis.set(lockKey, 'another-process', 'EX', 60)
374+
try {
375+
const staleRow = await storedAccount()
376+
const follower = refreshTokenIfNeeded('revocation-integration', staleRow, accountId)
377+
await db
378+
.update(account)
379+
.set({ refreshRevokedAt, refreshRevokedCode, refreshRevokedTokenHash })
380+
.where(eq(account.id, accountId))
381+
await expect(follower).rejects.toBeInstanceOf(CredentialRevokedError)
382+
} finally {
383+
await redis.del(lockKey)
384+
}
385+
expect(providerRequests).toBe(1)
380386
}
381-
expect(providerRequests).toBe(1)
382-
})
387+
)
383388

384389
it('records a Slack revocation for the whole installation until a connect fans out', async () => {
385390
await insertSlackInstallation()

‎apps/sim/lib/oauth/slack.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@ interface FanOutOptions {
5454
*/
5555
ifChainUnchangedSince?: Date
5656
/**
57-
* The provider just issued this chain (a refresh or a connect), so a revocation recorded
58-
* against the installation no longer holds. Omit when re-spreading a chain already stored.
57+
* The provider just issued this chain in a refresh, so a revocation recorded against the
58+
* installation no longer holds. Omit when re-spreading a chain already stored; a connect's
59+
* credential draft clears the installation's record itself.
5960
*/
6061
freshlyIssued?: boolean
6162
}

0 commit comments

Comments
 (0)