From fa4b65ad3bc207a88c79f50b981178290bef0f98 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Thu, 16 Jul 2026 15:56:58 -0700 Subject: [PATCH 1/6] fix(oauth): coalesce slack token refresh per installation and preserve provider refresh errors --- .../app/api/auth/oauth/token/route.test.ts | 33 +++++ apps/sim/app/api/auth/oauth/token/route.ts | 16 +- apps/sim/app/api/auth/oauth/utils.test.ts | 138 +++++++++++++++++- apps/sim/app/api/auth/oauth/utils.ts | 131 +++++++++++++---- apps/sim/lib/auth/auth.ts | 34 +++++ apps/sim/lib/oauth/errors.ts | 30 ++++ apps/sim/lib/oauth/oauth.ts | 12 ++ apps/sim/lib/oauth/slack.test.ts | 35 +++++ apps/sim/lib/oauth/slack.ts | 91 ++++++++++++ apps/sim/lib/oauth/terminal-errors.ts | 1 + 10 files changed, 485 insertions(+), 36 deletions(-) create mode 100644 apps/sim/lib/oauth/errors.ts create mode 100644 apps/sim/lib/oauth/slack.test.ts create mode 100644 apps/sim/lib/oauth/slack.ts diff --git a/apps/sim/app/api/auth/oauth/token/route.test.ts b/apps/sim/app/api/auth/oauth/token/route.test.ts index e1ef6105675..7bb4712dae4 100644 --- a/apps/sim/app/api/auth/oauth/token/route.test.ts +++ b/apps/sim/app/api/auth/oauth/token/route.test.ts @@ -27,6 +27,7 @@ vi.mock('@/lib/auth/credential-access', () => ({ })) import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors' +import { OAuthRefreshError } from '@/lib/oauth/errors' import { GET, POST } from '@/app/api/auth/oauth/token/route' describe('OAuth Token API Routes', () => { @@ -301,6 +302,38 @@ describe('OAuth Token API Routes', () => { ) }) + it('should surface the provider error detail on typed refresh failures', async () => { + mockAuthorizeCredentialUse.mockResolvedValueOnce({ + ok: true, + authType: 'session', + requesterUserId: 'test-user-id', + credentialOwnerUserId: 'owner-user-id', + }) + authOAuthUtilsMockFns.mockGetCredential.mockResolvedValueOnce({ + id: 'credential-id', + accessToken: 'test-token', + refreshToken: 'refresh-token', + accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000), + providerId: 'google', + }) + authOAuthUtilsMockFns.mockRefreshTokenIfNeeded.mockRejectedValueOnce( + new OAuthRefreshError('google', 'invalid_grant', 'Token has been expired or revoked.') + ) + + const req = createMockRequest('POST', { + credentialId: 'credential-id', + }) + + const response = await POST(req) + const data = await response.json() + + expect(response.status).toBe(401) + expect(data).toHaveProperty( + 'error', + 'Failed to refresh access token: invalid_grant (google: Token has been expired or revoked.)' + ) + }) + describe('credentialAccountUserId + providerId path', () => { it('should reject unauthenticated requests', async () => { hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValueOnce({ diff --git a/apps/sim/app/api/auth/oauth/token/route.ts b/apps/sim/app/api/auth/oauth/token/route.ts index b767b59325e..c4445d99ac2 100644 --- a/apps/sim/app/api/auth/oauth/token/route.ts +++ b/apps/sim/app/api/auth/oauth/token/route.ts @@ -12,6 +12,7 @@ import { AuthType, checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors' +import { OAuthRefreshError } from '@/lib/oauth/errors' import { captureServerEvent } from '@/lib/posthog/server' import { getCredential, @@ -300,7 +301,11 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } catch (error) { logger.error(`[${requestId}] Failed to refresh access token:`, error) - return NextResponse.json({ error: 'Failed to refresh access token' }, { status: 401 }) + const detail = error instanceof OAuthRefreshError ? `: ${error.message}` : '' + return NextResponse.json( + { error: `Failed to refresh access token${detail}` }, + { status: 401 } + ) } } catch (error) { logger.error(`[${requestId}] Error getting access token`, error) @@ -410,8 +415,13 @@ export const GET = withRouteHandler(async (request: NextRequest) => { }, { status: 200 } ) - } catch (_error) { - return NextResponse.json({ error: 'Failed to refresh access token' }, { status: 401 }) + } catch (error) { + logger.error(`[${requestId}] Failed to refresh access token:`, error) + const detail = error instanceof OAuthRefreshError ? `: ${error.message}` : '' + return NextResponse.json( + { error: `Failed to refresh access token${detail}` }, + { status: 401 } + ) } } catch (error) { logger.error(`[${requestId}] Error fetching access token`, error) diff --git a/apps/sim/app/api/auth/oauth/utils.test.ts b/apps/sim/app/api/auth/oauth/utils.test.ts index 0e6550d44ea..7b9c988e7f4 100644 --- a/apps/sim/app/api/auth/oauth/utils.test.ts +++ b/apps/sim/app/api/auth/oauth/utils.test.ts @@ -166,7 +166,28 @@ describe('OAuth Utils', () => { await expect( refreshTokenIfNeeded('request-id', mockCredential, 'credential-id') - ).rejects.toThrow('Failed to refresh token') + ).rejects.toThrow('invalid_grant (google)') + }) + + it('should preserve the provider error description in the thrown error', async () => { + const mockCredential = { + id: 'credential-id', + accessToken: 'expired-token', + refreshToken: 'refresh-token', + accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000), + providerId: 'google', + } + + mockRefreshOAuthToken.mockResolvedValueOnce({ + ok: false, + errorCode: 'invalid_grant', + errorDescription: 'Token has been expired or revoked.', + message: 'Failed', + }) + + await expect( + refreshTokenIfNeeded('request-id', mockCredential, 'credential-id') + ).rejects.toThrow('invalid_grant (google: Token has been expired or revoked.)') }) it('should not attempt refresh if no refresh token', async () => { @@ -282,6 +303,121 @@ describe('OAuth Utils', () => { }) }) + describe('Slack installation-scoped refresh', () => { + const SLACK_ACCOUNT_ID = 'T08CM6ZNYBE-usr_U08USBQ9B1T-cbf46a7e-ca75-4a2e-bef5-fd467299eaae' + const past = new Date(Date.now() - 3600 * 1000) + const future = new Date(Date.now() + 3600 * 1000) + + /** Select chain for getFreshestSlackChain: where() -> orderBy() -> limit(). */ + function mockSelectOrderedChain(limitResult: unknown[]) { + const mockLimit = vi.fn().mockReturnValue(limitResult) + const mockOrderBy = vi.fn().mockReturnValue({ limit: mockLimit }) + const mockWhere = vi.fn().mockReturnValue({ orderBy: mockOrderBy, limit: mockLimit }) + const mockFrom = vi.fn().mockReturnValue({ where: mockWhere }) + mockDb.select.mockReturnValueOnce({ from: mockFrom }) + return { mockWhere, mockOrderBy, mockLimit } + } + + function slackCredential(overrides: Record = {}) { + return { + id: 'row-1', + resolvedCredentialId: 'row-1', + accountId: SLACK_ACCOUNT_ID, + accessToken: 'stale-at', + refreshToken: 'stale-rt', + accessTokenExpiresAt: past, + providerId: 'slack', + ...overrides, + } + } + + it('locks per installation and refreshes with the freshest sibling refresh token', async () => { + mockSelectOrderedChain([ + { accessToken: 'stale-at', refreshToken: 'live-rt', accessTokenExpiresAt: past }, + ]) + mockRefreshOAuthToken.mockResolvedValueOnce({ + ok: true, + accessToken: 'new-at', + expiresIn: 43200, + refreshToken: 'new-rt', + }) + const { mockSet } = mockUpdateChain() + + const result = await refreshTokenIfNeeded('request-id', slackCredential(), 'row-1') + + expect(result).toEqual({ accessToken: 'new-at', refreshed: true }) + expect(redisConfigMockFns.mockAcquireLock.mock.calls[0][0]).toBe( + 'oauth:refresh:slack:T08CM6ZNYBE' + ) + expect(mockRefreshOAuthToken).toHaveBeenCalledWith('slack', 'live-rt') + expect(mockSet).toHaveBeenCalledWith( + expect.objectContaining({ accessToken: 'new-at', refreshToken: 'new-rt' }) + ) + }) + + it('returns the freshest sibling token without refreshing when it is still valid', async () => { + mockSelectOrderedChain([ + { accessToken: 'sibling-at', refreshToken: 'live-rt', accessTokenExpiresAt: future }, + ]) + const { mockSet } = mockUpdateChain() + + const result = await refreshTokenIfNeeded('request-id', slackCredential(), 'row-1') + + expect(result).toEqual({ accessToken: 'sibling-at', refreshed: true }) + expect(mockRefreshOAuthToken).not.toHaveBeenCalled() + expect(mockSet).toHaveBeenCalledWith( + expect.objectContaining({ accessToken: 'sibling-at', refreshToken: 'live-rt' }) + ) + }) + + it('keeps per-row behavior for pasted custom-bot account ids', async () => { + mockRefreshOAuthToken.mockResolvedValueOnce({ + ok: true, + accessToken: 'new-at', + expiresIn: 43200, + refreshToken: 'new-rt', + }) + mockUpdateChain() + + const result = await refreshTokenIfNeeded( + 'request-id', + slackCredential({ accountId: 'slack-bot-1764756583292' }), + 'row-1' + ) + + expect(result).toEqual({ accessToken: 'new-at', refreshed: true }) + expect(redisConfigMockFns.mockAcquireLock.mock.calls[0][0]).toBe('oauth:refresh:row-1') + expect(mockRefreshOAuthToken).toHaveBeenCalledWith('slack', 'stale-rt') + }) + + it('dead-flags the installation, not the row, on terminal refresh errors', async () => { + const fakeRedis = { + set: vi.fn().mockResolvedValue('OK'), + get: vi.fn().mockResolvedValue(null), + del: vi.fn().mockResolvedValue(1), + } + redisConfigMockFns.mockGetRedisClient.mockReturnValue(fakeRedis) + mockSelectOrderedChain([ + { accessToken: 'stale-at', refreshToken: 'live-rt', accessTokenExpiresAt: past }, + ]) + mockRefreshOAuthToken.mockResolvedValueOnce({ + ok: false, + errorCode: 'token_revoked', + }) + + await expect(refreshTokenIfNeeded('request-id', slackCredential(), 'row-1')).rejects.toThrow( + 'token_revoked (slack)' + ) + + expect(fakeRedis.set).toHaveBeenCalledWith( + 'oauth:dead:slack:T08CM6ZNYBE', + 'token_revoked', + 'EX', + 3600 + ) + }) + }) + describe('resolveServiceAccountToken', () => { it('throws loudly for an unknown provider (never silently attempts Google)', async () => { await expect(resolveServiceAccountToken('cred-1', 'mystery-provider')).rejects.toThrow( diff --git a/apps/sim/app/api/auth/oauth/utils.ts b/apps/sim/app/api/auth/oauth/utils.ts index 4fd56f0e0a7..fe521b7b52e 100644 --- a/apps/sim/app/api/auth/oauth/utils.ts +++ b/apps/sim/app/api/auth/oauth/utils.ts @@ -21,12 +21,19 @@ import { type TokenServiceAccountSecretBlob, } from '@/lib/credentials/token-service-accounts/server' import { refreshOAuthToken } from '@/lib/oauth' +import { OAuthRefreshError } from '@/lib/oauth/errors' import { isInstagramProvider, shouldProactivelyRefreshInstagramToken } from '@/lib/oauth/instagram' import { getMicrosoftRefreshTokenExpiry, isMicrosoftProvider, PROACTIVE_REFRESH_THRESHOLD_DAYS, } from '@/lib/oauth/microsoft' +import { + extractSlackTeamId, + fanOutSlackTokenChain, + getFreshestSlackChain, + isSlackProvider, +} from '@/lib/oauth/slack' import { getRecentTerminalError, isTerminalRefreshError, @@ -658,41 +665,83 @@ interface CoalescedRefreshOptions { accountId: string providerId: string refreshToken: string + /** External provider account id (`account.accountId`), used to scope Slack refreshes per installation. */ + providerAccountId?: string | null requestId?: string userId?: string } +interface CoalescedRefreshOutcome { + accessToken: string | null + /** Present when the refresh failed with a known provider error; absent on follower timeout / transient failure. */ + error?: OAuthRefreshError +} + async function performCoalescedRefresh({ accountId, providerId, refreshToken, + providerAccountId, requestId, userId, -}: CoalescedRefreshOptions): Promise { +}: CoalescedRefreshOptions): Promise { + /** + * Slack bot tokens are per-installation (team × app): every account row for + * one team holds a copy of the same rotating chain, so refreshes are locked, + * dead-flagged, and written per installation rather than per row. + */ + const slackTeamId = isSlackProvider(providerId) ? extractSlackTeamId(providerAccountId) : null + const scopeKey = slackTeamId ? `slack:${slackTeamId}` : accountId + const logContext = { ...(requestId ? { requestId } : {}), ...(userId ? { userId } : {}), + ...(slackTeamId ? { slackTeamId } : {}), providerId, accountId, } - const deadCode = await getRecentTerminalError(accountId) + const deadCode = await getRecentTerminalError(scopeKey) if (deadCode) { logger.warn('Skipping refresh: credential recently failed', { ...logContext, errorCode: deadCode, }) - return null + return { accessToken: null, error: new OAuthRefreshError(providerId, deadCode) } } - const lockKey = `oauth:refresh:${accountId}` + const lockKey = `oauth:refresh:${scopeKey}` const refreshPromise = coalesceLocally(lockKey, () => - withLeaderLock({ + withLeaderLock({ key: lockKey, onLeader: async () => { try { - const result = await refreshOAuthToken(providerId, refreshToken) + let refreshTokenToUse = refreshToken + if (slackTeamId) { + const freshest = await getFreshestSlackChain(slackTeamId) + if (!freshest) { + throw new Error( + `No refresh-capable account row found for Slack installation ${slackTeamId}` + ) + } + if ( + freshest.accessToken && + freshest.accessTokenExpiresAt && + freshest.accessTokenExpiresAt > new Date() + ) { + await fanOutSlackTokenChain(slackTeamId, { + accessToken: freshest.accessToken, + refreshToken: freshest.refreshToken, + accessTokenExpiresAt: freshest.accessTokenExpiresAt, + }) + logger.info('Reused freshest Slack installation token', logContext) + return { accessToken: freshest.accessToken } + } + refreshTokenToUse = freshest.refreshToken + } + + const result = await refreshOAuthToken(providerId, refreshTokenToUse) if (!result.ok) { logger.error('Failed to refresh token', { @@ -700,33 +749,46 @@ async function performCoalescedRefresh({ errorCode: result.errorCode, }) if (result.errorCode && isTerminalRefreshError(result.errorCode)) { - await markCredentialDead(accountId, result.errorCode) + await markCredentialDead(scopeKey, result.errorCode) + } + return { + accessToken: null, + error: new OAuthRefreshError(providerId, result.errorCode, result.errorDescription), } - return null } - const updateData: Record = { - accessToken: result.accessToken, - accessTokenExpiresAt: new Date(Date.now() + result.expiresIn * 1000), - updatedAt: new Date(), - } - if (result.refreshToken && result.refreshToken !== refreshToken) { - updateData.refreshToken = result.refreshToken - } - if (isMicrosoftProvider(providerId)) { - updateData.refreshTokenExpiresAt = getMicrosoftRefreshTokenExpiry() - } + const accessTokenExpiresAt = new Date(Date.now() + result.expiresIn * 1000) - await db.update(account).set(updateData).where(eq(account.id, accountId)) + if (slackTeamId) { + await fanOutSlackTokenChain(slackTeamId, { + accessToken: result.accessToken, + refreshToken: result.refreshToken || refreshTokenToUse, + accessTokenExpiresAt, + }) + } else { + const updateData: Record = { + accessToken: result.accessToken, + accessTokenExpiresAt, + updatedAt: new Date(), + } + if (result.refreshToken && result.refreshToken !== refreshToken) { + updateData.refreshToken = result.refreshToken + } + if (isMicrosoftProvider(providerId)) { + updateData.refreshTokenExpiresAt = getMicrosoftRefreshTokenExpiry() + } + + await db.update(account).set(updateData).where(eq(account.id, accountId)) + } logger.info('Successfully refreshed access token', logContext) - return result.accessToken + return { accessToken: result.accessToken } } catch (error) { logger.error('Refresh failed inside leader path', { ...logContext, error: toError(error).message, }) - return null + return { accessToken: null } } }, onFollower: async () => { @@ -745,7 +807,7 @@ async function performCoalescedRefresh({ row.accessTokenExpiresAt > new Date() ) { logger.info('Got fresh access token from coalesced refresh', logContext) - return row.accessToken + return { accessToken: row.accessToken } } return null } catch (error) { @@ -760,13 +822,13 @@ async function performCoalescedRefresh({ ) try { - return await refreshPromise + return (await refreshPromise) ?? { accessToken: null } } catch (error) { logger.error('Coalesced refresh did not settle', { ...logContext, error: toError(error).message, }) - return null + return { accessToken: null } } } @@ -774,6 +836,7 @@ export async function getOAuthToken(userId: string, providerId: string): Promise const connections = await db .select({ id: account.id, + providerAccountId: account.accountId, accessToken: account.accessToken, refreshToken: account.refreshToken, accessTokenExpiresAt: account.accessTokenExpiresAt, @@ -809,16 +872,18 @@ export async function getOAuthToken(userId: string, providerId: string): Promise }) if (accessTokenNeedsRefresh || instagramNeedsProactiveRefresh) { - const fresh = await performCoalescedRefresh({ + const outcome = await performCoalescedRefresh({ accountId: credential.id, providerId, refreshToken: credential.refreshToken!, + providerAccountId: credential.providerAccountId, userId, }) - if (fresh) return fresh + if (outcome.accessToken) return outcome.accessToken if (!accessTokenNeedsRefresh && credential.accessToken) { return credential.accessToken } + if (outcome.error) throw outcome.error return null } @@ -911,14 +976,15 @@ export async function resolveCredentialAccessToken( const resolvedCredentialId = (credential as { resolvedCredentialId?: string }).resolvedCredentialId ?? credentialId - const fresh = await performCoalescedRefresh({ + const outcome = await performCoalescedRefresh({ accountId: resolvedCredentialId, providerId: credential.providerId, refreshToken: credential.refreshToken!, + providerAccountId: credential.accountId, requestId, userId: credential.userId, }) - if (fresh) return { accessToken: fresh } + if (outcome.accessToken) return { accessToken: outcome.accessToken } // If refresh was only triggered proactively (Microsoft refresh-token aging / // Instagram long-lived nearing expiry), the still-valid access token is fine. @@ -1015,18 +1081,19 @@ export async function refreshTokenIfNeeded( return { accessToken: credential.accessToken, refreshed: false } } - const fresh = await performCoalescedRefresh({ + const outcome = await performCoalescedRefresh({ accountId: resolvedCredentialId, providerId: credential.providerId, refreshToken: credential.refreshToken!, + providerAccountId: credential.accountId, requestId, userId: credential.userId, }) - if (fresh) return { accessToken: fresh, refreshed: true } + if (outcome.accessToken) return { accessToken: outcome.accessToken, refreshed: true } if (!accessTokenNeedsRefresh && credential.accessToken) { logger.info(`[${requestId}] Refresh unavailable; reusing still-valid access token`) return { accessToken: credential.accessToken, refreshed: false } } - throw new Error('Failed to refresh token') + throw outcome.error ?? new Error('Failed to refresh token') } diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index a4aeafc2343..d5ae502fe2e 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -95,6 +95,8 @@ import { getMicrosoftRefreshTokenExpiry, isMicrosoftProvider, } from '@/lib/oauth/microsoft' +import { extractSlackTeamId, fanOutSlackTokenChain } from '@/lib/oauth/slack' +import { clearDeadFlag } from '@/lib/oauth/terminal-errors' import { getCanonicalScopesForProvider } from '@/lib/oauth/utils' import { captureServerEvent, getPostHogClient } from '@/lib/posthog/server' import { disableUserResources } from '@/lib/workflows/lifecycle' @@ -444,6 +446,38 @@ export const auth = betterAuth({ }) } + /** + * A fresh Slack connect re-issues the installation's rotating token + * chain, invalidating the copies held by sibling account rows for the + * same team (Slack bot tokens are per-installation, not per-grant). + * Propagate the new chain so every sibling is valid again, and clear + * the installation's dead flag. + */ + if (account.providerId === 'slack' && account.accessToken) { + try { + const teamId = extractSlackTeamId(account.accountId) + if (teamId) { + await fanOutSlackTokenChain(teamId, { + accessToken: account.accessToken, + refreshToken: account.refreshToken ?? null, + accessTokenExpiresAt: account.accessTokenExpiresAt ?? null, + }) + await clearDeadFlag(`slack:${teamId}`) + logger.info('[account.create.after] Propagated Slack installation token chain', { + userId: account.userId, + teamId, + newAccountId: account.id, + }) + } + } catch (error) { + logger.error('[account.create.after] Failed to propagate Slack token chain', { + userId: account.userId, + accountId: account.id, + error, + }) + } + } + try { await processCredentialDraft({ userId: account.userId, diff --git a/apps/sim/lib/oauth/errors.ts b/apps/sim/lib/oauth/errors.ts new file mode 100644 index 00000000000..11150808e0d --- /dev/null +++ b/apps/sim/lib/oauth/errors.ts @@ -0,0 +1,30 @@ +import { truncate } from '@sim/utils/string' + +const MAX_DESCRIPTION_LENGTH = 300 + +function buildSafeMessage( + providerId: string, + errorCode?: string, + errorDescription?: string +): string { + const code = errorCode || 'unknown_error' + return errorDescription + ? `${code} (${providerId}: ${truncate(errorDescription, MAX_DESCRIPTION_LENGTH)})` + : `${code} (${providerId})` +} + +/** + * A token-refresh failure with the provider's error preserved. The message is + * built from the provider error code and (truncated) error description only — + * never raw response bodies — so it is safe to surface to end users. + */ +export class OAuthRefreshError extends Error { + constructor( + readonly providerId: string, + readonly errorCode?: string, + errorDescription?: string + ) { + super(buildSafeMessage(providerId, errorCode, errorDescription)) + this.name = 'OAuthRefreshError' + } +} diff --git a/apps/sim/lib/oauth/oauth.ts b/apps/sim/lib/oauth/oauth.ts index c2cfad05096..8926a8a0a87 100644 --- a/apps/sim/lib/oauth/oauth.ts +++ b/apps/sim/lib/oauth/oauth.ts @@ -1634,6 +1634,7 @@ export interface RefreshTokenSuccess { export interface RefreshTokenFailure { ok: false errorCode?: string + errorDescription?: string message?: string } @@ -1651,6 +1652,14 @@ function extractErrorCode(value: unknown): string | undefined { return undefined } +function extractErrorDescription(value: unknown): string | undefined { + if (value && typeof value === 'object' && 'error_description' in value) { + const description = (value as { error_description: unknown }).error_description + if (typeof description === 'string' && description.trim()) return description + } + return undefined +} + /** * Hard deadline on the token-endpoint exchange. This function does not coalesce * on its own; its sole production caller (`performCoalescedRefresh` in the OAuth @@ -1698,6 +1707,7 @@ async function refreshInstagramLongLivedToken( return { ok: false, errorCode: extractErrorCode(responseData), + errorDescription: extractErrorDescription(responseData), message: `Failed to refresh token: ${response.status} ${errorSummary}`, } } @@ -1769,6 +1779,7 @@ export async function refreshOAuthToken( return { ok: false, errorCode: extractErrorCode(errorData), + errorDescription: extractErrorDescription(errorData), message: `Failed to refresh token: ${response.status} ${errorText}`, } } @@ -1790,6 +1801,7 @@ export async function refreshOAuthToken( return { ok: false, errorCode: typeof data.error === 'string' ? data.error : undefined, + errorDescription: extractErrorDescription(data), message: `Failed to refresh token: ${data.error ?? 'unknown'}`, } } diff --git a/apps/sim/lib/oauth/slack.test.ts b/apps/sim/lib/oauth/slack.test.ts new file mode 100644 index 00000000000..e38c919f8d3 --- /dev/null +++ b/apps/sim/lib/oauth/slack.test.ts @@ -0,0 +1,35 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { extractSlackTeamId } from '@/lib/oauth/slack' + +describe('extractSlackTeamId', () => { + it('extracts the team id from a scoped account id', () => { + expect( + extractSlackTeamId('T08CM6ZNYBE-usr_U08USBQ9B1T-cbf46a7e-ca75-4a2e-bef5-fd467299eaae') + ).toBe('T08CM6ZNYBE') + }) + + it('extracts the team id from a legacy bot-segment account id', () => { + expect(extractSlackTeamId('T08CM6ZNYBE-U08USBQ9B1T-599a2a79-2543-42fd-9a74-9e2c466a8b19')).toBe( + 'T08CM6ZNYBE' + ) + }) + + it('accepts enterprise grid ids', () => { + expect(extractSlackTeamId('E0123ABCD-usr_U1-aaaa')).toBe('E0123ABCD') + }) + + it('returns null for pasted custom-bot account ids', () => { + expect(extractSlackTeamId('slack-bot-1764756583292')).toBeNull() + }) + + it('returns null for lowercase or malformed ids', () => { + expect(extractSlackTeamId('t123-usr_U1-x')).toBeNull() + expect(extractSlackTeamId('T08CM6ZNYBE')).toBeNull() + expect(extractSlackTeamId('')).toBeNull() + expect(extractSlackTeamId(null)).toBeNull() + expect(extractSlackTeamId(undefined)).toBeNull() + }) +}) diff --git a/apps/sim/lib/oauth/slack.ts b/apps/sim/lib/oauth/slack.ts new file mode 100644 index 00000000000..73b36ea02f3 --- /dev/null +++ b/apps/sim/lib/oauth/slack.ts @@ -0,0 +1,91 @@ +import { db } from '@sim/db' +import { account } from '@sim/db/schema' +import { and, eq, isNotNull, like, sql } from 'drizzle-orm' + +/** + * Slack bot tokens belong to the installation (team × app), not to the OAuth + * grant: every connect of the same Slack workspace hands back the same rotating + * token chain, so all `account` rows for one team are copies of one credential. + * These helpers let the refresh path treat those rows as a single unit. + * + * External account ids are `${teamId}-${userSegment}-${uuid}` (see the Slack + * getUserInfo in lib/auth/auth.ts). The team segment charset `[TE][A-Z0-9]+` + * contains no LIKE metacharacters, and the trailing `-` in the prefix match + * prevents `T123` from matching `T1234-...`. + */ +const SLACK_TEAM_PREFIX_RE = /^([TE][A-Z0-9]+)-/ + +export function isSlackProvider(providerId: string): boolean { + return providerId === 'slack' +} + +/** + * Extracts the Slack installation (team/enterprise) id from an external account + * id. Returns null for ids that don't carry a team prefix (e.g. legacy pasted + * `slack-bot-...` rows), which keep per-row refresh behavior. + */ +export function extractSlackTeamId(externalAccountId: string | null | undefined): string | null { + if (!externalAccountId) return null + const match = SLACK_TEAM_PREFIX_RE.exec(externalAccountId) + return match ? match[1] : null +} + +function installationFilter(teamId: string) { + return and(eq(account.providerId, 'slack'), like(account.accountId, `${teamId}-%`)) +} + +interface SlackTokenChain { + accessToken: string + refreshToken: string | null + accessTokenExpiresAt: Date | null +} + +/** + * Writes a token chain to every account row of a Slack installation. Rotation + * revokes whatever the sibling rows were holding, so a successful refresh or a + * fresh connect must overwrite all copies or the stale ones fail with + * `token_revoked` at call time. + */ +export async function fanOutSlackTokenChain(teamId: string, chain: SlackTokenChain): Promise { + await db + .update(account) + .set({ + accessToken: chain.accessToken, + accessTokenExpiresAt: chain.accessTokenExpiresAt, + ...(chain.refreshToken ? { refreshToken: chain.refreshToken } : {}), + updatedAt: new Date(), + }) + .where(installationFilter(teamId)) +} + +interface FreshestSlackChain { + accessToken: string | null + refreshToken: string + accessTokenExpiresAt: Date | null +} + +/** + * Reads the freshest token chain for an installation: the sibling row with the + * latest access-token expiry that still holds a refresh token. The caller's own + * copy may already be rotated away; the installation's live refresh token is + * the most recently issued one. + */ +export async function getFreshestSlackChain(teamId: string): Promise { + const [row] = await db + .select({ + accessToken: account.accessToken, + refreshToken: account.refreshToken, + accessTokenExpiresAt: account.accessTokenExpiresAt, + }) + .from(account) + .where(and(installationFilter(teamId), isNotNull(account.refreshToken))) + .orderBy(sql`${account.accessTokenExpiresAt} DESC NULLS LAST`) + .limit(1) + + if (!row?.refreshToken) return null + return { + accessToken: row.accessToken, + refreshToken: row.refreshToken, + accessTokenExpiresAt: row.accessTokenExpiresAt, + } +} diff --git a/apps/sim/lib/oauth/terminal-errors.ts b/apps/sim/lib/oauth/terminal-errors.ts index 25fba73205c..81d915531f6 100644 --- a/apps/sim/lib/oauth/terminal-errors.ts +++ b/apps/sim/lib/oauth/terminal-errors.ts @@ -12,6 +12,7 @@ const TERMINAL_ERRORS = new Set([ 'invalid_client_id', 'invalid_client', 'bad_redirect_uri', + 'token_revoked', ]) const DEAD_CACHE_TTL_SEC = 60 * 60 From 2c649324a98183ac8d41d6377a07aef586454418 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Thu, 16 Jul 2026 16:06:10 -0700 Subject: [PATCH 2/6] fix(oauth): keep transient refresh failures errorless and size slack lock budgets past the provider timeout --- apps/sim/app/api/auth/oauth/utils.test.ts | 20 +++++++++++++++++ apps/sim/app/api/auth/oauth/utils.ts | 26 ++++++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/apps/sim/app/api/auth/oauth/utils.test.ts b/apps/sim/app/api/auth/oauth/utils.test.ts index 7b9c988e7f4..5a5f172068c 100644 --- a/apps/sim/app/api/auth/oauth/utils.test.ts +++ b/apps/sim/app/api/auth/oauth/utils.test.ts @@ -190,6 +190,25 @@ describe('OAuth Utils', () => { ).rejects.toThrow('invalid_grant (google: Token has been expired or revoked.)') }) + it('should throw the generic error on transient failures without a provider code', async () => { + const mockCredential = { + id: 'credential-id', + accessToken: 'expired-token', + refreshToken: 'refresh-token', + accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000), + providerId: 'google', + } + + mockRefreshOAuthToken.mockResolvedValueOnce({ + ok: false, + message: 'fetch failed', + }) + + await expect( + refreshTokenIfNeeded('request-id', mockCredential, 'credential-id') + ).rejects.toThrow('Failed to refresh token') + }) + it('should not attempt refresh if no refresh token', async () => { const mockCredential = { id: 'credential-id', @@ -349,6 +368,7 @@ describe('OAuth Utils', () => { expect(redisConfigMockFns.mockAcquireLock.mock.calls[0][0]).toBe( 'oauth:refresh:slack:T08CM6ZNYBE' ) + expect(redisConfigMockFns.mockAcquireLock.mock.calls[0][2]).toBe(20) expect(mockRefreshOAuthToken).toHaveBeenCalledWith('slack', 'live-rt') expect(mockSet).toHaveBeenCalledWith( expect.objectContaining({ accessToken: 'new-at', refreshToken: 'new-rt' }) diff --git a/apps/sim/app/api/auth/oauth/utils.ts b/apps/sim/app/api/auth/oauth/utils.ts index fe521b7b52e..829d3395446 100644 --- a/apps/sim/app/api/auth/oauth/utils.ts +++ b/apps/sim/app/api/auth/oauth/utils.ts @@ -677,6 +677,15 @@ interface CoalescedRefreshOutcome { error?: OAuthRefreshError } +/** + * Slack lock budgets sized past `TOKEN_REFRESH_TIMEOUT_MS` (15s) in + * lib/oauth/oauth.ts: installation-keyed locks make every sibling row's request + * a follower of one refresh, so followers must keep polling for the leader's + * full provider window and the lock must not expire under a live refresh. + */ +const SLACK_FOLLOWER_MAX_WAIT_MS = 16_000 +const SLACK_LOCK_TTL_SEC = 20 + async function performCoalescedRefresh({ accountId, providerId, @@ -715,6 +724,11 @@ async function performCoalescedRefresh({ const refreshPromise = coalesceLocally(lockKey, () => withLeaderLock({ key: lockKey, + // Installation-keyed Slack locks gather followers from every sibling row, + // so their wait and the lock TTL must outlast the 15s provider timeout — + // the 3s/10s defaults would fail followers early and let a second leader + // start a concurrent rotation mid-refresh. + ...(slackTeamId ? { maxWaitMs: SLACK_FOLLOWER_MAX_WAIT_MS, ttlSec: SLACK_LOCK_TTL_SEC } : {}), onLeader: async () => { try { let refreshTokenToUse = refreshToken @@ -753,7 +767,17 @@ async function performCoalescedRefresh({ } return { accessToken: null, - error: new OAuthRefreshError(providerId, result.errorCode, result.errorDescription), + // No errorCode = transient (timeout/network), not a provider rejection — + // stay errorless so callers keep their null-fallback behavior. + ...(result.errorCode + ? { + error: new OAuthRefreshError( + providerId, + result.errorCode, + result.errorDescription + ), + } + : {}), } } From c60a39be381181bf3ee58c4d5c721ea3e71d6b47 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Thu, 16 Jul 2026 16:12:15 -0700 Subject: [PATCH 3/6] fix(oauth): let slack refresh followers poll for the lock's full lifetime --- apps/sim/app/api/auth/oauth/utils.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/api/auth/oauth/utils.ts b/apps/sim/app/api/auth/oauth/utils.ts index 829d3395446..c6891b8808d 100644 --- a/apps/sim/app/api/auth/oauth/utils.ts +++ b/apps/sim/app/api/auth/oauth/utils.ts @@ -680,11 +680,13 @@ interface CoalescedRefreshOutcome { /** * Slack lock budgets sized past `TOKEN_REFRESH_TIMEOUT_MS` (15s) in * lib/oauth/oauth.ts: installation-keyed locks make every sibling row's request - * a follower of one refresh, so followers must keep polling for the leader's - * full provider window and the lock must not expire under a live refresh. + * a follower of one refresh. The lock TTL must not expire under a live refresh + * (15s provider call plus DB reads and the fan-out write), and followers poll + * for the lock's full lifetime so a slow-but-successful refresh is still + * observed rather than reported as a failure. */ -const SLACK_FOLLOWER_MAX_WAIT_MS = 16_000 const SLACK_LOCK_TTL_SEC = 20 +const SLACK_FOLLOWER_MAX_WAIT_MS = SLACK_LOCK_TTL_SEC * 1000 async function performCoalescedRefresh({ accountId, From 54a38bee037cb8bc5d6681f3172229a4d28cc68c Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Thu, 16 Jul 2026 18:00:04 -0700 Subject: [PATCH 4/6] chore(oauth): drop provider refresh-error surfacing to keep this PR slack-only --- .../app/api/auth/oauth/token/route.test.ts | 33 ----------- apps/sim/app/api/auth/oauth/token/route.ts | 13 +---- apps/sim/app/api/auth/oauth/utils.test.ts | 42 +------------- apps/sim/app/api/auth/oauth/utils.ts | 55 ++++++------------- apps/sim/lib/oauth/errors.ts | 30 ---------- apps/sim/lib/oauth/oauth.ts | 12 ---- 6 files changed, 20 insertions(+), 165 deletions(-) delete mode 100644 apps/sim/lib/oauth/errors.ts diff --git a/apps/sim/app/api/auth/oauth/token/route.test.ts b/apps/sim/app/api/auth/oauth/token/route.test.ts index 7bb4712dae4..e1ef6105675 100644 --- a/apps/sim/app/api/auth/oauth/token/route.test.ts +++ b/apps/sim/app/api/auth/oauth/token/route.test.ts @@ -27,7 +27,6 @@ vi.mock('@/lib/auth/credential-access', () => ({ })) import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors' -import { OAuthRefreshError } from '@/lib/oauth/errors' import { GET, POST } from '@/app/api/auth/oauth/token/route' describe('OAuth Token API Routes', () => { @@ -302,38 +301,6 @@ describe('OAuth Token API Routes', () => { ) }) - it('should surface the provider error detail on typed refresh failures', async () => { - mockAuthorizeCredentialUse.mockResolvedValueOnce({ - ok: true, - authType: 'session', - requesterUserId: 'test-user-id', - credentialOwnerUserId: 'owner-user-id', - }) - authOAuthUtilsMockFns.mockGetCredential.mockResolvedValueOnce({ - id: 'credential-id', - accessToken: 'test-token', - refreshToken: 'refresh-token', - accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000), - providerId: 'google', - }) - authOAuthUtilsMockFns.mockRefreshTokenIfNeeded.mockRejectedValueOnce( - new OAuthRefreshError('google', 'invalid_grant', 'Token has been expired or revoked.') - ) - - const req = createMockRequest('POST', { - credentialId: 'credential-id', - }) - - const response = await POST(req) - const data = await response.json() - - expect(response.status).toBe(401) - expect(data).toHaveProperty( - 'error', - 'Failed to refresh access token: invalid_grant (google: Token has been expired or revoked.)' - ) - }) - describe('credentialAccountUserId + providerId path', () => { it('should reject unauthenticated requests', async () => { hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValueOnce({ diff --git a/apps/sim/app/api/auth/oauth/token/route.ts b/apps/sim/app/api/auth/oauth/token/route.ts index c4445d99ac2..6b5c2adad6d 100644 --- a/apps/sim/app/api/auth/oauth/token/route.ts +++ b/apps/sim/app/api/auth/oauth/token/route.ts @@ -12,7 +12,6 @@ import { AuthType, checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors' -import { OAuthRefreshError } from '@/lib/oauth/errors' import { captureServerEvent } from '@/lib/posthog/server' import { getCredential, @@ -301,11 +300,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } catch (error) { logger.error(`[${requestId}] Failed to refresh access token:`, error) - const detail = error instanceof OAuthRefreshError ? `: ${error.message}` : '' - return NextResponse.json( - { error: `Failed to refresh access token${detail}` }, - { status: 401 } - ) + return NextResponse.json({ error: 'Failed to refresh access token' }, { status: 401 }) } } catch (error) { logger.error(`[${requestId}] Error getting access token`, error) @@ -417,11 +412,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => { ) } catch (error) { logger.error(`[${requestId}] Failed to refresh access token:`, error) - const detail = error instanceof OAuthRefreshError ? `: ${error.message}` : '' - return NextResponse.json( - { error: `Failed to refresh access token${detail}` }, - { status: 401 } - ) + return NextResponse.json({ error: 'Failed to refresh access token' }, { status: 401 }) } } catch (error) { logger.error(`[${requestId}] Error fetching access token`, error) diff --git a/apps/sim/app/api/auth/oauth/utils.test.ts b/apps/sim/app/api/auth/oauth/utils.test.ts index 5a5f172068c..a270ede978c 100644 --- a/apps/sim/app/api/auth/oauth/utils.test.ts +++ b/apps/sim/app/api/auth/oauth/utils.test.ts @@ -164,46 +164,6 @@ describe('OAuth Utils', () => { message: 'Failed', }) - await expect( - refreshTokenIfNeeded('request-id', mockCredential, 'credential-id') - ).rejects.toThrow('invalid_grant (google)') - }) - - it('should preserve the provider error description in the thrown error', async () => { - const mockCredential = { - id: 'credential-id', - accessToken: 'expired-token', - refreshToken: 'refresh-token', - accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000), - providerId: 'google', - } - - mockRefreshOAuthToken.mockResolvedValueOnce({ - ok: false, - errorCode: 'invalid_grant', - errorDescription: 'Token has been expired or revoked.', - message: 'Failed', - }) - - await expect( - refreshTokenIfNeeded('request-id', mockCredential, 'credential-id') - ).rejects.toThrow('invalid_grant (google: Token has been expired or revoked.)') - }) - - it('should throw the generic error on transient failures without a provider code', async () => { - const mockCredential = { - id: 'credential-id', - accessToken: 'expired-token', - refreshToken: 'refresh-token', - accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000), - providerId: 'google', - } - - mockRefreshOAuthToken.mockResolvedValueOnce({ - ok: false, - message: 'fetch failed', - }) - await expect( refreshTokenIfNeeded('request-id', mockCredential, 'credential-id') ).rejects.toThrow('Failed to refresh token') @@ -426,7 +386,7 @@ describe('OAuth Utils', () => { }) await expect(refreshTokenIfNeeded('request-id', slackCredential(), 'row-1')).rejects.toThrow( - 'token_revoked (slack)' + 'Failed to refresh token' ) expect(fakeRedis.set).toHaveBeenCalledWith( diff --git a/apps/sim/app/api/auth/oauth/utils.ts b/apps/sim/app/api/auth/oauth/utils.ts index c6891b8808d..09ff3b3f09a 100644 --- a/apps/sim/app/api/auth/oauth/utils.ts +++ b/apps/sim/app/api/auth/oauth/utils.ts @@ -21,7 +21,6 @@ import { type TokenServiceAccountSecretBlob, } from '@/lib/credentials/token-service-accounts/server' import { refreshOAuthToken } from '@/lib/oauth' -import { OAuthRefreshError } from '@/lib/oauth/errors' import { isInstagramProvider, shouldProactivelyRefreshInstagramToken } from '@/lib/oauth/instagram' import { getMicrosoftRefreshTokenExpiry, @@ -671,12 +670,6 @@ interface CoalescedRefreshOptions { userId?: string } -interface CoalescedRefreshOutcome { - accessToken: string | null - /** Present when the refresh failed with a known provider error; absent on follower timeout / transient failure. */ - error?: OAuthRefreshError -} - /** * Slack lock budgets sized past `TOKEN_REFRESH_TIMEOUT_MS` (15s) in * lib/oauth/oauth.ts: installation-keyed locks make every sibling row's request @@ -695,7 +688,7 @@ async function performCoalescedRefresh({ providerAccountId, requestId, userId, -}: CoalescedRefreshOptions): Promise { +}: CoalescedRefreshOptions): Promise { /** * Slack bot tokens are per-installation (team × app): every account row for * one team holds a copy of the same rotating chain, so refreshes are locked, @@ -718,13 +711,13 @@ async function performCoalescedRefresh({ ...logContext, errorCode: deadCode, }) - return { accessToken: null, error: new OAuthRefreshError(providerId, deadCode) } + return null } const lockKey = `oauth:refresh:${scopeKey}` const refreshPromise = coalesceLocally(lockKey, () => - withLeaderLock({ + withLeaderLock({ key: lockKey, // Installation-keyed Slack locks gather followers from every sibling row, // so their wait and the lock TTL must outlast the 15s provider timeout — @@ -752,7 +745,7 @@ async function performCoalescedRefresh({ accessTokenExpiresAt: freshest.accessTokenExpiresAt, }) logger.info('Reused freshest Slack installation token', logContext) - return { accessToken: freshest.accessToken } + return freshest.accessToken } refreshTokenToUse = freshest.refreshToken } @@ -767,20 +760,7 @@ async function performCoalescedRefresh({ if (result.errorCode && isTerminalRefreshError(result.errorCode)) { await markCredentialDead(scopeKey, result.errorCode) } - return { - accessToken: null, - // No errorCode = transient (timeout/network), not a provider rejection — - // stay errorless so callers keep their null-fallback behavior. - ...(result.errorCode - ? { - error: new OAuthRefreshError( - providerId, - result.errorCode, - result.errorDescription - ), - } - : {}), - } + return null } const accessTokenExpiresAt = new Date(Date.now() + result.expiresIn * 1000) @@ -808,13 +788,13 @@ async function performCoalescedRefresh({ } logger.info('Successfully refreshed access token', logContext) - return { accessToken: result.accessToken } + return result.accessToken } catch (error) { logger.error('Refresh failed inside leader path', { ...logContext, error: toError(error).message, }) - return { accessToken: null } + return null } }, onFollower: async () => { @@ -833,7 +813,7 @@ async function performCoalescedRefresh({ row.accessTokenExpiresAt > new Date() ) { logger.info('Got fresh access token from coalesced refresh', logContext) - return { accessToken: row.accessToken } + return row.accessToken } return null } catch (error) { @@ -848,13 +828,13 @@ async function performCoalescedRefresh({ ) try { - return (await refreshPromise) ?? { accessToken: null } + return await refreshPromise } catch (error) { logger.error('Coalesced refresh did not settle', { ...logContext, error: toError(error).message, }) - return { accessToken: null } + return null } } @@ -898,18 +878,17 @@ export async function getOAuthToken(userId: string, providerId: string): Promise }) if (accessTokenNeedsRefresh || instagramNeedsProactiveRefresh) { - const outcome = await performCoalescedRefresh({ + const fresh = await performCoalescedRefresh({ accountId: credential.id, providerId, refreshToken: credential.refreshToken!, providerAccountId: credential.providerAccountId, userId, }) - if (outcome.accessToken) return outcome.accessToken + if (fresh) return fresh if (!accessTokenNeedsRefresh && credential.accessToken) { return credential.accessToken } - if (outcome.error) throw outcome.error return null } @@ -1002,7 +981,7 @@ export async function resolveCredentialAccessToken( const resolvedCredentialId = (credential as { resolvedCredentialId?: string }).resolvedCredentialId ?? credentialId - const outcome = await performCoalescedRefresh({ + const fresh = await performCoalescedRefresh({ accountId: resolvedCredentialId, providerId: credential.providerId, refreshToken: credential.refreshToken!, @@ -1010,7 +989,7 @@ export async function resolveCredentialAccessToken( requestId, userId: credential.userId, }) - if (outcome.accessToken) return { accessToken: outcome.accessToken } + if (fresh) return { accessToken: fresh } // If refresh was only triggered proactively (Microsoft refresh-token aging / // Instagram long-lived nearing expiry), the still-valid access token is fine. @@ -1107,7 +1086,7 @@ export async function refreshTokenIfNeeded( return { accessToken: credential.accessToken, refreshed: false } } - const outcome = await performCoalescedRefresh({ + const fresh = await performCoalescedRefresh({ accountId: resolvedCredentialId, providerId: credential.providerId, refreshToken: credential.refreshToken!, @@ -1115,11 +1094,11 @@ export async function refreshTokenIfNeeded( requestId, userId: credential.userId, }) - if (outcome.accessToken) return { accessToken: outcome.accessToken, refreshed: true } + if (fresh) return { accessToken: fresh, refreshed: true } if (!accessTokenNeedsRefresh && credential.accessToken) { logger.info(`[${requestId}] Refresh unavailable; reusing still-valid access token`) return { accessToken: credential.accessToken, refreshed: false } } - throw outcome.error ?? new Error('Failed to refresh token') + throw new Error('Failed to refresh token') } diff --git a/apps/sim/lib/oauth/errors.ts b/apps/sim/lib/oauth/errors.ts deleted file mode 100644 index 11150808e0d..00000000000 --- a/apps/sim/lib/oauth/errors.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { truncate } from '@sim/utils/string' - -const MAX_DESCRIPTION_LENGTH = 300 - -function buildSafeMessage( - providerId: string, - errorCode?: string, - errorDescription?: string -): string { - const code = errorCode || 'unknown_error' - return errorDescription - ? `${code} (${providerId}: ${truncate(errorDescription, MAX_DESCRIPTION_LENGTH)})` - : `${code} (${providerId})` -} - -/** - * A token-refresh failure with the provider's error preserved. The message is - * built from the provider error code and (truncated) error description only — - * never raw response bodies — so it is safe to surface to end users. - */ -export class OAuthRefreshError extends Error { - constructor( - readonly providerId: string, - readonly errorCode?: string, - errorDescription?: string - ) { - super(buildSafeMessage(providerId, errorCode, errorDescription)) - this.name = 'OAuthRefreshError' - } -} diff --git a/apps/sim/lib/oauth/oauth.ts b/apps/sim/lib/oauth/oauth.ts index 8926a8a0a87..c2cfad05096 100644 --- a/apps/sim/lib/oauth/oauth.ts +++ b/apps/sim/lib/oauth/oauth.ts @@ -1634,7 +1634,6 @@ export interface RefreshTokenSuccess { export interface RefreshTokenFailure { ok: false errorCode?: string - errorDescription?: string message?: string } @@ -1652,14 +1651,6 @@ function extractErrorCode(value: unknown): string | undefined { return undefined } -function extractErrorDescription(value: unknown): string | undefined { - if (value && typeof value === 'object' && 'error_description' in value) { - const description = (value as { error_description: unknown }).error_description - if (typeof description === 'string' && description.trim()) return description - } - return undefined -} - /** * Hard deadline on the token-endpoint exchange. This function does not coalesce * on its own; its sole production caller (`performCoalescedRefresh` in the OAuth @@ -1707,7 +1698,6 @@ async function refreshInstagramLongLivedToken( return { ok: false, errorCode: extractErrorCode(responseData), - errorDescription: extractErrorDescription(responseData), message: `Failed to refresh token: ${response.status} ${errorSummary}`, } } @@ -1779,7 +1769,6 @@ export async function refreshOAuthToken( return { ok: false, errorCode: extractErrorCode(errorData), - errorDescription: extractErrorDescription(errorData), message: `Failed to refresh token: ${response.status} ${errorText}`, } } @@ -1801,7 +1790,6 @@ export async function refreshOAuthToken( return { ok: false, errorCode: typeof data.error === 'string' ? data.error : undefined, - errorDescription: extractErrorDescription(data), message: `Failed to refresh token: ${data.error ?? 'unknown'}`, } } From 3708198f41fad13e34dcd24313ef6cb2a121d4f5 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Thu, 16 Jul 2026 18:25:03 -0700 Subject: [PATCH 5/6] fix(oauth): version-guard slack chain writes against concurrent connects --- apps/sim/app/api/auth/oauth/utils.test.ts | 26 ++++++++++- apps/sim/app/api/auth/oauth/utils.ts | 57 ++++++++++++++++------- apps/sim/lib/oauth/slack.ts | 55 ++++++++++++++++++++-- 3 files changed, 118 insertions(+), 20 deletions(-) diff --git a/apps/sim/app/api/auth/oauth/utils.test.ts b/apps/sim/app/api/auth/oauth/utils.test.ts index a270ede978c..43ea672f9d1 100644 --- a/apps/sim/app/api/auth/oauth/utils.test.ts +++ b/apps/sim/app/api/auth/oauth/utils.test.ts @@ -328,7 +328,7 @@ describe('OAuth Utils', () => { expect(redisConfigMockFns.mockAcquireLock.mock.calls[0][0]).toBe( 'oauth:refresh:slack:T08CM6ZNYBE' ) - expect(redisConfigMockFns.mockAcquireLock.mock.calls[0][2]).toBe(20) + expect(redisConfigMockFns.mockAcquireLock.mock.calls[0][2]).toBe(30) expect(mockRefreshOAuthToken).toHaveBeenCalledWith('slack', 'live-rt') expect(mockSet).toHaveBeenCalledWith( expect.objectContaining({ accessToken: 'new-at', refreshToken: 'new-rt' }) @@ -384,6 +384,7 @@ describe('OAuth Utils', () => { ok: false, errorCode: 'token_revoked', }) + mockSelectChain([]) await expect(refreshTokenIfNeeded('request-id', slackCredential(), 'row-1')).rejects.toThrow( 'Failed to refresh token' @@ -396,6 +397,29 @@ describe('OAuth Utils', () => { 3600 ) }) + + it('skips the dead flag when the chain moved during the failed refresh', async () => { + const fakeRedis = { + set: vi.fn().mockResolvedValue('OK'), + get: vi.fn().mockResolvedValue(null), + del: vi.fn().mockResolvedValue(1), + } + redisConfigMockFns.mockGetRedisClient.mockReturnValue(fakeRedis) + mockSelectOrderedChain([ + { accessToken: 'stale-at', refreshToken: 'live-rt', accessTokenExpiresAt: past }, + ]) + mockRefreshOAuthToken.mockResolvedValueOnce({ + ok: false, + errorCode: 'token_revoked', + }) + mockSelectChain([{ moved: new Date() }]) + + await expect(refreshTokenIfNeeded('request-id', slackCredential(), 'row-1')).rejects.toThrow( + 'Failed to refresh token' + ) + + expect(fakeRedis.set).not.toHaveBeenCalled() + }) }) describe('resolveServiceAccountToken', () => { diff --git a/apps/sim/app/api/auth/oauth/utils.ts b/apps/sim/app/api/auth/oauth/utils.ts index 09ff3b3f09a..7f6fe1eb461 100644 --- a/apps/sim/app/api/auth/oauth/utils.ts +++ b/apps/sim/app/api/auth/oauth/utils.ts @@ -31,6 +31,7 @@ import { extractSlackTeamId, fanOutSlackTokenChain, getFreshestSlackChain, + hasSlackChainMoved, isSlackProvider, } from '@/lib/oauth/slack' import { @@ -673,12 +674,15 @@ interface CoalescedRefreshOptions { /** * Slack lock budgets sized past `TOKEN_REFRESH_TIMEOUT_MS` (15s) in * lib/oauth/oauth.ts: installation-keyed locks make every sibling row's request - * a follower of one refresh. The lock TTL must not expire under a live refresh - * (15s provider call plus DB reads and the fan-out write), and followers poll - * for the lock's full lifetime so a slow-but-successful refresh is still - * observed rather than reported as a failure. + * a follower of one refresh, so the TTL covers the provider call plus generous + * headroom for the surrounding DB reads and the fan-out write, and followers + * poll for the lock's full lifetime so a slow-but-successful refresh is still + * observed rather than reported as a failure. These budgets are latency knobs, + * not correctness guarantees — chain integrity under lock expiry or unlocked + * concurrent writers is enforced by the version-guarded fan-out + * (`ifChainUnchangedSince` in lib/oauth/slack.ts). */ -const SLACK_LOCK_TTL_SEC = 20 +const SLACK_LOCK_TTL_SEC = 30 const SLACK_FOLLOWER_MAX_WAIT_MS = SLACK_LOCK_TTL_SEC * 1000 async function performCoalescedRefresh({ @@ -727,6 +731,7 @@ async function performCoalescedRefresh({ onLeader: async () => { try { let refreshTokenToUse = refreshToken + let slackChainVersion: Date | null = null if (slackTeamId) { const freshest = await getFreshestSlackChain(slackTeamId) if (!freshest) { @@ -734,16 +739,21 @@ async function performCoalescedRefresh({ `No refresh-capable account row found for Slack installation ${slackTeamId}` ) } + slackChainVersion = freshest.chainVersion if ( freshest.accessToken && freshest.accessTokenExpiresAt && freshest.accessTokenExpiresAt > new Date() ) { - await fanOutSlackTokenChain(slackTeamId, { - accessToken: freshest.accessToken, - refreshToken: freshest.refreshToken, - accessTokenExpiresAt: freshest.accessTokenExpiresAt, - }) + await fanOutSlackTokenChain( + slackTeamId, + { + accessToken: freshest.accessToken, + refreshToken: freshest.refreshToken, + accessTokenExpiresAt: freshest.accessTokenExpiresAt, + }, + { ifChainUnchangedSince: freshest.chainVersion } + ) logger.info('Reused freshest Slack installation token', logContext) return freshest.accessToken } @@ -758,7 +768,18 @@ async function performCoalescedRefresh({ errorCode: result.errorCode, }) if (result.errorCode && isTerminalRefreshError(result.errorCode)) { - await markCredentialDead(scopeKey, result.errorCode) + // A refresh that lost a race with a concurrent connect fails with + // a revoked/rotated-out token even though the installation just + // got a live chain — dead-flagging then would take down a healthy + // credential for an hour. + if ( + slackChainVersion && + (await hasSlackChainMoved(slackTeamId!, slackChainVersion)) + ) { + logger.info('Skipping dead flag: Slack chain moved during refresh', logContext) + } else { + await markCredentialDead(scopeKey, result.errorCode) + } } return null } @@ -766,11 +787,15 @@ async function performCoalescedRefresh({ const accessTokenExpiresAt = new Date(Date.now() + result.expiresIn * 1000) if (slackTeamId) { - await fanOutSlackTokenChain(slackTeamId, { - accessToken: result.accessToken, - refreshToken: result.refreshToken || refreshTokenToUse, - accessTokenExpiresAt, - }) + await fanOutSlackTokenChain( + slackTeamId, + { + accessToken: result.accessToken, + refreshToken: result.refreshToken || refreshTokenToUse, + accessTokenExpiresAt, + }, + { ifChainUnchangedSince: slackChainVersion ?? undefined } + ) } else { const updateData: Record = { accessToken: result.accessToken, diff --git a/apps/sim/lib/oauth/slack.ts b/apps/sim/lib/oauth/slack.ts index 73b36ea02f3..25f5811859c 100644 --- a/apps/sim/lib/oauth/slack.ts +++ b/apps/sim/lib/oauth/slack.ts @@ -1,6 +1,6 @@ import { db } from '@sim/db' import { account } from '@sim/db/schema' -import { and, eq, isNotNull, like, sql } from 'drizzle-orm' +import { and, eq, gt, isNotNull, like, max, sql } from 'drizzle-orm' /** * Slack bot tokens belong to the installation (team × app), not to the OAuth @@ -40,13 +40,31 @@ interface SlackTokenChain { accessTokenExpiresAt: Date | null } +interface FanOutOptions { + /** + * Version guard: skip the write entirely when any sibling row was updated + * after this timestamp. A refresh leader holds its chain snapshot across a + * multi-second provider call while a concurrent OAuth connect (whose + * `account.create.after` fan-out takes no lock) may land the newly issued + * chain first — an unconditional write would then overwrite the live chain + * with a stale one. Omit for connect-time fan-out, whose chain is by + * definition the newest. + */ + ifChainUnchangedSince?: Date +} + /** * Writes a token chain to every account row of a Slack installation. Rotation * revokes whatever the sibling rows were holding, so a successful refresh or a * fresh connect must overwrite all copies or the stale ones fail with * `token_revoked` at call time. */ -export async function fanOutSlackTokenChain(teamId: string, chain: SlackTokenChain): Promise { +export async function fanOutSlackTokenChain( + teamId: string, + chain: SlackTokenChain, + options?: FanOutOptions +): Promise { + const since = options?.ifChainUnchangedSince await db .update(account) .set({ @@ -55,13 +73,40 @@ export async function fanOutSlackTokenChain(teamId: string, chain: SlackTokenCha ...(chain.refreshToken ? { refreshToken: chain.refreshToken } : {}), updatedAt: new Date(), }) - .where(installationFilter(teamId)) + .where( + and( + installationFilter(teamId), + since + ? sql`NOT EXISTS (SELECT 1 FROM ${account} sibling WHERE sibling.provider_id = 'slack' AND sibling.account_id LIKE ${`${teamId}-%`} AND sibling.updated_at > ${since})` + : undefined + ) + ) +} + +/** + * True when any account row of the installation was updated after `since` — + * i.e. another writer (a fresh connect or a competing refresh) landed a newer + * chain while the caller was working from an older snapshot. + */ +export async function hasSlackChainMoved(teamId: string, since: Date): Promise { + const [row] = await db + .select({ moved: max(account.updatedAt) }) + .from(account) + .where(and(installationFilter(teamId), gt(account.updatedAt, since))) + .limit(1) + return row?.moved != null } interface FreshestSlackChain { accessToken: string | null refreshToken: string accessTokenExpiresAt: Date | null + /** + * Max `updated_at` across the installation's rows at read time — the version + * guard passed back into {@link fanOutSlackTokenChain} / consulted via + * {@link hasSlackChainMoved} after the provider round-trip. + */ + chainVersion: Date } /** @@ -76,6 +121,9 @@ export async function getFreshestSlackChain(teamId: string): Promise`(SELECT max(sibling.updated_at) FROM ${account} sibling WHERE sibling.provider_id = 'slack' AND sibling.account_id LIKE ${`${teamId}-%`})`, }) .from(account) .where(and(installationFilter(teamId), isNotNull(account.refreshToken))) @@ -87,5 +135,6 @@ export async function getFreshestSlackChain(teamId: string): Promise Date: Thu, 16 Jul 2026 18:30:49 -0700 Subject: [PATCH 6/6] fix(oauth): clear slack dead flag before connect fan-out --- apps/sim/lib/auth/auth.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index d5ae502fe2e..a8d11f31cd5 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -457,12 +457,15 @@ export const auth = betterAuth({ try { const teamId = extractSlackTeamId(account.accountId) if (teamId) { + // Clear the dead flag before fanning out: the connect itself + // proves the installation has live tokens, and a fan-out + // failure must not leave the hour-long flag blocking refreshes. + await clearDeadFlag(`slack:${teamId}`) await fanOutSlackTokenChain(teamId, { accessToken: account.accessToken, refreshToken: account.refreshToken ?? null, accessTokenExpiresAt: account.accessTokenExpiresAt ?? null, }) - await clearDeadFlag(`slack:${teamId}`) logger.info('[account.create.after] Propagated Slack installation token chain', { userId: account.userId, teamId,