Skip to content

Commit 54a38be

Browse files
chore(oauth): drop provider refresh-error surfacing to keep this PR slack-only
1 parent c60a39b commit 54a38be

6 files changed

Lines changed: 20 additions & 165 deletions

File tree

apps/sim/app/api/auth/oauth/token/route.test.ts

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ vi.mock('@/lib/auth/credential-access', () => ({
2727
}))
2828

2929
import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors'
30-
import { OAuthRefreshError } from '@/lib/oauth/errors'
3130
import { GET, POST } from '@/app/api/auth/oauth/token/route'
3231

3332
describe('OAuth Token API Routes', () => {
@@ -302,38 +301,6 @@ describe('OAuth Token API Routes', () => {
302301
)
303302
})
304303

305-
it('should surface the provider error detail on typed refresh failures', async () => {
306-
mockAuthorizeCredentialUse.mockResolvedValueOnce({
307-
ok: true,
308-
authType: 'session',
309-
requesterUserId: 'test-user-id',
310-
credentialOwnerUserId: 'owner-user-id',
311-
})
312-
authOAuthUtilsMockFns.mockGetCredential.mockResolvedValueOnce({
313-
id: 'credential-id',
314-
accessToken: 'test-token',
315-
refreshToken: 'refresh-token',
316-
accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000),
317-
providerId: 'google',
318-
})
319-
authOAuthUtilsMockFns.mockRefreshTokenIfNeeded.mockRejectedValueOnce(
320-
new OAuthRefreshError('google', 'invalid_grant', 'Token has been expired or revoked.')
321-
)
322-
323-
const req = createMockRequest('POST', {
324-
credentialId: 'credential-id',
325-
})
326-
327-
const response = await POST(req)
328-
const data = await response.json()
329-
330-
expect(response.status).toBe(401)
331-
expect(data).toHaveProperty(
332-
'error',
333-
'Failed to refresh access token: invalid_grant (google: Token has been expired or revoked.)'
334-
)
335-
})
336-
337304
describe('credentialAccountUserId + providerId path', () => {
338305
it('should reject unauthenticated requests', async () => {
339306
hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValueOnce({

apps/sim/app/api/auth/oauth/token/route.ts

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import { AuthType, checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
1212
import { generateRequestId } from '@/lib/core/utils/request'
1313
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1414
import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors'
15-
import { OAuthRefreshError } from '@/lib/oauth/errors'
1615
import { captureServerEvent } from '@/lib/posthog/server'
1716
import {
1817
getCredential,
@@ -301,11 +300,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
301300
)
302301
} catch (error) {
303302
logger.error(`[${requestId}] Failed to refresh access token:`, error)
304-
const detail = error instanceof OAuthRefreshError ? `: ${error.message}` : ''
305-
return NextResponse.json(
306-
{ error: `Failed to refresh access token${detail}` },
307-
{ status: 401 }
308-
)
303+
return NextResponse.json({ error: 'Failed to refresh access token' }, { status: 401 })
309304
}
310305
} catch (error) {
311306
logger.error(`[${requestId}] Error getting access token`, error)
@@ -417,11 +412,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
417412
)
418413
} catch (error) {
419414
logger.error(`[${requestId}] Failed to refresh access token:`, error)
420-
const detail = error instanceof OAuthRefreshError ? `: ${error.message}` : ''
421-
return NextResponse.json(
422-
{ error: `Failed to refresh access token${detail}` },
423-
{ status: 401 }
424-
)
415+
return NextResponse.json({ error: 'Failed to refresh access token' }, { status: 401 })
425416
}
426417
} catch (error) {
427418
logger.error(`[${requestId}] Error fetching access token`, error)

apps/sim/app/api/auth/oauth/utils.test.ts

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -164,46 +164,6 @@ describe('OAuth Utils', () => {
164164
message: 'Failed',
165165
})
166166

167-
await expect(
168-
refreshTokenIfNeeded('request-id', mockCredential, 'credential-id')
169-
).rejects.toThrow('invalid_grant (google)')
170-
})
171-
172-
it('should preserve the provider error description in the thrown error', async () => {
173-
const mockCredential = {
174-
id: 'credential-id',
175-
accessToken: 'expired-token',
176-
refreshToken: 'refresh-token',
177-
accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000),
178-
providerId: 'google',
179-
}
180-
181-
mockRefreshOAuthToken.mockResolvedValueOnce({
182-
ok: false,
183-
errorCode: 'invalid_grant',
184-
errorDescription: 'Token has been expired or revoked.',
185-
message: 'Failed',
186-
})
187-
188-
await expect(
189-
refreshTokenIfNeeded('request-id', mockCredential, 'credential-id')
190-
).rejects.toThrow('invalid_grant (google: Token has been expired or revoked.)')
191-
})
192-
193-
it('should throw the generic error on transient failures without a provider code', async () => {
194-
const mockCredential = {
195-
id: 'credential-id',
196-
accessToken: 'expired-token',
197-
refreshToken: 'refresh-token',
198-
accessTokenExpiresAt: new Date(Date.now() - 3600 * 1000),
199-
providerId: 'google',
200-
}
201-
202-
mockRefreshOAuthToken.mockResolvedValueOnce({
203-
ok: false,
204-
message: 'fetch failed',
205-
})
206-
207167
await expect(
208168
refreshTokenIfNeeded('request-id', mockCredential, 'credential-id')
209169
).rejects.toThrow('Failed to refresh token')
@@ -426,7 +386,7 @@ describe('OAuth Utils', () => {
426386
})
427387

428388
await expect(refreshTokenIfNeeded('request-id', slackCredential(), 'row-1')).rejects.toThrow(
429-
'token_revoked (slack)'
389+
'Failed to refresh token'
430390
)
431391

432392
expect(fakeRedis.set).toHaveBeenCalledWith(

apps/sim/app/api/auth/oauth/utils.ts

Lines changed: 17 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import {
2121
type TokenServiceAccountSecretBlob,
2222
} from '@/lib/credentials/token-service-accounts/server'
2323
import { refreshOAuthToken } from '@/lib/oauth'
24-
import { OAuthRefreshError } from '@/lib/oauth/errors'
2524
import { isInstagramProvider, shouldProactivelyRefreshInstagramToken } from '@/lib/oauth/instagram'
2625
import {
2726
getMicrosoftRefreshTokenExpiry,
@@ -671,12 +670,6 @@ interface CoalescedRefreshOptions {
671670
userId?: string
672671
}
673672

674-
interface CoalescedRefreshOutcome {
675-
accessToken: string | null
676-
/** Present when the refresh failed with a known provider error; absent on follower timeout / transient failure. */
677-
error?: OAuthRefreshError
678-
}
679-
680673
/**
681674
* Slack lock budgets sized past `TOKEN_REFRESH_TIMEOUT_MS` (15s) in
682675
* lib/oauth/oauth.ts: installation-keyed locks make every sibling row's request
@@ -695,7 +688,7 @@ async function performCoalescedRefresh({
695688
providerAccountId,
696689
requestId,
697690
userId,
698-
}: CoalescedRefreshOptions): Promise<CoalescedRefreshOutcome> {
691+
}: CoalescedRefreshOptions): Promise<string | null> {
699692
/**
700693
* Slack bot tokens are per-installation (team × app): every account row for
701694
* one team holds a copy of the same rotating chain, so refreshes are locked,
@@ -718,13 +711,13 @@ async function performCoalescedRefresh({
718711
...logContext,
719712
errorCode: deadCode,
720713
})
721-
return { accessToken: null, error: new OAuthRefreshError(providerId, deadCode) }
714+
return null
722715
}
723716

724717
const lockKey = `oauth:refresh:${scopeKey}`
725718

726719
const refreshPromise = coalesceLocally(lockKey, () =>
727-
withLeaderLock<CoalescedRefreshOutcome>({
720+
withLeaderLock<string>({
728721
key: lockKey,
729722
// Installation-keyed Slack locks gather followers from every sibling row,
730723
// so their wait and the lock TTL must outlast the 15s provider timeout —
@@ -752,7 +745,7 @@ async function performCoalescedRefresh({
752745
accessTokenExpiresAt: freshest.accessTokenExpiresAt,
753746
})
754747
logger.info('Reused freshest Slack installation token', logContext)
755-
return { accessToken: freshest.accessToken }
748+
return freshest.accessToken
756749
}
757750
refreshTokenToUse = freshest.refreshToken
758751
}
@@ -767,20 +760,7 @@ async function performCoalescedRefresh({
767760
if (result.errorCode && isTerminalRefreshError(result.errorCode)) {
768761
await markCredentialDead(scopeKey, result.errorCode)
769762
}
770-
return {
771-
accessToken: null,
772-
// No errorCode = transient (timeout/network), not a provider rejection —
773-
// stay errorless so callers keep their null-fallback behavior.
774-
...(result.errorCode
775-
? {
776-
error: new OAuthRefreshError(
777-
providerId,
778-
result.errorCode,
779-
result.errorDescription
780-
),
781-
}
782-
: {}),
783-
}
763+
return null
784764
}
785765

786766
const accessTokenExpiresAt = new Date(Date.now() + result.expiresIn * 1000)
@@ -808,13 +788,13 @@ async function performCoalescedRefresh({
808788
}
809789

810790
logger.info('Successfully refreshed access token', logContext)
811-
return { accessToken: result.accessToken }
791+
return result.accessToken
812792
} catch (error) {
813793
logger.error('Refresh failed inside leader path', {
814794
...logContext,
815795
error: toError(error).message,
816796
})
817-
return { accessToken: null }
797+
return null
818798
}
819799
},
820800
onFollower: async () => {
@@ -833,7 +813,7 @@ async function performCoalescedRefresh({
833813
row.accessTokenExpiresAt > new Date()
834814
) {
835815
logger.info('Got fresh access token from coalesced refresh', logContext)
836-
return { accessToken: row.accessToken }
816+
return row.accessToken
837817
}
838818
return null
839819
} catch (error) {
@@ -848,13 +828,13 @@ async function performCoalescedRefresh({
848828
)
849829

850830
try {
851-
return (await refreshPromise) ?? { accessToken: null }
831+
return await refreshPromise
852832
} catch (error) {
853833
logger.error('Coalesced refresh did not settle', {
854834
...logContext,
855835
error: toError(error).message,
856836
})
857-
return { accessToken: null }
837+
return null
858838
}
859839
}
860840

@@ -898,18 +878,17 @@ export async function getOAuthToken(userId: string, providerId: string): Promise
898878
})
899879

900880
if (accessTokenNeedsRefresh || instagramNeedsProactiveRefresh) {
901-
const outcome = await performCoalescedRefresh({
881+
const fresh = await performCoalescedRefresh({
902882
accountId: credential.id,
903883
providerId,
904884
refreshToken: credential.refreshToken!,
905885
providerAccountId: credential.providerAccountId,
906886
userId,
907887
})
908-
if (outcome.accessToken) return outcome.accessToken
888+
if (fresh) return fresh
909889
if (!accessTokenNeedsRefresh && credential.accessToken) {
910890
return credential.accessToken
911891
}
912-
if (outcome.error) throw outcome.error
913892
return null
914893
}
915894

@@ -1002,15 +981,15 @@ export async function resolveCredentialAccessToken(
1002981
const resolvedCredentialId =
1003982
(credential as { resolvedCredentialId?: string }).resolvedCredentialId ?? credentialId
1004983

1005-
const outcome = await performCoalescedRefresh({
984+
const fresh = await performCoalescedRefresh({
1006985
accountId: resolvedCredentialId,
1007986
providerId: credential.providerId,
1008987
refreshToken: credential.refreshToken!,
1009988
providerAccountId: credential.accountId,
1010989
requestId,
1011990
userId: credential.userId,
1012991
})
1013-
if (outcome.accessToken) return { accessToken: outcome.accessToken }
992+
if (fresh) return { accessToken: fresh }
1014993

1015994
// If refresh was only triggered proactively (Microsoft refresh-token aging /
1016995
// Instagram long-lived nearing expiry), the still-valid access token is fine.
@@ -1107,19 +1086,19 @@ export async function refreshTokenIfNeeded(
11071086
return { accessToken: credential.accessToken, refreshed: false }
11081087
}
11091088

1110-
const outcome = await performCoalescedRefresh({
1089+
const fresh = await performCoalescedRefresh({
11111090
accountId: resolvedCredentialId,
11121091
providerId: credential.providerId,
11131092
refreshToken: credential.refreshToken!,
11141093
providerAccountId: credential.accountId,
11151094
requestId,
11161095
userId: credential.userId,
11171096
})
1118-
if (outcome.accessToken) return { accessToken: outcome.accessToken, refreshed: true }
1097+
if (fresh) return { accessToken: fresh, refreshed: true }
11191098

11201099
if (!accessTokenNeedsRefresh && credential.accessToken) {
11211100
logger.info(`[${requestId}] Refresh unavailable; reusing still-valid access token`)
11221101
return { accessToken: credential.accessToken, refreshed: false }
11231102
}
1124-
throw outcome.error ?? new Error('Failed to refresh token')
1103+
throw new Error('Failed to refresh token')
11251104
}

apps/sim/lib/oauth/errors.ts

Lines changed: 0 additions & 30 deletions
This file was deleted.

apps/sim/lib/oauth/oauth.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1634,7 +1634,6 @@ export interface RefreshTokenSuccess {
16341634
export interface RefreshTokenFailure {
16351635
ok: false
16361636
errorCode?: string
1637-
errorDescription?: string
16381637
message?: string
16391638
}
16401639

@@ -1652,14 +1651,6 @@ function extractErrorCode(value: unknown): string | undefined {
16521651
return undefined
16531652
}
16541653

1655-
function extractErrorDescription(value: unknown): string | undefined {
1656-
if (value && typeof value === 'object' && 'error_description' in value) {
1657-
const description = (value as { error_description: unknown }).error_description
1658-
if (typeof description === 'string' && description.trim()) return description
1659-
}
1660-
return undefined
1661-
}
1662-
16631654
/**
16641655
* Hard deadline on the token-endpoint exchange. This function does not coalesce
16651656
* on its own; its sole production caller (`performCoalescedRefresh` in the OAuth
@@ -1707,7 +1698,6 @@ async function refreshInstagramLongLivedToken(
17071698
return {
17081699
ok: false,
17091700
errorCode: extractErrorCode(responseData),
1710-
errorDescription: extractErrorDescription(responseData),
17111701
message: `Failed to refresh token: ${response.status} ${errorSummary}`,
17121702
}
17131703
}
@@ -1779,7 +1769,6 @@ export async function refreshOAuthToken(
17791769
return {
17801770
ok: false,
17811771
errorCode: extractErrorCode(errorData),
1782-
errorDescription: extractErrorDescription(errorData),
17831772
message: `Failed to refresh token: ${response.status} ${errorText}`,
17841773
}
17851774
}
@@ -1801,7 +1790,6 @@ export async function refreshOAuthToken(
18011790
return {
18021791
ok: false,
18031792
errorCode: typeof data.error === 'string' ? data.error : undefined,
1804-
errorDescription: extractErrorDescription(data),
18051793
message: `Failed to refresh token: ${data.error ?? 'unknown'}`,
18061794
}
18071795
}

0 commit comments

Comments
 (0)