diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dc85176c..fac898575 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Remove references to demo from docs. [#734](https://github.com/sourcebot-dev/sourcebot/pull/734) - Add docs for GitHub App connection auth. [#735](https://github.com/sourcebot-dev/sourcebot/pull/735) +- Improved error messaging around oauth scope errors with user driven permission syncing. [#639](https://github.com/sourcebot-dev/sourcebot/pull/639) ### Fixed - Fixed issue where 403 errors were being raised during a user driven permission sync against a self-hosted code host. [#729](https://github.com/sourcebot-dev/sourcebot/pull/729) diff --git a/docs/docs/features/permission-syncing.mdx b/docs/docs/features/permission-syncing.mdx index fb7d88afd..ac3250bbe 100644 --- a/docs/docs/features/permission-syncing.mdx +++ b/docs/docs/features/permission-syncing.mdx @@ -27,6 +27,10 @@ docker run \ ghcr.io/sourcebot-dev/sourcebot:latest ``` + +Enabling permission syncing on an **existing** deployment may result in errors that look like **"User does not have an OAuth access token..."**. This is because the OAuth access token associated with the user's existing account does not have the correct scopes necessary for permission syncing. To fix, have the user re-authenticate to refresh their access token by either logging out of Sourcebot and logging in again or unlinking and re-linking their account. + + ## Platform support We are actively working on supporting more code hosts. If you'd like to see a specific code host supported, please [reach out](https://www.sourcebot.dev/contact). diff --git a/packages/backend/src/ee/accountPermissionSyncer.ts b/packages/backend/src/ee/accountPermissionSyncer.ts index 315c86811..60e27a178 100644 --- a/packages/backend/src/ee/accountPermissionSyncer.ts +++ b/packages/backend/src/ee/accountPermissionSyncer.ts @@ -4,8 +4,16 @@ import { env, hasEntitlement, createLogger, loadConfig } from "@sourcebot/shared import { Job, Queue, Worker } from "bullmq"; import { Redis } from "ioredis"; import { PERMISSION_SYNC_SUPPORTED_CODE_HOST_TYPES } from "../constants.js"; -import { createOctokitFromToken, getReposForAuthenticatedUser } from "../github.js"; -import { createGitLabFromOAuthToken, getProjectsForAuthenticatedUser } from "../gitlab.js"; +import { + createOctokitFromToken, + getOAuthScopesForAuthenticatedUser as getGitHubOAuthScopesForAuthenticatedUser, + getReposForAuthenticatedUser, +} from "../github.js"; +import { + createGitLabFromOAuthToken, + getOAuthScopesForAuthenticatedUser as getGitLabOAuthScopesForAuthenticatedUser, + getProjectsForAuthenticatedUser, +} from "../gitlab.js"; import { Settings } from "../types.js"; import { setIntervalAsync } from "../utils.js"; @@ -158,7 +166,7 @@ export class AccountPermissionSyncer { if (account.provider === 'github') { if (!account.access_token) { - throw new Error(`User '${account.user.email}' does not have an GitHub OAuth access token associated with their GitHub account.`); + 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. @@ -170,6 +178,12 @@ export class AccountPermissionSyncer { token: account.access_token, url: baseUrl, }); + + const scopes = await getGitHubOAuthScopesForAuthenticatedUser(octokit); + if (!scopes.includes('repo')) { + throw new Error(`OAuth token with scopes [${scopes.join(', ')}] is missing the 'repo' scope required for permission syncing.`); + } + // @note: we only care about the private repos since we don't need to build a mapping // for public repos. // @see: packages/web/src/prisma.ts @@ -188,7 +202,7 @@ export class AccountPermissionSyncer { repos.forEach(repo => aggregatedRepoIds.add(repo.id)); } else if (account.provider === 'gitlab') { if (!account.access_token) { - throw new Error(`User '${account.user.email}' does not have a GitLab OAuth access token associated with their GitLab account.`); + 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. @@ -201,6 +215,11 @@ export class AccountPermissionSyncer { url: baseUrl, }); + const scopes = await getGitLabOAuthScopesForAuthenticatedUser(api); + if (!scopes.includes('read_api')) { + throw new Error(`OAuth token with scopes [${scopes.join(', ')}] is missing the 'read_api' scope required for permission syncing.`); + } + // @note: we only care about the private and internal repos since we don't need to build a mapping // for public repos. // @see: packages/web/src/prisma.ts diff --git a/packages/backend/src/github.ts b/packages/backend/src/github.ts index 49ce1d37e..85169058c 100644 --- a/packages/backend/src/github.ts +++ b/packages/backend/src/github.ts @@ -197,6 +197,20 @@ export const getReposForAuthenticatedUser = async (visibility: 'all' | 'private' } } +// Gets oauth scopes +// @see: https://github.com/octokit/auth-token.js/?tab=readme-ov-file#find-out-what-scopes-are-enabled-for-oauth-tokens +export const getOAuthScopesForAuthenticatedUser = async (octokit: Octokit) => { + try { + const response = await octokit.request("HEAD /"); + const scopes = response.headers["x-oauth-scopes"]?.split(/,\s+/) || []; + return scopes; + } catch (error) { + Sentry.captureException(error); + logger.error(`Failed to fetch OAuth scopes for authenticated user.`, error); + throw error; + } +} + const getReposOwnedByUsers = async (users: string[], octokit: Octokit, signal: AbortSignal, url?: string) => { const results = await Promise.allSettled(users.map((user) => githubQueryLimit(async () => { try { diff --git a/packages/backend/src/gitlab.ts b/packages/backend/src/gitlab.ts index 44685424a..5d07985e7 100644 --- a/packages/backend/src/gitlab.ts +++ b/packages/backend/src/gitlab.ts @@ -41,8 +41,8 @@ export const getGitLabReposFromConfig = async (config: GitlabConnectionConfig) = const token = config.token ? await getTokenFromConfig(config.token) : hostname === GITLAB_CLOUD_HOSTNAME ? - env.FALLBACK_GITLAB_CLOUD_TOKEN : - undefined; + env.FALLBACK_GITLAB_CLOUD_TOKEN : + undefined; const api = await createGitLabFromPersonalAccessToken({ token, @@ -207,7 +207,7 @@ export const getGitLabReposFromConfig = async (config: GitlabConnectionConfig) = return !isExcluded; }); - + logger.debug(`Found ${repos.length} total repositories.`); return { @@ -316,4 +316,28 @@ export const getProjectsForAuthenticatedUser = async (visibility: 'private' | 'i logger.error(`Failed to fetch projects for authenticated user.`, error); throw error; } +} + +// Fetches OAuth scopes for the authenticated user. +// @see: https://github.com/doorkeeper-gem/doorkeeper/wiki/API-endpoint-descriptions-and-examples#get----oauthtokeninfo +// @see: https://docs.gitlab.com/api/oauth2/#retrieve-the-token-information +export const getOAuthScopesForAuthenticatedUser = async (api: InstanceType) => { + try { + const response = await api.requester.get('/oauth/token/info'); + if ( + response && + typeof response.body === 'object' && + response.body !== null && + 'scope' in response.body && + Array.isArray(response.body.scope) + ) { + return response.body.scope; + } + + throw new Error('/oauth/token_info response body is not in the expected format.'); + } catch (error) { + Sentry.captureException(error); + logger.error('Failed to fetch OAuth scopes for authenticated user.', error); + throw error; + } } \ No newline at end of file diff --git a/packages/web/src/app/components/authMethodSelector.tsx b/packages/web/src/app/components/authMethodSelector.tsx index 442fb919a..540d8b0a2 100644 --- a/packages/web/src/app/components/authMethodSelector.tsx +++ b/packages/web/src/app/components/authMethodSelector.tsx @@ -29,9 +29,12 @@ export const AuthMethodSelector = ({ // Call the optional analytics callback first onProviderClick?.(provider); - signIn(provider, { - redirectTo: callbackUrl ?? "/" - }); + signIn( + provider, + { + redirectTo: callbackUrl ?? "/", + } + ); }, [callbackUrl, onProviderClick]); // Separate OAuth providers from special auth methods diff --git a/packages/web/src/auth.ts b/packages/web/src/auth.ts index f61219d7b..b1f9c720b 100644 --- a/packages/web/src/auth.ts +++ b/packages/web/src/auth.ts @@ -60,88 +60,92 @@ export const getProviders = () => { const providers: IdentityProvider[] = eeIdentityProviders; if (env.SMTP_CONNECTION_URL && env.EMAIL_FROM_ADDRESS && env.AUTH_EMAIL_CODE_LOGIN_ENABLED === 'true') { - providers.push({ provider: EmailProvider({ - server: env.SMTP_CONNECTION_URL, - from: env.EMAIL_FROM_ADDRESS, - maxAge: 60 * 10, - generateVerificationToken: async () => { - const token = String(Math.floor(100000 + Math.random() * 900000)); - return token; - }, - sendVerificationRequest: async ({ identifier, provider, token }) => { - const transport = createTransport(provider.server); - const html = await render(MagicLinkEmail({ token: token })); - const result = await transport.sendMail({ - to: identifier, - from: provider.from, - subject: 'Log in to Sourcebot', - html, - text: `Log in to Sourcebot using this code: ${token}` - }); + providers.push({ + provider: EmailProvider({ + server: env.SMTP_CONNECTION_URL, + from: env.EMAIL_FROM_ADDRESS, + maxAge: 60 * 10, + generateVerificationToken: async () => { + const token = String(Math.floor(100000 + Math.random() * 900000)); + return token; + }, + sendVerificationRequest: async ({ identifier, provider, token }) => { + const transport = createTransport(provider.server); + const html = await render(MagicLinkEmail({ token: token })); + const result = await transport.sendMail({ + to: identifier, + from: provider.from, + subject: 'Log in to Sourcebot', + html, + text: `Log in to Sourcebot using this code: ${token}` + }); - const failed = result.rejected.concat(result.pending).filter(Boolean); - if (failed.length) { - throw new Error(`Email(s) (${failed.join(", ")}) could not be sent`); + const failed = result.rejected.concat(result.pending).filter(Boolean); + if (failed.length) { + throw new Error(`Email(s) (${failed.join(", ")}) could not be sent`); + } } - } - }), purpose: "sso"}); + }), purpose: "sso" + }); } if (env.AUTH_CREDENTIALS_LOGIN_ENABLED === 'true') { - providers.push({ provider: Credentials({ - credentials: { - email: {}, - password: {} - }, - type: "credentials", - authorize: async (credentials) => { - const body = verifyCredentialsRequestSchema.safeParse(credentials); - if (!body.success) { - return null; - } - const { email, password } = body.data; + providers.push({ + provider: Credentials({ + credentials: { + email: {}, + password: {} + }, + type: "credentials", + authorize: async (credentials) => { + const body = verifyCredentialsRequestSchema.safeParse(credentials); + if (!body.success) { + return null; + } + const { email, password } = body.data; - const user = await prisma.user.findUnique({ - where: { email } - }); + const user = await prisma.user.findUnique({ + where: { email } + }); + + // The user doesn't exist, so create a new one. + if (!user) { + const hashedPassword = bcrypt.hashSync(password, 10); + const newUser = await prisma.user.create({ + data: { + email, + hashedPassword, + } + }); - // The user doesn't exist, so create a new one. - if (!user) { - const hashedPassword = bcrypt.hashSync(password, 10); - const newUser = await prisma.user.create({ - data: { - email, - hashedPassword, + const authJsUser: AuthJsUser = { + id: newUser.id, + email: newUser.email, } - }); - const authJsUser: AuthJsUser = { - id: newUser.id, - email: newUser.email, - } + onCreateUser({ user: authJsUser }); + return authJsUser; - onCreateUser({ user: authJsUser }); - return authJsUser; + // Otherwise, the user exists, so verify the password. + } else { + if (!user.hashedPassword) { + return null; + } - // Otherwise, the user exists, so verify the password. - } else { - if (!user.hashedPassword) { - return null; - } + if (!bcrypt.compareSync(password, user.hashedPassword)) { + return null; + } - if (!bcrypt.compareSync(password, user.hashedPassword)) { - return null; + return { + id: user.id, + email: user.email, + name: user.name ?? undefined, + image: user.image ?? undefined, + }; } - - return { - id: user.id, - email: user.email, - name: user.name ?? undefined, - image: user.image ?? undefined, - }; } - } - }), purpose: "sso"}); + }), purpose: "sso" + }); } return providers; @@ -156,7 +160,29 @@ export const { handlers, signIn, signOut, auth } = NextAuth({ trustHost: true, events: { createUser: onCreateUser, - signIn: async ({ user }) => { + signIn: async ({ user, account }) => { + // Explicitly update the Account record with the OAuth token details. + // This is necessary to update the access token when the user + // re-authenticates. + if (account && account.provider && account.provider !== 'credentials' && account.providerAccountId) { + await prisma.account.update({ + where: { + provider_providerAccountId: { + provider: account.provider, + providerAccountId: account.providerAccountId, + }, + }, + data: { + refresh_token: account.refresh_token, + access_token: account.access_token, + expires_at: account.expires_at, + token_type: account.token_type, + scope: account.scope, + id_token: account.id_token, + } + }) + } + if (user.id) { await auditService.createAudit({ action: "user.signed_in", @@ -225,7 +251,7 @@ export const { handlers, signIn, signOut, auth } = NextAuth({ // Propagate the userId to the session. id: token.userId, } - + // Pass only linked account provider errors to the session (not sensitive tokens) if (token.linkedAccountTokens) { const errors: Record = {};