From 0730afa26a14240baae060f4d68aeeea125ca056 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 17 Sep 2026 12:58:08 -0700 Subject: [PATCH] fix(oauth): refresh access tokens before expiry --- apps/sim/lib/oauth/credential-service.test.ts | 237 ++++++++++++++++++ apps/sim/lib/oauth/credential-service.ts | 33 ++- 2 files changed, 259 insertions(+), 11 deletions(-) diff --git a/apps/sim/lib/oauth/credential-service.test.ts b/apps/sim/lib/oauth/credential-service.test.ts index cd388def9c3..cf88960c4d9 100644 --- a/apps/sim/lib/oauth/credential-service.test.ts +++ b/apps/sim/lib/oauth/credential-service.test.ts @@ -83,6 +83,9 @@ import { resolveServiceAccountToken, ServiceAccountTokenError, } from '@/lib/oauth/credential-service' +import { isInstagramProvider, shouldProactivelyRefreshInstagramToken } from '@/lib/oauth/instagram' +import { isMicrosoftProvider } from '@/lib/oauth/microsoft' +import { fanOutSlackTokenChain } from '@/lib/oauth/slack' import { GOOGLE_SERVICE_ACCOUNT_PROVIDER_ID } from '@/lib/oauth/types' const RAW_CREDENTIAL_ID = 'credential-raw-secret-id' @@ -340,6 +343,240 @@ describe('non-refreshable OAuth token expiry', () => { ) }) +describe('OAuth access-token refresh headroom', () => { + const now = new Date('2026-09-17T12:00:00.000Z') + + function createOAuthAccount(remainingMs: number | null = 6_000) { + return { + id: RAW_ACCOUNT_ID, + accountId: 'provider-subject', + providerId: 'google-drive', + userId: RAW_USER_ID, + accessToken: 'original-access-token', + refreshToken: 'original-refresh-token' as string | null, + accessTokenExpiresAt: remainingMs === null ? null : new Date(now.getTime() + remainingMs), + refreshTokenExpiresAt: null as Date | null, + updatedAt: new Date(now.getTime() - 30 * 24 * 60 * 60_000), + } + } + + type OAuthAccount = ReturnType + + function queueCredentialAccount(row: OAuthAccount) { + queueTableRows(credential, [ + { id: RAW_CREDENTIAL_ID, type: 'oauth', accountId: RAW_ACCOUNT_ID }, + ]) + queueTableRows(account, [row]) + } + + const readers = [ + { + name: 'credential bundle', + throwsOnFailure: false, + read: async (row: OAuthAccount) => { + queueCredentialAccount(row) + const bundle = await resolveCredentialTokenBundle(RAW_CREDENTIAL_ID, RAW_USER_ID, 'test') + return bundle?.accessToken ?? null + }, + }, + { + name: 'provider token', + throwsOnFailure: false, + read: async (row: OAuthAccount) => { + queueTableRows(account, [row]) + return getOAuthToken(RAW_USER_ID, row.providerId) + }, + }, + { + name: 'refresh result', + throwsOnFailure: true, + read: async (row: OAuthAccount) => + (await refreshTokenIfNeeded('test', row, RAW_ACCOUNT_ID)).accessToken, + }, + ] + + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + vi.useFakeTimers() + vi.setSystemTime(now) + vi.mocked(isInstagramProvider).mockReturnValue(false) + vi.mocked(shouldProactivelyRefreshInstagramToken).mockReturnValue(false) + vi.mocked(isMicrosoftProvider).mockReturnValue(false) + mocks.getRecentTerminalError.mockResolvedValue(null) + mocks.coalesceLocally.mockImplementation( + async (_key: string, producer: () => Promise) => producer() + ) + mocks.withLeaderLock.mockImplementation(async (options: { onLeader: () => Promise }) => + options.onLeader() + ) + mocks.refreshOAuthToken.mockResolvedValue({ + ok: true, + accessToken: 'refreshed-access-token', + refreshToken: 'rotated-refresh-token', + expiresIn: 3600, + }) + }) + + afterEach(() => { + vi.useRealTimers() + vi.mocked(isInstagramProvider).mockReturnValue(false) + vi.mocked(shouldProactivelyRefreshInstagramToken).mockReturnValue(false) + vi.mocked(isMicrosoftProvider).mockReturnValue(false) + }) + + describe.each(readers)('$name', ({ read, throwsOnFailure }) => { + it.each(['google-drive', 'jira', 'microsoft-teams'])( + 'refreshes %s with six seconds remaining before the first request', + async (providerId) => { + const row = { ...createOAuthAccount(), providerId } + await expect(read(row)).resolves.toBe('refreshed-access-token') + expect(mocks.refreshOAuthToken).toHaveBeenCalledExactlyOnceWith( + providerId, + 'original-refresh-token' + ) + expect(dbChainMockFns.set).toHaveBeenCalledWith( + expect.objectContaining({ + accessToken: 'refreshed-access-token', + refreshToken: 'rotated-refresh-token', + accessTokenExpiresAt: new Date(now.getTime() + 3_600_000), + }) + ) + } + ) + + it.each([ + { remainingMs: -1, refresh: true }, + { remainingMs: 0, refresh: true }, + { remainingMs: 300_000, refresh: true }, + { remainingMs: 300_001, refresh: false }, + { remainingMs: 3_600_000, refresh: false }, + { remainingMs: null, refresh: false }, + ])('handles $remainingMs milliseconds remaining', async ({ remainingMs, refresh }) => { + await expect(read(createOAuthAccount(remainingMs))).resolves.toBe( + refresh ? 'refreshed-access-token' : 'original-access-token' + ) + expect(mocks.refreshOAuthToken).toHaveBeenCalledTimes(refresh ? 1 : 0) + }) + + it('still refreshes a missing access token without a known expiry', async () => { + await expect(read({ ...createOAuthAccount(null), accessToken: '' })).resolves.toBe( + 'refreshed-access-token' + ) + }) + + it('preserves still-valid tokens without refresh capability', async () => { + await expect(read({ ...createOAuthAccount(), refreshToken: null })).resolves.toBe( + 'original-access-token' + ) + expect(mocks.refreshOAuthToken).not.toHaveBeenCalled() + }) + + it.each([6_000, -1])( + 'does not fall back to a token with %i milliseconds remaining when refresh fails', + async (remainingMs) => { + mocks.refreshOAuthToken.mockResolvedValue({ ok: false, errorCode: 'invalid_grant' }) + const result = read(createOAuthAccount(remainingMs)) + if (throwsOnFailure) await expect(result).rejects.toThrow('Failed to refresh token') + else await expect(result).resolves.toBeNull() + expect(mocks.refreshOAuthToken).toHaveBeenCalledTimes(1) + expect(dbChainMockFns.set).not.toHaveBeenCalled() + } + ) + + it('preserves Instagram proactive refresh and its healthy-token fallback', async () => { + vi.mocked(isInstagramProvider).mockReturnValue(true) + vi.mocked(shouldProactivelyRefreshInstagramToken).mockReturnValue(true) + mocks.refreshOAuthToken.mockResolvedValue({ ok: false, errorCode: 'temporarily_unavailable' }) + await expect( + read({ ...createOAuthAccount(3_600_000), providerId: 'instagram' }) + ).resolves.toBe('original-access-token') + expect(mocks.refreshOAuthToken).toHaveBeenCalledTimes(1) + }) + + it('does not bypass Instagram minimum token age with the generic refresh window', async () => { + vi.mocked(isInstagramProvider).mockReturnValue(true) + vi.mocked(shouldProactivelyRefreshInstagramToken).mockReturnValue(false) + await expect( + read({ ...createOAuthAccount(), providerId: 'instagram', updatedAt: now }) + ).resolves.toBe('original-access-token') + expect(shouldProactivelyRefreshInstagramToken).toHaveBeenCalledWith( + expect.objectContaining({ updatedAt: now }) + ) + expect(mocks.refreshOAuthToken).not.toHaveBeenCalled() + }) + }) + + it('waits for the refresh leader instead of accepting its near-expired stored token', async () => { + queueCredentialAccount(createOAuthAccount()) + queueTableRows(account, [createOAuthAccount()]) + queueTableRows(account, [{ ...createOAuthAccount(3_600_000), accessToken: 'leader-token' }]) + mocks.withLeaderLock.mockImplementation( + async (options: { onFollower: () => Promise }) => { + expect(await options.onFollower()).toBeNull() + return options.onFollower() + } + ) + await expect( + resolveCredentialTokenBundle(RAW_CREDENTIAL_ID, RAW_USER_ID, 'test') + ).resolves.toEqual({ accessToken: 'leader-token' }) + expect(mocks.refreshOAuthToken).not.toHaveBeenCalled() + }) + + it.each([ + { remainingMs: 6_000, refresh: true }, + { remainingMs: 300_000, refresh: true }, + { remainingMs: 300_001, refresh: false }, + ])( + 'applies headroom to the shared Slack token with $remainingMs left', + async ({ remainingMs, refresh }) => { + const row = { ...createOAuthAccount(-1), providerId: 'slack', accountId: 'TEXAMPLE-usr_U1' } + const chainVersion = new Date(0) + mocks.getFreshestSlackChain.mockResolvedValue({ + accessToken: 'installation-access-token', + refreshToken: 'installation-refresh-token', + accessTokenExpiresAt: new Date(now.getTime() + remainingMs), + chainVersion, + }) + queueCredentialAccount(row) + await expect( + resolveCredentialTokenBundle(RAW_CREDENTIAL_ID, RAW_USER_ID, 'test') + ).resolves.toEqual({ + accessToken: refresh ? 'refreshed-access-token' : 'installation-access-token', + }) + expect(mocks.refreshOAuthToken).toHaveBeenCalledTimes(refresh ? 1 : 0) + if (refresh) + expect(mocks.refreshOAuthToken).toHaveBeenCalledWith('slack', 'installation-refresh-token') + expect(fanOutSlackTokenChain).toHaveBeenCalledWith( + 'TEXAMPLE', + expect.objectContaining({ + accessToken: refresh ? 'refreshed-access-token' : 'installation-access-token', + }), + { ifChainUnchangedSince: chainVersion } + ) + } + ) + + it('preserves Microsoft refresh-token aging without rejecting a healthy access token', async () => { + vi.mocked(isMicrosoftProvider).mockReturnValue(true) + mocks.refreshOAuthToken.mockResolvedValue({ ok: false, errorCode: 'temporarily_unavailable' }) + const row = { + ...createOAuthAccount(3_600_000), + providerId: 'microsoft-teams', + refreshTokenExpiresAt: new Date(now.getTime() + 24 * 60 * 60_000), + } + queueCredentialAccount(row) + await expect( + resolveCredentialTokenBundle(RAW_CREDENTIAL_ID, RAW_USER_ID, 'test') + ).resolves.toEqual({ accessToken: 'original-access-token' }) + await expect(refreshTokenIfNeeded('test', row, RAW_ACCOUNT_ID)).resolves.toEqual({ + accessToken: 'original-access-token', + refreshed: false, + }) + expect(mocks.refreshOAuthToken).toHaveBeenCalledTimes(2) + }) +}) + describe('Google service-account token minting', () => { const { privateKey, publicKey } = generateKeyPairSync('rsa', { modulusLength: 2048 }) const fetchMock = vi.fn() diff --git a/apps/sim/lib/oauth/credential-service.ts b/apps/sim/lib/oauth/credential-service.ts index ebfb51a8118..d14c4d3921a 100644 --- a/apps/sim/lib/oauth/credential-service.ts +++ b/apps/sim/lib/oauth/credential-service.ts @@ -63,6 +63,7 @@ import { } from '@/lib/slack-search/app-configuration' const logger = createLogger('OAuthCredentialService') +const OAUTH_ACCESS_TOKEN_REFRESH_WINDOW_MS = 5 * 60 * 1000 export interface CredentialTokenResolutionOptions { /** @@ -857,6 +858,19 @@ interface CoalescedRefreshOptions { privacyMode?: 'selector' } +/** + * Leave time for request preparation and transit, including when reusing another worker's token. + * Instagram instead uses its age-gated long-lived token refresh policy. + */ +function isOAuthAccessTokenExpiring( + expiresAt: Date | null | undefined, + providerId: string, + now = new Date() +): boolean { + const refreshWindowMs = isInstagramProvider(providerId) ? 0 : OAUTH_ACCESS_TOKEN_REFRESH_WINDOW_MS + return expiresAt != null && expiresAt.getTime() <= now.getTime() + refreshWindowMs +} + /** * Slack lock budgets sized past `TOKEN_REFRESH_TIMEOUT_MS` (15s) in * lib/oauth/oauth.ts: installation-keyed locks make every sibling row's request @@ -932,7 +946,7 @@ async function performCoalescedRefresh({ if ( freshest.accessToken && freshest.accessTokenExpiresAt && - freshest.accessTokenExpiresAt > new Date() + !isOAuthAccessTokenExpiring(freshest.accessTokenExpiresAt, providerId) ) { await fanOutSlackTokenChain( slackTeamId, @@ -1039,7 +1053,7 @@ async function performCoalescedRefresh({ if ( row?.accessToken && row.accessTokenExpiresAt && - row.accessTokenExpiresAt > new Date() + !isOAuthAccessTokenExpiring(row.accessTokenExpiresAt, providerId) ) { logger.info('Got fresh access token from coalesced refresh', logContext) return row.accessToken @@ -1092,8 +1106,6 @@ export async function getOAuthToken(userId: string, providerId: string): Promise const credential = connections[0] - // Determine whether we should refresh: missing/expired token, or Instagram - // long-lived token nearing expiry (Meta cannot refresh after expiry). const now = new Date() const tokenExpiry = credential.accessTokenExpiresAt if (!credential.refreshToken && tokenExpiry && tokenExpiry <= now) { @@ -1101,7 +1113,8 @@ export async function getOAuthToken(userId: string, providerId: string): Promise return null } const accessTokenNeedsRefresh = - !!credential.refreshToken && (!credential.accessToken || (tokenExpiry && tokenExpiry < now)) + !!credential.refreshToken && + (!credential.accessToken || isOAuthAccessTokenExpiring(tokenExpiry, providerId, now)) const instagramNeedsProactiveRefresh = !!credential.refreshToken && isInstagramProvider(providerId) && @@ -1178,7 +1191,6 @@ export async function resolveCredentialTokenBundle( return null } - // Decide if we should refresh: token missing OR expired const accessTokenExpiresAt = credential.accessTokenExpiresAt const refreshTokenExpiresAt = credential.refreshTokenExpiresAt const now = new Date() @@ -1188,10 +1200,10 @@ export async function resolveCredentialTokenBundle( return null } - // Check if access token needs refresh (missing or expired) const accessTokenNeedsRefresh = !!credential.refreshToken && - (!credential.accessToken || (accessTokenExpiresAt && accessTokenExpiresAt <= now)) + (!credential.accessToken || + isOAuthAccessTokenExpiring(accessTokenExpiresAt, credential.providerId, now)) // Check if we should proactively refresh to prevent refresh token expiry // This applies to Microsoft providers whose refresh tokens expire after 90 days of inactivity @@ -1292,7 +1304,6 @@ export async function refreshTokenIfNeeded( ): Promise<{ accessToken: string; refreshed: boolean }> { const resolvedCredentialId = credential.resolvedCredentialId ?? credentialId - // Decide if we should refresh: token missing OR expired const accessTokenExpiresAt = credential.accessTokenExpiresAt const refreshTokenExpiresAt = credential.refreshTokenExpiresAt const now = new Date() @@ -1301,10 +1312,10 @@ export async function refreshTokenIfNeeded( throw new Error('OAuth access token expired and cannot be refreshed; reconnect the account') } - // Check if access token needs refresh (missing or expired) const accessTokenNeedsRefresh = !!credential.refreshToken && - (!credential.accessToken || (accessTokenExpiresAt && accessTokenExpiresAt <= now)) + (!credential.accessToken || + isOAuthAccessTokenExpiring(accessTokenExpiresAt, credential.providerId, now)) // Check if we should proactively refresh to prevent refresh token expiry // This applies to Microsoft providers whose refresh tokens expire after 90 days of inactivity