diff --git a/CHANGELOG.md b/CHANGELOG.md index c1efc4102..430f1b62c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - [EE] Fixed account-driven permission sync silently wiping all Bitbucket Server repository permissions when the OAuth token expires on instances with anonymous access enabled. [#998](https://github.com/sourcebot-dev/sourcebot/pull/998) - [EE] Fixed Bitbucket Server repos being incorrectly treated as public in Sourcebot when the instance-level `feature.public.access` flag is disabled but per-repo public flags were not reset. [#999](https://github.com/sourcebot-dev/sourcebot/pull/999) +- [EE] Fixed account-driven permission sync jobs failing when OAuth tokens expire between user visits by moving token refresh into the backend sync flow. [#1000](https://github.com/sourcebot-dev/sourcebot/pull/1000) ## [4.15.5] - 2026-03-12 diff --git a/packages/backend/src/ee/accountPermissionSyncer.ts b/packages/backend/src/ee/accountPermissionSyncer.ts index 87a49cc42..f1bfe9d05 100644 --- a/packages/backend/src/ee/accountPermissionSyncer.ts +++ b/packages/backend/src/ee/accountPermissionSyncer.ts @@ -1,6 +1,7 @@ import * as Sentry from "@sentry/node"; import { PrismaClient, AccountPermissionSyncJobStatus, Account, PermissionSyncSource} from "@sourcebot/db"; -import { env, hasEntitlement, createLogger, loadConfig, decryptOAuthToken, PERMISSION_SYNC_SUPPORTED_IDENTITY_PROVIDERS } from "@sourcebot/shared"; +import { env, hasEntitlement, createLogger, loadConfig, PERMISSION_SYNC_SUPPORTED_IDENTITY_PROVIDERS } from "@sourcebot/shared"; +import { ensureFreshAccountToken } from "./tokenRefresh.js"; import { Job, Queue, Worker } from "bullmq"; import { Redis } from "ioredis"; import { @@ -182,18 +183,15 @@ export class AccountPermissionSyncer { logger.info(`Syncing permissions for ${account.provider} account (id: ${account.id}) for user ${account.user.email}...`); - // Decrypt tokens (stored encrypted in the database) - const accessToken = decryptOAuthToken(account.access_token); + // Ensure the OAuth token is fresh, refreshing it if it is expired or near expiry. + // Throws and sets Account.tokenRefreshErrorMessage if the refresh fails. + 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 () => { const aggregatedRepoIds: Set = new Set(); if (account.provider === 'github') { - if (!accessToken) { - throw new Error(`User '${account.user.email}' does not have an GitHub OAuth access token associated with their GitHub account. Please re-authenticate with GitHub to refresh the token.`); - } - // @hack: we don't have a way of identifying specific identity providers in the config file. // Instead, we'll use the first connection of type 'github' and hope for the best. const baseUrl = Array.from(Object.values(config.connections ?? {})) @@ -244,10 +242,6 @@ export class AccountPermissionSyncer { repos.forEach(repo => aggregatedRepoIds.add(repo.id)); } else if (account.provider === 'gitlab') { - if (!accessToken) { - throw new Error(`User '${account.user.email}' does not have a GitLab OAuth access token associated with their GitLab account. Please re-authenticate with GitLab to refresh the token.`); - } - // @hack: we don't have a way of identifying specific identity providers in the config file. // Instead, we'll use the first connection of type 'gitlab' and hope for the best. const baseUrl = Array.from(Object.values(config.connections ?? {})) @@ -284,10 +278,6 @@ export class AccountPermissionSyncer { repos.forEach(repo => aggregatedRepoIds.add(repo.id)); } else if (account.provider === 'bitbucket-cloud') { - if (!accessToken) { - throw new Error(`User '${account.user.email}' does not have a Bitbucket Cloud OAuth access token associated with their account. Please re-authenticate with Bitbucket Cloud to refresh the token.`); - } - // @note: we don't pass a user here since we want to use a bearer token // for authentication. const client = createBitbucketCloudClient(/* user = */ undefined, accessToken) @@ -305,10 +295,6 @@ export class AccountPermissionSyncer { repos.forEach(repo => aggregatedRepoIds.add(repo.id)); } else if (account.provider === 'bitbucket-server') { - if (!accessToken) { - throw new Error(`User '${account.user.email}' does not have a Bitbucket Server OAuth access token associated with their account. Please re-authenticate with Bitbucket Server to refresh the token.`); - } - // @hack: we don't have a way of identifying specific identity providers in the config file. // Instead, we'll use the first Bitbucket Server connection's URL as the base URL. const baseUrl = Array.from(Object.values(config.connections ?? {})) diff --git a/packages/web/src/ee/features/sso/tokenRefresh.ts b/packages/backend/src/ee/tokenRefresh.ts similarity index 60% rename from packages/web/src/ee/features/sso/tokenRefresh.ts rename to packages/backend/src/ee/tokenRefresh.ts index 60ce7e955..82f162083 100644 --- a/packages/web/src/ee/features/sso/tokenRefresh.ts +++ b/packages/backend/src/ee/tokenRefresh.ts @@ -1,11 +1,22 @@ -import { loadConfig, decryptOAuthToken } from "@sourcebot/shared"; -import { getTokenFromConfig, createLogger, env, encryptOAuthToken } from "@sourcebot/shared"; -import { BitbucketCloudIdentityProviderConfig, BitbucketServerIdentityProviderConfig, GitHubIdentityProviderConfig, GitLabIdentityProviderConfig } from "@sourcebot/schemas/v3/index.type"; -import { IdentityProviderType } from "@sourcebot/shared"; +import { Account, PrismaClient } from '@sourcebot/db'; +import { + BitbucketCloudIdentityProviderConfig, + BitbucketServerIdentityProviderConfig, + GitHubIdentityProviderConfig, + GitLabIdentityProviderConfig, +} from '@sourcebot/schemas/v3/index.type'; +import { + createLogger, + decryptOAuthToken, + encryptOAuthToken, + env, + getTokenFromConfig, + IdentityProviderType, + loadConfig, +} from '@sourcebot/shared'; import { z } from 'zod'; -import { prisma } from '@/prisma'; -const logger = createLogger('web-ee-token-refresh'); +const logger = createLogger('backend-ee-token-refresh'); const SUPPORTED_PROVIDERS = [ 'github', @@ -16,117 +27,124 @@ const SUPPORTED_PROVIDERS = [ type SupportedProvider = (typeof SUPPORTED_PROVIDERS)[number]; -const isSupportedProvider = (provider: string): provider is SupportedProvider => { - return SUPPORTED_PROVIDERS.includes(provider as SupportedProvider); -} +const isSupportedProvider = (provider: string): provider is SupportedProvider => + SUPPORTED_PROVIDERS.includes(provider as SupportedProvider); -// Map of providerAccountId -> error message -export type LinkedAccountErrors = Record; +// @see: https://datatracker.ietf.org/doc/html/rfc6749#section-5.1 +const OAuthTokenResponseSchema = z.object({ + access_token: z.string(), + token_type: z.string().optional(), + expires_in: z.number().optional(), + refresh_token: z.string().optional(), + scope: z.string().optional(), +}); -// In-memory lock to prevent concurrent refresh attempts for the same user -const refreshLocks = new Map>(); +type OAuthTokenResponse = z.infer; + +type ProviderCredentials = { + clientId: string; + clientSecret: string; + baseUrl?: string; +}; + +const EXPIRY_BUFFER_S = 5 * 60; // 5 minutes /** - * Refreshes expiring OAuth tokens for all linked accounts of a user. - * Loads accounts from database, refreshes tokens as needed, and returns any errors. - * Uses an in-memory lock to prevent concurrent refresh attempts for the same user. + * Ensures the OAuth access token for a given account is fresh. + * + * - If the token is not expired (or has no expiry), decrypts and returns it as-is. + * - If the token is expired or near expiry, attempts a refresh using the OAuth + * client credentials from the config file (or deprecated env vars). + * - On successful refresh: persists the new tokens to the DB, clears any + * tokenRefreshErrorMessage, and returns the fresh access token. + * - On failure: sets tokenRefreshErrorMessage on the account and throws, so + * the calling job fails with a clear error. */ -export const refreshLinkedAccountTokens = async (userId: string): Promise => { - // Check if there's already an in-flight refresh for this user - const existingRefresh = refreshLocks.get(userId); - if (existingRefresh) { - return existingRefresh; +export const ensureFreshAccountToken = async ( + account: Account, + db: PrismaClient, +): Promise => { + if (!account.access_token) { + throw new Error(`Account ${account.id} (${account.provider}) has no access token.`); } - // Create the refresh promise and store it in the lock map - const refreshPromise = doRefreshLinkedAccountTokens(userId); - refreshLocks.set(userId, refreshPromise); + if (!isSupportedProvider(account.provider)) { + // Non-refreshable provider — just decrypt and return whatever is stored. + const token = decryptOAuthToken(account.access_token); + if (!token) { + throw new Error(`Failed to decrypt access token for account ${account.id}.`); + } + return token; + } - try { - return await refreshPromise; - } finally { - refreshLocks.delete(userId); + const now = Math.floor(Date.now() / 1000); + const isExpiredOrNearExpiry = + account.expires_at !== null && + account.expires_at > 0 && + now >= account.expires_at - EXPIRY_BUFFER_S; + + if (!isExpiredOrNearExpiry) { + const token = decryptOAuthToken(account.access_token); + if (!token) { + throw new Error(`Failed to decrypt access token for account ${account.id}.`); + } + return token; } -}; -const doRefreshLinkedAccountTokens = async (userId: string): Promise => { - // Only grab accounts that can be refreshed (i.e., have an access token, refresh token, and expires_at). - const accounts = await prisma.account.findMany({ - where: { - userId, - access_token: { not: null }, - refresh_token: { not: null }, - expires_at: { not: null }, - }, - select: { - provider: true, - providerAccountId: true, - access_token: true, - refresh_token: true, - expires_at: true, - }, - }); + if (!account.refresh_token) { + const message = `Account ${account.id} (${account.provider}) token is expired and has no refresh token.`; + logger.error(message); + await setTokenRefreshError(account.id, message, db); + throw new Error(message); + } - const now = Math.floor(Date.now() / 1000); - const bufferTimeS = 5 * 60; // 5 minutes - const errors: LinkedAccountErrors = {}; + const refreshToken = decryptOAuthToken(account.refresh_token); + if (!refreshToken) { + const message = `Failed to decrypt refresh token for account ${account.id} (${account.provider}).`; + logger.error(message); + await setTokenRefreshError(account.id, message, db); + throw new Error(message); + } - await Promise.all( - accounts.map(async (account) => { - const { provider, providerAccountId, expires_at } = account; + logger.debug(`Refreshing OAuth token for account ${account.id} (${account.provider})...`); - if (!isSupportedProvider(provider)) { - return; - } + const refreshResponse = await refreshOAuthToken(account.provider, refreshToken); + if (!refreshResponse) { + const message = `OAuth token refresh failed for account ${account.id} (${account.provider}).`; + logger.error(message); + await setTokenRefreshError(account.id, message, db); + throw new Error(message); + } - if (expires_at !== null && expires_at > 0 && now >= (expires_at - bufferTimeS)) { - const refreshToken = decryptOAuthToken(account.refresh_token); - if (!refreshToken) { - logger.error(`Failed to decrypt refresh token for providerAccountId: ${providerAccountId}`); - errors[providerAccountId] = 'RefreshTokenError'; - return; - } + const newExpiresAt = refreshResponse.expires_in + ? Math.floor(Date.now() / 1000) + refreshResponse.expires_in + : null; + + await db.account.update({ + where: { id: account.id }, + data: { + access_token: encryptOAuthToken(refreshResponse.access_token), + // Only update refresh_token if a new one was provided; preserve the + // existing one otherwise (some providers use rotating refresh tokens, + // others reuse the same one). + ...(refreshResponse.refresh_token !== undefined + ? { refresh_token: encryptOAuthToken(refreshResponse.refresh_token) } + : {}), + expires_at: newExpiresAt, + tokenRefreshErrorMessage: null, + }, + }); - try { - logger.info(`Refreshing token for providerAccountId: ${providerAccountId} (${provider})`); - const refreshTokenResponse = await refreshOAuthToken(provider, refreshToken); - - if (refreshTokenResponse) { - const expires_at = refreshTokenResponse.expires_in ? Math.floor(Date.now() / 1000) + refreshTokenResponse.expires_in : null; - - await prisma.account.update({ - where: { - provider_providerAccountId: { - provider, - providerAccountId, - } - }, - data: { - access_token: encryptOAuthToken(refreshTokenResponse.access_token), - // Only update refresh_token if a new one was provided. - // This will preserve an existing refresh token if the provider - // does not return a new one. - ...(refreshTokenResponse.refresh_token !== undefined ? { - refresh_token: encryptOAuthToken(refreshTokenResponse.refresh_token), - } : {}), - expires_at, - }, - }); - logger.info(`Successfully refreshed token for provider: ${provider}`); - } else { - logger.error(`Failed to refresh token for provider: ${provider}`); - errors[providerAccountId] = 'RefreshTokenError'; - } - } catch (error) { - logger.error(`Error refreshing token for provider ${provider}:`, error); - errors[providerAccountId] = 'RefreshTokenError'; - } - } - }) - ); + logger.debug(`Successfully refreshed OAuth token for account ${account.id} (${account.provider}).`); + return refreshResponse.access_token; +}; - return errors; -} +const setTokenRefreshError = async (accountId: string, message: string, db: PrismaClient) => { + await db.account.update({ + where: { id: accountId }, + data: { tokenRefreshErrorMessage: message }, + }); +}; const refreshOAuthToken = async ( provider: SupportedProvider, @@ -135,10 +153,9 @@ const refreshOAuthToken = async ( try { const config = await loadConfig(env.CONFIG_PATH); const identityProviders = config?.identityProviders ?? []; - const providerConfigs = identityProviders.filter(idp => idp.provider === provider); - // If no provider configs in the config file, try deprecated env vars + // If no provider configs in the config file, try deprecated env vars. if (providerConfigs.length === 0) { const envCredentials = getDeprecatedEnvCredentials(provider); if (envCredentials) { @@ -150,7 +167,7 @@ const refreshOAuthToken = async ( logger.error(`Failed to refresh ${provider} token using deprecated env credentials`); return null; } - logger.error(`Provider config not found or invalid for: ${provider}`); + logger.error(`No provider config or env credentials found for: ${provider}`); return null; } @@ -172,7 +189,9 @@ const refreshOAuthToken = async ( // Get client credentials from config const clientId = await getTokenFromConfig(linkedAccountProviderConfig.clientId); const clientSecret = await getTokenFromConfig(linkedAccountProviderConfig.clientSecret); - const baseUrl = 'baseUrl' in linkedAccountProviderConfig ? linkedAccountProviderConfig.baseUrl : undefined; + const baseUrl = 'baseUrl' in linkedAccountProviderConfig + ? linkedAccountProviderConfig.baseUrl + : undefined; const result = await tryRefreshToken(provider, refreshToken, { clientId, clientSecret, baseUrl }); if (result) { @@ -186,29 +205,12 @@ const refreshOAuthToken = async ( logger.error(`All provider configs failed for: ${provider}`); return null; - } catch (error) { - logger.error(`Error refreshing ${provider} token:`, error); + } catch (e) { + logger.error(`Error refreshing ${provider} token:`, e); return null; } -} - -type ProviderCredentials = { - clientId: string; - clientSecret: string; - baseUrl?: string; }; -// @see: https://datatracker.ietf.org/doc/html/rfc6749#section-5.1 -const OAuthTokenResponseSchema = z.object({ - access_token: z.string(), - token_type: z.string().optional(), - expires_in: z.number().optional(), - refresh_token: z.string().optional(), - scope: z.string().optional(), -}); - -type OAuthTokenResponse = z.infer; - const tryRefreshToken = async ( provider: SupportedProvider, refreshToken: string, diff --git a/packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql b/packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql new file mode 100644 index 000000000..a1fdd02b2 --- /dev/null +++ b/packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "Account" ADD COLUMN "tokenRefreshErrorMessage" TEXT; diff --git a/packages/db/prisma/schema.prisma b/packages/db/prisma/schema.prisma index 04f310209..609c71b89 100644 --- a/packages/db/prisma/schema.prisma +++ b/packages/db/prisma/schema.prisma @@ -460,6 +460,10 @@ model Account { permissionSyncJobs AccountPermissionSyncJob[] permissionSyncedAt DateTime? + /// Set when an OAuth token refresh fails and the account needs to be re-linked by the user. + /// Cleared when the user successfully re-authenticates. + tokenRefreshErrorMessage String? + createdAt DateTime @default(now()) updatedAt DateTime @updatedAt diff --git a/packages/web/src/auth.ts b/packages/web/src/auth.ts index 0e4ec06ad..a35563a3e 100644 --- a/packages/web/src/auth.ts +++ b/packages/web/src/auth.ts @@ -17,7 +17,6 @@ import { hasEntitlement } from '@sourcebot/shared'; import { onCreateUser } from '@/lib/authUtils'; import { getAuditService } from '@/ee/features/audit/factory'; import { SINGLE_TENANT_ORG_ID } from './lib/constants'; -import { refreshLinkedAccountTokens, LinkedAccountErrors } from '@/ee/features/sso/tokenRefresh'; import { EncryptedPrismaAdapter, encryptAccountData } from '@/lib/encryptedPrismaAdapter'; const auditService = getAuditService(); @@ -39,14 +38,12 @@ export type SessionUser = { declare module 'next-auth' { interface Session { user: SessionUser; - linkedAccountProviderErrors?: LinkedAccountErrors; } } declare module 'next-auth/jwt' { interface JWT { userId: string; - linkedAccountErrors?: LinkedAccountErrors; } } @@ -182,6 +179,8 @@ export const { handlers, signIn, signOut, auth } = NextAuth({ scope: account.scope, id_token: account.id_token, issuerUrl, + // Clear any token refresh error since the user has successfully re-authenticated. + tokenRefreshErrorMessage: null, }) }) } @@ -256,12 +255,6 @@ export const { handlers, signIn, signOut, auth } = NextAuth({ } } - // Refresh expiring tokens and capture any errors. - if (hasEntitlement('sso') && token.userId) { - const errors = await refreshLinkedAccountTokens(token.userId); - token.linkedAccountErrors = Object.keys(errors).length > 0 ? errors : undefined; - } - return token; }, async session({ session, token }) { @@ -273,11 +266,6 @@ export const { handlers, signIn, signOut, auth } = NextAuth({ id: token.userId, } - // Pass linked account errors to the session for UI display - if (token.linkedAccountErrors) { - session.linkedAccountProviderErrors = token.linkedAccountErrors; - } - return session; }, }, diff --git a/packages/web/src/ee/features/sso/actions.ts b/packages/web/src/ee/features/sso/actions.ts index 8101c8d72..adb3d0fa8 100644 --- a/packages/web/src/ee/features/sso/actions.ts +++ b/packages/web/src/ee/features/sso/actions.ts @@ -1,7 +1,6 @@ 'use server'; import { sew } from "@/actions"; -import { auth } from "@/auth"; import { OPTIONAL_PROVIDERS_LINK_SKIPPED_COOKIE_NAME } from "@/lib/constants"; import { withAuthV2, withMinimumOrgRole } from "@/withAuthV2"; import { OrgRole } from "@sourcebot/db"; @@ -33,15 +32,13 @@ export const getLinkedAccounts = async () => sew(() => id: true, provider: true, providerAccountId: true, + tokenRefreshErrorMessage: true, }, }); const config = await loadConfig(env.CONFIG_PATH); const identityProviderConfigs = config.identityProviders ?? []; - const session = await auth(); - const providerErrors = session?.linkedAccountProviderErrors; - const permissionSyncEnabled = env.PERMISSION_SYNC_ENABLED === 'true' && hasEntitlement('permission-syncing'); @@ -59,7 +56,7 @@ export const getLinkedAccounts = async () => sew(() => isLinked: true, accountId: account.id, providerAccountId: account.providerAccountId, - error: providerErrors?.[account.providerAccountId], + error: account.tokenRefreshErrorMessage ?? undefined, isAccountLinkingProvider: isAccountLinking, required: isAccountLinking ? (providerConfig?.accountLinkingRequired ?? false) : false, supportsPermissionSync: permissionSyncEnabled && PERMISSION_SYNC_SUPPORTED_IDENTITY_PROVIDERS.includes(account.provider as IdentityProviderType), diff --git a/packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx b/packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx index d0ef2e0a0..41defc7eb 100644 --- a/packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx +++ b/packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx @@ -52,8 +52,9 @@ export function LinkedAccountProviderCard({ if (syncJobId && syncStatusData !== undefined && !syncStatusData.isSyncing) { setSyncJobId(null); toast({ description: `✅ Permissions refreshed for ${providerInfo.displayName}.` }); + router.refresh(); } - }, [syncJobId, syncStatusData, providerInfo.displayName, toast]); + }, [syncJobId, syncStatusData, providerInfo.displayName, toast, router]); const handleConnect = () => { signIn(linkedAccount.provider, { redirectTo: callbackUrl ?? window.location.href }); diff --git a/packages/web/src/lib/encryptedPrismaAdapter.ts b/packages/web/src/lib/encryptedPrismaAdapter.ts index 35c108d35..3a8780e07 100644 --- a/packages/web/src/lib/encryptedPrismaAdapter.ts +++ b/packages/web/src/lib/encryptedPrismaAdapter.ts @@ -40,6 +40,7 @@ export function encryptAccountData(data: { token_type?: string | null; scope?: string | null; issuerUrl?: string | null; + tokenRefreshErrorMessage?: string | null; }) { return { ...data,