Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
- Reduced Sentry span sampling to 10% outside development. [#1475](https://github.com/sourcebot-dev/sourcebot/pull/1475)

### Fixed
- [EE] Preserved cached repository permissions when OAuth token refresh fails because of a transient code host outage. [#1481](https://github.com/sourcebot-dev/sourcebot/pull/1481)

## [5.1.3] - 2026-07-20

### Added
Expand Down
56 changes: 56 additions & 0 deletions packages/backend/src/ee/accountPermissionSyncer.test.ts
Original file line number Diff line number Diff line change
@@ -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',
});
});
});
76 changes: 49 additions & 27 deletions packages/backend/src/ee/accountPermissionSyncer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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>;
Expand Down Expand Up @@ -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;
Expand All @@ -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 () => {
Expand Down
Loading
Loading