Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 97 additions & 12 deletions crates/buzz-agent/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -521,13 +532,42 @@ fn random_state() -> Result<String, AgentError> {
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.
async fn browser_pkce_flow(
http: &Client,
cfg: &PkceOAuthConfig,
endpoints: &OidcEndpoints,
force_account_selection: bool,
) -> Result<CachedToken, AgentError> {
use axum::{extract::Query, response::Html, routing::get, Router};
use std::collections::HashMap;
Expand Down Expand Up @@ -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}");
Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 12 additions & 3 deletions crates/buzz-agent/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,16 @@ pub fn run() -> Result<(), Box<dyn std::error::Error>> {
/// `buzz-agent auth <provider>` — 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<dyn std::error::Error>> {
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")
Expand All @@ -143,12 +150,14 @@ async fn auth_subcommand(args: &[String]) -> Result<(), Box<dyn std::error::Erro
cache_dir_override: None,
};
let src = auth::PkceOAuthTokenSource::new(pkce)?;
src.interactive_login().await?;
src.interactive_login(force_account_selection).await?;
eprintln!("Authenticated. Token cached under ~/.config/buzz-agent/oauth/databricks/.");
Ok(())
}
Some(other) => 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(),
),
}
}

Expand Down
1 change: 1 addition & 0 deletions desktop/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export default defineConfig({
"**/channel-browser.spec.ts",
"**/channel-add-screenshots.spec.ts",
"**/add-community-screenshots.spec.ts",
"**/hosted-communities-account-selection.spec.ts",
"**/hosted-communities-settings-screenshots.spec.ts",
"**/invites-settings-screenshots.spec.ts",
"**/messaging.spec.ts",
Expand Down
44 changes: 41 additions & 3 deletions desktop/src-tauri/src/builderlab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,26 @@ fn api_url(path: &str) -> Result<Url, String> {
.map_err(|error| format!("invalid Builderlab API URL: {error}"))
}

fn login_url(return_to: &str) -> Result<Url, String> {
/// 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<Url, String> {
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)
}

Expand All @@ -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<bool>,
) -> Result<BuilderlabAuthInfo, String> {
let listener = TcpListener::bind("127.0.0.1:0")
.await
Expand All @@ -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}"));
Expand Down Expand Up @@ -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"));
Expand All @@ -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")
);
}
}
14 changes: 12 additions & 2 deletions desktop/src/features/communities/hostedCommunityApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,18 @@ export function clearBuilderlabAuth() {
return invoke<void>("clear_builderlab_auth");
}

export function startBuilderlabLogin() {
return invoke<BuilderlabAuth>("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<BuilderlabAuth>("start_builderlab_login", {
forceAccountSelection,
});
}

export async function loadHostedCommunityAccount(): Promise<HostedCommunityAccount> {
Expand Down
Loading