Skip to content

Commit 87863dd

Browse files
authored
fix(oauth): scope unauthorized_client as terminal to Atlassian refreshes (#8171)
* fix(oauth): scope unauthorized_client as terminal to Atlassian refreshes * fix(oauth): look up provider terminal codes in a Map
1 parent 100d26b commit 87863dd

4 files changed

Lines changed: 31 additions & 11 deletions

File tree

‎apps/sim/lib/oauth/__tests__/terminal-errors.test.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,21 @@ describe('isTerminalRefreshError', () => {
5050
'invalid_client',
5151
'bad_redirect_uri',
5252
'token_revoked',
53-
'unauthorized_client',
5453
])('returns true for %s', (code) => {
5554
expect(isTerminalRefreshError(code)).toBe(true)
5655
})
5756

57+
it.each(['confluence', 'jira'])('treats unauthorized_client as terminal for %s', (providerId) => {
58+
expect(isTerminalRefreshError('unauthorized_client', providerId)).toBe(true)
59+
})
60+
61+
it.each([undefined, 'microsoft', 'salesforce', 'google-email', 'constructor', '__proto__'])(
62+
'does not treat unauthorized_client as terminal for %s',
63+
(providerId) => {
64+
expect(isTerminalRefreshError('unauthorized_client', providerId)).toBe(false)
65+
}
66+
)
67+
5868
it.each(['ratelimited', 'internal_error', 'service_unavailable', undefined, null, ''])(
5969
'returns false for %s',
6070
(code) => {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ describe('OAuth access-token refresh headroom', () => {
591591
resolveCredentialTokenBundle(RAW_CREDENTIAL_ID, RAW_USER_ID, 'test')
592592
).resolves.toBeNull()
593593
expect(markCredentialDead).toHaveBeenCalledWith(expect.any(String), 'invalid_grant')
594+
expect(isTerminalRefreshError).toHaveBeenCalledWith('invalid_grant', 'google-drive')
594595
})
595596

596597
it('uses the stored chain when the rotation write loses to a newer one', async () => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1047,7 +1047,7 @@ async function performCoalescedRefresh({
10471047
errorCode: result.errorCode,
10481048
message: result.message,
10491049
})
1050-
if (result.errorCode && isTerminalRefreshError(result.errorCode)) {
1050+
if (result.errorCode && isTerminalRefreshError(result.errorCode, providerId)) {
10511051
// A refresh that lost a race with a concurrent connect or a newer
10521052
// rotation fails with a revoked/rotated-out token even though the
10531053
// account just got a live chain — dead-flagging then would take

‎apps/sim/lib/oauth/terminal-errors.ts‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,7 @@ import { getRedisClient } from '@/lib/core/config/redis'
44

55
const logger = createLogger('OAuthTerminalErrors')
66

7-
/**
8-
* Refresh error codes that no retry can recover from: the credential stays dead until
9-
* its owner reconnects. `unauthorized_client` is how Atlassian rejects a revoked or
10-
* rotated-out refresh token, and under RFC 6749 section 5.2 it otherwise means the
11-
* client may not use the refresh grant, which is equally persistent.
12-
*/
7+
/** Refresh error codes that no retry can recover from: the credential stays dead until its owner reconnects. */
138
const TERMINAL_ERRORS = new Set<string>([
149
'invalid_refresh_token',
1510
'bad_refresh_token',
@@ -20,7 +15,17 @@ const TERMINAL_ERRORS = new Set<string>([
2015
'invalid_client',
2116
'bad_redirect_uri',
2217
'token_revoked',
23-
'unauthorized_client',
18+
])
19+
20+
/**
21+
* Codes terminal only for the providers listed. Atlassian rejects a revoked or rotated-out
22+
* refresh token with `unauthorized_client`; elsewhere that code usually describes the app
23+
* registration, and treating it as terminal would send every credential of the provider to
24+
* reauthorization over one configuration fault.
25+
*/
26+
const PROVIDER_TERMINAL_ERRORS: ReadonlyMap<string, ReadonlySet<string>> = new Map([
27+
['confluence', new Set(['unauthorized_client'])],
28+
['jira', new Set(['unauthorized_client'])],
2429
])
2530

2631
const DEAD_CACHE_TTL_SEC = 60 * 60
@@ -29,9 +34,13 @@ function deadKey(accountId: string): string {
2934
return `oauth:dead:${accountId}`
3035
}
3136

32-
export function isTerminalRefreshError(code: string | undefined | null): boolean {
37+
export function isTerminalRefreshError(
38+
code: string | undefined | null,
39+
providerId?: string
40+
): boolean {
3341
if (!code) return false
34-
return TERMINAL_ERRORS.has(code)
42+
if (TERMINAL_ERRORS.has(code)) return true
43+
return providerId !== undefined && (PROVIDER_TERMINAL_ERRORS.get(providerId)?.has(code) ?? false)
3544
}
3645

3746
export async function markCredentialDead(accountId: string, code: string): Promise<void> {

0 commit comments

Comments
 (0)