Skip to content

Commit 0730afa

Browse files
committed
fix(oauth): refresh access tokens before expiry
1 parent 0b893d7 commit 0730afa

2 files changed

Lines changed: 259 additions & 11 deletions

File tree

‎apps/sim/lib/oauth/credential-service.test.ts‎

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ import {
8383
resolveServiceAccountToken,
8484
ServiceAccountTokenError,
8585
} from '@/lib/oauth/credential-service'
86+
import { isInstagramProvider, shouldProactivelyRefreshInstagramToken } from '@/lib/oauth/instagram'
87+
import { isMicrosoftProvider } from '@/lib/oauth/microsoft'
88+
import { fanOutSlackTokenChain } from '@/lib/oauth/slack'
8689
import { GOOGLE_SERVICE_ACCOUNT_PROVIDER_ID } from '@/lib/oauth/types'
8790

8891
const RAW_CREDENTIAL_ID = 'credential-raw-secret-id'
@@ -340,6 +343,240 @@ describe('non-refreshable OAuth token expiry', () => {
340343
)
341344
})
342345

346+
describe('OAuth access-token refresh headroom', () => {
347+
const now = new Date('2026-09-17T12:00:00.000Z')
348+
349+
function createOAuthAccount(remainingMs: number | null = 6_000) {
350+
return {
351+
id: RAW_ACCOUNT_ID,
352+
accountId: 'provider-subject',
353+
providerId: 'google-drive',
354+
userId: RAW_USER_ID,
355+
accessToken: 'original-access-token',
356+
refreshToken: 'original-refresh-token' as string | null,
357+
accessTokenExpiresAt: remainingMs === null ? null : new Date(now.getTime() + remainingMs),
358+
refreshTokenExpiresAt: null as Date | null,
359+
updatedAt: new Date(now.getTime() - 30 * 24 * 60 * 60_000),
360+
}
361+
}
362+
363+
type OAuthAccount = ReturnType<typeof createOAuthAccount>
364+
365+
function queueCredentialAccount(row: OAuthAccount) {
366+
queueTableRows(credential, [
367+
{ id: RAW_CREDENTIAL_ID, type: 'oauth', accountId: RAW_ACCOUNT_ID },
368+
])
369+
queueTableRows(account, [row])
370+
}
371+
372+
const readers = [
373+
{
374+
name: 'credential bundle',
375+
throwsOnFailure: false,
376+
read: async (row: OAuthAccount) => {
377+
queueCredentialAccount(row)
378+
const bundle = await resolveCredentialTokenBundle(RAW_CREDENTIAL_ID, RAW_USER_ID, 'test')
379+
return bundle?.accessToken ?? null
380+
},
381+
},
382+
{
383+
name: 'provider token',
384+
throwsOnFailure: false,
385+
read: async (row: OAuthAccount) => {
386+
queueTableRows(account, [row])
387+
return getOAuthToken(RAW_USER_ID, row.providerId)
388+
},
389+
},
390+
{
391+
name: 'refresh result',
392+
throwsOnFailure: true,
393+
read: async (row: OAuthAccount) =>
394+
(await refreshTokenIfNeeded('test', row, RAW_ACCOUNT_ID)).accessToken,
395+
},
396+
]
397+
398+
beforeEach(() => {
399+
vi.clearAllMocks()
400+
resetDbChainMock()
401+
vi.useFakeTimers()
402+
vi.setSystemTime(now)
403+
vi.mocked(isInstagramProvider).mockReturnValue(false)
404+
vi.mocked(shouldProactivelyRefreshInstagramToken).mockReturnValue(false)
405+
vi.mocked(isMicrosoftProvider).mockReturnValue(false)
406+
mocks.getRecentTerminalError.mockResolvedValue(null)
407+
mocks.coalesceLocally.mockImplementation(
408+
async (_key: string, producer: () => Promise<unknown>) => producer()
409+
)
410+
mocks.withLeaderLock.mockImplementation(async (options: { onLeader: () => Promise<unknown> }) =>
411+
options.onLeader()
412+
)
413+
mocks.refreshOAuthToken.mockResolvedValue({
414+
ok: true,
415+
accessToken: 'refreshed-access-token',
416+
refreshToken: 'rotated-refresh-token',
417+
expiresIn: 3600,
418+
})
419+
})
420+
421+
afterEach(() => {
422+
vi.useRealTimers()
423+
vi.mocked(isInstagramProvider).mockReturnValue(false)
424+
vi.mocked(shouldProactivelyRefreshInstagramToken).mockReturnValue(false)
425+
vi.mocked(isMicrosoftProvider).mockReturnValue(false)
426+
})
427+
428+
describe.each(readers)('$name', ({ read, throwsOnFailure }) => {
429+
it.each(['google-drive', 'jira', 'microsoft-teams'])(
430+
'refreshes %s with six seconds remaining before the first request',
431+
async (providerId) => {
432+
const row = { ...createOAuthAccount(), providerId }
433+
await expect(read(row)).resolves.toBe('refreshed-access-token')
434+
expect(mocks.refreshOAuthToken).toHaveBeenCalledExactlyOnceWith(
435+
providerId,
436+
'original-refresh-token'
437+
)
438+
expect(dbChainMockFns.set).toHaveBeenCalledWith(
439+
expect.objectContaining({
440+
accessToken: 'refreshed-access-token',
441+
refreshToken: 'rotated-refresh-token',
442+
accessTokenExpiresAt: new Date(now.getTime() + 3_600_000),
443+
})
444+
)
445+
}
446+
)
447+
448+
it.each([
449+
{ remainingMs: -1, refresh: true },
450+
{ remainingMs: 0, refresh: true },
451+
{ remainingMs: 300_000, refresh: true },
452+
{ remainingMs: 300_001, refresh: false },
453+
{ remainingMs: 3_600_000, refresh: false },
454+
{ remainingMs: null, refresh: false },
455+
])('handles $remainingMs milliseconds remaining', async ({ remainingMs, refresh }) => {
456+
await expect(read(createOAuthAccount(remainingMs))).resolves.toBe(
457+
refresh ? 'refreshed-access-token' : 'original-access-token'
458+
)
459+
expect(mocks.refreshOAuthToken).toHaveBeenCalledTimes(refresh ? 1 : 0)
460+
})
461+
462+
it('still refreshes a missing access token without a known expiry', async () => {
463+
await expect(read({ ...createOAuthAccount(null), accessToken: '' })).resolves.toBe(
464+
'refreshed-access-token'
465+
)
466+
})
467+
468+
it('preserves still-valid tokens without refresh capability', async () => {
469+
await expect(read({ ...createOAuthAccount(), refreshToken: null })).resolves.toBe(
470+
'original-access-token'
471+
)
472+
expect(mocks.refreshOAuthToken).not.toHaveBeenCalled()
473+
})
474+
475+
it.each([6_000, -1])(
476+
'does not fall back to a token with %i milliseconds remaining when refresh fails',
477+
async (remainingMs) => {
478+
mocks.refreshOAuthToken.mockResolvedValue({ ok: false, errorCode: 'invalid_grant' })
479+
const result = read(createOAuthAccount(remainingMs))
480+
if (throwsOnFailure) await expect(result).rejects.toThrow('Failed to refresh token')
481+
else await expect(result).resolves.toBeNull()
482+
expect(mocks.refreshOAuthToken).toHaveBeenCalledTimes(1)
483+
expect(dbChainMockFns.set).not.toHaveBeenCalled()
484+
}
485+
)
486+
487+
it('preserves Instagram proactive refresh and its healthy-token fallback', async () => {
488+
vi.mocked(isInstagramProvider).mockReturnValue(true)
489+
vi.mocked(shouldProactivelyRefreshInstagramToken).mockReturnValue(true)
490+
mocks.refreshOAuthToken.mockResolvedValue({ ok: false, errorCode: 'temporarily_unavailable' })
491+
await expect(
492+
read({ ...createOAuthAccount(3_600_000), providerId: 'instagram' })
493+
).resolves.toBe('original-access-token')
494+
expect(mocks.refreshOAuthToken).toHaveBeenCalledTimes(1)
495+
})
496+
497+
it('does not bypass Instagram minimum token age with the generic refresh window', async () => {
498+
vi.mocked(isInstagramProvider).mockReturnValue(true)
499+
vi.mocked(shouldProactivelyRefreshInstagramToken).mockReturnValue(false)
500+
await expect(
501+
read({ ...createOAuthAccount(), providerId: 'instagram', updatedAt: now })
502+
).resolves.toBe('original-access-token')
503+
expect(shouldProactivelyRefreshInstagramToken).toHaveBeenCalledWith(
504+
expect.objectContaining({ updatedAt: now })
505+
)
506+
expect(mocks.refreshOAuthToken).not.toHaveBeenCalled()
507+
})
508+
})
509+
510+
it('waits for the refresh leader instead of accepting its near-expired stored token', async () => {
511+
queueCredentialAccount(createOAuthAccount())
512+
queueTableRows(account, [createOAuthAccount()])
513+
queueTableRows(account, [{ ...createOAuthAccount(3_600_000), accessToken: 'leader-token' }])
514+
mocks.withLeaderLock.mockImplementation(
515+
async (options: { onFollower: () => Promise<string | null> }) => {
516+
expect(await options.onFollower()).toBeNull()
517+
return options.onFollower()
518+
}
519+
)
520+
await expect(
521+
resolveCredentialTokenBundle(RAW_CREDENTIAL_ID, RAW_USER_ID, 'test')
522+
).resolves.toEqual({ accessToken: 'leader-token' })
523+
expect(mocks.refreshOAuthToken).not.toHaveBeenCalled()
524+
})
525+
526+
it.each([
527+
{ remainingMs: 6_000, refresh: true },
528+
{ remainingMs: 300_000, refresh: true },
529+
{ remainingMs: 300_001, refresh: false },
530+
])(
531+
'applies headroom to the shared Slack token with $remainingMs left',
532+
async ({ remainingMs, refresh }) => {
533+
const row = { ...createOAuthAccount(-1), providerId: 'slack', accountId: 'TEXAMPLE-usr_U1' }
534+
const chainVersion = new Date(0)
535+
mocks.getFreshestSlackChain.mockResolvedValue({
536+
accessToken: 'installation-access-token',
537+
refreshToken: 'installation-refresh-token',
538+
accessTokenExpiresAt: new Date(now.getTime() + remainingMs),
539+
chainVersion,
540+
})
541+
queueCredentialAccount(row)
542+
await expect(
543+
resolveCredentialTokenBundle(RAW_CREDENTIAL_ID, RAW_USER_ID, 'test')
544+
).resolves.toEqual({
545+
accessToken: refresh ? 'refreshed-access-token' : 'installation-access-token',
546+
})
547+
expect(mocks.refreshOAuthToken).toHaveBeenCalledTimes(refresh ? 1 : 0)
548+
if (refresh)
549+
expect(mocks.refreshOAuthToken).toHaveBeenCalledWith('slack', 'installation-refresh-token')
550+
expect(fanOutSlackTokenChain).toHaveBeenCalledWith(
551+
'TEXAMPLE',
552+
expect.objectContaining({
553+
accessToken: refresh ? 'refreshed-access-token' : 'installation-access-token',
554+
}),
555+
{ ifChainUnchangedSince: chainVersion }
556+
)
557+
}
558+
)
559+
560+
it('preserves Microsoft refresh-token aging without rejecting a healthy access token', async () => {
561+
vi.mocked(isMicrosoftProvider).mockReturnValue(true)
562+
mocks.refreshOAuthToken.mockResolvedValue({ ok: false, errorCode: 'temporarily_unavailable' })
563+
const row = {
564+
...createOAuthAccount(3_600_000),
565+
providerId: 'microsoft-teams',
566+
refreshTokenExpiresAt: new Date(now.getTime() + 24 * 60 * 60_000),
567+
}
568+
queueCredentialAccount(row)
569+
await expect(
570+
resolveCredentialTokenBundle(RAW_CREDENTIAL_ID, RAW_USER_ID, 'test')
571+
).resolves.toEqual({ accessToken: 'original-access-token' })
572+
await expect(refreshTokenIfNeeded('test', row, RAW_ACCOUNT_ID)).resolves.toEqual({
573+
accessToken: 'original-access-token',
574+
refreshed: false,
575+
})
576+
expect(mocks.refreshOAuthToken).toHaveBeenCalledTimes(2)
577+
})
578+
})
579+
343580
describe('Google service-account token minting', () => {
344581
const { privateKey, publicKey } = generateKeyPairSync('rsa', { modulusLength: 2048 })
345582
const fetchMock = vi.fn<typeof fetch>()

‎apps/sim/lib/oauth/credential-service.ts‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ import {
6363
} from '@/lib/slack-search/app-configuration'
6464

6565
const logger = createLogger('OAuthCredentialService')
66+
const OAUTH_ACCESS_TOKEN_REFRESH_WINDOW_MS = 5 * 60 * 1000
6667

6768
export interface CredentialTokenResolutionOptions {
6869
/**
@@ -857,6 +858,19 @@ interface CoalescedRefreshOptions {
857858
privacyMode?: 'selector'
858859
}
859860

861+
/**
862+
* Leave time for request preparation and transit, including when reusing another worker's token.
863+
* Instagram instead uses its age-gated long-lived token refresh policy.
864+
*/
865+
function isOAuthAccessTokenExpiring(
866+
expiresAt: Date | null | undefined,
867+
providerId: string,
868+
now = new Date()
869+
): boolean {
870+
const refreshWindowMs = isInstagramProvider(providerId) ? 0 : OAUTH_ACCESS_TOKEN_REFRESH_WINDOW_MS
871+
return expiresAt != null && expiresAt.getTime() <= now.getTime() + refreshWindowMs
872+
}
873+
860874
/**
861875
* Slack lock budgets sized past `TOKEN_REFRESH_TIMEOUT_MS` (15s) in
862876
* lib/oauth/oauth.ts: installation-keyed locks make every sibling row's request
@@ -932,7 +946,7 @@ async function performCoalescedRefresh({
932946
if (
933947
freshest.accessToken &&
934948
freshest.accessTokenExpiresAt &&
935-
freshest.accessTokenExpiresAt > new Date()
949+
!isOAuthAccessTokenExpiring(freshest.accessTokenExpiresAt, providerId)
936950
) {
937951
await fanOutSlackTokenChain(
938952
slackTeamId,
@@ -1039,7 +1053,7 @@ async function performCoalescedRefresh({
10391053
if (
10401054
row?.accessToken &&
10411055
row.accessTokenExpiresAt &&
1042-
row.accessTokenExpiresAt > new Date()
1056+
!isOAuthAccessTokenExpiring(row.accessTokenExpiresAt, providerId)
10431057
) {
10441058
logger.info('Got fresh access token from coalesced refresh', logContext)
10451059
return row.accessToken
@@ -1092,16 +1106,15 @@ export async function getOAuthToken(userId: string, providerId: string): Promise
10921106

10931107
const credential = connections[0]
10941108

1095-
// Determine whether we should refresh: missing/expired token, or Instagram
1096-
// long-lived token nearing expiry (Meta cannot refresh after expiry).
10971109
const now = new Date()
10981110
const tokenExpiry = credential.accessTokenExpiresAt
10991111
if (!credential.refreshToken && tokenExpiry && tokenExpiry <= now) {
11001112
logger.warn('OAuth access token expired and cannot be refreshed; reconnect the account')
11011113
return null
11021114
}
11031115
const accessTokenNeedsRefresh =
1104-
!!credential.refreshToken && (!credential.accessToken || (tokenExpiry && tokenExpiry < now))
1116+
!!credential.refreshToken &&
1117+
(!credential.accessToken || isOAuthAccessTokenExpiring(tokenExpiry, providerId, now))
11051118
const instagramNeedsProactiveRefresh =
11061119
!!credential.refreshToken &&
11071120
isInstagramProvider(providerId) &&
@@ -1178,7 +1191,6 @@ export async function resolveCredentialTokenBundle(
11781191
return null
11791192
}
11801193

1181-
// Decide if we should refresh: token missing OR expired
11821194
const accessTokenExpiresAt = credential.accessTokenExpiresAt
11831195
const refreshTokenExpiresAt = credential.refreshTokenExpiresAt
11841196
const now = new Date()
@@ -1188,10 +1200,10 @@ export async function resolveCredentialTokenBundle(
11881200
return null
11891201
}
11901202

1191-
// Check if access token needs refresh (missing or expired)
11921203
const accessTokenNeedsRefresh =
11931204
!!credential.refreshToken &&
1194-
(!credential.accessToken || (accessTokenExpiresAt && accessTokenExpiresAt <= now))
1205+
(!credential.accessToken ||
1206+
isOAuthAccessTokenExpiring(accessTokenExpiresAt, credential.providerId, now))
11951207

11961208
// Check if we should proactively refresh to prevent refresh token expiry
11971209
// This applies to Microsoft providers whose refresh tokens expire after 90 days of inactivity
@@ -1292,7 +1304,6 @@ export async function refreshTokenIfNeeded(
12921304
): Promise<{ accessToken: string; refreshed: boolean }> {
12931305
const resolvedCredentialId = credential.resolvedCredentialId ?? credentialId
12941306

1295-
// Decide if we should refresh: token missing OR expired
12961307
const accessTokenExpiresAt = credential.accessTokenExpiresAt
12971308
const refreshTokenExpiresAt = credential.refreshTokenExpiresAt
12981309
const now = new Date()
@@ -1301,10 +1312,10 @@ export async function refreshTokenIfNeeded(
13011312
throw new Error('OAuth access token expired and cannot be refreshed; reconnect the account')
13021313
}
13031314

1304-
// Check if access token needs refresh (missing or expired)
13051315
const accessTokenNeedsRefresh =
13061316
!!credential.refreshToken &&
1307-
(!credential.accessToken || (accessTokenExpiresAt && accessTokenExpiresAt <= now))
1317+
(!credential.accessToken ||
1318+
isOAuthAccessTokenExpiring(accessTokenExpiresAt, credential.providerId, now))
13081319

13091320
// Check if we should proactively refresh to prevent refresh token expiry
13101321
// This applies to Microsoft providers whose refresh tokens expire after 90 days of inactivity

0 commit comments

Comments
 (0)