From 1432e2a9878b0d9b65a757706c9613c22d78d304 Mon Sep 17 00:00:00 2001 From: Tom Smallwood Date: Fri, 24 Jul 2026 18:37:51 -0700 Subject: [PATCH 1/2] fix(auth): let users choose which account to sign in with MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Browser-handoff auth opened the login URL with no `prompt` parameter, so the identity provider silently reused whatever session the default browser already held. A user whose browser is signed into a personal account had no way to authenticate as a different (work) account, and signing out did not help: `clear_builderlab_auth` drops only Buzz's own session, never the browser's, so sign out -> sign in returned the same account without ever showing a chooser. Add an explicit account-selection path that sends `prompt=select_account`: - desktop: `login_url` takes `force_account_selection`; `start_builderlab_login` accepts it as an optional command argument (defaults to today's behavior). - desktop UI: "Use a different account" next to Sign out, and alongside the signed-out Sign in button. The signed-in path clears the local session first so a cancelled switch doesn't leave a stale account on screen. - buzz-agent: extract the authorize-URL construction into `authorize_url` and thread the flag through `browser_pkce_flow`; `interactive_login` takes it, exposed as `buzz-agent auth --select-account`. Implicit first-run auth is unchanged — no chooser is forced on a path the user did not initiate, and providers that reject the parameter never see it. Fixes #2802 Signed-off-by: Tom Smallwood --- crates/buzz-agent/src/auth.rs | 109 ++++++++++++++++-- crates/buzz-agent/src/lib.rs | 15 ++- desktop/src-tauri/src/builderlab.rs | 44 ++++++- .../communities/hostedCommunityApi.ts | 14 ++- .../ui/HostedCommunitiesSettingsCard.tsx | 94 +++++++++++---- 5 files changed, 233 insertions(+), 43 deletions(-) diff --git a/crates/buzz-agent/src/auth.rs b/crates/buzz-agent/src/auth.rs index 34974fbf0b..2270922486 100644 --- a/crates/buzz-agent/src/auth.rs +++ b/crates/buzz-agent/src/auth.rs @@ -232,9 +232,18 @@ impl PkceOAuthTokenSource { /// Run the full browser-mediated Authorization Code + PKCE flow. /// Caller must hold a TTY/browser: this opens a window and blocks. - pub async fn interactive_login(&self) -> Result<(), AgentError> { + /// + /// `force_account_selection` adds `prompt=select_account` to the + /// authorization request. Without it the provider silently reuses whatever + /// session the default browser already holds, so a user signed into the + /// wrong account has no way to reach a different one. Pass `true` for + /// explicitly user-initiated logins ("use a different account"); leave it + /// `false` on implicit first-run auth, where an unnecessary account chooser + /// is friction and some providers reject the parameter. + pub async fn interactive_login(&self, force_account_selection: bool) -> Result<(), AgentError> { let endpoints = self.endpoints().await?; - let token = browser_pkce_flow(&self.http, &self.cfg, &endpoints).await?; + let token = + browser_pkce_flow(&self.http, &self.cfg, &endpoints, force_account_selection).await?; let mut state = self.state.lock().await; self.save(&mut state, token)?; Ok(()) @@ -289,8 +298,10 @@ impl TokenSource for PkceOAuthTokenSource { } } - // 5. No usable cache: full browser dance. - let fresh = browser_pkce_flow(&self.http, &self.cfg, &endpoints).await?; + // 5. No usable cache: full browser dance. No forced account chooser — + // this path is reached implicitly mid-request, not by a user who + // asked to switch accounts (see `interactive_login`). + let fresh = browser_pkce_flow(&self.http, &self.cfg, &endpoints, false).await?; let bearer = fresh.access_token.clone(); self.save(&mut state, fresh)?; Ok(bearer) @@ -521,6 +532,34 @@ fn random_state() -> Result { Ok(base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(bytes)) } +/// Build the authorization-endpoint URL for one PKCE attempt. +/// +/// When `force_account_selection` is set the request carries +/// `prompt=select_account`, which tells the provider to show its account +/// chooser instead of silently reusing the browser's current session. +fn authorize_url( + endpoints: &OidcEndpoints, + cfg: &PkceOAuthConfig, + redirect_uri: &str, + state: &str, + challenge: &str, + force_account_selection: bool, +) -> String { + let mut url = format!( + "{}?response_type=code&client_id={}&redirect_uri={}&scope={}&state={}&code_challenge={}&code_challenge_method=S256", + endpoints.authorization_endpoint, + urlencoding::encode(&cfg.client_id), + urlencoding::encode(redirect_uri), + urlencoding::encode(&cfg.scopes.join(" ")), + urlencoding::encode(state), + urlencoding::encode(challenge), + ); + if force_account_selection { + url.push_str("&prompt=select_account"); + } + url +} + /// Spin up a localhost callback server, open the authorize URL in a /// browser, wait up to [`BROWSER_AUTH_TIMEOUT`] for the redirect, then /// exchange the code for a token. @@ -528,6 +567,7 @@ async fn browser_pkce_flow( http: &Client, cfg: &PkceOAuthConfig, endpoints: &OidcEndpoints, + force_account_selection: bool, ) -> Result { use axum::{extract::Query, response::Html, routing::get, Router}; use std::collections::HashMap; @@ -585,14 +625,13 @@ async fn browser_pkce_flow( let _ = axum::serve(listener, app).await; })); - let auth_url = format!( - "{}?response_type=code&client_id={}&redirect_uri={}&scope={}&state={}&code_challenge={}&code_challenge_method=S256", - endpoints.authorization_endpoint, - urlencoding::encode(&cfg.client_id), - urlencoding::encode(&redirect_uri), - urlencoding::encode(&cfg.scopes.join(" ")), - urlencoding::encode(&state), - urlencoding::encode(&challenge), + let auth_url = authorize_url( + endpoints, + cfg, + &redirect_uri, + &state, + &challenge, + force_account_selection, ); eprintln!("Opening browser for authentication. If it doesn't open, visit:\n {auth_url}"); @@ -642,6 +681,52 @@ mod tests { assert_eq!(expected, challenge); } + fn test_authorize_url(force_account_selection: bool) -> String { + let endpoints = OidcEndpoints { + authorization_endpoint: "https://accounts.example.com/authorize".into(), + token_endpoint: "https://accounts.example.com/token".into(), + }; + let cfg = PkceOAuthConfig { + discovery_url: "https://accounts.example.com/.well-known/openid-configuration".into(), + client_id: "client-123".into(), + scopes: vec!["openid".into(), "email".into()], + cache_namespace: "example".into(), + cache_dir_override: None, + }; + authorize_url( + &endpoints, + &cfg, + "http://localhost:1234", + "state-abc", + "challenge-xyz", + force_account_selection, + ) + } + + #[test] + fn authorize_url_omits_prompt_by_default() { + let url = test_authorize_url(false); + assert!(url.starts_with("https://accounts.example.com/authorize?")); + assert!(url.contains("client_id=client-123")); + assert!(url.contains("scope=openid%20email")); + assert!( + !url.contains("prompt="), + "implicit auth must not force an account chooser: {url}" + ); + } + + #[test] + fn authorize_url_requests_account_chooser_when_forced() { + let url = test_authorize_url(true); + assert!( + url.contains("&prompt=select_account"), + "explicit re-auth must let the user pick an account: {url}" + ); + // The rest of the request is unchanged by the added parameter. + assert!(url.contains("code_challenge_method=S256")); + assert!(url.contains("state=state-abc")); + } + #[test] fn cached_token_no_expiry_is_not_expired() { let t = CachedToken { diff --git a/crates/buzz-agent/src/lib.rs b/crates/buzz-agent/src/lib.rs index f5faafe853..27a6760440 100644 --- a/crates/buzz-agent/src/lib.rs +++ b/crates/buzz-agent/src/lib.rs @@ -125,9 +125,16 @@ pub fn run() -> Result<(), Box> { /// `buzz-agent auth ` — run the interactive auth flow for a /// provider and persist the result, then exit. Today this supports Databricks /// OAuth 2.0 PKCE. Reads `DATABRICKS_HOST` from env; needs a browser on the -/// machine. +/// machine. Pass `--select-account` to force the provider's account chooser +/// rather than reusing whatever session the browser already holds. async fn auth_subcommand(args: &[String]) -> Result<(), Box> { let provider = args.first().map(String::as_str); + // `--select-account` forces the provider's account chooser instead of + // silently reusing the browser's current session — the only way to + // authenticate as an account other than the one already signed in. + let force_account_selection = args + .iter() + .any(|arg| arg == "--select-account" || arg == "--switch-account"); match provider { Some("databricks" | "databricks_v2" | "databricks-v2") => { let host = std::env::var("DATABRICKS_HOST") @@ -143,12 +150,14 @@ async fn auth_subcommand(args: &[String]) -> Result<(), Box Err(format!("auth: unknown provider {other:?}").into()), - None => Err("auth: provider required (try: buzz-agent auth databricks)".into()), + None => Err( + "auth: provider required (try: buzz-agent auth databricks [--select-account])".into(), + ), } } diff --git a/desktop/src-tauri/src/builderlab.rs b/desktop/src-tauri/src/builderlab.rs index 1252946c58..f5b340b59a 100644 --- a/desktop/src-tauri/src/builderlab.rs +++ b/desktop/src-tauri/src/builderlab.rs @@ -214,13 +214,26 @@ fn api_url(path: &str) -> Result { .map_err(|error| format!("invalid Builderlab API URL: {error}")) } -fn login_url(return_to: &str) -> Result { +/// Build the Builderlab login URL Buzz hands to the OS browser. +/// +/// With `force_account_selection` the request carries `prompt=select_account`, +/// which asks the identity provider for its account chooser. Without it the +/// browser's existing session is reused silently: a user whose browser is +/// signed into the wrong account (personal vs. work) lands back in Buzz as +/// that account with no chance to switch, and local sign-out cannot help +/// because it clears only Buzz's own session, never the browser's. +fn login_url(return_to: &str, force_account_selection: bool) -> Result { let mut login_url = api_url("/v1/auth/login")?; login_url .query_pairs_mut() .append_pair("type", "cli") .append_pair("product", "buzz") .append_pair("returnTo", return_to); + if force_account_selection { + login_url + .query_pairs_mut() + .append_pair("prompt", "select_account"); + } Ok(login_url) } @@ -247,12 +260,18 @@ async fn authenticated_user( .map_err(|error| format!("invalid Builderlab session response: {error}")) } +/// Start the browser-mediated Builderlab login. +/// +/// `force_account_selection` (default `false`) requests the identity +/// provider's account chooser instead of reusing the browser's current +/// session — the path behind the UI's "Use a different account". #[tauri::command] pub(crate) async fn start_builderlab_login( app: tauri::AppHandle, app_state: tauri::State<'_, crate::app_state::AppState>, session: tauri::State<'_, BuilderlabSession>, login: tauri::State<'_, BuilderlabLogin>, + force_account_selection: Option, ) -> Result { let listener = TcpListener::bind("127.0.0.1:0") .await @@ -275,7 +294,7 @@ pub(crate) async fn start_builderlab_login( let _ = axum::serve(listener, router).await; }); - let login_url = login_url(&return_to)?; + let login_url = login_url(&return_to, force_account_selection.unwrap_or(false))?; if let Err(error) = app.opener().open_url(login_url.as_str(), None::<&str>) { server.abort(); return Err(format!("could not open Builderlab authentication: {error}")); @@ -673,7 +692,7 @@ mod tests { #[test] fn login_defaults_to_auth0_login() { - let login = login_url("http://127.0.0.1:1234/callback/nonce").unwrap(); + let login = login_url("http://127.0.0.1:1234/callback/nonce", false).unwrap(); let query: HashMap<_, _> = login.query_pairs().into_owned().collect(); assert_eq!(query.get("type").map(String::as_str), Some("cli")); @@ -683,5 +702,24 @@ mod tests { Some("http://127.0.0.1:1234/callback/nonce") ); assert!(!query.contains_key("screen_hint")); + assert!(!query.contains_key("prompt")); + } + + #[test] + fn login_can_force_the_account_chooser() { + let login = login_url("http://127.0.0.1:1234/callback/nonce", true).unwrap(); + let query: HashMap<_, _> = login.query_pairs().into_owned().collect(); + + assert_eq!( + query.get("prompt").map(String::as_str), + Some("select_account") + ); + // Forcing the chooser must not disturb the rest of the request. + assert_eq!(query.get("type").map(String::as_str), Some("cli")); + assert_eq!(query.get("product").map(String::as_str), Some("buzz")); + assert_eq!( + query.get("returnTo").map(String::as_str), + Some("http://127.0.0.1:1234/callback/nonce") + ); } } diff --git a/desktop/src/features/communities/hostedCommunityApi.ts b/desktop/src/features/communities/hostedCommunityApi.ts index 16f9f92567..1ad480985e 100644 --- a/desktop/src/features/communities/hostedCommunityApi.ts +++ b/desktop/src/features/communities/hostedCommunityApi.ts @@ -102,8 +102,18 @@ export function clearBuilderlabAuth() { return invoke("clear_builderlab_auth"); } -export function startBuilderlabLogin() { - return invoke("start_builderlab_login"); +/** + * Start the browser sign-in flow. + * + * `forceAccountSelection` asks the identity provider for its account chooser + * instead of silently reusing the browser's current session — without it a + * browser signed into the wrong account can never reach a different one, since + * signing out of Buzz clears only Buzz's session. + */ +export function startBuilderlabLogin(forceAccountSelection = false) { + return invoke("start_builderlab_login", { + forceAccountSelection, + }); } export async function loadHostedCommunityAccount(): Promise { diff --git a/desktop/src/features/settings/ui/HostedCommunitiesSettingsCard.tsx b/desktop/src/features/settings/ui/HostedCommunitiesSettingsCard.tsx index 9c16359b45..06250d962c 100644 --- a/desktop/src/features/settings/ui/HostedCommunitiesSettingsCard.tsx +++ b/desktop/src/features/settings/ui/HostedCommunitiesSettingsCard.tsx @@ -11,6 +11,7 @@ import { LogOut, RefreshCw, Unlink, + UserRoundCog, } from "lucide-react"; import { useIdentityQuery } from "@/shared/api/hooks"; @@ -135,9 +136,36 @@ export function HostedCommunitiesSettingsCard() { } }; - const signIn = () => - run("Signing in…", async () => { - const nextAuth = await invoke("start_builderlab_login"); + // `forceAccountSelection` asks the identity provider for its account chooser. + // Without it the browser's existing session is reused silently, so a browser + // signed into the wrong account (personal vs. work) can never reach another + // one — signing out clears Buzz's session, not the browser's. + const signIn = (forceAccountSelection = false) => + run( + forceAccountSelection ? "Choosing account…" : "Signing in…", + async () => { + const nextAuth = await invoke( + "start_builderlab_login", + { + forceAccountSelection, + }, + ); + setAuth(nextAuth); + await loadAccount(); + }, + ); + + const switchAccount = () => + run("Choosing account…", async () => { + // Drop the local session first so a cancelled or failed switch doesn't + // leave the UI showing an account the user meant to leave. + await invoke("clear_builderlab_auth"); + setAuth(null); + setIdentity(null); + setCommunities([]); + const nextAuth = await invoke("start_builderlab_login", { + forceAccountSelection: true, + }); setAuth(nextAuth); await loadAccount(); }); @@ -427,18 +455,28 @@ export function HostedCommunitiesSettingsCard() { Authentication opens in your browser and returns securely to Buzz. You can use every other part of the app without signing in.

- +
+ + +
+

+ Your browser may already be signed into an account. Use a different + account to pick which one Buzz signs in with. +

) : ( <> @@ -451,14 +489,24 @@ export function HostedCommunitiesSettingsCard() {

{auth.email}

) : null} - +
+ + +
{!identity ? ( From 9af56144cc97e1b6f01d402fd904a4ff940cdfeb Mon Sep 17 00:00:00 2001 From: Tom Smallwood Date: Fri, 24 Jul 2026 22:49:56 -0700 Subject: [PATCH 2/2] test(desktop): cover the account-selection sign-in path Regression coverage for the account chooser: asserts the plain sign-in still sends `forceAccountSelection: false`, that "Use a different account" sends `true` from both the signed-out and signed-in states, and that the signed-in path clears the local session before opening the browser. Also captures the two settings-card states for review, following the existing *-screenshots.spec.ts pattern, and adds `hosted-communities` to the `openSettings` helper's section union so the section is reachable from tests like every other one. Signed-off-by: Tom Smallwood --- desktop/playwright.config.ts | 1 + ...sted-communities-account-selection.spec.ts | 81 +++++++++++++++++++ desktop/tests/helpers/settings.ts | 1 + 3 files changed, 83 insertions(+) create mode 100644 desktop/tests/e2e/hosted-communities-account-selection.spec.ts diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 5046b29688..9495f668de 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -30,6 +30,7 @@ export default defineConfig({ "**/badge.spec.ts", "**/channel-browser.spec.ts", "**/channel-add-screenshots.spec.ts", + "**/hosted-communities-account-selection.spec.ts", "**/messaging.spec.ts", "**/custom-emoji.spec.ts", "**/profile-custom-emoji-status.spec.ts", diff --git a/desktop/tests/e2e/hosted-communities-account-selection.spec.ts b/desktop/tests/e2e/hosted-communities-account-selection.spec.ts new file mode 100644 index 0000000000..9fc3d55f04 --- /dev/null +++ b/desktop/tests/e2e/hosted-communities-account-selection.spec.ts @@ -0,0 +1,81 @@ +import { expect, test } from "@playwright/test"; + +import { installMockBridge } from "../helpers/bridge"; +import { openSettings } from "../helpers/settings"; +import { waitForAnimations } from "../helpers/animations"; + +const OUTDIR = "test-results/hosted-communities-account-selection"; + +const MOCK_AUTH = { + email: "personal@example.com", + name: "Personal Account", + expiresAt: "2099-01-01T00:00:00Z", +}; + +/** Payload the app sent with the most recent `start_builderlab_login`. */ +async function lastLoginPayload(page: import("@playwright/test").Page) { + return page.evaluate(() => { + const log = window.__BUZZ_E2E_COMMAND_LOG__ ?? []; + const entry = [...log] + .reverse() + .find((call) => call.command === "start_builderlab_login"); + return (entry?.payload ?? null) as { + forceAccountSelection?: boolean; + } | null; + }); +} + +async function openHostedCommunities(page: import("@playwright/test").Page) { + await page.goto("/"); + await openSettings(page, "hosted-communities"); +} + +test("signed out: plain sign-in reuses the browser session", async ({ + page, +}) => { + await installMockBridge(page); + await openHostedCommunities(page); + + const signIn = page.getByRole("button", { name: "Sign in with Builderlab" }); + await signIn.waitFor(); + await waitForAnimations(page); + await page.screenshot({ path: `${OUTDIR}/01-signed-out.png` }); + + await signIn.click(); + expect(await lastLoginPayload(page)).toEqual({ + forceAccountSelection: false, + }); +}); + +test("signed out: 'Use a different account' asks for the account chooser", async ({ + page, +}) => { + await installMockBridge(page); + await openHostedCommunities(page); + + await page.getByRole("button", { name: "Use a different account" }).click(); + expect(await lastLoginPayload(page)).toEqual({ forceAccountSelection: true }); +}); + +test("signed in: 'Use a different account' clears the session and asks for the chooser", async ({ + page, +}) => { + await installMockBridge(page, { builderlabAuth: MOCK_AUTH }); + await openHostedCommunities(page); + + await expect(page.getByText(MOCK_AUTH.name)).toBeVisible(); + await waitForAnimations(page); + await page.screenshot({ path: `${OUTDIR}/02-signed-in.png` }); + + await page.getByRole("button", { name: "Use a different account" }).click(); + + expect(await lastLoginPayload(page)).toEqual({ forceAccountSelection: true }); + // The local session is dropped first, so a cancelled switch cannot leave the + // previous account on screen. + const commands = await page.evaluate(() => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).map((call) => call.command), + ); + expect(commands.indexOf("clear_builderlab_auth")).toBeLessThan( + commands.lastIndexOf("start_builderlab_login"), + ); +}); diff --git a/desktop/tests/helpers/settings.ts b/desktop/tests/helpers/settings.ts index 301e622719..a42a886f80 100644 --- a/desktop/tests/helpers/settings.ts +++ b/desktop/tests/helpers/settings.ts @@ -10,6 +10,7 @@ type SettingsSection = | "shortcuts" | "tokens" | "community-members" + | "hosted-communities" | "mobile" | "updates";