fix(worker): preserve permissions on transient token refresh failures

Classify OAuth token refresh failures so transient provider outages are retried without clearing cached permissions, while invalid_grant remains fail-closed.

Fixes SOU-1560

Fixes SOU-1177

Author: brendan-kellam

packages/backend/src/ee/accountPermissionSyncer.test.ts

@@ -0,0 +1,56 @@
+import { describe, expect, test } from 'vitest';
+import { classifyPermissionSyncFailure } from './accountPermissionSyncer.js';
+import { TokenRefreshError, type TokenRefreshErrorKind } from './tokenRefresh.js';
+
+const tokenRefreshError = (
+    kind: TokenRefreshErrorKind,
+    status?: number,
+): TokenRefreshError => new TokenRefreshError(`Token refresh failed: ${kind}`, {
+    kind,
+    status,
+});
+
+describe('classifyPermissionSyncFailure', () => {
+    test('fails closed for invalid_grant', () => {
+        expect(classifyPermissionSyncFailure(tokenRefreshError('invalid_grant', 400))).toEqual({
+            action: 'clear_permissions',
+            reason: 'oauth_invalid_grant',
+        });
+    });
+
+    test.each([
+        ['transient', 500],
+        ['configuration', 400],
+        ['invalid_response', undefined],
+        ['local_credential', undefined],
+    ] satisfies Array<[TokenRefreshErrorKind, number | undefined]>)('keeps permissions for a %s token refresh failure', (kind, status) => {
+        expect(classifyPermissionSyncFailure(tokenRefreshError(kind, status))).toEqual({
+            action: 'preserve_permissions',
+        });
+    });
+
+    test('does not treat a token refresh configuration error with HTTP 401 as an API authorization failure', () => {
+        expect(classifyPermissionSyncFailure(tokenRefreshError('configuration', 401))).toEqual({
+            action: 'preserve_permissions',
+        });
+    });
+
+    test.each([
+        [401, 'http_unauthorized'],
+        [403, 'http_forbidden'],
+        [410, 'http_gone'],
+    ] as const)('preserves fail-closed behavior for an API HTTP %s response', (status, reason) => {
+        const error = Object.assign(new Error(reason), { status });
+        expect(classifyPermissionSyncFailure(error)).toEqual({
+            action: 'clear_permissions',
+            reason,
+        });
+    });
+
+    test('keeps permissions for an unrelated API failure', () => {
+        const error = Object.assign(new Error('Internal Server Error'), { status: 500 });
+        expect(classifyPermissionSyncFailure(error)).toEqual({
+            action: 'preserve_permissions',
+        });
+    });
+});

packages/backend/src/ee/accountPermissionSyncer.ts

@@ -2,7 +2,7 @@ import * as Sentry from "@sentry/node";
 import { PrismaClient, AccountPermissionSyncJobStatus, Account, PermissionSyncSource} from "@sourcebot/db";
 import { env, createLogger, getIdentityProviderConfig, PERMISSION_SYNC_SUPPORTED_IDENTITY_PROVIDERS } from "@sourcebot/shared";
 import { hasEntitlement } from "../entitlements.js";
-import { ensureFreshAccountToken } from "./tokenRefresh.js";
+import { ensureFreshAccountToken, TokenRefreshError } from "./tokenRefresh.js";
 import { DelayedError, Job, Queue, Worker } from "bullmq";
 import { Redis } from "ioredis";
 import {
@@ -32,11 +32,50 @@ type AccountPermissionSyncJob = {
     jobId: string;
 }
 
-class RefreshTokenError extends Error {
-    constructor(message: string) {
-        super(message);
+export type PermissionCleanupReason =
+    | 'oauth_invalid_grant'
+    | 'http_unauthorized'
+    | 'http_forbidden'
+    | 'http_gone';
+
+export type PermissionCleanupDecision =
+    | {
+        action: 'clear_permissions';
+        reason: PermissionCleanupReason;
     }
-}
+    | {
+        action: 'preserve_permissions';
+    };
+
+const PERMISSION_CLEANUP_REASON_MESSAGES: Record<PermissionCleanupReason, string> = {
+    oauth_invalid_grant: 'OAuth invalid_grant',
+    http_unauthorized: 'HTTP 401 Unauthorized',
+    http_forbidden: 'HTTP 403 Forbidden',
+    http_gone: 'HTTP 410 Gone',
+};
+
+export const classifyPermissionSyncFailure = (error: unknown): PermissionCleanupDecision => {
+    // Token refresh failures have their own classification. Do not fall through
+    // to the generic HTTP checks because a non-invalid_grant response may also
+    // carry a 401 or 403 status.
+    if (error instanceof TokenRefreshError) {
+        return error.kind === 'invalid_grant'
+            ? { action: 'clear_permissions', reason: 'oauth_invalid_grant' }
+            : { action: 'preserve_permissions' };
+    }
+
+    if (isUnauthorized(error)) {
+        return { action: 'clear_permissions', reason: 'http_unauthorized' };
+    }
+    if (isForbidden(error)) {
+        return { action: 'clear_permissions', reason: 'http_forbidden' };
+    }
+    if (isGone(error)) {
+        return { action: 'clear_permissions', reason: 'http_gone' };
+    }
+
+    return { action: 'preserve_permissions' };
+};
 
 export class AccountPermissionSyncer {
     private queue: Queue<AccountPermissionSyncJob>;
@@ -202,18 +241,14 @@ export class AccountPermissionSyncer {
             // on is gone (e.g. Bitbucket Cloud's CHANGE-2770), clear the
             // account's existing permission rows so the read-side filter stops
             // matching through them.
-            const reason =
-                error instanceof RefreshTokenError ? 'token refresh failure' :
-                isUnauthorized(error) ? 'HTTP 401 Unauthorized' :
-                isForbidden(error) ? 'HTTP 403 Forbidden' :
-                isGone(error) ? 'HTTP 410 Gone' :
-                null;
-
-            if (reason !== null) {
+            const cleanupDecision = classifyPermissionSyncFailure(error);
+
+            if (cleanupDecision.action === 'clear_permissions') {
                 const { count } = await this.db.accountToRepoPermission.deleteMany({
                     where: { accountId: account.id },
                 });
                 const message = error instanceof Error ? error.message : String(error);
+                const reason = PERMISSION_CLEANUP_REASON_MESSAGES[cleanupDecision.reason];
                 logger.warn(`Cleared ${count} permission row(s) for account ${account.id} (user ${account.user.email ?? 'unknown'}) — fail-closed cleanup triggered by ${reason}: ${message}`);
             }
             throw error;
@@ -227,20 +262,7 @@ export class AccountPermissionSyncer {
         logger.debug(`Syncing permissions for ${account.providerId} account (id: ${account.id}) for user ${account.user.email}...`);
 
         // Ensure the OAuth token is fresh, refreshing it if it is expired or near expiry.
-        //
-        // @note(SOU-1177) re-throwing as a RefreshTokenError here is required to flag to the caller
-        // (runJob) that the account's permissions should be cleared. The side-effect with this
-        // approach is that permissions will be cleared for any error thrown in the
-        // ensureFreshAccountToken path. A better approach would be to look at the response
-        // from the oauth call and determining if the host returned a invalid_grant.
-        //
-        // @see: https://datatracker.ietf.org/doc/html/rfc6749#section-5.2
-        let accessToken;
-        try {
-            accessToken = await ensureFreshAccountToken(account, this.db);
-        } catch (error) {
-            throw new RefreshTokenError(error instanceof Error ? error.message : 'Failed to refresh token with unknown error.');
-        }
+        const accessToken = await ensureFreshAccountToken(account, this.db);
 
         // Get a list of all repos that the user has access to from all connected accounts.
         const repoIds = await (async () => {