From f3dbe52607c095f0385169fba7785ac18182b845 Mon Sep 17 00:00:00 2001 From: Bryan Maass Date: Mon, 6 Jul 2026 14:14:58 -0600 Subject: [PATCH 1/3] feat(auth): mb auth login --workspace requests a workspace-manager-scoped grant The scoped token is the containment primitive for Option B: a parent credential on an agent's machine that can only do workspace CRUD, enforced server-side by default-deny scope middleware. - scope threads through discovery gating, dynamic client registration, the authorize URL, the stored credential, and refresh (which sends no scope parameter, so a refresh can never widen the grant) - pre-scope profile records resolve to mb:full, which is what every legacy login was minted with - workspace login is browser-OAuth only: --api-key, MB_API_KEY, and non-TTY contexts are refused so a key can't masquerade as scoped - a 403 on a scope-narrowed profile is enriched to name the scope and the fix instead of a bare "Forbidden." Refs GHY-4053 --- README.md | 17 ++++--- src/commands/auth/login.ts | 76 +++++++++++++++++++++++++++-- src/commands/auth/logout.test.ts | 1 + src/commands/auth/status.test.ts | 1 + src/commands/runtime.test.ts | 71 ++++++++++++++++++++++++++- src/commands/runtime.ts | 21 ++++++++ src/core/auth/credential.test.ts | 1 + src/core/auth/credential.ts | 3 ++ src/core/auth/oauth-login.test.ts | 34 ++++++++++++- src/core/auth/oauth-login.ts | 11 +++-- src/core/auth/oauth-session.test.ts | 3 ++ src/core/auth/oauth-session.ts | 7 ++- src/core/auth/profile-record.ts | 4 ++ src/core/auth/storage.test.ts | 32 ++++++++++++ src/core/auth/storage.ts | 2 + src/core/config.test.ts | 2 + src/core/http/client.test.ts | 1 + src/core/http/oauth.test.ts | 30 ++++++++++++ src/core/http/oauth.ts | 27 ++++++---- tests/e2e/auth.e2e.test.ts | 55 +++++++++++++++++++++ 20 files changed, 370 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index e44a001..f8edc72 100644 --- a/README.md +++ b/README.md @@ -52,16 +52,19 @@ Against a server older than v62 the CLI detects the missing OAuth support and fa On success the server is probed once — the rendered output shows the user, role (`Admin`/`User`), and Metabase version, and the same values are cached in `/profiles.json` so later commands skip re-probing. Failure of either the auth probe (`/api/user/current`) or the server probe (`/api/session/properties`) rejects the login; an existing profile keeps its last-known-good credential and gains a `lastFailure` entry. -| Flag | Description | -| ------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------- | -| `--url ` | Metabase URL, including any subpath if the instance is hosted under one (`https://my.org.com/metabase`). Falls back to `MB_URL`, then prompts. | -| `--api-key ` | API key. Skips the browser flow. Visible in shell history — pipe on stdin instead. | -| `--client-id ` | Pre-registered OAuth client id (only needed when dynamic client registration is disabled on the server). | -| `--profile `, `-p` | Profile to write to (default: `default`). | -| `--skip-verify` | Save without contacting the server (no probe, no cache). | +| Flag | Description | +| ------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `--url ` | Metabase URL, including any subpath if the instance is hosted under one (`https://my.org.com/metabase`). Falls back to `MB_URL`, then prompts. | +| `--api-key ` | API key. Skips the browser flow. Visible in shell history — pipe on stdin instead. | +| `--client-id ` | Pre-registered OAuth client id (only needed when dynamic client registration is disabled on the server). | +| `--profile `, `-p` | Profile to write to (default: `default`). | +| `--skip-verify` | Save without contacting the server (no probe, no cache). | +| `--workspace` | Request a workspace-manager-scoped login: the stored token can only manage workspaces — every other endpoint 403s server-side. Browser OAuth only (an API key cannot carry a scope), so it requires a TTY and refuses `--api-key`/`MB_API_KEY`. Requires a server advertising the `mb:workspace-manager` scope. | Non-interactive (non-TTY) login requires an API key; resolution order: `--api-key` → piped stdin → `MB_API_KEY` (first non-empty wins). Without one, non-interactive login fails rather than prompting. +A workspace-scoped profile that hits a 403 on a content command gets an error naming the scope and the fix (re-login without `--workspace`, or use a workspace profile) instead of a bare "Forbidden." + ```sh mb auth login # interactive: browser or API key echo "$MB_KEY" | mb auth login --url https://m.example.com diff --git a/src/commands/auth/login.ts b/src/commands/auth/login.ts index 41cc854..5773bc5 100644 --- a/src/commands/auth/login.ts +++ b/src/commands/auth/login.ts @@ -21,7 +21,12 @@ import { normalizeUrl } from "../../core/url"; import { ParsedVersionSchema } from "../../core/version/tag"; import { ProbedUser } from "../../core/auth/profile-record"; import type { ResourceView } from "../../domain/view"; -import { tryDiscoverMetadata, type OAuthServerMetadata } from "../../core/http/oauth"; +import { + OAUTH_SCOPE, + tryDiscoverMetadata, + WORKSPACE_MANAGER_SCOPE, + type OAuthServerMetadata, +} from "../../core/http/oauth"; import { warn } from "../../output/notice"; import { promptPassword, promptSelect, promptText } from "../../output/prompt"; import { renderSummary } from "../../output/render"; @@ -38,6 +43,7 @@ export const LoginResult = z.object({ authenticated: z.boolean(), user: ProbedUser.nullable(), version: ParsedVersionSchema.nullable(), + scope: z.string().nullable(), }); export type LoginResultJson = z.infer; @@ -54,6 +60,11 @@ const loginView: ResourceView = { { key: "user", label: "Logged in as", format: (value) => renderUserName(value) }, { key: "user", label: "Role", format: (value) => renderUserRole(value) }, { key: "version", label: "Version", format: (value) => renderVersionTag(value) }, + { + key: "scope", + label: "Scope", + format: (value) => (typeof value === "string" ? value : EMPTY_CELL), + }, ], }; @@ -76,12 +87,19 @@ export default defineMetabaseCommand({ default: false, description: "Save without contacting the server", }, + workspace: { + type: "boolean", + default: false, + description: + "Request a workspace-manager-scoped login (workspace CRUD only, browser OAuth required)", + }, }, outputSchema: LoginResult, examples: [ "mb auth login --url https://metabase.example.com", "echo $MB_API_KEY | mb auth login --url https://metabase.example.com", "mb auth login --profile staging --url https://staging.example.com", + "mb auth login --workspace --profile parent --url https://metabase.example.com", ], async run({ args, ctx }) { const profileName = await resolveLoginProfile(args.profile); @@ -94,6 +112,30 @@ export default defineMetabaseCommand({ } const url = await resolveUrl(args.url, env.url); + + if (args.workspace) { + assertWorkspaceLoginPossible(args.apiKey, env.apiKey); + const metadata = await probeOAuthSupport(url, WORKSPACE_MANAGER_SCOPE); + if (metadata === null) { + throw new ConfigError( + `this Metabase does not support workspace-scoped login (no ${WORKSPACE_MANAGER_SCOPE} scope advertised)`, + ); + } + const credential = await oauthLogin( + { + baseUrl: url, + metadata, + scope: WORKSPACE_MANAGER_SCOPE, + ...(args.clientId !== undefined && { clientId: args.clientId }), + }, + { openBrowser, onAuthorizeUrl: announceAuthorizeUrl, now: () => Date.now() }, + ); + await completeLogin(profileName, url, credential, args["skip-verify"], ctx, () => + writeOAuthProfile(url, credential, profileName), + ); + return; + } + const apiKey = await nonInteractiveApiKey(args.apiKey, env.apiKey); if (apiKey !== null) { @@ -146,11 +188,32 @@ export default defineMetabaseCommand({ type LoginMethod = "oauth" | "apiKey"; +// A workspace-scoped grant exists to keep content-mutating credentials off the machine; an API +// key can't carry a scope, so every key-shaped path is refused rather than silently ignored. +function assertWorkspaceLoginPossible(flagKey: string | undefined, envKey: string | null): void { + if (flagKey !== undefined) { + throw new ConfigError( + "--workspace login is browser-OAuth only; an API key cannot carry a scope — omit --api-key", + ); + } + if (envKey !== null) { + throw new ConfigError( + "--workspace login is browser-OAuth only; unset MB_API_KEY so it cannot shadow the scoped login", + ); + } + if (!process.stdin.isTTY) { + throw new ConfigError("--workspace login opens a browser and requires a TTY"); + } +} + // Reaching the server but finding no CLI-capable OAuth server (pre-v63) degrades to the API key // prompt; not reaching it at all is an error worth stopping on before any credential is collected. -async function probeOAuthSupport(url: string): Promise { +async function probeOAuthSupport( + url: string, + requiredScope?: string, +): Promise { try { - return await tryDiscoverMetadata(url); + return await tryDiscoverMetadata(url, requiredScope); } catch (error) { if (error instanceof ConfigError) { throw error; @@ -213,10 +276,11 @@ async function completeLogin( ctx: CommonContext, persist: PersistCredential, ): Promise { + const scope = credential.kind === "oauth" ? credential.scope : null; if (skipVerify) { await persistWithWarning(persist); renderSummary( - { profile: profileName, url, authenticated: false, user: null, version: null }, + { profile: profileName, url, authenticated: false, user: null, version: null, scope }, loginView, `Saved credentials for profile "${profileName}" (${url}) without verifying.`, ctx, @@ -238,6 +302,7 @@ async function completeLogin( const role = renderUserRole(result.user); const versionTag = renderVersionTag(result.server.version); const serverClause = versionTag === EMPTY_CELL ? "" : ` Server ${versionTag}.`; + const scopeClause = scope === null || scope === OAUTH_SCOPE ? "" : ` Scope: ${scope}.`; renderSummary( { profile: profileName, @@ -245,9 +310,10 @@ async function completeLogin( authenticated: true, user: result.user, version: result.server.version, + scope, }, loginView, - `Logged in to ${url} as ${who} (${role}). Saved to profile "${profileName}".${serverClause}`, + `Logged in to ${url} as ${who} (${role}). Saved to profile "${profileName}".${serverClause}${scopeClause}`, ctx, ); } diff --git a/src/commands/auth/logout.test.ts b/src/commands/auth/logout.test.ts index 6c7a0bf..78ffde1 100644 --- a/src/commands/auth/logout.test.ts +++ b/src/commands/auth/logout.test.ts @@ -33,6 +33,7 @@ const OAUTH_CRED: OAuthCredential = { refreshToken: "ref-1", expiresAt: "2099-01-01T00:00:00.000Z", clientId: "client-1", + scope: "mb:full", }; const METADATA: OAuthServerMetadata = { diff --git a/src/commands/auth/status.test.ts b/src/commands/auth/status.test.ts index a73a570..e82b43a 100644 --- a/src/commands/auth/status.test.ts +++ b/src/commands/auth/status.test.ts @@ -92,6 +92,7 @@ describe("auth status command", () => { refreshToken: "ref", expiresAt: "2026-06-08T13:00:00.000Z", clientId: "c1", + scope: "mb:full", }); const capture = captureStdout(); await runCommand(authStatusCommand, { rawArgs: ["--profile", "default", "--json"] }); diff --git a/src/commands/runtime.test.ts b/src/commands/runtime.test.ts index 14528c9..69edb29 100644 --- a/src/commands/runtime.test.ts +++ b/src/commands/runtime.test.ts @@ -2,6 +2,7 @@ import { runCommand } from "citty"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { setupTempConfigHome, type TempConfigHome } from "../core/auth/temp-config-home"; +import type { ResolvedConfig } from "../core/config"; import type { ServerInfo } from "../core/version/probe"; const hoisted = vi.hoisted(() => ({ @@ -14,7 +15,10 @@ vi.mock("@napi-rs/keyring", async () => { return createKeyringMockModule(hoisted); }); -const { defineMetabaseCommand, SKIP_PREFLIGHT_ENV } = await import("./runtime"); +const { defineMetabaseCommand, enrichScopeForbiddenError, SKIP_PREFLIGHT_ENV } = + await import("./runtime"); +const { ConfigError, errorMessage } = await import("../core/errors"); +const { HttpError } = await import("../core/http/errors"); const { connectionFlags, outputFlags, profileFlag } = await import("./flags"); const { writeProbeResult, writeProfile } = await import("../core/auth/storage"); @@ -316,3 +320,68 @@ describe("defineMetabaseCommand", () => { expect(ran).toHaveBeenCalledOnce(); }); }); + +describe("enrichScopeForbiddenError", () => { + const FORBIDDEN = new HttpError({ + status: 403, + statusText: "Forbidden", + method: "POST", + url: "https://m.example.com/api/card", + responseHeaders: {}, + rawBody: null, + }); + + function oauthConfig(scope: string): ResolvedConfig { + return { + url: "https://m.example.com", + profile: "parent", + source: "stored", + credential: { + kind: "oauth", + accessToken: "acc", + refreshToken: "ref", + expiresAt: "2099-01-01T00:00:00.000Z", + clientId: "c1", + scope, + }, + }; + } + + it("converts a 403 on a workspace-scoped profile into a ConfigError naming the scope", () => { + const result = enrichScopeForbiddenError(FORBIDDEN, oauthConfig("mb:workspace-manager")); + expect(result).toBeInstanceOf(ConfigError); + expect(errorMessage(result)).toBe( + `${FORBIDDEN.userMessage} This profile's login is scoped to mb:workspace-manager, which only allows workspace commands against this server. Run \`mb auth login\` for a full-access login, or point --profile at a workspace profile.`, + ); + }); + + it("passes a 403 on a full-scope profile through untouched", () => { + expect(enrichScopeForbiddenError(FORBIDDEN, oauthConfig("mb:full"))).toBe(FORBIDDEN); + }); + + it("passes a 403 on an api-key profile through untouched", () => { + const config: ResolvedConfig = { + url: "https://m.example.com", + profile: "default", + source: "stored", + credential: { kind: "apiKey", apiKey: "mb_secret" }, + }; + expect(enrichScopeForbiddenError(FORBIDDEN, config)).toBe(FORBIDDEN); + }); + + it("passes a non-403 error through untouched even on a workspace-scoped profile", () => { + const notFound = new HttpError({ + status: 404, + statusText: "Not Found", + method: "GET", + url: "https://m.example.com/api/card/1", + responseHeaders: {}, + rawBody: null, + }); + expect(enrichScopeForbiddenError(notFound, oauthConfig("mb:workspace-manager"))).toBe(notFound); + }); + + it("passes the error through when no config was resolved yet", () => { + expect(enrichScopeForbiddenError(FORBIDDEN, null)).toBe(FORBIDDEN); + }); +}); diff --git a/src/commands/runtime.ts b/src/commands/runtime.ts index 9c984c2..236e7da 100644 --- a/src/commands/runtime.ts +++ b/src/commands/runtime.ts @@ -16,7 +16,10 @@ import { type ResolvedConfig, } from "../core/config"; import { consumeLegacyEnvWarnings } from "../core/env"; +import { ConfigError } from "../core/errors"; import { createClient, type Client } from "../core/http/client"; +import { HttpError } from "../core/http/errors"; +import { OAUTH_SCOPE } from "../core/http/oauth"; import { BASELINE_CAPABILITIES, checkCapabilities, @@ -112,6 +115,8 @@ export function defineMetabaseCommand( getResolvedConfig, getServerInfo, }); + } catch (error) { + throw enrichScopeForbiddenError(error, cachedConfig); } finally { emitPendingWarnings(); } @@ -129,6 +134,22 @@ export function defineMetabaseCommand( return cmd; } +// A server-side 403 on a scope-narrowed profile is almost always the scope working as designed +// (the containment model 403s everything outside workspace CRUD), so name the real fix instead +// of letting the bare "Forbidden." suggest a permissions bug. +export function enrichScopeForbiddenError(error: unknown, config: ResolvedConfig | null): unknown { + if (!(error instanceof HttpError) || error.status !== 403 || config === null) { + return error; + } + const { credential } = config; + if (credential.kind !== "oauth" || credential.scope === OAUTH_SCOPE) { + return error; + } + return new ConfigError( + `${error.userMessage} This profile's login is scoped to ${credential.scope}, which only allows workspace commands against this server. Run \`mb auth login\` for a full-access login, or point --profile at a workspace profile.`, + ); +} + function emitPendingWarnings(): void { for (const message of consumeLegacyEnvWarnings()) { warn(message); diff --git a/src/core/auth/credential.test.ts b/src/core/auth/credential.test.ts index cccd4d9..d667c04 100644 --- a/src/core/auth/credential.test.ts +++ b/src/core/auth/credential.test.ts @@ -18,6 +18,7 @@ function oauth(overrides: Partial = {}): OAuthCredential { refreshToken: "refresh-tok", expiresAt: "2026-01-01T00:00:00.000Z", clientId: "client-123", + scope: "mb:full", ...overrides, }; } diff --git a/src/core/auth/credential.ts b/src/core/auth/credential.ts index f30a7da..2b47058 100644 --- a/src/core/auth/credential.ts +++ b/src/core/auth/credential.ts @@ -11,6 +11,7 @@ export interface OAuthCredential { refreshToken: string; expiresAt: string; clientId: string; + scope: string; } export type Credential = ApiKeyCredential | OAuthCredential; @@ -68,6 +69,7 @@ export function oauthCredentialFromTokens( tokens: Pick, refreshToken: string, clientId: string, + scope: string, nowMs: number, ): OAuthCredential { return { @@ -76,5 +78,6 @@ export function oauthCredentialFromTokens( refreshToken, expiresAt: expiresAtFromNow(tokens.expires_in ?? DEFAULT_EXPIRES_IN_SECONDS, nowMs), clientId, + scope, }; } diff --git a/src/core/auth/oauth-login.test.ts b/src/core/auth/oauth-login.test.ts index b3a9fe3..5591e7f 100644 --- a/src/core/auth/oauth-login.test.ts +++ b/src/core/auth/oauth-login.test.ts @@ -3,12 +3,13 @@ import { createHash } from "node:crypto"; import { afterEach, assert, describe, expect, it, vi } from "vitest"; import { ConfigError } from "../errors"; -import type { CodeExchange, OAuthTokens } from "../http/oauth"; +import type { ClientRegistration, CodeExchange, OAuthTokens } from "../http/oauth"; const hoisted = vi.hoisted<{ tokens: OAuthTokens; metadata: OAuthServerMetadata; registerCalls: number; + registeredScope: string | null; discoverCalls: number; exchange: CodeExchange | null; }>(() => ({ @@ -25,6 +26,7 @@ const hoisted = vi.hoisted<{ registration_endpoint: "https://mb.example.com/oauth/register", }, registerCalls: 0, + registeredScope: null, discoverCalls: 0, exchange: null, })); @@ -37,8 +39,9 @@ vi.mock("../http/oauth", async (importOriginal) => { hoisted.discoverCalls += 1; return hoisted.metadata; }, - registerClient: async () => { + registerClient: async (input: ClientRegistration) => { hoisted.registerCalls += 1; + hoisted.registeredScope = input.scope; return { client_id: "client-xyz" }; }, exchangeCode: async (input: CodeExchange) => { @@ -95,6 +98,7 @@ describe("oauthLogin", () => { hoisted.tokens = { ...DEFAULT_TOKENS }; hoisted.metadata = { ...DEFAULT_METADATA }; hoisted.registerCalls = 0; + hoisted.registeredScope = null; hoisted.discoverCalls = 0; hoisted.exchange = null; }); @@ -115,6 +119,7 @@ describe("oauthLogin", () => { refreshToken: "ref", expiresAt: "2026-06-08T13:00:00.000Z", clientId: "client-xyz", + scope: "mb:full", }); expect(announced).toHaveLength(1); expect(announced[0]).toContain("https://mb.example.com/oauth/authorize?"); @@ -139,6 +144,30 @@ describe("oauthLogin", () => { ); }); + it("threads a narrowed scope through registration, the authorize URL, and the credential", async () => { + const announced: string[] = []; + const credential = await oauthLogin( + { baseUrl: "https://mb.example.com", scope: "mb:workspace-manager" }, + { + openBrowser: browserDriver(), + onAuthorizeUrl: (url) => announced.push(url), + now: () => NOW, + }, + ); + expect(credential).toEqual({ + kind: "oauth", + accessToken: "acc", + refreshToken: "ref", + expiresAt: "2026-06-08T13:00:00.000Z", + clientId: "client-xyz", + scope: "mb:workspace-manager", + }); + expect(hoisted.registeredScope).toBe("mb:workspace-manager"); + assert(announced[0] !== undefined); + const authorizeParams = new URL(announced[0]).searchParams; + expect(authorizeParams.get("scope")).toBe("mb:workspace-manager"); + }); + it("joins authorize params with & when the endpoint already carries a query", async () => { hoisted.metadata = { ...DEFAULT_METADATA, @@ -183,6 +212,7 @@ describe("oauthLogin", () => { refreshToken: "ref", expiresAt: "2026-06-08T13:00:00.000Z", clientId: "client-xyz", + scope: "mb:full", }); }); diff --git a/src/core/auth/oauth-login.ts b/src/core/auth/oauth-login.ts index 1295270..2d23413 100644 --- a/src/core/auth/oauth-login.ts +++ b/src/core/auth/oauth-login.ts @@ -19,6 +19,7 @@ export interface OAuthLoginInput { // when omitted it is discovered here. metadata?: OAuthServerMetadata; clientId?: string; + scope?: string; timeoutMs?: number; } @@ -38,6 +39,7 @@ async function resolveClientId( registrationEndpoint: string | undefined, redirectUri: string, provided: string | undefined, + scope: string, ): Promise { if (provided !== undefined) { return provided; @@ -51,6 +53,7 @@ async function resolveClientId( registrationEndpoint, redirectUri, clientName: CLIENT_NAME, + scope, }); return registered.client_id; } @@ -59,7 +62,8 @@ export async function oauthLogin( input: OAuthLoginInput, deps: OAuthLoginDeps, ): Promise { - const metadata = input.metadata ?? (await discoverMetadata(input.baseUrl)); + const scope = input.scope ?? OAUTH_SCOPE; + const metadata = input.metadata ?? (await discoverMetadata(input.baseUrl, scope)); const pkce = generatePkce(); const state = randomState(); // The server validates state in-handler, so a forged callback can't consume the slot. @@ -69,6 +73,7 @@ export async function oauthLogin( metadata.registration_endpoint, server.redirectUri, input.clientId, + scope, ); const authorizeUrl = buildAuthorizeUrl(metadata.authorization_endpoint, { response_type: "code", @@ -77,7 +82,7 @@ export async function oauthLogin( code_challenge: pkce.challenge, code_challenge_method: "S256", state, - scope: OAUTH_SCOPE, + scope, }); const opened = await deps.openBrowser(authorizeUrl); @@ -97,7 +102,7 @@ export async function oauthLogin( throw new ConfigError("token endpoint did not return a refresh token"); } - return oauthCredentialFromTokens(tokens, tokens.refresh_token, clientId, deps.now()); + return oauthCredentialFromTokens(tokens, tokens.refresh_token, clientId, scope, deps.now()); } finally { server.close(); } diff --git a/src/core/auth/oauth-session.test.ts b/src/core/auth/oauth-session.test.ts index c094369..c063bf5 100644 --- a/src/core/auth/oauth-session.test.ts +++ b/src/core/auth/oauth-session.test.ts @@ -12,6 +12,7 @@ const CREDENTIAL: OAuthCredential = { refreshToken: "ref-1", expiresAt: "2026-06-08T12:00:00.000Z", clientId: "client-1", + scope: "mb:full", }; const METADATA: OAuthServerMetadata = { @@ -48,6 +49,7 @@ describe("refreshOAuthCredential", () => { refreshToken: "ref-2", expiresAt: "2026-06-08T13:00:00.000Z", clientId: "client-1", + scope: "mb:full", }); }); @@ -63,6 +65,7 @@ describe("refreshOAuthCredential", () => { refreshToken: "ref-1", expiresAt: "2026-06-08T13:00:00.000Z", clientId: "client-1", + scope: "mb:full", }); }); }); diff --git a/src/core/auth/oauth-session.ts b/src/core/auth/oauth-session.ts index 4d73840..6c246fd 100644 --- a/src/core/auth/oauth-session.ts +++ b/src/core/auth/oauth-session.ts @@ -9,16 +9,19 @@ export async function refreshOAuthCredential( credential: OAuthCredential, nowMs: number, ): Promise { - const metadata = await discoverMetadata(baseUrl); + const metadata = await discoverMetadata(baseUrl, credential.scope); const tokens = await refreshTokens({ tokenEndpoint: metadata.token_endpoint, refreshToken: credential.refreshToken, clientId: credential.clientId, }); + // The refresh request carries no scope parameter, so the new grant inherits the original + // scope — a refresh can never widen it. return oauthCredentialFromTokens( tokens, tokens.refresh_token ?? credential.refreshToken, credential.clientId, + credential.scope, nowMs, ); } @@ -32,7 +35,7 @@ export async function revokeOAuthCredential( baseUrl: string, credential: OAuthCredential, ): Promise { - const metadata = await discoverMetadata(baseUrl); + const metadata = await discoverMetadata(baseUrl, credential.scope); const revocationEndpoint = metadata.revocation_endpoint; if (revocationEndpoint === undefined) { return false; diff --git a/src/core/auth/profile-record.ts b/src/core/auth/profile-record.ts index b4be959..9bd6ed4 100644 --- a/src/core/auth/profile-record.ts +++ b/src/core/auth/profile-record.ts @@ -1,6 +1,7 @@ import { z } from "zod"; import { TokenFeatures } from "../../domain/session-properties"; +import { OAUTH_SCOPE } from "../http/oauth"; import { ParsedVersionSchema } from "../version/tag"; export const ProbedUser = z.object({ @@ -37,6 +38,9 @@ export const ProfileOAuth = z refreshToken: z.string().nullable(), expiresAt: z.iso.datetime(), clientId: z.string(), + // Records written before scoped logins existed were all minted with the full-access scope, + // so defaulting an absent field to it is a fact, not a guess. + scope: z.string().default(OAUTH_SCOPE), }) .loose(); export type ProfileOAuth = z.infer; diff --git a/src/core/auth/storage.test.ts b/src/core/auth/storage.test.ts index 0173506..cf4c9c1 100644 --- a/src/core/auth/storage.test.ts +++ b/src/core/auth/storage.test.ts @@ -48,6 +48,7 @@ const OAUTH: OAuthCredential = { refreshToken: "refresh-1", expiresAt: "2026-06-08T13:00:00.000Z", clientId: "client-1", + scope: "mb:full", }; import { join } from "node:path"; @@ -447,6 +448,7 @@ describe("OAuth profiles (keyring backend)", () => { refreshToken: null, expiresAt: OAUTH.expiresAt, clientId: "client-1", + scope: "mb:full", }, lastProbe: null, lastFailure: null, @@ -545,6 +547,7 @@ describe("OAuth profiles (file fallback)", () => { refreshToken: "refresh-1", expiresAt: OAUTH.expiresAt, clientId: "client-1", + scope: "mb:full", }); expect(await readProfileCredential()).toEqual({ url: "https://m.example.com", @@ -552,6 +555,35 @@ describe("OAuth profiles (file fallback)", () => { }); }); + it("resolves a pre-scope profile record to the full-access scope", async () => { + const profilesPath = join(configDir(), "profiles.json"); + mkdirSync(dirname(profilesPath), { recursive: true }); + writeFileSync( + profilesPath, + JSON.stringify({ + profiles: [ + { + name: "default", + url: "https://m.example.com", + apiKey: null, + oauth: { + accessToken: "access-1", + refreshToken: "refresh-1", + expiresAt: OAUTH.expiresAt, + clientId: "client-1", + }, + lastProbe: null, + lastFailure: null, + }, + ], + }), + ); + expect(await readProfileCredential()).toEqual({ + url: "https://m.example.com", + credential: OAUTH, + }); + }); + it("does not flag a residual secret for an inline (file-fallback) profile", async () => { await writeOAuthProfile("https://m.example.com", OAUTH); // inlined, never in the keyring expect(await clearProfile()).toBe(true); diff --git a/src/core/auth/storage.ts b/src/core/auth/storage.ts index 4182fbc..d715b58 100644 --- a/src/core/auth/storage.ts +++ b/src/core/auth/storage.ts @@ -341,6 +341,7 @@ export function resolveRecordCredential(record: ProfileRecord): ResolvedCredenti refreshToken, expiresAt: record.oauth.expiresAt, clientId: record.oauth.clientId, + scope: record.oauth.scope, }, }; } @@ -425,6 +426,7 @@ export async function writeOAuthProfile( refreshToken: onFile ? credential.refreshToken : null, expiresAt: credential.expiresAt, clientId: credential.clientId, + scope: credential.scope, }; const file = await readProfilesFile(); const existing = findRecord(file, name); diff --git a/src/core/config.test.ts b/src/core/config.test.ts index df61d4d..3567654 100644 --- a/src/core/config.test.ts +++ b/src/core/config.test.ts @@ -59,6 +59,7 @@ const STORED_OAUTH: OAuthCredential = { refreshToken: "old-refresh", expiresAt: "2000-01-01T00:00:00.000Z", clientId: "c1", + scope: "mb:full", }; const REFRESHED_OAUTH: OAuthCredential = { @@ -67,6 +68,7 @@ const REFRESHED_OAUTH: OAuthCredential = { refreshToken: "refreshed-refresh", expiresAt: "2099-01-01T00:00:00.000Z", clientId: "c1", + scope: "mb:full", }; function clearConfigEnv(): void { diff --git a/src/core/http/client.test.ts b/src/core/http/client.test.ts index 57c647a..67b43be 100644 --- a/src/core/http/client.test.ts +++ b/src/core/http/client.test.ts @@ -479,6 +479,7 @@ const OAUTH: OAuthCredential = { refreshToken: "ref-1", expiresAt: "2026-06-08T13:00:00.000Z", clientId: "c1", + scope: "mb:full", }; function unauthorizedResponse(): Response { diff --git a/src/core/http/oauth.test.ts b/src/core/http/oauth.test.ts index 8c6c5e9..07962c6 100644 --- a/src/core/http/oauth.test.ts +++ b/src/core/http/oauth.test.ts @@ -13,6 +13,7 @@ import { refreshTokens, revokeToken, tryDiscoverMetadata, + WORKSPACE_MANAGER_SCOPE, } from "./oauth"; function installFetch(script: FetchScript): FetchCapture { @@ -82,6 +83,35 @@ describe("oauth HTTP boundary", () => { expect(await tryDiscoverMetadata("https://mb.example.com")).toBeNull(); }); + it("treats a server that does not advertise the requested narrow scope as unsupported", async () => { + installFetch([ + jsonResponse({ + issuer: "https://mb.example.com", + authorization_endpoint: "https://mb.example.com/oauth/authorize", + token_endpoint: TOKEN_ENDPOINT, + scopes_supported: ["agent:sql:read", OAUTH_SCOPE], + }), + ]); + expect(await tryDiscoverMetadata("https://mb.example.com", WORKSPACE_MANAGER_SCOPE)).toBeNull(); + }); + + it("accepts a server advertising the requested narrow scope", async () => { + installFetch([ + jsonResponse({ + issuer: "https://mb.example.com", + authorization_endpoint: "https://mb.example.com/oauth/authorize", + token_endpoint: TOKEN_ENDPOINT, + scopes_supported: [OAUTH_SCOPE, WORKSPACE_MANAGER_SCOPE], + }), + ]); + expect(await tryDiscoverMetadata("https://mb.example.com", WORKSPACE_MANAGER_SCOPE)).toEqual({ + issuer: "https://mb.example.com", + authorization_endpoint: "https://mb.example.com/oauth/authorize", + token_endpoint: TOKEN_ENDPOINT, + scopes_supported: [OAUTH_SCOPE, WORKSPACE_MANAGER_SCOPE], + }); + }); + it("accepts a discovery document that omits scopes_supported", async () => { installFetch([ jsonResponse({ diff --git a/src/core/http/oauth.ts b/src/core/http/oauth.ts index 20f1c75..dd33994 100644 --- a/src/core/http/oauth.ts +++ b/src/core/http/oauth.ts @@ -47,9 +47,10 @@ async function oauthFetch(request: OAuthRequest): Promise { } } -// The CLI always authorizes with the single full-access scope; it never requests anything narrower, -// so scope is a fixed constant rather than a per-call parameter. +// Default login requests the full-access scope; `mb auth login --workspace` narrows the grant to +// workspace CRUD so the token on an agent's machine structurally cannot mutate parent content. export const OAUTH_SCOPE = "mb:full"; +export const WORKSPACE_MANAGER_SCOPE = "mb:workspace-manager"; export const OAuthServerMetadata = z .object({ @@ -84,6 +85,7 @@ export interface ClientRegistration { registrationEndpoint: string; redirectUri: string; clientName: string; + scope: string; } export interface CodeExchange { @@ -177,7 +179,10 @@ export const OAUTH_UNSUPPORTED_MESSAGE = // The well-known path is appended after any subpath (base + /.well-known/...), not inserted // between host and path as RFC 8414 prescribes — Metabase routes it like /api/*, so a // subpath-hosted instance serves discovery under its prefix. -export async function tryDiscoverMetadata(baseUrl: string): Promise { +export async function tryDiscoverMetadata( + baseUrl: string, + requiredScope: string = OAUTH_SCOPE, +): Promise { const url = `${baseUrl}${DISCOVERY_PATH}`; const response = await oauthFetch({ url, @@ -190,10 +195,11 @@ export async function tryDiscoverMetadata(baseUrl: string): Promise { - const metadata = await tryDiscoverMetadata(baseUrl); +export async function discoverMetadata( + baseUrl: string, + requiredScope: string = OAUTH_SCOPE, +): Promise { + const metadata = await tryDiscoverMetadata(baseUrl, requiredScope); if (metadata === null) { throw new ConfigError(OAUTH_UNSUPPORTED_MESSAGE); } @@ -230,7 +239,7 @@ export async function registerClient(input: ClientRegistration): Promise { aborted: false, }); }); + + it("login --workspace refuses without a TTY (browser OAuth is the only scoped path)", async () => { + const configHome = await makeIsolatedConfigHome(); + + const login = await runCli({ + args: ["auth", "login", "--workspace", "--url", bootstrap.baseUrl, "--json"], + configHome, + }); + + expect(login.exitCode).toBe(2); + expect(cliErrorMessage(login.stderr)).toContain( + "--workspace login opens a browser and requires a TTY", + ); + expect(login.stdout).toBe(""); + }); + + it("login --workspace refuses an explicit --api-key (a key cannot carry a scope)", async () => { + const configHome = await makeIsolatedConfigHome(); + + const login = await runCli({ + args: [ + "auth", + "login", + "--workspace", + "--url", + bootstrap.baseUrl, + "--api-key", + bootstrap.adminApiKey, + "--json", + ], + configHome, + }); + + expect(login.exitCode).toBe(2); + expect(cliErrorMessage(login.stderr)).toContain( + "--workspace login is browser-OAuth only; an API key cannot carry a scope — omit --api-key", + ); + expect(login.stdout).toBe(""); + }); + + it("login --workspace refuses when MB_API_KEY is set rather than silently ignoring it", async () => { + const configHome = await makeIsolatedConfigHome(); + + const login = await runCli({ + args: ["auth", "login", "--workspace", "--url", bootstrap.baseUrl, "--json"], + configHome, + env: { MB_API_KEY: bootstrap.adminApiKey }, + }); + + expect(login.exitCode).toBe(2); + expect(cliErrorMessage(login.stderr)).toContain( + "--workspace login is browser-OAuth only; unset MB_API_KEY so it cannot shadow the scoped login", + ); + expect(login.stdout).toBe(""); + }); }); From bae194a3ee50b08012e275988f4863be87d09328 Mon Sep 17 00:00:00 2001 From: Bryan Maass Date: Mon, 6 Jul 2026 14:48:07 -0600 Subject: [PATCH 2/3] feat(workspace): stale-credential sweep at workspace create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tier 2 of the containment ladder: a stale full-power credential for the same parent (any api-key profile, any OAuth grant wider than mb:workspace-manager) would let a confused agent bypass the scoped token, so create refuses while one exists. Interactive runs offer revocation (local clear + best-effort server-side token revocation); non-interactive runs hard-refuse — --keep-existing-auth is the human-only override and is itself refused without a TTY. The profile the command runs as is exempt: it is in deliberate use, not lying around. connect gets the same sweep when it lands (GHY-4052). Refs GHY-4054 --- README.md | 11 +- src/commands/workspace/create.ts | 14 ++- src/commands/workspace/credential-sweep.ts | 95 ++++++++++++++++++ src/core/auth/stale-credentials.test.ts | 99 ++++++++++++++++++ src/core/auth/stale-credentials.ts | 51 ++++++++++ tests/e2e/workspace.e2e.test.ts | 111 +++++++++++++++++++++ 6 files changed, 375 insertions(+), 6 deletions(-) create mode 100644 src/commands/workspace/credential-sweep.ts create mode 100644 src/core/auth/stale-credentials.test.ts create mode 100644 src/core/auth/stale-credentials.ts diff --git a/README.md b/README.md index c50171b..c25b7e6 100644 --- a/README.md +++ b/README.md @@ -1325,10 +1325,13 @@ mb workspace create --name ws-reports --database-ids 1 mb workspace create --name ws-etl --database-ids 1,2 --json ``` -| Flag | Description | -| ---------------------- | ---------------------------------------- | -| `--name ` | Workspace name. | -| `--database-ids ` | Database ids to attach, comma separated. | +| Flag | Description | +| ---------------------- | ----------------------------------------------------------------------------------------------------------------------------- | +| `--name ` | Workspace name. | +| `--database-ids ` | Database ids to attach, comma separated. | +| `--keep-existing-auth` | Proceed despite broader same-server credentials in the profile store. Interactive only — refused in non-interactive contexts. | + +Before creating, the profile store is swept for broader credentials against the same server: any API key profile, or any OAuth profile wider than `mb:workspace-manager`. Interactive runs offer to revoke the offenders (local clear + best-effort server-side token revocation; API keys must additionally be deleted in Admin settings); non-interactive runs hard-refuse so an agent can't proceed while a full-power credential is in reach. ### `mb workspace destroy ` diff --git a/src/commands/workspace/create.ts b/src/commands/workspace/create.ts index af3bae3..f49d097 100644 --- a/src/commands/workspace/create.ts +++ b/src/commands/workspace/create.ts @@ -4,15 +4,18 @@ import { connectionFlags, outputFlags, profileFlag } from "../flags"; import { parseIdCsv } from "../parse-id"; import { defineMetabaseCommand } from "../runtime"; +import { enforceCredentialSweep, keepExistingAuthFlag } from "./credential-sweep"; + export default defineMetabaseCommand({ meta: { name: "create", description: "Create and provision a workspace" }, details: - "Attaches the given databases and provisions warehouse isolation (a temporary schema + user per database) before returning. Each database must be eligible for workspaces; provisioning is blocking, so the response carries the final per-database status.", + "Attaches the given databases and provisions warehouse isolation (a temporary schema + user per database) before returning. Each database must be eligible for workspaces; provisioning is blocking, so the response carries the final per-database status. Before creating, the profile store is swept for broader same-server credentials (any API key, any OAuth grant wider than mb:workspace-manager): interactive runs offer to revoke them, non-interactive runs refuse — --keep-existing-auth is the human-only override.", capabilities: { minVersion: 62, tokenFeature: "workspaces" }, args: { ...outputFlags, ...profileFlag, ...connectionFlags, + ...keepExistingAuthFlag, name: { type: "string", description: "Workspace name", required: true }, "database-ids": { type: "string", @@ -25,8 +28,15 @@ export default defineMetabaseCommand({ "mb workspace create --name ws-reports --database-ids 1", "mb workspace create --name ws-etl --database-ids 1,2 --json", ], - async run({ args, ctx, getClient }) { + async run({ args, ctx, getClient, getResolvedConfig }) { const databaseIds = parseIdCsv(args["database-ids"], "database id"); + const resolved = await getResolvedConfig(); + await enforceCredentialSweep({ + url: resolved.url, + profile: resolved.profile, + keepExistingAuth: args.keepExistingAuth === true, + action: "create a workspace", + }); const client = await getClient(); const created = await client.requestParsed(Workspace, "/api/ee/workspace-manager/", { method: "POST", diff --git a/src/commands/workspace/credential-sweep.ts b/src/commands/workspace/credential-sweep.ts new file mode 100644 index 0000000..24098d3 --- /dev/null +++ b/src/commands/workspace/credential-sweep.ts @@ -0,0 +1,95 @@ +import { revokeOAuthCredential } from "../../core/auth/oauth-session"; +import { + describeStaleCredential, + findStaleParentCredentials, + type StaleCredential, +} from "../../core/auth/stale-credentials"; +import { clearProfile, listProfileRecords, readProfileCredential } from "../../core/auth/storage"; +import { ConfigError, errorMessage } from "../../core/errors"; +import { warn } from "../../output/notice"; +import { promptConfirm } from "../../output/prompt"; + +export const keepExistingAuthFlag = { + keepExistingAuth: { + type: "boolean", + description: + "Proceed despite broader same-server credentials in the profile store (interactive only)", + alias: "keep-existing-auth", + default: false, + }, +} as const; + +export interface CredentialSweepInput { + url: string; + profile: string; + keepExistingAuth: boolean; + action: string; +} + +// Tier 2 of the containment ladder: a stale full-power credential for the same parent would let a +// confused agent bypass the workspace-scoped token, so workspace create/connect refuses to proceed +// while one exists. Interactive runs offer revocation; non-interactive (agent) runs hard-refuse — +// the override is deliberately human-only. +export async function enforceCredentialSweep(input: CredentialSweepInput): Promise { + const records = await listProfileRecords(); + const stale = findStaleParentCredentials(records, input.url, input.profile); + if (stale.length === 0) { + return; + } + const listing = stale.map(describeStaleCredential).join(", "); + const interactive = process.stdin.isTTY === true; + + if (input.keepExistingAuth) { + if (!interactive) { + throw new ConfigError( + "--keep-existing-auth requires an interactive terminal; refusing to proceed with broader credentials in a non-interactive context", + ); + } + warn(`proceeding with broader credentials left in the profile store: ${listing}`); + return; + } + + if (!interactive) { + throw new ConfigError( + `refusing to ${input.action}: broader credentials for this server exist in the profile store (${listing}) — revoke them with \`mb auth logout --profile \` first; a human can override with --keep-existing-auth`, + ); + } + + const ok = await promptConfirm({ + message: `Broader credentials for this server exist (${listing}). Revoke them before continuing?`, + initialValue: true, + }); + if (!ok) { + throw new ConfigError( + `aborted: revoke the broader credentials (${listing}) with \`mb auth logout --profile \` or pass --keep-existing-auth`, + ); + } + for (const credential of stale) { + await revokeStaleCredential(credential); + } +} + +// Mirrors logout: durable local clear first, then best-effort server-side revocation for OAuth. +// API keys cannot be revoked from here — the server-side key must be deleted in Admin settings. +async function revokeStaleCredential(stale: StaleCredential): Promise { + const resolved = await readProfileCredential(stale.profile); + await clearProfile(stale.profile); + if (resolved === null || resolved.credential.kind !== "oauth") { + warn( + `cleared profile "${stale.profile}"; its API key is still active server-side — delete it in Admin settings → Authentication → API keys`, + ); + return; + } + try { + const revoked = await revokeOAuthCredential(resolved.url, resolved.credential); + if (!revoked) { + warn( + `cleared profile "${stale.profile}", but the server advertises no revocation endpoint; its tokens remain valid until they expire`, + ); + } + } catch (error) { + warn( + `cleared profile "${stale.profile}", but revoking server-side failed: ${errorMessage(error)}`, + ); + } +} diff --git a/src/core/auth/stale-credentials.test.ts b/src/core/auth/stale-credentials.test.ts new file mode 100644 index 0000000..cd7f497 --- /dev/null +++ b/src/core/auth/stale-credentials.test.ts @@ -0,0 +1,99 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import type { ProfileRecord } from "./profile-record"; +import { describeStaleCredential, findStaleParentCredentials } from "./stale-credentials"; + +const PARENT_URL = "https://parent.example.com"; + +function apiKeyRecord(name: string, url: string): ProfileRecord { + return { name, url, apiKey: "mb_stale_key", oauth: null, lastProbe: null, lastFailure: null }; +} + +function oauthRecord(name: string, url: string, scope: string): ProfileRecord { + return { + name, + url, + apiKey: null, + oauth: { + accessToken: "acc", + refreshToken: "ref", + expiresAt: "2099-01-01T00:00:00.000Z", + clientId: "c1", + scope, + }, + lastProbe: null, + lastFailure: null, + }; +} + +describe("findStaleParentCredentials", () => { + beforeEach(() => { + process.env["MB_CLI_DISABLE_KEYRING"] = "1"; + }); + + afterEach(() => { + delete process.env["MB_CLI_DISABLE_KEYRING"]; + }); + + it("flags a full-scope OAuth profile and an api-key profile for the same server", () => { + const records = [ + oauthRecord("old-login", PARENT_URL, "mb:full"), + apiKeyRecord("ci-admin", PARENT_URL), + ]; + expect(findStaleParentCredentials(records, PARENT_URL, "agent")).toEqual([ + { profile: "old-login", kind: "oauth", scope: "mb:full" }, + { profile: "ci-admin", kind: "apiKey" }, + ]); + }); + + it("does not flag a workspace-scoped OAuth profile", () => { + const records = [oauthRecord("agent-safe", PARENT_URL, "mb:workspace-manager")]; + expect(findStaleParentCredentials(records, PARENT_URL, "agent")).toEqual([]); + }); + + it("ignores credentials for a different server", () => { + const records = [ + apiKeyRecord("other-host", "https://other.example.com"), + oauthRecord("other-login", "https://other.example.com", "mb:full"), + ]; + expect(findStaleParentCredentials(records, PARENT_URL, "agent")).toEqual([]); + }); + + it("matches the same server across trailing-slash differences", () => { + const records = [apiKeyRecord("slashed", `${PARENT_URL}/`)]; + expect(findStaleParentCredentials(records, PARENT_URL, "agent")).toEqual([ + { profile: "slashed", kind: "apiKey" }, + ]); + }); + + it("exempts the profile the command runs as", () => { + const records = [apiKeyRecord("agent", PARENT_URL)]; + expect(findStaleParentCredentials(records, PARENT_URL, "agent")).toEqual([]); + }); + + it("ignores a record whose credential was already cleared", () => { + const cleared: ProfileRecord = { + name: "logged-out", + url: PARENT_URL, + apiKey: null, + oauth: null, + lastProbe: null, + lastFailure: null, + }; + expect(findStaleParentCredentials([cleared], PARENT_URL, "agent")).toEqual([]); + }); +}); + +describe("describeStaleCredential", () => { + it("names an api-key offender", () => { + expect(describeStaleCredential({ profile: "ci-admin", kind: "apiKey" })).toBe( + "ci-admin (api key)", + ); + }); + + it("names an oauth offender with its scope", () => { + expect(describeStaleCredential({ profile: "old-login", kind: "oauth", scope: "mb:full" })).toBe( + "old-login (mb:full)", + ); + }); +}); diff --git a/src/core/auth/stale-credentials.ts b/src/core/auth/stale-credentials.ts new file mode 100644 index 0000000..38faf51 --- /dev/null +++ b/src/core/auth/stale-credentials.ts @@ -0,0 +1,51 @@ +import { WORKSPACE_MANAGER_SCOPE } from "../http/oauth"; +import { normalizeUrl } from "../url"; + +import type { ProfileRecord } from "./profile-record"; +import { resolveRecordCredential } from "./storage"; + +export interface StaleApiKeyCredential { + profile: string; + kind: "apiKey"; +} + +export interface StaleOAuthCredential { + profile: string; + kind: "oauth"; + scope: string; +} + +export type StaleCredential = StaleApiKeyCredential | StaleOAuthCredential; + +// A credential is "stale" for workspace purposes when it targets the same parent instance and is +// broader than workspace CRUD: any API key (keys are unscoped, so full power) or any OAuth grant +// wider than the workspace-manager scope. The profile the command itself runs as is exempt — it +// is in deliberate use, not lying around. +export function findStaleParentCredentials( + records: readonly ProfileRecord[], + targetUrl: string, + currentProfile: string, +): StaleCredential[] { + const target = normalizeUrl(targetUrl); + const stale: StaleCredential[] = []; + for (const record of records) { + if (record.name === currentProfile || normalizeUrl(record.url) !== target) { + continue; + } + const resolved = resolveRecordCredential(record); + if (resolved === null) { + continue; + } + if (resolved.credential.kind === "apiKey") { + stale.push({ profile: record.name, kind: "apiKey" }); + } else if (resolved.credential.scope !== WORKSPACE_MANAGER_SCOPE) { + stale.push({ profile: record.name, kind: "oauth", scope: resolved.credential.scope }); + } + } + return stale; +} + +export function describeStaleCredential(credential: StaleCredential): string { + const detail = credential.kind === "apiKey" ? "api key" : credential.scope; + return `${credential.profile} (${detail})`; +} diff --git a/tests/e2e/workspace.e2e.test.ts b/tests/e2e/workspace.e2e.test.ts index 1dcd50e..26e3ddf 100644 --- a/tests/e2e/workspace.e2e.test.ts +++ b/tests/e2e/workspace.e2e.test.ts @@ -233,6 +233,117 @@ describe("workspace arg validation e2e (no Metabase contact required)", () => { }); }); +describe("workspace credential sweep e2e", () => { + let bootstrap: E2EBootstrap; + const tempDirs: string[] = []; + + beforeAll(async () => { + bootstrap = await readBootstrap(); + }); + + afterEach(async () => { + await Promise.all(tempDirs.splice(0).map(cleanupConfigHome)); + }); + + async function makeIsolatedConfigHome(): Promise { + const dir = await mkTempConfigHome(); + tempDirs.push(dir); + return dir; + } + + async function seedStaleAdminProfile(configHome: string): Promise { + const login = await runCli({ + args: ["auth", "login", "--profile", "stale-admin", "--url", bootstrap.baseUrl, "--json"], + configHome, + stdin: bootstrap.adminApiKey, + }); + expect(login.exitCode, login.stderr).toBe(0); + } + + it("create hard-refuses non-interactively while a broader same-server credential exists", async () => { + const configHome = await makeIsolatedConfigHome(); + await seedStaleAdminProfile(configHome); + + const result = await runCli({ + args: ["workspace", "create", "--name", "ws", "--database-ids", "1", "--json"], + configHome, + env: { + MB_URL: bootstrap.baseUrl, + MB_API_KEY: bootstrap.adminApiKey, + }, + }); + + expect(result.exitCode).toBe(2); + expect(cliErrorMessage(result.stderr)).toContain( + "refusing to create a workspace: broader credentials for this server exist in the profile store (stale-admin (api key)) — revoke them with `mb auth logout --profile ` first; a human can override with --keep-existing-auth", + ); + expect(result.stdout).toBe(""); + }); + + it("--keep-existing-auth is refused in a non-interactive context (human-only override)", async () => { + const configHome = await makeIsolatedConfigHome(); + await seedStaleAdminProfile(configHome); + + const result = await runCli({ + args: [ + "workspace", + "create", + "--name", + "ws", + "--database-ids", + "1", + "--keep-existing-auth", + "--json", + ], + configHome, + env: { + MB_URL: bootstrap.baseUrl, + MB_API_KEY: bootstrap.adminApiKey, + }, + }); + + expect(result.exitCode).toBe(2); + expect(cliErrorMessage(result.stderr)).toContain( + "--keep-existing-auth requires an interactive terminal; refusing to proceed with broader credentials in a non-interactive context", + ); + expect(result.stdout).toBe(""); + }); + + it("a credential for a different server does not trip the sweep (fails later, not on the sweep)", async () => { + const configHome = await makeIsolatedConfigHome(); + const login = await runCli({ + args: [ + "auth", + "login", + "--profile", + "other-host", + "--url", + "https://other.example.com", + "--skip-verify", + "--json", + ], + configHome, + stdin: bootstrap.adminApiKey, + }); + expect(login.exitCode, login.stderr).toBe(0); + + const result = await runCli({ + args: ["workspace", "create", "--name", "ws", "--database-ids", "1", "--json"], + configHome, + env: { + MB_URL: bootstrap.baseUrl, + MB_API_KEY: bootstrap.adminApiKey, + }, + }); + + // The sweep passes; the create then fails server-side (ineligible database or missing + // endpoint depending on the booted image), so the only stable assertions are "not the + // sweep's refusal" and the HttpError exit code. + expect(result.exitCode).toBe(1); + expect(cliErrorMessage(result.stderr)).not.toContain("broader credentials"); + }); +}); + describe.skipIf(!serverVersionBelow(WORKSPACE_MIN_VERSION))( "workspace capability gate against a sub-v62 server", () => { From e5dc844c846f3e4e095897177a2ab47c1c4b71c0 Mon Sep 17 00:00:00 2001 From: Bryan Maass Date: Wed, 8 Jul 2026 09:19:28 -0600 Subject: [PATCH 3/3] test: hoist enrichScopeForbiddenError fixtures to module scope Newer oxlint flags helpers that capture nothing from their describe. Refs GHY-4053 --- src/commands/runtime.test.ts | 48 ++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/commands/runtime.test.ts b/src/commands/runtime.test.ts index 69edb29..41d49cf 100644 --- a/src/commands/runtime.test.ts +++ b/src/commands/runtime.test.ts @@ -321,32 +321,32 @@ describe("defineMetabaseCommand", () => { }); }); -describe("enrichScopeForbiddenError", () => { - const FORBIDDEN = new HttpError({ - status: 403, - statusText: "Forbidden", - method: "POST", - url: "https://m.example.com/api/card", - responseHeaders: {}, - rawBody: null, - }); +const FORBIDDEN = new HttpError({ + status: 403, + statusText: "Forbidden", + method: "POST", + url: "https://m.example.com/api/card", + responseHeaders: {}, + rawBody: null, +}); - function oauthConfig(scope: string): ResolvedConfig { - return { - url: "https://m.example.com", - profile: "parent", - source: "stored", - credential: { - kind: "oauth", - accessToken: "acc", - refreshToken: "ref", - expiresAt: "2099-01-01T00:00:00.000Z", - clientId: "c1", - scope, - }, - }; - } +function oauthConfig(scope: string): ResolvedConfig { + return { + url: "https://m.example.com", + profile: "parent", + source: "stored", + credential: { + kind: "oauth", + accessToken: "acc", + refreshToken: "ref", + expiresAt: "2099-01-01T00:00:00.000Z", + clientId: "c1", + scope, + }, + }; +} +describe("enrichScopeForbiddenError", () => { it("converts a 403 on a workspace-scoped profile into a ConfigError naming the scope", () => { const result = enrichScopeForbiddenError(FORBIDDEN, oauthConfig("mb:workspace-manager")); expect(result).toBeInstanceOf(ConfigError);