diff --git a/apps/cli-docs/src/content/docs/agent-guidance.md b/apps/cli-docs/src/content/docs/agent-guidance.md index 6668e02e2..7dc9d82e9 100644 --- a/apps/cli-docs/src/content/docs/agent-guidance.md +++ b/apps/cli-docs/src/content/docs/agent-guidance.md @@ -325,7 +325,7 @@ When querying the Events API (directly or via `sentry api`), valid dataset value ## Common Mistakes - **Wrong issue ID format**: Use `PROJECT-123` (short ID), not the numeric ID `123456789`. The short ID includes the project prefix. -- **Pre-authenticating unnecessarily**: Don't run `sentry auth login` before every command. The CLI detects missing/expired auth and prompts automatically. Only run `sentry auth login` to add an account; use `sentry auth switch` to change which stored account is used. +- **Pre-authenticating unnecessarily**: Don't run `sentry auth login` before every command. The CLI detects missing/expired auth and prompts automatically. Only run `sentry auth login` to add an account. With several stored accounts, commands pick the one that knows the target organization or server automatically; use `sentry auth switch` only to change the default. - **Missing `--json` for piping**: Human-readable output includes formatting. Use `--json` when parsing output programmatically. - **Specifying org/project when not needed**: Auto-detection resolves org/project from `.sentryclirc` config files, DSNs, env vars, and directory names. Let it work first — only add `/` if the CLI says it can't detect the target or detects the wrong one. - **Manually discovering the project before running a command**: Don't list the orgs you belong to, then list every project in each, to match the local checkout to a project — the CLI already does exactly this resolution internally on each command. Skip the fan-out and run the command directly; correct the target afterwards only if the output shows the wrong org/project. diff --git a/apps/cli-docs/src/fragments/commands/auth.md b/apps/cli-docs/src/fragments/commands/auth.md index 1fff71d7a..82d40993f 100644 --- a/apps/cli-docs/src/fragments/commands/auth.md +++ b/apps/cli-docs/src/fragments/commands/auth.md @@ -66,7 +66,8 @@ account, or a SaaS and a self-hosted login. When you're already logged in, `sentry auth login` asks whether to add another account and makes it active; logging in again as the same user on the same server replaces that account's credentials instead of adding a duplicate. `--force` skips the prompt and -replaces the active account's login instead. +replaces the stored login for that server instead (or adds one when that +server has none); accounts on other servers are never replaced. ```bash # Add (or log in again to) a specific account without prompting @@ -91,6 +92,30 @@ Select an account by email, username, user ID, or account ID (from only candidate. If the same email is stored on several servers, add `--url`: `sentry auth switch jane@example.com --url https://sentry.example.com`. +Most of the time you don't need to switch: commands pick the stored account +that fits what they target, without changing the active account: + +- **Organization** — when the target organization isn't in the active + account's organization list (cached at login and by `sentry org list`) but + another stored account has used it, that account is used. This includes + `oNNNN` IDs from DSNs. +- **Server** — `SENTRY_HOST`/`SENTRY_URL`, `.sentryclirc`, or a pasted Sentry + URL for another account's server (for example a self-hosted issue link) uses + the account stored for that server. + +```bash +# Active account is personal; this uses the work account that knows acme-corp +sentry issue list acme-corp/ + +# Uses the self-hosted account without switching +sentry issue view https://sentry.example.com/organizations/acme/issues/123/ +``` + +Otherwise the active account is used. One command never mixes accounts; when +several other accounts know the organization, the most recently used wins. +This works even when the active account is logged out: the target can still +pick a stored account that can authenticate. + Switching also points the default URL (`sentry cli defaults url`) at the account's server. `SENTRY_HOST`/`SENTRY_URL` and `SENTRY_AUTH_TOKEN` (with `SENTRY_FORCE_ENV_TOKEN=1`) still take precedence over the active account. diff --git a/packages/cli/plugins/sentry-cli/skills/sentry-cli/SKILL.md b/packages/cli/plugins/sentry-cli/skills/sentry-cli/SKILL.md index 355da0ded..b98a64094 100644 --- a/packages/cli/plugins/sentry-cli/skills/sentry-cli/SKILL.md +++ b/packages/cli/plugins/sentry-cli/skills/sentry-cli/SKILL.md @@ -337,7 +337,7 @@ When querying the Events API (directly or via `sentry api`), valid dataset value ### Common Mistakes - **Wrong issue ID format**: Use `PROJECT-123` (short ID), not the numeric ID `123456789`. The short ID includes the project prefix. -- **Pre-authenticating unnecessarily**: Don't run `sentry auth login` before every command. The CLI detects missing/expired auth and prompts automatically. Only run `sentry auth login` to add an account; use `sentry auth switch` to change which stored account is used. +- **Pre-authenticating unnecessarily**: Don't run `sentry auth login` before every command. The CLI detects missing/expired auth and prompts automatically. Only run `sentry auth login` to add an account. With several stored accounts, commands pick the one that knows the target organization or server automatically; use `sentry auth switch` only to change the default. - **Missing `--json` for piping**: Human-readable output includes formatting. Use `--json` when parsing output programmatically. - **Specifying org/project when not needed**: Auto-detection resolves org/project from `.sentryclirc` config files, DSNs, env vars, and directory names. Let it work first — only add `/` if the CLI says it can't detect the target or detects the wrong one. - **Manually discovering the project before running a command**: Don't list the orgs you belong to, then list every project in each, to match the local checkout to a project — the CLI already does exactly this resolution internally on each command. Skip the fan-out and run the command directly; correct the target afterwards only if the output shows the wrong org/project. diff --git a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/auth.md b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/auth.md index 66362a61a..cc197b032 100644 --- a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/auth.md +++ b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/auth.md @@ -22,7 +22,7 @@ Authenticate with Sentry **Flags:** - `--token - Authenticate using an API token instead of OAuth` - `--timeout - Timeout for OAuth flow in seconds (default: 900) - (default: "900")` -- `--force - Replace the active account's login without prompting, instead of adding another account` +- `--force - Replace the stored login for this server without prompting, instead of adding another account` - `--url - Sentry instance URL to authenticate against (e.g. https://sentry.example.com). Required for self-hosted; defaults to SaaS (https://sentry.io).` - `--read-only - Request only read-only OAuth scopes (project:read, org:read, event:read, member:read, team:read). Useful for handing tokens to AI agents or CI jobs that should not be able to mutate Sentry state.` - `-s, --scope ... - Request specific OAuth scopes (repeatable, comma-separated). E.g. --scope project:read --scope org:read. Overrides the default scope set.` diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 6bc08621e..1248818cc 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -58,6 +58,8 @@ async function preloadProjectContext(cwd: string): Promise { const url = getDefaultUrl(); if (url) { env.SENTRY_URL = url; + const { markPersistedDefaultUrl } = await import("./lib/constants.js"); + markPersistedDefaultUrl(url); } } catch { // DB not available — skip diff --git a/packages/cli/src/commands/auth/login.ts b/packages/cli/src/commands/auth/login.ts index 1d2fcacee..d62e12325 100644 --- a/packages/cli/src/commands/auth/login.ts +++ b/packages/cli/src/commands/auth/login.ts @@ -11,6 +11,7 @@ import { normalizeAuthToken } from "../../lib/auth-header.js"; import { buildCommand, numberParser } from "../../lib/command.js"; import { normalizeUrl } from "../../lib/constants.js"; import { + getAccountIdToReplace, getActiveAccountId, getActiveEnvVarName, hasStoredAuthCredentials, @@ -345,7 +346,8 @@ export function applyLoginUrl(url: string | undefined): string { * command should exit early. Without `--force` the new login is added * alongside the current account and becomes active (logging in again as * the same user replaces that user's old account); with `--force` it - * replaces the active account's credentials. + * replaces the stored login for the same server (or adds one when that + * server has none). Accounts on other servers are never touched. * * - Env-var auth only: proceeds (stored OAuth is kept separately). * - `--force`: proceeds without prompting. @@ -474,7 +476,8 @@ export const loginCommand = buildCommand({ "currently-authenticated host.\n\n" + "When already logged in, the new login is stored as another account\n" + "(see 'sentry auth list' / 'sentry auth switch'); --force instead\n" + - "replaces the active account's login. Pass an email to add or log in\n" + + "replaces the stored login for that server (accounts on other servers\n" + + "are kept). Pass an email to add or log in\n" + "again to that account without prompting: Sentry approves the request\n" + "as whoever is signed in to the browser, so a login approved as another\n" + "user is refused and nothing is saved.\n\n" + @@ -501,7 +504,7 @@ export const loginCommand = buildCommand({ force: { kind: "boolean", brief: - "Replace the active account's login without prompting, instead of adding another account", + "Replace the stored login for this server without prompting, instead of adding another account", default: false, }, url: { @@ -586,16 +589,20 @@ export const loginCommand = buildCommand({ // Non-fatal: cache directory may not exist } - // `--force` replaces the active account; otherwise the login is stored - // as another account (see handleExistingAuth). + // `--force` replaces the stored login for this server; otherwise the + // login is stored as another account (see handleExistingAuth). const newAccount = email !== undefined || !flags.force; if (token !== undefined) { // Save token first (with host scope), then validate by fetching user // regions. It is always stored as its own account so a rejected token - // never touches the stored ones; with `--force` the previously active - // account is replaced only once the new token checks out. + // never touches the stored ones; with `--force` the stored login for + // this server (never another server's) is replaced only once the new + // token checks out. const previousAccountId = getActiveAccountId(); + const replacedAccountId = newAccount + ? undefined + : getAccountIdToReplace(effectiveHost); const accountId = setAuthToken(token, undefined, undefined, { host: effectiveHost, newAccount: true, @@ -645,11 +652,11 @@ export const loginCommand = buildCommand({ } result.user = toLoginUser(user); } - // Replace the previous account only now. After setUserInfo, so a - // re-login as the same user folds that account in (keeping a known - // username) instead of losing it; then this is a no-op. - if (!newAccount && previousAccountId && previousAccountId !== accountId) { - await removeAccount(previousAccountId); + // Replace this server's previous login only now. After setUserInfo, + // so a re-login as the same user folds that account in (keeping a + // known username) instead of losing it; then this is a no-op. + if (replacedAccountId && replacedAccountId !== accountId) { + await removeAccount(replacedAccountId); } // Warm the org + region cache so the first real command is fast. diff --git a/packages/cli/src/lib/command.ts b/packages/cli/src/lib/command.ts index 470748071..33d6de8f7 100644 --- a/packages/cli/src/lib/command.ts +++ b/packages/cli/src/lib/command.ts @@ -38,7 +38,7 @@ import { } from "@stricli/core"; import type { Writer } from "../types/index.js"; import { appendCacheHint } from "./cache-hint.js"; -import { getAuthConfig } from "./db/auth.js"; +import { getAuthConfig, hasUsableStoredAccount } from "./db/auth.js"; import { getEnv } from "./env.js"; import { AuthError, CliError, OutputError } from "./errors.js"; import { warning } from "./formatters/colors.js"; @@ -785,7 +785,9 @@ export function buildCommand< // `sentry issue list help`). Without this, the auth prompt would fire // before the help-recovery path could show the command's help text. try { - if (requiresAuth && !getAuthConfig()) { + // A usable stored account defers the check: the command's target may + // select it even when the account in effect is logged out. + if (requiresAuth && !getAuthConfig() && !hasUsableStoredAccount()) { throw new AuthError("not_authenticated"); } diff --git a/packages/cli/src/lib/constants.ts b/packages/cli/src/lib/constants.ts index 14612a29f..64d69790f 100644 --- a/packages/cli/src/lib/constants.ts +++ b/packages/cli/src/lib/constants.ts @@ -101,6 +101,39 @@ export function getConfiguredSentryUrl(): string | undefined { } } +/** `SENTRY_URL` values the CLI applied from `defaults.url`, per invocation env. */ +const persistedDefaultUrls = new WeakMap(); + +/** + * Record that `env.SENTRY_URL` was set from the persisted default URL (not + * by the user), so account selection may route to another stored + * account's server instead of treating it as an explicit choice. + */ +export function markPersistedDefaultUrl(url: string): void { + persistedDefaultUrls.set(getEnv(), url); +} + +/** + * Forget which URL came from `defaults.url`, so the configured URL counts as + * an explicit server choice from now on (e.g. a pasted SaaS URL pinning the + * server to the same value as the persisted default). Tests also use it to + * reset state. + */ +export function clearPersistedDefaultUrl(): void { + persistedDefaultUrls.delete(getEnv()); +} + +/** Whether the configured URL is still the one applied from `defaults.url`. */ +export function isPersistedDefaultUrlInEffect(): boolean { + const env = getEnv(); + const applied = persistedDefaultUrls.get(env); + return ( + applied !== undefined && + !env.SENTRY_HOST?.trim() && + env.SENTRY_URL?.trim() === applied + ); +} + /** CLI version string, available for help output and other uses */ export const CLI_VERSION = typeof SENTRY_CLI_VERSION !== "undefined" ? SENTRY_CLI_VERSION : "0.0.0-dev"; diff --git a/packages/cli/src/lib/db/account-scope.ts b/packages/cli/src/lib/db/account-scope.ts new file mode 100644 index 000000000..75cd439d4 --- /dev/null +++ b/packages/cli/src/lib/db/account-scope.ts @@ -0,0 +1,55 @@ +/** + * Per-invocation account selection. + * + * Commands can resolve which stored account to use from what they target + * (see `selectAccountForOrg` in `auth.ts`). That choice becomes the + * invocation's default account: every credential read for the rest of the + * invocation uses it. It is never persisted — the stored active account + * (`sentry auth login` / `sentry auth switch`) is untouched, so the next + * invocation starts from scratch and may pick a different account. + * + * Why this is keyed by the environment object instead of being a plain + * module-level variable: + * + * - CLI mode runs one command per process. `getEnv()` always returns + * `process.env`, so there is exactly one scope and this behaves like a + * module-level variable. + * - Library mode (`createSentrySDK()`, see `sdk-invoke.ts`) runs many + * commands inside the consumer's long-lived process, and they + * can overlap: each call builds its own isolated env copy and runs under + * `withEnv()` (AsyncLocalStorage), so `getEnv()` returns that copy for + * everything the call awaits. A module-level variable would be shared by + * all of them — a call targeting org A would select account 1, and a + * concurrent (or later) call targeting org B would silently reuse account + * 1 instead of resolving its own. Keying by the env object gives every + * call its own scope with no explicit setup or teardown. + * - A `WeakMap` lets a finished call's scope be garbage-collected together + * with its env copy, so long-running library consumers do not accumulate + * one entry per call. + */ + +import { getEnv } from "../env.js"; + +type AccountScope = { + /** Default account for the rest of this invocation, chosen from its target. */ + accountId?: string; +}; + +/** One scope per invocation env; see the module comment for why. */ +let scopes = new WeakMap(); + +/** The mutable account selection for the current invocation. */ +export function getAccountScope(): AccountScope { + const env = getEnv(); + let scope = scopes.get(env); + if (!scope) { + scope = {}; + scopes.set(env, scope); + } + return scope; +} + +/** Forget every invocation's account selection. Tests only. */ +export function resetAccountScopes(): void { + scopes = new WeakMap(); +} diff --git a/packages/cli/src/lib/db/accounts.ts b/packages/cli/src/lib/db/accounts.ts index 2c42d01af..198b78856 100644 --- a/packages/cli/src/lib/db/accounts.ts +++ b/packages/cli/src/lib/db/accounts.ts @@ -7,12 +7,13 @@ * credential is active go through `auth.ts` so its memoized state resets. */ -import { createHash } from "node:crypto"; import { isSaaSTrustOrigin } from "@sentry/toolkit-core/sentry-origin"; import { uuidv7 } from "uuidv7"; import { fuzzyMatch } from "../fuzzy.js"; import { normalizeHttpOrigin } from "../sentry-urls.js"; +import { storedCredentialIdentity } from "./identity-hash.js"; import { getDatabase } from "./index.js"; +import { ORG_CACHE_TTL_MS } from "./regions.js"; /** Raw `accounts` row. */ export type AccountRow = { @@ -118,31 +119,15 @@ export function activateAccountRow(accountId: string): void { })(); } -/** - * Stable SHA-256 namespace for high-entropy OAuth and organization tokens. - * A collision would mix credentials' cached responses, so retain the full - * digest. This fingerprint is never a password verifier or a bearer token. - */ -export function hashIdentity(kind: string, secret: string): string { - return createHash("sha256") - .update(kind) - .update("\0") - .update(secret) - .digest("hex"); -} - /** Cache namespace for a stored account (matches `getIdentityFingerprint`). */ export function accountIdentity(row: AccountRow): string | undefined { - if (row.refresh_token) { - return hashIdentity("oauth", row.refresh_token); - } - if (row.token) { - return hashIdentity("oauth-access", row.token); - } - return; + return storedCredentialIdentity(row); } -/** True when the row's credential is usable now or can be refreshed. */ +/** + * True when the row's credential is usable now or can be refreshed. + * `USABLE_ACCOUNT_SQL` is the query form; keep the two in sync. + */ export function isAccountRowUsable(row: AccountRow): boolean { if (!row.token) { return false; @@ -161,17 +146,28 @@ export function deleteAccountRows(rows: readonly AccountRow[]): void { const db = getDatabase(); db.transaction(() => { for (const row of rows) { - const identity = accountIdentity(row); - if (identity) { - db.query("DELETE FROM org_regions WHERE credential_identity = ?").run( - identity, - ); - } + deleteAccountOrgRegions(row); db.query("DELETE FROM accounts WHERE account_id = ?").run(row.account_id); } })(); } +/** + * Delete an account's cached org memberships: rows keyed by its current + * credential identity, and rows recorded against the account under an + * earlier identity (before a refresh-token rotation or re-login). + */ +export function deleteAccountOrgRegions(row: AccountRow): void { + const db = getDatabase(); + const identity = accountIdentity(row); + if (identity) { + db.query("DELETE FROM org_regions WHERE credential_identity = ?").run( + identity, + ); + } + db.query("DELETE FROM org_regions WHERE account_id = ?").run(row.account_id); +} + /** Convert a row into its secret-free summary. */ export function toAccountSummary(row: AccountRow): AccountSummary { return { @@ -273,17 +269,10 @@ function matchesIdentity(account: AccountSummary, needle: string): boolean { * or both SaaS (`*.sentry.io`). `false` for an account without a host. */ export function isAccountOnServer( - account: Pick, + account: { readonly host?: string | null }, url: string, ): boolean { - const host = account.host ? normalizeHttpOrigin(account.host) : undefined; - const target = normalizeHttpOrigin(url); - if (!(host && target)) { - return false; - } - return ( - host === target || (isSaaSTrustOrigin(host) && isSaaSTrustOrigin(target)) - ); + return account.host ? isSameServer(account.host, url) : false; } /** Hostname of a stored account origin, lowercased. */ @@ -295,3 +284,113 @@ export function hostnameOf(host: string | undefined): string | undefined { ? new URL(host).hostname.toLowerCase() : host.toLowerCase(); } + +/** + * Whether two server URLs belong to the same Sentry installation: the same + * origin, or both SaaS (`*.sentry.io`). Mirrors the token trust model in + * `token-host.ts`, so an account is only ever chosen for hosts its token + * is already allowed to reach. + */ +export function isSameServer(a: string, b: string): boolean { + const left = normalizeHttpOrigin(a); + const right = normalizeHttpOrigin(b); + if (!(left && right)) { + return false; + } + return ( + left === right || (isSaaSTrustOrigin(left) && isSaaSTrustOrigin(right)) + ); +} + +/** + * Most recently used first. Ties (same `last_used_at`) fall back to the + * latest update, then to creation order, so picks are deterministic. + */ +const BY_RECENCY = + "ORDER BY last_used_at DESC, updated_at DESC, created_at ASC, account_id ASC"; + +/** + * SQL form of {@link isAccountRowUsable}: a token that is unexpired, + * never expires, or can be refreshed. Bind `Date.now()` as its only + * parameter. Keep the two in sync. + */ +const USABLE_ACCOUNT_SQL = `token IS NOT NULL AND token != '' + AND (expires_at IS NULL OR expires_at = 0 OR expires_at >= ? + OR (refresh_token IS NOT NULL AND refresh_token != ''))`; + +/** The most recently used stored account, usable or not. */ +export function getMostRecentAccountRow(): AccountRow | undefined { + return ( + (getDatabase() + .query(`SELECT * FROM accounts ${BY_RECENCY} LIMIT 1`) + .get() as AccountRow | null) ?? undefined + ); +} + +/** All stored accounts, usable or not, most recently used first. */ +export function listAccountRowsByRecency(): AccountRow[] { + return getDatabase() + .query(`SELECT * FROM accounts ${BY_RECENCY}`) + .all() as AccountRow[]; +} + +/** Usable stored accounts, most recently used first. */ +export function listUsableAccountRowsByRecency(): AccountRow[] { + return getDatabase() + .query(`SELECT * FROM accounts WHERE ${USABLE_ACCOUNT_SQL} ${BY_RECENCY}`) + .all(Date.now()) as AccountRow[]; +} + +/** + * Usable stored accounts that have seen an organization, by slug or + * numeric id, most recently used first — the first row is the pick. + * + * Org membership is already cached per account: `org_regions.account_id` + * is recorded whenever an account resolves or lists an organization, so + * this is one query over that cache rather than a lookup per account. + * + * Resolving one org by slug caches no numeric id (only org lists do), so a + * numeric id also matches rows for the slug that any cached row on the + * same server maps that id to. + */ +export function listUsableAccountRowsForOrg( + orgSlug: string, + orgId?: string, +): AccountRow[] { + const id = orgId ?? null; + return getDatabase() + .query( + `SELECT * FROM accounts + WHERE account_id IN ( + SELECT r.account_id FROM org_regions r + WHERE r.account_id IS NOT NULL AND ( + r.org_slug = ? OR r.org_id = ? OR EXISTS ( + SELECT 1 FROM org_regions m + WHERE m.org_id = ? AND m.org_slug = r.org_slug + AND m.source_origin = r.source_origin + ) + ) + ) AND ${USABLE_ACCOUNT_SQL} + ${BY_RECENCY}`, + ) + .all(orgSlug, id, id, Date.now()) as AccountRow[]; +} + +/** + * Whether a fresh full organization list is cached for an account (rows + * written by `listOrganizations`, which carry the numeric org id, updated + * within {@link ORG_CACHE_TTL_MS} — the TTL `getCachedOrganizations` uses). + * Only then can an org's absence mean the account is not a member; an + * expired list may predate a new membership. + */ +export function hasCachedOrgList(accountId: string): boolean { + const row = getDatabase() + .query( + "SELECT 1 AS found FROM org_regions WHERE account_id = ? AND org_id IS NOT NULL AND updated_at > ? LIMIT 1", + ) + .get(accountId, Date.now() - ORG_CACHE_TTL_MS) as + | { found: number } + | null + | undefined; + return Boolean(row); +} diff --git a/packages/cli/src/lib/db/auth.ts b/packages/cli/src/lib/db/auth.ts index 1f428ee2f..314707d42 100644 --- a/packages/cli/src/lib/db/auth.ts +++ b/packages/cli/src/lib/db/auth.ts @@ -2,19 +2,33 @@ * Authentication credential storage. * * Credentials live in the multi-row `accounts` table (see `accounts.ts`). - * Every read and refresh here targets the single active account, so the - * rest of the CLI keeps a one-credential view while users can store and - * switch between several logins. + * Every read and refresh here targets one *effective* account, so the rest + * of the CLI keeps a one-credential view while users store several logins. + * + * The effective account is chosen per invocation, most specific first: + * 1. The invocation's default account, chosen because it knows the + * targeted organization ({@link selectAccountForOrg}). Each invocation + * chooses again, so different commands can use different accounts. + * 2. An account on the configured server (`SENTRY_HOST`/`SENTRY_URL`, + * `--url`, a pasted Sentry URL, or `.sentryclirc`) when the active + * account is on a different one. + * 3. The active account (`sentry auth login` / `sentry auth switch`). */ import { normalizeAuthToken, trimAuthToken } from "../auth-header.js"; -import { DEFAULT_SENTRY_URL, getConfiguredSentryUrl } from "../constants.js"; +import { + clearPersistedDefaultUrl, + DEFAULT_SENTRY_URL, + getConfiguredSentryUrl, + isPersistedDefaultUrlInEffect, + markPersistedDefaultUrl, +} from "../constants.js"; import { getEnv } from "../env.js"; import { getBootConfiguredSentryUrl, getEnvTokenHost, } from "../env-token-host.js"; -import { ConfigError } from "../errors.js"; +import { AuthError, ConfigError } from "../errors.js"; import { logger } from "../logger.js"; import { normalizeHttpOrigin } from "../sentry-urls.js"; import { withDbSpan } from "../telemetry.js"; @@ -22,13 +36,23 @@ import { type AccountRow, accountIdentity, activateAccountRow, + deleteAccountOrgRegions, deleteAccountRows, getAccountRow, getActiveAccountRow, - hashIdentity, + getMostRecentAccountRow, + hasCachedOrgList, insertActiveAccountRow, + isAccountOnServer, + isAccountRowUsable, + isSameServer, listAccountRows, + listAccountRowsByRecency, + listUsableAccountRowsByRecency, + listUsableAccountRowsForOrg, } from "./accounts.js"; +import { getAccountScope, resetAccountScopes } from "./account-scope.js"; +import { hashIdentity } from "./identity-hash.js"; import { getDatabase } from "./index.js"; import { clearAllIssueOrgCache } from "./issue-org-cache.js"; import { clearTrustedHostState } from "./regions.js"; @@ -47,9 +71,88 @@ type AuthRow = AccountRow; const log = logger.withTag("auth"); -/** Read the active account row. Returns `undefined` when none is active. */ +/** `SENTRY_HOST`/`SENTRY_URL`, or `undefined` when unset or malformed. */ +function getConfiguredUrlOrUndefined(): string | undefined { + try { + return getConfiguredSentryUrl(); + } catch (error) { + // Malformed values surface where requests are built; selection ignores them. + log.debug( + "Ignoring invalid SENTRY_HOST/SENTRY_URL for account selection", + error, + ); + return; + } +} + +/** + * The server the user chose explicitly (`SENTRY_HOST`/`SENTRY_URL`, `--url`, + * `.sentryclirc`, a pasted URL), as opposed to the persisted default URL + * (`defaults.url`, applied at startup), which yields to account selection. + */ +function getExplicitServerUrl(): string | undefined { + const configured = getConfiguredUrlOrUndefined(); + return configured && !isPersistedDefaultUrlInEffect() + ? configured + : undefined; +} + +/** The most recently used usable stored account on `url`'s server. */ +function findUsableAccountOnServer(url: string): AuthRow | undefined { + // Same-server matching (origin normalization, `*.sentry.io` as one + // server) stays in TypeScript; SQL already filtered and ordered the rows. + return listUsableAccountRowsByRecency().find((row) => + isAccountOnServer(row, url), + ); +} + +/** + * Prefer an account on the configured server when the active account is + * on another one. Without a configured server, or without a stored + * account for it, the active account stays in effect (and the host trust + * checks report the mismatch exactly as before). + */ +function selectAccountForConfiguredHost( + active: AuthRow | undefined, +): AuthRow | undefined { + const configured = getConfiguredUrlOrUndefined(); + if (!configured) { + return active; + } + // NULL host: pre-v16 row, scoped lazily by migrateNullHost. Keep it. + if (active && (!active.host || isAccountOnServer(active, configured))) { + return active; + } + return findUsableAccountOnServer(configured) ?? active; +} + +/** + * The effective account row for credential reads: the invocation's default + * account, else one on the configured server, else the active one. + * Account management (switch, logout) uses the stored active row directly. + */ function getAuthRow(): AuthRow | undefined { - return getActiveAccountRow(); + const defaultAccountId = getAccountScope().accountId; + if (defaultAccountId) { + const row = getAccountRow(defaultAccountId); + if (row) { + return row; + } + } + return selectAccountForConfiguredHost(getActiveAccountRow()); +} + +/** The stored account whose credentials this invocation uses, if any. */ +export function getEffectiveAccountRow(): AccountRow | undefined { + return getAuthRow(); +} + +/** + * Key for memoized credential state. The effective account depends on the + * invocation's default account and configured server, so the memo must too. + */ +function selectionKey(): string { + return `${getAccountScope().accountId ?? ""}\0${getConfiguredUrlOrUndefined() ?? ""}`; } /** @@ -95,7 +198,7 @@ function migrateNullHost(row: AuthRow): string { * URL at a concrete server instead of keeping a previous account's. */ function backfillActiveNullHost(): void { - const row = getAuthRow(); + const row = getActiveAccountRow(); if (row && !row.host) { migrateNullHost(row); } @@ -320,10 +423,26 @@ export function getUsableStoredTokenHost(): string | undefined { } const authCacheState = { - tokens: new WeakMap(), - fingerprints: new WeakMap(), + tokens: new WeakMap< + NodeJS.ProcessEnv, + Map + >(), + fingerprints: new WeakMap>(), }; +/** The per-env, per-selection memo map in `store`, created on first use. */ +function memoFor( + store: WeakMap>, +): Map { + const env = getEnv(); + let memo = store.get(env); + if (!memo) { + memo = new Map(); + store.set(env, memo); + } + return memo; +} + /** * Get the active auth token. * @@ -331,13 +450,14 @@ const authCacheState = { * With `SENTRY_FORCE_ENV_TOKEN=1`: checks env vars first (old behavior). */ export function getAuthToken(): string | undefined { - const env = getEnv(); - const cached = authCacheState.tokens.get(env); + const memo = memoFor(authCacheState.tokens); + const key = selectionKey(); + const cached = memo.get(key); if (cached !== undefined) { return cached.value; } const value = computeAuthToken(); - authCacheState.tokens.set(env, { value }); + memo.set(key, { value }); return value; } @@ -380,29 +500,42 @@ export function resetAuthTokenCache(): void { authCacheState.tokens = new WeakMap(); } -/** Memoized result for {@link hasStoredAuthCredentials}. */ -let cachedHasStoredCreds: { value: boolean } | undefined; +/** Memoized results for {@link hasStoredAuthCredentials}, by selection. */ +const cachedHasStoredCreds = new Map(); -/** Memoized full auth row for {@link refreshToken}. */ -let cachedAuthRow: { value: AuthRow | undefined } | undefined; +/** Memoized effective auth rows for {@link refreshToken}, by selection. */ +const cachedAuthRows = new Map(); function getCachedAuthRow(): AuthRow | undefined { - if (cachedAuthRow !== undefined) { - return cachedAuthRow.value; + const key = selectionKey(); + const cached = cachedAuthRows.get(key); + if (cached !== undefined) { + return cached.value; } const row = getAuthRow(); - cachedAuthRow = { value: row }; + cachedAuthRows.set(key, { value: row }); return row; } /** Reset the memoized auth row. Tests only — call between auth-state mutations. */ export function resetAuthRowCache(): void { - cachedAuthRow = undefined; + cachedAuthRows.clear(); } /** Reset the memoized stored-credentials flag. Tests only — call between auth-state mutations. */ export function resetHasStoredCredsCache(): void { - cachedHasStoredCreds = undefined; + cachedHasStoredCreds.clear(); +} + +/** + * Forget default accounts chosen by {@link selectAccountForOrg}. Tests only: + * in the CLI the choice lasts for the invocation, and library calls get a + * fresh env. + */ +export function resetAccountSelection(): void { + resetAccountScopes(); + clearPersistedDefaultUrl(); + resetActiveCredentialState(); } /** @@ -417,10 +550,16 @@ export function resetHasStoredCredsCache(): void { * flows set this so earlier logins stay available to `sentry auth switch`. * Recording user info afterwards ({@link setUserInfo}) folds a re-login to * the same user on the same host into one account. + * @property accountId - Re-authorize this stored account in place instead + * of the active one: scope recovery for the account an invocation picked + * for its target org (see {@link getReauthorizationAccountId}). It stays + * in effect for the rest of the invocation; the stored active account is + * unchanged. Must be on `host`'s server. */ export type SetAuthTokenOptions = { host?: string; newAccount?: boolean; + accountId?: string; }; /** Drop all memoized active-credential state after an account mutation. */ @@ -443,13 +582,28 @@ export function setAuthToken( options?: SetAuthTokenOptions, ): string { const normalizedToken = normalizeAuthToken(token); + const reauthorizeId = options?.accountId; + if (!reauthorizeId) { + // A login, not a refresh: it targets the stored active account (or a + // new one), never the account this invocation picked for its target + // org, and the new credentials are what the rest of the invocation uses. + getAccountScope().accountId = undefined; + } const accountId = withDbSpan("setAuthToken", () => { const db = getDatabase(); const now = Date.now(); const expiresAt = expiresIn ? now + expiresIn * 1000 : null; const issuedAt = expiresIn ? now : null; - const existing = options?.newAccount ? undefined : getAuthRow(); + // Replacing credentials in place targets the stored login for the + // login's server — never an account on another server, whatever + // SENTRY_HOST or `.sentryclirc` says. With none there, a new account. + // Re-authorizing a specific account targets exactly that account. + const existing = reauthorizeId + ? findAccountToReauthorize(reauthorizeId, options?.host) + : options?.newAccount + ? undefined + : findAccountToReplace(options?.host); // Host resolution precedence: // 1. Explicit `options.host` (login command, tests) // 2. Existing row's `host` (refresh flow preserves the original scope) @@ -488,6 +642,13 @@ export function setAuthToken( now, existing.account_id, ); + if (existing.is_active !== 1 && !reauthorizeId) { + // Logging in to another server's stored account makes it active, + // like adding an account does; it is a different identity. + backfillActiveNullHost(); + clearUnscopedIdentityCaches(); + activateAccountRow(existing.account_id); + } return existing.account_id; }); // Active credential changed — drop memoized fingerprint, token, row, and @@ -496,6 +657,84 @@ export function setAuthToken( return accountId; } +/** + * The stored account a login to `host` replaces in place (`sentry auth + * login --force`, automatic re-login): the active account when it is on + * that server, else the most recently used account on it. Accounts are + * tied to their server, so a login never overwrites another server's + * account; `undefined` when no stored account is on `host` (the login is + * then stored as a new account). Without a host (credential updates that + * keep the existing scope), the active account. + */ +function findAccountToReplace(host: string | undefined): AuthRow | undefined { + const active = getActiveAccountRow(); + if (!host) { + return active; + } + const loginHost = normalizeCredentialHost(host); + if ( + active && + isSameServer(active.host ?? migrateNullHost(active), loginHost) + ) { + return active; + } + return listAccountRowsByRecency().find((row) => + isAccountOnServer(row, loginHost), + ); +} + +/** + * The stored account {@link setAuthToken} re-authorizes for `accountId`. + * + * @throws {AuthError} when it is gone or on another server than the login, + * so the new credentials are never stored under a different account + */ +function findAccountToReauthorize( + accountId: string, + host: string | undefined, +): AuthRow { + const row = getAccountRow(accountId); + if (row && (!host || isAccountOnServer(row, normalizeCredentialHost(host)))) { + return row; + } + throw new AuthError( + "not_authenticated", + "The account being re-authorized is no longer stored for this server. Nothing was saved.", + { skipAutoAuth: true }, + ); +} + +/** + * The account a 401/403 re-authorization must target: the one this + * invocation picked for its target org ({@link selectAccountForOrg}) when + * it isn't the stored active account. A plain login would replace the + * active account and leave the picked one's old grant in place, so the + * retried command would pick it again and fail the same way. `undefined` + * when the active account (or no stored account) is in effect. + */ +export function getReauthorizationAccountId(): string | undefined { + const accountId = getAccountScope().accountId; + if (!accountId) { + return; + } + try { + const row = getAccountRow(accountId); + return row && row.is_active !== 1 ? accountId : undefined; + } catch (error) { + log.debug("Could not read the invocation's account", error); + return; + } +} + +/** + * Id of the stored account a login to `host` replaces (see + * {@link findAccountToReplace}), or `undefined` when it adds one. + */ +export function getAccountIdToReplace(host: string): string | undefined { + return withDbSpan("getAccountIdToReplace", () => findAccountToReplace(host)) + ?.account_id; +} + /** * Clear caches whose entries are scoped to one identity's permissions but * not keyed by it (pagination cursors, issue-id → org mappings), plus the @@ -535,7 +774,7 @@ export async function clearAuth(): Promise { /** The active stored account's id, or `undefined` when none is active. */ export function getActiveAccountId(): string | undefined { - return getAuthRow()?.account_id; + return getActiveAccountRow()?.account_id; } /** @@ -549,7 +788,7 @@ export function setActiveAccount(accountId: string): void { if (!getAccountRow(accountId)) { throw new ConfigError(`No stored account with id '${accountId}'.`); } - if (getAuthRow()?.account_id === accountId) { + if (getActiveAccountRow()?.account_id === accountId) { return; } backfillActiveNullHost(); @@ -574,19 +813,17 @@ export async function removeAccount( const nextActive = withDbSpan("removeAccount", () => { const row = getAccountRow(accountId); if (!row) { - return getAuthRow()?.account_id; + return getActiveAccountRow()?.account_id; } deleteAccountRows([row]); if (row.is_active !== 1) { - return getAuthRow()?.account_id; + return getActiveAccountRow()?.account_id; } clearUnscopedIdentityCaches(); if (!options.promoteNext) { return; } - const [next] = listAccountRows().sort( - (a, b) => b.last_used_at - a.last_used_at || b.updated_at - a.updated_at, - ); + const next = getMostRecentAccountRow(); if (next) { activateAccountRow(next.account_id); backfillActiveNullHost(); @@ -611,12 +848,7 @@ async function invalidateAccountCredentials(accountId: string): Promise { return; } const db = getDatabase(); - const identity = accountIdentity(row); - if (identity) { - db.query("DELETE FROM org_regions WHERE credential_identity = ?").run( - identity, - ); - } + deleteAccountOrgRegions(row); db.query( "UPDATE accounts SET token = NULL, refresh_token = NULL, expires_at = NULL, issued_at = NULL, updated_at = ? WHERE account_id = ?", ).run(Date.now(), accountId); @@ -626,6 +858,167 @@ async function invalidateAccountCredentials(accountId: string): Promise { await clearResponseCacheBestEffort(); } +/** + * Point the invocation's server at its newly chosen default account, but only when + * no server was chosen explicitly: an unset URL or the persisted default + * (`defaults.url`, applied at startup) yields to the account's own host. + */ +function followDefaultAccountHost(host: string): void { + if (getExplicitServerUrl()) { + return; + } + if (isSameServer(getConfiguredUrlOrUndefined() ?? DEFAULT_SENTRY_URL, host)) { + return; + } + getEnv().SENTRY_URL = host; + markPersistedDefaultUrl(host); +} + +/** + * Use the stored account that knows `orgSlug` (or numeric `orgId`) for the + * rest of this invocation, so commands targeting another login's + * organization work without `sentry auth switch`. + * + * No-op when the effective account knows the org, or is the active account + * and has no cached org list to rule it out, when no stored account knows it, or when an earlier + * org already chose the invocation's default account — one invocation + * never mixes identities. An explicit server (`SENTRY_HOST`/`SENTRY_URL`, + * `--url`, `.sentryclirc`) limits candidates to accounts on it; when several + * accounts qualify, the most recently used wins. Forced env tokens (`SENTRY_FORCE_ENV_TOKEN`) are never overridden. + */ +export function selectAccountForOrg(orgSlug: string, orgId?: string): void { + if (getEnv().SENTRY_FORCE_ENV_TOKEN?.trim() && getRawEnvToken()) { + return; + } + const scope = getAccountScope(); + if (scope.accountId) { + return; + } + try { + // Usable accounts that know the org, most recently used first. + const owners = listUsableAccountRowsForOrg(orgSlug, orgId); + if (owners.length === 0) { + return; + } + const current = getAuthRow(); + // Respect the active account unless its fresh cached org list shows it + // is not a member: after `sentry auth switch`, an account that simply + // hasn't used the org yet must not be overridden by one that has. That + // deference is for the user's choice only: an account the configured + // server picked by recency yields to one on it that knows the org. An + // account that can no longer authenticate (credentials invalidated or + // expired) is never kept over an owner that can. + if ( + current && + (owners.some((o) => o.account_id === current.account_id) || + (current.is_active === 1 && + isAccountRowUsable(current) && + !hasCachedOrgList(current.account_id))) + ) { + return; + } + const explicitHost = getExplicitServerUrl(); + const candidates = explicitHost + ? owners.filter((o) => isAccountOnServer(o, explicitHost)) + : owners; + const pick = candidates[0]; + if (!pick?.host) { + return; + } + scope.accountId = pick.account_id; + followDefaultAccountHost(pick.host); + log.debug( + `Using stored account ${pick.account_id} (${pick.host}) for organization '${orgSlug}'`, + ); + } catch (error) { + // Best-effort: the active account stays in effect. + log.debug("Could not select an account for the organization", error); + } +} + +/** + * The stored account whose credentials hash to `identity` (following + * in-process refresh-token rotations), so cached org data can record which + * account it belongs to. `undefined` for env tokens and unknown identities. + */ +export function getAccountIdForIdentity(identity: string): string | undefined { + try { + const match = listAccountRows().find((row) => { + const rowIdentity = accountIdentity(row); + if (!rowIdentity) { + return false; + } + if (rowIdentity === identity) { + return true; + } + const host = row.host ? normalizeHttpOrigin(row.host) : undefined; + return ( + host !== undefined && + matchesRefreshIdentity(host, identity, rowIdentity) + ); + }); + return match?.account_id; + } catch (error) { + // The mapping is an optimization; a miss just skips recording it. + log.debug("Could not map credential identity to an account", error); + return; + } +} + +/** + * Whether a usable stored account is on `url`'s server. Lets a pasted + * Sentry URL for another login's server select that login instead of + * failing the host check against the active account. + */ +/** + * Whether a usable stored account is on `url`'s server. Lets a pasted + * Sentry URL for another login's server select that login instead of + * failing the host check against the active account. + */ +export function hasStoredAccountForHost(url: string): boolean { + try { + return findUsableAccountOnServer(url) !== undefined; + } catch (error) { + log.debug("Could not read stored accounts", error); + return false; + } +} + +/** + * Whether a pasted SaaS URL must pin the server to SaaS. With a SaaS account + * stored (logged in or not), or accounts on several servers, org-based + * account selection could otherwise route the URL to a self-hosted account + * that shares the org slug or id. A single non-SaaS login keeps the + * single-account behavior: its own server serves the request. + */ +export function shouldPinSaaSForPastedUrl(): boolean { + try { + const rows = listAccountRows(); + return ( + rows.length > 1 || + rows.some((row) => isAccountOnServer(row, DEFAULT_SENTRY_URL)) + ); + } catch (error) { + log.debug("Could not read stored accounts", error); + return false; + } +} + +/** + * Whether any stored account can authenticate. The command auth guard + * defers to it when the account in effect cannot: the command's target (an + * organization, a pasted URL) may still select that stored account, and a + * request without one fails with `AuthError` when it is made. + */ +export function hasUsableStoredAccount(): boolean { + try { + return listUsableAccountRowsByRecency().length > 0; + } catch (error) { + log.debug("Could not read stored accounts", error); + return false; + } +} + export function isAuthenticated(): boolean { const token = getAuthToken(); return !!token; @@ -650,13 +1043,14 @@ export const ANON_IDENTITY = ""; * {@link resetIdentityFingerprintCache}. */ export function getIdentityFingerprint(): string { - const env = getEnv(); - const cached = authCacheState.fingerprints.get(env); + const memo = memoFor(authCacheState.fingerprints); + const key = selectionKey(); + const cached = memo.get(key); if (cached !== undefined) { return cached; } const fingerprint = computeIdentityFingerprint(); - authCacheState.fingerprints.set(env, fingerprint); + memo.set(key, fingerprint); return fingerprint; } @@ -769,8 +1163,10 @@ export function getActiveAuthHost(): string | undefined { * when an env token is present. */ export function hasStoredAuthCredentials(): boolean { - if (cachedHasStoredCreds !== undefined) { - return cachedHasStoredCreds.value; + const key = selectionKey(); + const cached = cachedHasStoredCreds.get(key); + if (cached !== undefined) { + return cached; } const row = getAuthRow(); let result = false; @@ -783,7 +1179,7 @@ export function hasStoredAuthCredentials(): boolean { result = !!row.refresh_token; } } - cachedHasStoredCreds = { value: result }; + cachedHasStoredCreds.set(key, result); return result; } diff --git a/packages/cli/src/lib/db/identity-hash.ts b/packages/cli/src/lib/db/identity-hash.ts new file mode 100644 index 000000000..ce736a8e6 --- /dev/null +++ b/packages/cli/src/lib/db/identity-hash.ts @@ -0,0 +1,33 @@ +/** + * Credential identity hashing, shared by the account store and schema + * migrations (which cannot import the DB-backed modules). + */ + +import { createHash } from "node:crypto"; + +/** + * Stable SHA-256 namespace for high-entropy OAuth and organization tokens. + * A collision would mix credentials' cached responses, so retain the full + * digest. This fingerprint is never a password verifier or a bearer token. + */ +export function hashIdentity(kind: string, secret: string): string { + return createHash("sha256") + .update(kind) + .update("\0") + .update(secret) + .digest("hex"); +} + +/** Cache namespace for stored OAuth credentials (matches `getIdentityFingerprint`). */ +export function storedCredentialIdentity(row: { + token: string | null; + refresh_token: string | null; +}): string | undefined { + if (row.refresh_token) { + return hashIdentity("oauth", row.refresh_token); + } + if (row.token) { + return hashIdentity("oauth-access", row.token); + } + return; +} diff --git a/packages/cli/src/lib/db/regions.ts b/packages/cli/src/lib/db/regions.ts index ec45876fe..4fa60b2ab 100644 --- a/packages/cli/src/lib/db/regions.ts +++ b/packages/cli/src/lib/db/regions.ts @@ -14,7 +14,11 @@ import { DEFAULT_SENTRY_URL, getConfiguredSentryUrl } from "../constants.js"; import { logger } from "../logger.js"; import { normalizeHttpOrigin } from "../sentry-urls.js"; import { recordCacheHit } from "../telemetry.js"; -import { getCredentialContext, getIdentityFingerprint } from "./auth.js"; +import { + getAccountIdForIdentity, + getCredentialContext, + getIdentityFingerprint, +} from "./auth.js"; import { getDatabase } from "./index.js"; import { runUpsert } from "./utils.js"; @@ -220,6 +224,7 @@ type OrgRegionRow = { org_role: string | null; region_url: string; credential_identity: string; + account_id: string | null; source_origin: string; response_origin: string; updated_at: number; @@ -326,6 +331,7 @@ export function setOrgRegion( org_slug: orgSlug, region_url: region, credential_identity: identity, + account_id: getAccountIdForIdentity(identity) ?? null, source_origin: source, response_origin: response, updated_at: now, @@ -369,6 +375,14 @@ export function setOrgRegions(entries: OrgRegionEntry[]): void { }); const db = getDatabase(); const now = Date.now(); + // Record which stored account saw each org, so later commands targeting + // it can pick that account automatically (see `selectAccountForOrg`). + const accountIds = new Map(); + for (const { identity } of normalized) { + if (!accountIds.has(identity)) { + accountIds.set(identity, getAccountIdForIdentity(identity) ?? null); + } + } db.transaction(() => { for (const item of normalized) { @@ -377,6 +391,7 @@ export function setOrgRegions(entries: OrgRegionEntry[]): void { org_slug: entry.slug, region_url: item.region, credential_identity: item.identity, + account_id: accountIds.get(item.identity) ?? null, source_origin: item.source, response_origin: item.response, updated_at: now, @@ -465,7 +480,7 @@ export type CachedOrg = { * fresh API fetch. 7 days balances offline usability with picking up * new org memberships. */ -const ORG_CACHE_TTL_MS = 7 * 24 * 60 * 60 * 1000; +export const ORG_CACHE_TTL_MS = 7 * 24 * 60 * 60 * 1000; /** * Get all cached organizations with id, slug, and name. diff --git a/packages/cli/src/lib/db/schema.ts b/packages/cli/src/lib/db/schema.ts index d631d9f8d..7ed756c5f 100644 --- a/packages/cli/src/lib/db/schema.ts +++ b/packages/cli/src/lib/db/schema.ts @@ -16,11 +16,12 @@ import { uuidv7 } from "uuidv7"; import { getEnv } from "../env.js"; import { stringifyUnknown } from "../errors.js"; import { logger } from "../logger.js"; +import { storedCredentialIdentity } from "./identity-hash.js"; import type { Database } from "./sqlite.js"; const _require = createRequire(import.meta.url); -export const CURRENT_SCHEMA_VERSION = 18; +export const CURRENT_SCHEMA_VERSION = 19; /** Environment variable to disable auto-repair */ const NO_AUTO_REPAIR_ENV = "SENTRY_CLI_NO_AUTO_REPAIR"; @@ -197,6 +198,7 @@ export const TABLE_SCHEMAS: Record = { org_role: { type: "TEXT", addedInVersion: 10 }, region_url: { type: "TEXT", notNull: true }, credential_identity: { type: "TEXT", notNull: true, addedInVersion: 17 }, + account_id: { type: "TEXT", addedInVersion: 19 }, source_origin: { type: "TEXT", notNull: true, addedInVersion: 17 }, response_origin: { type: "TEXT", notNull: true, addedInVersion: 17 }, updated_at: { @@ -771,6 +773,29 @@ type LegacyUserRow = { * A NULL `host` (rows from before v16) is carried over as-is; the auth * layer backfills it lazily from the boot-time URL snapshot. */ +/** Tag cached org rows with the stored account whose credentials wrote them. */ +function backfillOrgRegionAccounts(db: Database): void { + if (!tableExists(db, "accounts")) { + return; + } + const accounts = db + .query("SELECT account_id, token, refresh_token FROM accounts") + .all() as { + account_id: string; + token: string | null; + refresh_token: string | null; + }[]; + const update = db.query( + "UPDATE org_regions SET account_id = ? WHERE credential_identity = ? AND account_id IS NULL", + ); + for (const account of accounts) { + const identity = storedCredentialIdentity(account); + if (identity) { + update.run(account.account_id, identity); + } + } +} + function migrateSingleAuthToAccounts(db: Database): void { db.exec(EXPECTED_TABLES.accounts as string); @@ -978,6 +1003,15 @@ export function runMigrations(db: Database): void { db.transaction(() => migrateSingleAuthToAccounts(db))(); } + // Migration 18 -> 19: Record which account each cached org belongs to, so + // commands targeting an org can use that account without a manual switch. + if (currentVersion < 19) { + db.transaction(() => { + addColumnIfMissing(db, "org_regions", "account_id", "TEXT"); + backfillOrgRegionAccounts(db); + })(); + } + if (currentVersion < CURRENT_SCHEMA_VERSION) { db.query("UPDATE schema_version SET version = ?").run( CURRENT_SCHEMA_VERSION, diff --git a/packages/cli/src/lib/db/user.ts b/packages/cli/src/lib/db/user.ts index dad38ddc1..657a90720 100644 --- a/packages/cli/src/lib/db/user.ts +++ b/packages/cli/src/lib/db/user.ts @@ -1,17 +1,18 @@ /** - * User identity of the active stored account. + * User identity of the stored account in effect. * - * Stores user info fetched from Sentry API on the active `accounts` row, - * used for display and to set Sentry user context. + * Stores user info fetched from Sentry API on the `accounts` row whose + * credentials the invocation uses (usually the active one; see + * `getEffectiveAccountRow`), for display and to set Sentry user context. */ import { withDbSpan } from "../telemetry.js"; import { deleteAccountRows, - getActiveAccountRow, isAccountOnServer, listAccountRows, } from "./accounts.js"; +import { getEffectiveAccountRow } from "./auth.js"; import { getDatabase } from "./index.js"; export type UserInfo = { @@ -23,11 +24,11 @@ export type UserInfo = { }; /** - * Get the active account's user info. - * Returns undefined when no account is active or its user is not known yet. + * Get the user info of the account in effect. + * Returns undefined when no stored account is in use or its user is not known yet. */ export function getUserInfo(): UserInfo | undefined { - const row = getActiveAccountRow(); + const row = getEffectiveAccountRow(); if (!row?.user_id) { return; } @@ -41,13 +42,14 @@ export function getUserInfo(): UserInfo | undefined { } /** - * Record who the active account's credentials belong to. + * Record who the in-effect account's credentials belong to. * * Any other stored account for the same user on the same server is * removed: logging in again to an existing account replaces its old * credentials instead of adding a duplicate. "Same server" matches `--url` * selection, so every `*.sentry.io` region counts as SaaS. No-op when no - * account is active (for example, when only an env-var token is in use). + * stored account is in use (for example, when only an env-var token is in + * use). * * Leaves `updated_at` alone: it versions the stored credential for refresh * compare-and-swap, and identity writes must not invalidate an in-flight @@ -55,7 +57,7 @@ export function getUserInfo(): UserInfo | undefined { */ export function setUserInfo(info: UserInfo): void { withDbSpan("setUserInfo", () => { - const active = getActiveAccountRow(); + const active = getEffectiveAccountRow(); if (!active) { return; } @@ -87,14 +89,14 @@ export function setUserInfo(info: UserInfo): void { } /** - * Forget who the active account's credentials belong to. Used after its + * Forget who the in-effect account's credentials belong to. Used after its * credentials were replaced in place without user info for the new ones, * so list/switch/status never show the previous user against a new token. * Leaves `updated_at` alone (see {@link setUserInfo}). */ export function clearUserInfo(): void { withDbSpan("clearUserInfo", () => { - const active = getActiveAccountRow(); + const active = getEffectiveAccountRow(); if (!active) { return; } diff --git a/packages/cli/src/lib/interactive-login.ts b/packages/cli/src/lib/interactive-login.ts index fe9d17d70..cc5d0abb8 100644 --- a/packages/cli/src/lib/interactive-login.ts +++ b/packages/cli/src/lib/interactive-login.ts @@ -9,6 +9,7 @@ import * as Sentry from "@sentry/node-core/light"; import { openBrowser } from "./browser.js"; import { setupCopyKeyListener } from "./clipboard.js"; +import { getAccountRow } from "./db/accounts.js"; import { getDbPath } from "./db/index.js"; import { clearUserInfo, setUserInfo } from "./db/user.js"; import { formatError, ValidationError } from "./errors.js"; @@ -70,6 +71,13 @@ export type InteractiveLoginOptions = { * the check happens here: a mismatching token is discarded, not stored. */ expectedEmail?: string; + /** + * Re-authorize this stored account in place instead of the active one + * (scope recovery for the account an invocation picked for its target + * org). The login must be approved as that account's user; a different + * user's token is discarded, not stored. + */ + accountId?: string; }; /** @@ -100,6 +108,36 @@ export function assertExpectedLoginEmail( } } +/** + * Refuse re-authorizing a stored account as a different user: its token + * would be stored under that account, and recording the user would then + * fold away the other user's own account. + * + * @throws {ValidationError} when the approving user isn't the account's + * user, or either identity is unknown + */ +export function assertReauthorizingSameUser( + accountId: string | undefined, + user: { id: string; email?: string | null } | undefined, +): void { + if (!accountId) { + return; + } + const account = getAccountRow(accountId); + if (account?.user_id && user?.id === account.user_id) { + return; + } + const expected = account?.email ?? account?.username ?? "the account in use"; + const actual = user?.email + ? `Signed in as ${user.email}, not ${expected}` + : `Could not confirm the login is for ${expected}`; + throw new ValidationError( + `${actual}. Nothing was saved.\n\n` + + `Sentry authorizes whichever user is signed in to the browser. Sign in as ${expected} ` + + "there (or use a private window) and run the command again.", + ); +} + /** * Build the user-facing display lines for the device flow verification step. * @@ -232,10 +270,12 @@ export async function runInteractiveLogin( const user = tokenResponse.user; assertExpectedLoginEmail(expectedEmail, user?.email); + assertReauthorizingSameUser(options?.accountId, user); // Store the token await completeOAuthFlow(tokenResponse, { newAccount: options?.newAccount, + accountId: options?.accountId, }); // Store user info from token response for telemetry and display diff --git a/packages/cli/src/lib/oauth.ts b/packages/cli/src/lib/oauth.ts index db9070cec..eb19580b4 100644 --- a/packages/cli/src/lib/oauth.ts +++ b/packages/cli/src/lib/oauth.ts @@ -491,6 +491,11 @@ export type StoreTokenOptions = { * automatic login) expects. */ newAccount?: boolean; + /** + * Re-authorize this stored account in place instead of the active one + * (scope recovery for an account picked for the target org). + */ + accountId?: string; }; /** @@ -509,7 +514,11 @@ export async function completeOAuthFlow( tokenResponse.access_token, tokenResponse.expires_in, tokenResponse.refresh_token, - { host: getSentryUrl(), newAccount: options.newAccount }, + { + host: getSentryUrl(), + newAccount: options.newAccount, + accountId: options.accountId, + }, ); } diff --git a/packages/cli/src/lib/region.ts b/packages/cli/src/lib/region.ts index da4ad3d65..accceac2c 100644 --- a/packages/cli/src/lib/region.ts +++ b/packages/cli/src/lib/region.ts @@ -7,7 +7,11 @@ import { getOrganization } from "@sentry/api"; import { resolveCachedRegion } from "@sentry/toolkit-core/region-cache"; -import { type CredentialContext, getCredentialContext } from "./db/auth.js"; +import { + type CredentialContext, + getCredentialContext, + selectAccountForOrg, +} from "./db/auth.js"; import { getOrgByNumericId, getOrgRegion, setOrgRegion } from "./db/regions.js"; import { stripDsnOrgPrefix } from "./dsn/index.js"; import { AuthError, withAuthGuard } from "./errors.js"; @@ -56,6 +60,7 @@ const regionCache = new Map>(); * @returns The region URL for the organization */ export function resolveOrgRegion(orgSlug: string): Promise { + useAccountForOrg(orgSlug); const credential = getCredentialContext(); if (!credential) { return Promise.reject(new AuthError("not_authenticated")); @@ -70,6 +75,23 @@ export function resolveOrgRegion(orgSlug: string): Promise { ).then((resolution) => resolution.url); } +/** Sentry org slugs are never entirely numeric, so digits are an org id. */ +const NUMERIC_ORG_ID = /^\d+$/; + +/** + * Switch this invocation to the stored account that knows the org, if the + * current one doesn't. Accepts DSN-style `oNNNN` identifiers and the bare + * numeric ids DSN resolution can yield too. + */ +function useAccountForOrg(orgSlug: string): void { + const numericId = stripDsnOrgPrefix(orgSlug); + const orgId = + numericId !== orgSlug || NUMERIC_ORG_ID.test(orgSlug) + ? numericId + : undefined; + selectAccountForOrg(orgSlug, orgId); +} + /** * Coerce a regionUrl from the API into an absolute URL. * @@ -246,6 +268,7 @@ function resolveOrgFromCache(orgSlug: string): string | undefined { * @returns The org slug to use for API calls (may be normalized) */ export async function resolveEffectiveOrg(orgSlug: string): Promise { + useAccountForOrg(orgSlug); // First attempt: use local cache const fromCache = resolveOrgFromCache(orgSlug); if (fromCache) { diff --git a/packages/cli/src/lib/scope-recovery.ts b/packages/cli/src/lib/scope-recovery.ts index 18d9614be..9717615fc 100644 --- a/packages/cli/src/lib/scope-recovery.ts +++ b/packages/cli/src/lib/scope-recovery.ts @@ -1,14 +1,23 @@ import { isatty } from "node:tty"; import { getCurrentAuthScopes } from "./api/auth.js"; import { assertAutoLoginHostTrusted } from "./auto-auth.js"; -import { type AuthSource, getAuthConfig } from "./db/auth.js"; +import { + type AuthSource, + getAuthConfig, + getReauthorizationAccountId, +} from "./db/auth.js"; import { ApiError, AuthError } from "./errors.js"; -import type { LoginResult } from "./interactive-login.js"; +import type { + InteractiveLoginOptions, + LoginResult, +} from "./interactive-login.js"; import { interactivePromptsAllowed } from "./interactive-prompts.js"; import { logger } from "./logger.js"; import { OAUTH_SCOPES } from "./oauth.js"; -type InteractiveLogin = () => Promise; +type InteractiveLogin = ( + options?: InteractiveLoginOptions, +) => Promise; type OAuthScopeState = | { kind: "current" } @@ -22,6 +31,12 @@ export type ScopeRecoveryRuntime = { inputIsTty: () => boolean; promptsAllowed: () => boolean; write: (message: string) => void; + /** + * The stored account to re-authorize when it isn't the active one (the + * account the invocation picked for its target org). Optional so test + * runtimes default to re-authorizing the active account. + */ + getReauthorizationAccountId?: () => string | undefined; }; const defaultRuntime: ScopeRecoveryRuntime = { @@ -31,8 +46,23 @@ const defaultRuntime: ScopeRecoveryRuntime = { inputIsTty: () => isatty(0), promptsAllowed: interactivePromptsAllowed, write: (message) => process.stderr.write(message), + getReauthorizationAccountId, }; +/** + * Run the login that re-authorizes the account in effect. When that is an + * account the invocation picked for its target org, the new grant must + * replace that account's credentials: a plain login replaces the active + * account, so the retried command would pick the unchanged account again. + */ +function reauthorize( + runInteractiveLogin: InteractiveLogin, + runtime: ScopeRecoveryRuntime, +): Promise { + const accountId = runtime.getReauthorizationAccountId?.(); + return accountId ? runInteractiveLogin({ accountId }) : runInteractiveLogin(); +} + async function inspectOAuthScopes( runtime: ScopeRecoveryRuntime, startedWithOAuth = false, @@ -133,7 +163,7 @@ async function refreshOAuthScopes( "Your CLI authorization is no longer valid. Starting authorization...\n\n", ); } - return Boolean(await runInteractiveLogin()); + return Boolean(await reauthorize(runInteractiveLogin, runtime)); } /** Refresh a stored OAuth grant when it lacks any scope requested by this CLI. */ @@ -206,7 +236,9 @@ export async function ensureScopesForAction( runtime.write( `${action} needs elevated permissions (${missing.join(", ")}). Starting authorization...\n\n`, ); - return (await runInteractiveLogin()) ? "elevated" : "declined"; + return (await reauthorize(runInteractiveLogin, runtime)) + ? "elevated" + : "declined"; } /** Check an OAuth token after a 401/403, re-authorize if needed, and retry once. */ diff --git a/packages/cli/src/lib/sentry-url-parser.ts b/packages/cli/src/lib/sentry-url-parser.ts index 8081e9582..f25500555 100644 --- a/packages/cli/src/lib/sentry-url-parser.ts +++ b/packages/cli/src/lib/sentry-url-parser.ts @@ -9,7 +9,15 @@ */ import { isSaaSTrustOrigin } from "@sentry/toolkit-core/sentry-origin"; -import { DEFAULT_SENTRY_HOST } from "./constants.js"; +import { + clearPersistedDefaultUrl, + DEFAULT_SENTRY_HOST, + DEFAULT_SENTRY_URL, +} from "./constants.js"; +import { + hasStoredAccountForHost, + shouldPinSaaSForPastedUrl, +} from "./db/auth.js"; import { getEnv } from "./env.js"; import { HostScopeError } from "./errors.js"; import { tryNormalizeHexId } from "./hex-id.js"; @@ -421,11 +429,32 @@ export function applySentryUrlContext(baseUrl: string): void { delete env.SENTRY_HOST; // env registry requires delete to truly unset; assignment coerces to string in Node.js delete env.SENTRY_URL; + // Pin the server to SaaS explicitly when other stored accounts could + // claim the URL (see `shouldPinSaaSForPastedUrl`): a usable SaaS account + // is used even when the active one is self-hosted (db/auth `getAuthRow`), + // and org-based selection only considers SaaS accounts, so the URL can't + // route to a self-hosted account sharing the org slug or id — even when + // the SaaS login is logged out (the request then fails to authenticate). + // Checks stored accounts only: reading the active token host would + // snapshot an env token's host (as SaaS, now that the URL is cleared). + if (shouldPinSaaSForPastedUrl()) { + env.SENTRY_URL = DEFAULT_SENTRY_URL; + // The pin is the user's choice even when `defaults.url` is already + // SaaS: otherwise it would still read as the persisted default, which + // org-based selection lets yield to a self-hosted account. + clearPersistedDefaultUrl(); + } return; } + // A stored account on this server is used for it (see db/auth `getAuthRow`). const tokenHost = getActiveTokenHost(); - if (!(tokenHost && isHostTrusted(baseUrl, tokenHost))) { + if ( + !( + (tokenHost && isHostTrusted(baseUrl, tokenHost)) || + hasStoredAccountForHost(baseUrl) + ) + ) { throw new HostScopeError("URL argument", baseUrl, tokenHost); } diff --git a/packages/cli/src/lib/sentryclirc.ts b/packages/cli/src/lib/sentryclirc.ts index 2a8567bac..c10550b58 100644 --- a/packages/cli/src/lib/sentryclirc.ts +++ b/packages/cli/src/lib/sentryclirc.ts @@ -21,6 +21,7 @@ import { homedir } from "node:os"; import { join } from "node:path"; import { isSaaSTrustOrigin } from "@sentry/toolkit-core/sentry-origin"; import { normalizeUrl } from "./constants.js"; +import { hasStoredAccountForHost } from "./db/auth.js"; import { getConfigDir } from "./db/index.js"; import { getEnv } from "./env.js"; import { HostScopeError } from "./errors.js"; @@ -386,8 +387,8 @@ export async function applySentryCliRcEnvShim(cwd: string): Promise { * command — commands that establish or tear down trust (`auth login`, * `auth logout`) opt out via `skipRcUrlCheck: true`. * - * No-op when: no rc URL was loaded, the rc URL is SaaS, or the rc URL - * matches the active token's host. + * No-op when: no rc URL was loaded, the rc URL is SaaS, the rc URL + * matches the active token's host, or a usable stored account is on it. * * @throws {HostScopeError} On non-SaaS URL that doesn't match the token */ @@ -398,10 +399,16 @@ export async function assertRcUrlTrusted(cwd: string): Promise { if (!normalizedRcUrl || isSaaSTrustOrigin(normalizedRcUrl)) { return; } - // Non-SaaS rc URL: must match active token's host. No token at all is - // also a refusal — the rc URL by itself isn't a trust source. + // Non-SaaS rc URL: must match active token's host, or a usable stored + // account must be on it (logging in with `--url` established that trust; + // account selection then uses that account for the server, like a pasted + // URL). No token at all is also a refusal — the rc URL by itself isn't a + // trust source. const tokenHost = getActiveTokenHost(); - if (tokenHost && isHostTrusted(normalizedRcUrl, tokenHost)) { + if ( + (tokenHost && isHostTrusted(normalizedRcUrl, tokenHost)) || + hasStoredAccountForHost(normalizedRcUrl) + ) { return; } const source = config.sources.url diff --git a/packages/cli/test/commands/auth/login-force.test.ts b/packages/cli/test/commands/auth/login-force.test.ts index c1cff1753..4c00800ec 100644 --- a/packages/cli/test/commands/auth/login-force.test.ts +++ b/packages/cli/test/commands/auth/login-force.test.ts @@ -1,6 +1,7 @@ /** * `sentry auth login --force --token` against a real test database: the - * active account is replaced only once the new token checks out. + * stored login for the token's server is replaced only once the new token + * checks out, and accounts on other servers are never touched. */ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; @@ -48,6 +49,7 @@ useEnvSandbox([ ]); const SAAS = "https://sentry.io"; +const ACME = "https://sentry.acme.com"; function createContext() { return { @@ -127,6 +129,57 @@ describe("login --force --token", () => { ]); }); + test("--url for another server keeps the active account and adds one there", async () => { + const saas = addAccount("saas-token", "1", "jane@example.com"); + vi.spyOn(apiClient, "getUserRegions").mockResolvedValue([]); + vi.spyOn(apiClient, "getCurrentUser").mockResolvedValue({ + id: "7", + email: "jane@acme.com", + }); + + await func.call(createContext(), { + token: "acme-token", + force: true, + timeout: 900, + url: ACME, + }); + + expect(listAccounts().map((a) => [a.id === saas, a.email, a.host])).toEqual( + [ + [true, "jane@example.com", SAAS], + [false, "jane@acme.com", ACME], + ], + ); + }); + + test("--url replaces that server's stored login, not the active account", async () => { + const acme = setAuthToken("old-acme-token", undefined, undefined, { + host: ACME, + newAccount: true, + }); + setUserInfo({ userId: "7", email: "jane@acme.com" }); + const saas = addAccount("saas-token", "1", "jane@example.com"); + vi.spyOn(apiClient, "getUserRegions").mockResolvedValue([]); + vi.spyOn(apiClient, "getCurrentUser").mockResolvedValue({ + id: "8", + email: "bob@acme.com", + }); + + await func.call(createContext(), { + token: "new-acme-token", + force: true, + timeout: 900, + url: ACME, + }); + + const accounts = listAccounts(); + expect(accounts.map((a) => [a.id === saas, a.email, a.host])).toEqual([ + [true, "jane@example.com", SAAS], + [false, "bob@acme.com", ACME], + ]); + expect(accounts.some((a) => a.id === acme)).toBe(false); + }); + test("a transient validation failure keeps the previous account active", async () => { const active = addAccount("old-token", "2", "jane@example.com"); const cause = new ApiError("Server error", 503); diff --git a/packages/cli/test/helpers.ts b/packages/cli/test/helpers.ts index 195f91e31..66fee55d2 100644 --- a/packages/cli/test/helpers.ts +++ b/packages/cli/test/helpers.ts @@ -9,6 +9,7 @@ import { mkdtemp, rm } from "node:fs/promises"; import { join } from "node:path"; import { afterEach, beforeEach } from "vitest"; import { + resetAccountSelection, resetAuthRowCache, resetAuthTokenCache, resetHasStoredCredsCache, @@ -118,6 +119,7 @@ export function useTestConfigDir( resetHasStoredCredsCache(); resetIdentityFingerprintCache(); resetTipsCache(); + resetAccountSelection(); dir = await createTestConfigDir(prefix, options); process.env[CONFIG_DIR_ENV_VAR] = dir; }); @@ -129,6 +131,7 @@ export function useTestConfigDir( resetHasStoredCredsCache(); resetIdentityFingerprintCache(); resetTipsCache(); + resetAccountSelection(); // Always restore the previous value — never delete. // Deleting process.env.SENTRY_CONFIG_DIR causes failures in test files // that load after this afterEach runs, because their module-level code diff --git a/packages/cli/test/lib/command-auth-guard.test.ts b/packages/cli/test/lib/command-auth-guard.test.ts new file mode 100644 index 000000000..d89d8f3ee --- /dev/null +++ b/packages/cli/test/lib/command-auth-guard.test.ts @@ -0,0 +1,95 @@ +/** + * The command auth guard with several stored accounts: a logged-out active + * account must not stop a command whose target selects another stored + * account that can still authenticate. + */ + +import { buildApplication, type CommandContext, run } from "@stricli/core"; +import { describe, expect, test } from "vitest"; +import { buildCommand } from "../../src/lib/command.js"; +import { + getAuthToken, + selectAccountForOrg, + setAuthToken, +} from "../../src/lib/db/auth.js"; +import { getDatabase } from "../../src/lib/db/index.js"; +import { setOrgRegion } from "../../src/lib/db/regions.js"; +import { setUserInfo } from "../../src/lib/db/user.js"; +import { buildRouteMap } from "../../src/lib/route-map.js"; +import { useEnvSandbox, useTestConfigDir } from "../helpers.js"; + +useTestConfigDir("command-auth-guard-"); +useEnvSandbox([ + "SENTRY_AUTH_TOKEN", + "SENTRY_TOKEN", + "SENTRY_FORCE_ENV_TOKEN", + "SENTRY_HOST", + "SENTRY_URL", +]); + +const SAAS = "https://sentry.io"; +const ACME = "https://sentry.acme.com"; + +type TestContext = CommandContext & { + process: { stdout: { write: (s: string) => boolean } }; + stdout: { write: (s: string) => boolean }; +}; + +function login(token: string, host: string, userId: string): string { + const id = setAuthToken(token, undefined, undefined, { + host, + newAccount: true, + }); + setUserInfo({ userId, email: `${userId}@example.com` }); + return id; +} + +function logOut(accountId: string): void { + getDatabase() + .query("UPDATE accounts SET token = NULL WHERE account_id = ?") + .run(accountId); +} + +/** Run an auth-required command that targets `org`; returns the token it used. */ +async function runTargeting(org: string): Promise { + const seen: (string | undefined)[] = []; + const command = buildCommand, [], TestContext>({ + docs: { brief: "Targets an org" }, + parameters: { flags: {} }, + // oxlint-disable-next-line require-yield -- test command only observes auth state + async *func() { + selectAccountForOrg(org); + seen.push(getAuthToken()); + }, + }); + const app = buildApplication( + buildRouteMap({ routes: { cmd: command }, docs: { brief: "Test" } }), + { name: "test" }, + ); + const write = () => true; + const stdout = { write }; + await run(app, ["cmd"], { + process: { stdout, stderr: { write } }, + stdout, + } as unknown as TestContext); + return seen.length > 0 ? (seen[0] ?? "") : undefined; +} + +describe("command auth guard", () => { + test("a logged-out active account still lets the target select a usable stored account", async () => { + login("acme-token", ACME, "acme"); + setOrgRegion("acme-internal", ACME, ACME); + const saas = login("saas-token", SAAS, "saas"); + logOut(saas); + + expect(await runTargeting("acme-internal")).toBe("acme-token"); + }); + + test("still stops the command when no stored account can authenticate", async () => { + const acme = login("acme-token", ACME, "acme"); + setOrgRegion("acme-internal", ACME, ACME); + logOut(acme); + + expect(await runTargeting("acme-internal")).toBeUndefined(); + }); +}); diff --git a/packages/cli/test/lib/db/account-routing.test.ts b/packages/cli/test/lib/db/account-routing.test.ts new file mode 100644 index 000000000..411d1b095 --- /dev/null +++ b/packages/cli/test/lib/db/account-routing.test.ts @@ -0,0 +1,761 @@ +/** + * Automatic account selection tests. + * + * With several stored logins, commands use the account that matches what + * they target — the configured server or the organization — without + * `sentry auth switch`, and never change the stored active account. + */ + +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, test, vi } from "vitest"; +import { markPersistedDefaultUrl } from "../../../src/lib/constants.js"; +import { + getActiveAccountId, + getAuthToken, + getEffectiveAccountRow, + getReauthorizationAccountId, + refreshToken, + resetAccountSelection, + selectAccountForOrg, + setActiveAccount, + setAuthToken, +} from "../../../src/lib/db/auth.js"; +import { closeDatabase, getDatabase } from "../../../src/lib/db/index.js"; +import { getEnvTokenHost } from "../../../src/lib/env-token-host.js"; +import { AuthError } from "../../../src/lib/errors.js"; +import { assertReauthorizingSameUser } from "../../../src/lib/interactive-login.js"; +import { + ORG_CACHE_TTL_MS, + setOrgRegion, + setOrgRegions, +} from "../../../src/lib/db/regions.js"; +import { CURRENT_SCHEMA_VERSION } from "../../../src/lib/db/schema.js"; +import { getUserInfo, setUserInfo } from "../../../src/lib/db/user.js"; +import { resolveOrgRegion } from "../../../src/lib/region.js"; +import { applySentryUrlContext } from "../../../src/lib/sentry-url-parser.js"; +import { + applySentryCliRcEnvShim, + assertRcUrlTrusted, + clearSentryCliRcCache, +} from "../../../src/lib/sentryclirc.js"; +import { useEnvSandbox, useTestConfigDir } from "../../helpers.js"; + +useTestConfigDir("account-routing-"); +useEnvSandbox([ + "SENTRY_AUTH_TOKEN", + "SENTRY_TOKEN", + "SENTRY_FORCE_ENV_TOKEN", + "SENTRY_HOST", + "SENTRY_URL", +]); + +const SAAS = "https://sentry.io"; +const ACME = "https://sentry.acme.com"; + +function login(token: string, host: string, userId: string): string { + const id = setAuthToken(token, undefined, undefined, { + host, + newAccount: true, + }); + setUserInfo({ userId, email: `${userId}@example.com` }); + return id; +} + +/** Cache `org` (looked up on `host`) as seen by the account in effect. */ +function seeOrg(org: string, host: string, region = host): void { + setOrgRegion(org, region, host); +} + +/** An account's stored access token; `undefined` once the account is gone. */ +function tokenOf(accountId: string): string | null | undefined { + const row = getDatabase() + .query("SELECT token FROM accounts WHERE account_id = ?") + .get(accountId) as { token: string | null } | null; + return row?.token; +} + +let nextOrgId = 1000; + +/** Cache a full org list for the account in effect, like `org list`. */ +function seeOrgList(host: string, orgs: string[]): void { + setOrgRegions( + orgs.map((slug) => { + nextOrgId += 1; + return { + slug, + regionUrl: host, + sourceOrigin: host, + orgId: String(nextOrgId), + orgName: slug, + }; + }), + ); +} + +describe("server-based selection", () => { + test("uses the stored account on the configured server", () => { + login("acme-token", ACME, "acme-user"); + const saas = login("saas-token", SAAS, "saas-user"); + + process.env.SENTRY_URL = ACME; + + expect(getAuthToken()).toBe("acme-token"); + expect(getUserInfo()?.userId).toBe("acme-user"); + expect(getActiveAccountId()).toBe(saas); + }); + + test("keeps the active account when no stored account is on that server", () => { + login("saas-token", SAAS, "saas-user"); + + process.env.SENTRY_URL = "https://sentry.other.com"; + + expect(getAuthToken()).toBe("saas-token"); + }); + + test("treats every sentry.io subdomain as the SaaS server", () => { + login("saas-token", SAAS, "saas-user"); + login("acme-token", ACME, "acme-user"); + + process.env.SENTRY_URL = "https://us.sentry.io"; + + expect(getAuthToken()).toBe("saas-token"); + }); +}); + +describe("organization-based selection", () => { + test("uses the account that knows the org without switching", () => { + const work = login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + const personal = login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + + selectAccountForOrg("work-org"); + + expect(getAuthToken()).toBe("work-token"); + expect(getActiveAccountId()).toBe(personal); + expect(work).not.toBe(personal); + }); + + test("keeps an account whose org list isn't cached, e.g. right after a switch", () => { + login("work-token", SAAS, "work"); + seeOrg("shared-org", SAAS); + login("personal-token", SAAS, "personal"); + + selectAccountForOrg("shared-org"); + + expect(getAuthToken()).toBe("personal-token"); + }); + + test("keeps an account whose cached org list has expired", () => { + login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + const personal = login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + getDatabase() + .query("UPDATE org_regions SET updated_at = ? WHERE account_id = ?") + .run(Date.now() - ORG_CACHE_TTL_MS - 1000, personal); + + selectAccountForOrg("work-org"); + + expect(getAuthToken()).toBe("personal-token"); + }); + + test("matches DSN-style numeric org ids", () => { + login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + getDatabase() + .query("UPDATE org_regions SET org_id = '42' WHERE org_slug = 'work-org'") + .run(); + login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + + selectAccountForOrg("o42", "42"); + + expect(getAuthToken()).toBe("work-token"); + }); + + test("matches a DSN org id against an account that only resolved the slug", () => { + const work = login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + const db = getDatabase(); + // Only an org list (here: an env-token's, not tied to an account) knows + // that org 42 is `work-org`; the work account resolved it by slug. + db.query( + `INSERT INTO org_regions (org_slug, org_id, region_url, credential_identity, account_id, source_origin, response_origin, updated_at) + SELECT org_slug, '42', region_url, 'env-token-identity', NULL, source_origin, response_origin, updated_at + FROM org_regions WHERE account_id = ? AND org_slug = 'work-org'`, + ).run(work); + + selectAccountForOrg("o42", "42"); + + expect(getAuthToken()).toBe("work-token"); + }); + + test("keeps the current account when it knows the org too", () => { + login("work-token", SAAS, "work"); + seeOrg("shared-org", SAAS); + login("personal-token", SAAS, "personal"); + seeOrg("shared-org", SAAS); + + selectAccountForOrg("shared-org"); + + expect(getAuthToken()).toBe("personal-token"); + }); + + test("leaves unknown orgs on the active account", () => { + login("work-token", SAAS, "work"); + login("personal-token", SAAS, "personal"); + + selectAccountForOrg("never-seen"); + + expect(getAuthToken()).toBe("personal-token"); + }); + + test("never mixes accounts within one invocation", () => { + const work = login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + const other = login("other-token", SAAS, "other"); + seeOrg("other-org", SAAS); + login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + + selectAccountForOrg("work-org"); + selectAccountForOrg("other-org"); + + expect(getAuthToken()).toBe("work-token"); + expect(other).not.toBe(work); + }); + + test("an explicit server limits candidates to accounts on it", () => { + login("acme-token", ACME, "acme"); + seeOrg("acme-org", ACME); + login("saas-token", SAAS, "saas"); + seeOrgList(SAAS, ["saas-org"]); + + process.env.SENTRY_HOST = SAAS; + selectAccountForOrg("acme-org"); + + expect(getAuthToken()).toBe("saas-token"); + expect(process.env.SENTRY_HOST).toBe(SAAS); + }); + + test("the persisted default URL yields to the org's account server", () => { + login("saas-token", SAAS, "saas"); + seeOrg("saas-org", SAAS); + login("acme-token", ACME, "acme"); + seeOrgList(ACME, ["acme-org"]); + process.env.SENTRY_URL = ACME; + markPersistedDefaultUrl(ACME); + + selectAccountForOrg("saas-org"); + + expect(getAuthToken()).toBe("saas-token"); + expect(process.env.SENTRY_URL).toBe(SAAS); + }); + + test("a self-hosted org's account brings its server along", () => { + login("acme-token", ACME, "acme"); + seeOrg("acme-org", ACME); + login("saas-token", SAAS, "saas"); + seeOrgList(SAAS, ["saas-org"]); + + selectAccountForOrg("acme-org"); + + expect(getAuthToken()).toBe("acme-token"); + expect(process.env.SENTRY_URL).toBe(ACME); + }); + + test("a forced env token is never overridden", () => { + login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + process.env.SENTRY_AUTH_TOKEN = "env-token"; + process.env.SENTRY_FORCE_ENV_TOKEN = "1"; + + selectAccountForOrg("work-org"); + + expect(getAuthToken()).toBe("env-token"); + }); + + test("skips accounts whose credentials were invalidated", () => { + const work = login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + getDatabase() + .query("UPDATE accounts SET token = NULL WHERE account_id = ?") + .run(work); + + selectAccountForOrg("work-org"); + + expect(getAuthToken()).toBe("personal-token"); + }); + + test("moves off an account in effect whose credentials were invalidated", () => { + login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + const personal = login("personal-token", SAAS, "personal"); + // Invalidated: token cleared and its org cache dropped, still active. + const db = getDatabase(); + db.query("UPDATE accounts SET token = NULL WHERE account_id = ?").run( + personal, + ); + db.query("DELETE FROM org_regions WHERE account_id = ?").run(personal); + + selectAccountForOrg("work-org"); + + expect(getAuthToken()).toBe("work-token"); + expect(getActiveAccountId()).toBe(personal); + }); + + test("skips expired accounts that cannot refresh", () => { + const work = login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + getDatabase() + .query( + "UPDATE accounts SET expires_at = ?, refresh_token = NULL WHERE account_id = ?", + ) + .run(Date.now() - 1000, work); + + selectAccountForOrg("work-org"); + + expect(getAuthToken()).toBe("personal-token"); + }); + + test("picks the most recently used account when several know the org", () => { + // Created first, so plain insertion order would pick it. + const stale = login("stale-token", SAAS, "stale"); + seeOrg("shared-org", SAAS); + const recent = login("recent-token", SAAS, "recent"); + seeOrg("shared-org", SAAS); + login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + const setLastUsed = getDatabase().query( + "UPDATE accounts SET last_used_at = ? WHERE account_id = ?", + ); + setLastUsed.run(1000, stale); + setLastUsed.run(2000, recent); + + selectAccountForOrg("shared-org"); + + expect(getAuthToken()).toBe("recent-token"); + }); + + test("an account the configured server picked yields to one on it that knows the org", () => { + const peer = login("peer-token", ACME, "peer"); + seeOrg("acme-org", ACME); + // More recently used on ACME, but has never seen the org. + login("other-token", ACME, "other"); + const saas = login("saas-token", SAAS, "saas"); + process.env.SENTRY_URL = ACME; + expect(getAuthToken()).toBe("other-token"); + + selectAccountForOrg("acme-org"); + + expect(getAuthToken()).toBe("peer-token"); + expect(getEffectiveAccountRow()?.account_id).toBe(peer); + expect(getActiveAccountId()).toBe(saas); + }); + + test("resolveOrgRegion routes through the org's account", async () => { + login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS, "https://de.sentry.io"); + const personal = login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + + await expect(resolveOrgRegion("work-org")).resolves.toBe( + "https://de.sentry.io", + ); + expect(getAuthToken()).toBe("work-token"); + expect(getActiveAccountId()).toBe(personal); + }); + + test("resolveOrgRegion routes a bare numeric DSN org id through its account", async () => { + login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS, "https://de.sentry.io"); + getDatabase() + .query("UPDATE org_regions SET org_id = '42' WHERE org_slug = 'work-org'") + .run(); + const personal = login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + + // DSN resolution can hand over the id without its `o` prefix. The + // region lookup itself isn't under test: keep it off the network. + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockRejectedValue(new Error("offline")); + try { + await resolveOrgRegion("42").catch(() => undefined); + } finally { + fetchSpy.mockRestore(); + } + + expect(getAuthToken()).toBe("work-token"); + expect(getActiveAccountId()).toBe(personal); + }); + + test("account switch keeps the org mapping for each account", () => { + const work = login("work-token", SAAS, "work"); + seeOrgList(SAAS, ["work-org"]); + const personal = login("personal-token", SAAS, "personal"); + seeOrg("personal-org", SAAS); + + setActiveAccount(work); + selectAccountForOrg("personal-org"); + + expect(getAuthToken()).toBe("personal-token"); + expect(personal).not.toBe(work); + }); +}); + +describe("logging in after an org picked the account", () => { + function pinWorkAccount(): { work: string; personal: string } { + const work = login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + const personal = login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + selectAccountForOrg("work-org"); + expect(getAuthToken()).toBe("work-token"); + return { work, personal }; + } + + test("re-authenticating replaces the active account, not the picked one", () => { + const { work, personal } = pinWorkAccount(); + + // Automatic login / `--force`: replace the active account in place. + setAuthToken("personal-new", undefined, undefined, { host: SAAS }); + setUserInfo({ userId: "personal", email: "personal@example.com" }); + + expect(tokenOf(work)).toBe("work-token"); + expect(tokenOf(personal)).toBe("personal-new"); + expect(getAuthToken()).toBe("personal-new"); + expect(getUserInfo()?.userId).toBe("personal"); + }); + + test("re-authenticating replaces the stored login for the login's server", () => { + const acme = login("acme-token", ACME, "acme"); + const saas = login("saas-token", SAAS, "saas"); + + // `sentry auth login --force --url ` while SaaS is active. + const replaced = setAuthToken("replaced-token", undefined, undefined, { + host: ACME, + }); + + expect(replaced).toBe(acme); + expect(tokenOf(acme)).toBe("replaced-token"); + expect(tokenOf(saas)).toBe("saas-token"); + expect(getActiveAccountId()).toBe(acme); + }); + + test("re-authenticating to a server with no stored login adds an account", () => { + const saas = login("saas-token", SAAS, "saas"); + + const added = setAuthToken("acme-token", undefined, undefined, { + host: ACME, + }); + + expect(added).not.toBe(saas); + expect(tokenOf(saas)).toBe("saas-token"); + expect(getActiveAccountId()).toBe(added); + }); + + test("a configured server never redirects re-authentication to its account", () => { + const acme = login("acme-token", ACME, "acme"); + const saas = login("saas-token", SAAS, "saas"); + process.env.SENTRY_URL = ACME; + expect(getAuthToken()).toBe("acme-token"); + + // A SaaS login replaces the SaaS account, whatever SENTRY_URL says. + setAuthToken("replaced-token", undefined, undefined, { host: SAAS }); + + expect(tokenOf(acme)).toBe("acme-token"); + expect(tokenOf(saas)).toBe("replaced-token"); + }); + + test("adding an account records its user on the new account", () => { + const { work } = pinWorkAccount(); + + const added = login("new-token", SAAS, "work"); + + expect(getActiveAccountId()).toBe(added); + expect(getAuthToken()).toBe("new-token"); + // Same user: the new login replaces the old work account. + expect(tokenOf(work)).toBeUndefined(); + }); +}); + +describe("re-authorizing the account an org picked", () => { + function pinWorkAccount(): { work: string; personal: string } { + const work = login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + const personal = login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + selectAccountForOrg("work-org"); + expect(getAuthToken()).toBe("work-token"); + return { work, personal }; + } + + test("targets the picked account, which the retry then uses with the new grant", () => { + const { work, personal } = pinWorkAccount(); + expect(getReauthorizationAccountId()).toBe(work); + + // Scope recovery: the new grant replaces the picked account in place. + setAuthToken("work-new", undefined, undefined, { + host: SAAS, + accountId: work, + }); + setUserInfo({ userId: "work", email: "work@example.com" }); + + expect(tokenOf(work)).toBe("work-new"); + expect(tokenOf(personal)).toBe("personal-token"); + expect(getActiveAccountId()).toBe(personal); + expect(getAuthToken()).toBe("work-new"); + + // The retried command resolves the org again and gets the new grant. + resetAccountSelection(); + selectAccountForOrg("work-org"); + expect(getAuthToken()).toBe("work-new"); + }); + + test("only accepts a login approved as the picked account's user", () => { + const { work } = pinWorkAccount(); + + expect(() => + assertReauthorizingSameUser(work, { id: "work", email: null }), + ).not.toThrow(); + // Signed in to the browser as the other user: nothing may be stored. + expect(() => + assertReauthorizingSameUser(work, { + id: "personal", + email: "personal@example.com", + }), + ).toThrow(/not work@example.com/); + expect(() => assertReauthorizingSameUser(work, undefined)).toThrow( + /Could not confirm/, + ); + }); + + test("is not needed while the active account is in effect", () => { + login("saas-token", SAAS, "saas"); + + expect(getReauthorizationAccountId()).toBeUndefined(); + }); + + test("refuses an account on another server and saves nothing", () => { + const acme = login("acme-token", ACME, "acme"); + const saas = login("saas-token", SAAS, "saas"); + + expect(() => + setAuthToken("new-token", undefined, undefined, { + host: SAAS, + accountId: acme, + }), + ).toThrow(AuthError); + expect(tokenOf(acme)).toBe("acme-token"); + expect(tokenOf(saas)).toBe("saas-token"); + }); +}); + +describe("credential invalidation", () => { + test("drops org memberships cached under an earlier identity", async () => { + login("personal-token", SAAS, "personal"); + seeOrgList(SAAS, ["personal-org"]); + const work = login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + const db = getDatabase(); + // As if written before a refresh-token rotation. + db.query( + "UPDATE org_regions SET credential_identity = 'rotated-away' WHERE account_id = ?", + ).run(work); + db.query( + "UPDATE accounts SET expires_at = ?, refresh_token = NULL WHERE account_id = ?", + ).run(Date.now() - 1000, work); + + await refreshToken().catch(() => undefined); + + const left = db + .query("SELECT COUNT(*) AS n FROM org_regions WHERE account_id = ?") + .get(work) as { n: number }; + expect(left.n).toBe(0); + expect(tokenOf(work)).toBeNull(); + }); +}); + +describe("pasted Sentry URLs", () => { + test("a URL for another stored account's server selects it", () => { + login("acme-token", ACME, "acme"); + login("saas-token", SAAS, "saas"); + + applySentryUrlContext(ACME); + + expect(getAuthToken()).toBe("acme-token"); + }); + + test("a SaaS URL selects the SaaS account when self-hosted is active", () => { + login("saas-token", SAAS, "saas"); + login("acme-token", ACME, "acme"); + process.env.SENTRY_URL = ACME; + + applySentryUrlContext("https://acme-org.sentry.io"); + + expect(getAuthToken()).toBe("saas-token"); + }); + + test("a SaaS URL pins the server so org selection can't switch to a self-hosted account", () => { + login("acme-token", ACME, "acme"); + // The self-hosted account knows an org whose slug SaaS also uses. + seeOrg("shared", ACME); + login("saas-token", SAAS, "saas"); + // SaaS is active, with a fresh org list that doesn't include it yet. + seeOrgList(SAAS, ["alpha"]); + + applySentryUrlContext("https://shared.sentry.io"); + selectAccountForOrg("shared"); + + expect(process.env.SENTRY_URL).toBe(SAAS); + expect(getAuthToken()).toBe("saas-token"); + }); + + test("a SaaS URL pins the server even when the persisted default is already SaaS", () => { + login("acme-token", ACME, "acme"); + seeOrg("shared", ACME); + login("saas-token", SAAS, "saas"); + seeOrgList(SAAS, ["alpha"]); + // `defaults.url` is SaaS and was applied at startup. + process.env.SENTRY_URL = SAAS; + markPersistedDefaultUrl(SAAS); + + applySentryUrlContext("https://shared.sentry.io"); + selectAccountForOrg("shared"); + + expect(process.env.SENTRY_URL).toBe(SAAS); + expect(getAuthToken()).toBe("saas-token"); + }); + + test("a SaaS URL never routes to a self-hosted account when the SaaS login is logged out", () => { + login("acme-token", ACME, "acme"); + seeOrg("shared", ACME); + const saas = login("saas-token", SAAS, "saas"); + getDatabase() + .query("UPDATE accounts SET token = NULL WHERE account_id = ?") + .run(saas); + + applySentryUrlContext("https://shared.sentry.io"); + selectAccountForOrg("shared"); + + expect(process.env.SENTRY_URL).toBe(SAAS); + expect(getAuthToken()).not.toBe("acme-token"); + }); + + test("a SaaS URL never routes to a self-hosted account when no SaaS account is stored", () => { + const OTHER = "https://sentry.other.com"; + login("other-token", OTHER, "other"); + seeOrg("shared", OTHER); + login("acme-token", ACME, "acme"); + seeOrgList(ACME, ["alpha"]); + + applySentryUrlContext("https://shared.sentry.io"); + selectAccountForOrg("shared"); + + // Pinned to SaaS: the self-hosted account that knows the slug is never + // picked; the active one stays and the SaaS host check refuses it. + expect(process.env.SENTRY_URL).toBe(SAAS); + expect(getAuthToken()).toBe("acme-token"); + }); + + test("a SaaS URL with a single self-hosted login keeps using that login", () => { + login("acme-token", ACME, "acme"); + process.env.SENTRY_URL = ACME; + + applySentryUrlContext("https://acme-org.sentry.io"); + + expect(process.env.SENTRY_URL).toBeUndefined(); + expect(getAuthToken()).toBe("acme-token"); + }); + + test("a SaaS URL never snapshots an env token's host", () => { + login("saas-token", SAAS, "saas"); + process.env.SENTRY_AUTH_TOKEN = "env-token"; + process.env.SENTRY_FORCE_ENV_TOKEN = "1"; + process.env.SENTRY_URL = ACME; + + applySentryUrlContext("https://acme-org.sentry.io"); + + // Had the URL handling read the env token's host after clearing the + // URL, it would be pinned to SaaS; captured now, it is still ACME. + process.env.SENTRY_URL = ACME; + expect(getEnvTokenHost()).toBe(ACME); + }); + + test("a URL for a server with no stored account is still refused", () => { + login("saas-token", SAAS, "saas"); + + expect(() => applySentryUrlContext("https://evil.example.com")).toThrow(); + }); +}); + +describe(".sentryclirc URLs", () => { + async function withRcUrl(url: string, run: (dir: string) => Promise) { + const dir = mkdtempSync(join(tmpdir(), "account-routing-rc-")); + try { + writeFileSync(join(dir, ".sentryclirc"), `[defaults]\nurl = ${url}\n`); + clearSentryCliRcCache(); + await applySentryCliRcEnvShim(dir); + await run(dir); + } finally { + clearSentryCliRcCache(); + rmSync(dir, { recursive: true, force: true }); + } + } + + test("a logged-out active account on the rc server defers to a usable peer that knows the org", async () => { + login("peer-token", ACME, "peer"); + seeOrg("acme-org", ACME); + const active = login("active-token", ACME, "active"); + getDatabase() + .query("UPDATE accounts SET token = NULL WHERE account_id = ?") + .run(active); + + await withRcUrl(ACME, async (dir) => { + await expect(assertRcUrlTrusted(dir)).resolves.toBeUndefined(); + selectAccountForOrg("acme-org"); + expect(getAuthToken()).toBe("peer-token"); + }); + }); + + test("an rc server with no usable stored account is still refused", async () => { + login("saas-token", SAAS, "saas"); + + await withRcUrl("https://evil.example.com", async (dir) => { + await expect(assertRcUrlTrusted(dir)).rejects.toThrow(); + }); + }); +}); + +describe("schema v19 migration", () => { + test("tags cached orgs with the account whose credentials wrote them", () => { + const work = login("work-token", SAAS, "work"); + seeOrg("work-org", SAAS); + const db = getDatabase(); + db.query("UPDATE org_regions SET account_id = NULL").run(); + db.query("UPDATE schema_version SET version = 18").run(); + closeDatabase(); + + const reopened = getDatabase(); + expect(reopened.query("SELECT version FROM schema_version").get()).toEqual({ + version: CURRENT_SCHEMA_VERSION, + }); + expect( + reopened + .query("SELECT account_id FROM org_regions WHERE org_slug = 'work-org'") + .get(), + ).toEqual({ account_id: work }); + }); +}); diff --git a/packages/cli/test/lib/db/auth-concurrency.focused.test.ts b/packages/cli/test/lib/db/auth-concurrency.focused.test.ts index 7da6e0dd9..907bbbb0d 100644 --- a/packages/cli/test/lib/db/auth-concurrency.focused.test.ts +++ b/packages/cli/test/lib/db/auth-concurrency.focused.test.ts @@ -90,8 +90,10 @@ describe("OAuth refresh compare-and-swap", () => { const response = deferred(); const { pending, started } = startPausedRefresh(response); expect((await started).origin).toBe("https://old.example.com"); + // A newer login replaces the same server's account in place (a login + // to another server would add its own account instead). setAuthToken("new-access", 3600, "new-refresh", { - host: "https://new.example.com", + host: "https://old.example.com", }); response.resolve( Response.json({ @@ -106,15 +108,17 @@ describe("OAuth refresh compare-and-swap", () => { token: "new-access", refreshToken: "new-refresh", }); - expect(getCredentialContext()?.host).toBe("https://new.example.com"); + expect(getCredentialContext()?.host).toBe("https://old.example.com"); }); test("a late invalid_grant never clears a newer login", async () => { const response = deferred(); const { pending, started } = startPausedRefresh(response); await started; + // A newer login replaces the same server's account in place (a login + // to another server would add its own account instead). setAuthToken("new-access", 3600, "new-refresh", { - host: "https://new.example.com", + host: "https://old.example.com", }); response.resolve( Response.json( @@ -127,7 +131,7 @@ describe("OAuth refresh compare-and-swap", () => { token: "new-access", refreshToken: "new-refresh", }); - expect(getCredentialContext()?.host).toBe("https://new.example.com"); + expect(getCredentialContext()?.host).toBe("https://old.example.com"); }); test("an unexpected refresh failure retains the stored credential", async () => { diff --git a/packages/cli/test/lib/scope-recovery.test.ts b/packages/cli/test/lib/scope-recovery.test.ts index 2b0e22186..00220c225 100644 --- a/packages/cli/test/lib/scope-recovery.test.ts +++ b/packages/cli/test/lib/scope-recovery.test.ts @@ -55,6 +55,28 @@ describe("runWithScopeRecovery", () => { ); }); + test("re-authorizes the account picked for the target org, not the active one", async () => { + const error = new ApiError("Forbidden", 403); + const proceed = vi + .fn<(argv: string[]) => Promise>() + .mockRejectedValueOnce(error) + .mockResolvedValueOnce(); + const login = vi.fn().mockResolvedValue({ method: "oauth" }); + + await runWithScopeRecovery( + proceed, + [], + login, + runtime({ + getAuthScopes: vi.fn().mockResolvedValue([]), + getReauthorizationAccountId: () => "work-account", + }), + ); + + expect(login).toHaveBeenCalledWith({ accountId: "work-account" }); + expect(proceed).toHaveBeenCalledTimes(2); + }); + test("does not re-authorize a role or policy 403 when the token has every scope", async () => { const error = new ApiError("Forbidden", 403); const proceed = vi.fn().mockRejectedValue(error); @@ -378,6 +400,23 @@ describe("ensureScopesForAction", () => { ); }); + test("elevates the account picked for the target org", async () => { + const login = vi.fn().mockResolvedValue({ method: "oauth" }); + + await expect( + ensureScopesForAction( + ["org:integrations"], + "Removing an integration", + login, + runtime({ + getAuthScopes: vi.fn().mockResolvedValue(withoutIntegrations), + getReauthorizationAccountId: () => "work-account", + }), + ), + ).resolves.toBe("elevated"); + expect(login).toHaveBeenCalledWith({ accountId: "work-account" }); + }); + test("reports declined when the login does not complete", async () => { const testRuntime = runtime({ getAuthScopes: vi.fn().mockResolvedValue(withoutIntegrations),