Skip to content

Commit f320b6c

Browse files
eddietejedaclaude
andcommitted
refactor(auth): extract run_browser_auth helper; add tests for exchange_cli_register_code
- Add doc comment to receive_callback noting success_title/success_body are interpolated into HTML without escaping — callers must pass static strings only - Extract run_browser_auth() to collapse the ~80% duplication between login() and register(); the two functions now each reduce to the signed-in guard plus a run_browser_auth call with distinct URL and exchange closures - Add four tests for exchange_cli_register_code: success (two-step mock: /v1/auth/token then /o/token/), http_error, malformed_response, connection_error — matching the pattern used by mint_from_pkce_code and mint_from_api_token Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 1f51883 commit f320b6c

2 files changed

Lines changed: 181 additions & 143 deletions

File tree

src/auth.rs

Lines changed: 108 additions & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,10 @@ struct WsListResponse { workspaces: Vec<WsItem> }
169169
struct WsItem { public_id: String, name: String }
170170

171171
/// Wait for the browser callback, verify state, and extract the authorization code.
172-
/// `success_title` and `success_body` are rendered in the browser tab on success.
172+
///
173+
/// `success_title` and `success_body` are interpolated directly into HTML
174+
/// without escaping. Callers **must** pass static, trusted strings — never
175+
/// dynamic or user-supplied content.
173176
fn receive_callback(
174177
server: &tiny_http::Server,
175178
expected_state: &str,
@@ -262,104 +265,85 @@ fn is_already_signed_in(profile_config: &config::ProfileConfig) -> bool {
262265
check_status(profile_config) == AuthStatus::Authenticated
263266
}
264267

265-
pub fn login() {
266-
let profile_config = config::load("default").unwrap_or_default();
268+
/// Shared PKCE browser-handoff loop used by both `login` and `register`.
269+
///
270+
/// 1. Generates PKCE params and starts the local loopback callback server.
271+
/// 2. Calls `build_url(app_url, code_challenge, state, port)` to construct
272+
/// the browser URL.
273+
/// 3. Opens the browser and waits for the OAuth/registration callback.
274+
/// 4. Calls `exchange(code, code_verifier, port)` to mint a JWT session.
275+
/// 5. Saves the session, prints `success_print`, and displays the workspace.
276+
fn run_browser_auth(
277+
profile_config: &config::ProfileConfig,
278+
opening_msg: &str,
279+
waiting_msg: &str,
280+
success_print: &str,
281+
success_title: &str,
282+
success_body: &str,
283+
build_url: impl Fn(&str, &str, &str, u16) -> String,
284+
exchange: impl Fn(&str, &str, u16) -> Result<crate::jwt::Session, String>,
285+
) {
267286
let app_url = profile_config.app_url.to_string();
268-
269-
// Check if already authenticated
270-
if is_already_signed_in(&profile_config) {
271-
println!("{}", "You are already signed in.".green());
272-
if !crate::util::is_interactive() {
273-
return;
274-
}
275-
print!("Do you want to log in again? [y/N] ");
276-
use std::io::Write;
277-
std::io::stdout().flush().unwrap();
278-
let mut input = String::new();
279-
std::io::stdin().read_line(&mut input).unwrap();
280-
if !input.trim().eq_ignore_ascii_case("y") {
281-
return;
282-
}
283-
}
284-
285287
let code_verifier = generate_code_verifier();
286288
let code_challenge = generate_code_challenge(&code_verifier);
287289
let state = generate_random_string(32);
288290

289-
// Bind to port 0 so the OS picks an available port. DOT's consent
290-
// page will redirect here with `?code=...&state=...`.
291291
let server =
292292
tiny_http::Server::http("127.0.0.1:0").expect("failed to start local callback server");
293293
let port = server.server_addr().to_ip().unwrap().port();
294-
let redirect_uri = format!("http://127.0.0.1:{port}/");
295294

296-
// DOT's `/o/authorize/` endpoint is mounted off the app URL (the
297-
// browser-facing one; allauth session cookies live here). We send
298-
// no `scope` parameter — the consent page picks permissions and
299-
// workspace scope interactively, then composes the scope string
300-
// server-side (see HotdataAllowForm).
301-
let login_url = format!(
302-
"{app_url}/o/authorize/\
303-
?client_id=hotdata-cli\
304-
&response_type=code\
305-
&redirect_uri={redirect_uri}\
306-
&code_challenge={code_challenge}\
307-
&code_challenge_method=S256\
308-
&state={state}",
309-
app_url = app_url.trim_end_matches('/'),
310-
);
295+
let url = build_url(app_url.trim_end_matches('/'), &code_challenge, &state, port);
311296

312-
println!("Opening browser to log in...");
297+
println!("{opening_msg}");
313298
stdout()
314299
.execute(Print("If your browser does not open, visit:\n "))
315300
.unwrap()
316301
.execute(SetForegroundColor(Color::DarkGrey))
317302
.unwrap()
318-
.execute(Print(format!("{login_url}\n")))
303+
.execute(Print(format!("{url}\n")))
319304
.unwrap()
320305
.execute(ResetColor)
321306
.unwrap();
322307

323-
if let Err(e) = open::that(&login_url) {
308+
if let Err(e) = open::that(&url) {
324309
eprintln!("failed to open browser: {e}");
325310
}
326311

327-
println!("Waiting for login callback...");
312+
println!("{waiting_msg}");
328313

329-
let code = match receive_callback(
330-
&server,
331-
&state,
332-
"Login successful",
333-
"You're now authenticated with Hotdata.<br/>You can close this tab and return to the terminal.",
334-
) {
314+
let code = match receive_callback(&server, &state, success_title, success_body) {
335315
Ok(c) => c,
336316
Err(e) => {
337317
eprintln!("error: {e}");
338318
std::process::exit(1);
339319
}
340320
};
341321

342-
match crate::jwt::mint_from_pkce_code(&profile_config, &code, &code_verifier, &redirect_uri) {
322+
match exchange(&code, &code_verifier, port) {
343323
Ok(session) => {
344324
if let Err(e) = crate::jwt::save_session(&session) {
345325
eprintln!("warning: could not save session: {e}");
346326
}
347327
stdout()
348328
.execute(SetForegroundColor(Color::Green))
349329
.unwrap()
350-
.execute(Print("Logged in successfully.\n"))
330+
.execute(Print(format!("{success_print}\n")))
351331
.unwrap()
352332
.execute(ResetColor)
353333
.unwrap();
354334

355-
// Best-effort workspace cache using the freshly minted JWT.
356-
// Fall back to the existing on-disk list if the fetch fails.
357-
let workspaces = cache_workspaces(&profile_config, &session.access_token)
358-
.unwrap_or(profile_config.workspaces);
335+
let workspaces = cache_workspaces(profile_config, &session.access_token)
336+
.unwrap_or_else(|_| profile_config.workspaces.clone());
359337
match workspaces.first() {
360338
Some(w) => {
361-
print_row("Workspace", &format!("{} {}", w.name.as_str().cyan(), format!("({})", w.public_id).dark_grey()));
362-
print_row("", &"use 'hotdata workspaces set' to switch workspaces".dark_grey().to_string());
339+
print_row(
340+
"Workspace",
341+
&format!("{} {}", w.name.as_str().cyan(), format!("({})", w.public_id).dark_grey()),
342+
);
343+
print_row(
344+
"",
345+
&"use 'hotdata workspaces set' to switch workspaces".dark_grey().to_string(),
346+
);
363347
}
364348
None => print_row("Workspace", &"None".dark_grey().to_string()),
365349
}
@@ -371,9 +355,57 @@ pub fn login() {
371355
}
372356
}
373357

358+
pub fn login() {
359+
let profile_config = config::load("default").unwrap_or_default();
360+
361+
if is_already_signed_in(&profile_config) {
362+
println!("{}", "You are already signed in.".green());
363+
if !crate::util::is_interactive() {
364+
return;
365+
}
366+
print!("Do you want to log in again? [y/N] ");
367+
use std::io::Write;
368+
std::io::stdout().flush().unwrap();
369+
let mut input = String::new();
370+
std::io::stdin().read_line(&mut input).unwrap();
371+
if !input.trim().eq_ignore_ascii_case("y") {
372+
return;
373+
}
374+
}
375+
376+
// DOT's `/o/authorize/` endpoint is mounted off the app URL (the
377+
// browser-facing one; allauth session cookies live here). We send
378+
// no `scope` parameter — the consent page picks permissions and
379+
// workspace scope interactively, then composes the scope string
380+
// server-side (see HotdataAllowForm).
381+
run_browser_auth(
382+
&profile_config,
383+
"Opening browser to log in...",
384+
"Waiting for login callback...",
385+
"Logged in successfully.",
386+
"Login successful",
387+
"You're now authenticated with Hotdata.<br/>You can close this tab and return to the terminal.",
388+
|app_url, code_challenge, state, port| {
389+
let redirect_uri = format!("http://127.0.0.1:{port}/");
390+
format!(
391+
"{app_url}/o/authorize/\
392+
?client_id=hotdata-cli\
393+
&response_type=code\
394+
&redirect_uri={redirect_uri}\
395+
&code_challenge={code_challenge}\
396+
&code_challenge_method=S256\
397+
&state={state}"
398+
)
399+
},
400+
|code, code_verifier, port| {
401+
let redirect_uri = format!("http://127.0.0.1:{port}/");
402+
crate::jwt::mint_from_pkce_code(&profile_config, code, code_verifier, &redirect_uri)
403+
},
404+
);
405+
}
406+
374407
pub fn register(use_email: bool) {
375408
let profile_config = config::load("default").unwrap_or_default();
376-
let app_url = profile_config.app_url.to_string();
377409

378410
if is_already_signed_in(&profile_config) {
379411
println!(
@@ -383,95 +415,28 @@ pub fn register(use_email: bool) {
383415
return;
384416
}
385417

386-
let code_verifier = generate_code_verifier();
387-
let code_challenge = generate_code_challenge(&code_verifier);
388-
let state = generate_random_string(32);
389-
390-
let server =
391-
tiny_http::Server::http("127.0.0.1:0").expect("failed to start local callback server");
392-
let port = server.server_addr().to_ip().unwrap().port();
393-
394418
let method = if use_email { "email" } else { "github" };
395-
let register_url = format!(
396-
"{app_url}/auth/cli-register/\
397-
?code_challenge={code_challenge}\
398-
&code_challenge_method=S256\
399-
&state={state}\
400-
&callback_port={port}\
401-
&method={method}",
402-
app_url = app_url.trim_end_matches('/'),
403-
);
404-
405-
println!("Opening browser to create your account...");
406-
stdout()
407-
.execute(Print("If your browser does not open, visit:\n "))
408-
.unwrap()
409-
.execute(SetForegroundColor(Color::DarkGrey))
410-
.unwrap()
411-
.execute(Print(format!("{register_url}\n")))
412-
.unwrap()
413-
.execute(ResetColor)
414-
.unwrap();
415-
416-
if let Err(e) = open::that(&register_url) {
417-
eprintln!("failed to open browser: {e}");
418-
}
419-
420-
println!("Waiting for account setup to complete...");
421-
422-
let code = match receive_callback(
423-
&server,
424-
&state,
419+
run_browser_auth(
420+
&profile_config,
421+
"Opening browser to create your account...",
422+
"Waiting for account setup to complete...",
423+
"Account created and logged in.",
425424
"Account created",
426425
"Your Hotdata account is ready.<br/>You can close this tab and return to the terminal.",
427-
) {
428-
Ok(c) => c,
429-
Err(e) => {
430-
eprintln!("error: {e}");
431-
std::process::exit(1);
432-
}
433-
};
434-
435-
match crate::jwt::exchange_cli_register_code(&profile_config, &code, &code_verifier) {
436-
Ok(session) => {
437-
if let Err(e) = crate::jwt::save_session(&session) {
438-
eprintln!("warning: could not save session: {e}");
439-
}
440-
stdout()
441-
.execute(SetForegroundColor(Color::Green))
442-
.unwrap()
443-
.execute(Print("Account created and logged in.\n"))
444-
.unwrap()
445-
.execute(ResetColor)
446-
.unwrap();
447-
448-
let workspaces = cache_workspaces(&profile_config, &session.access_token)
449-
.unwrap_or(profile_config.workspaces);
450-
match workspaces.first() {
451-
Some(w) => {
452-
print_row(
453-
"Workspace",
454-
&format!(
455-
"{} {}",
456-
w.name.as_str().cyan(),
457-
format!("({})", w.public_id).dark_grey()
458-
),
459-
);
460-
print_row(
461-
"",
462-
&"use 'hotdata workspaces set' to switch workspaces"
463-
.dark_grey()
464-
.to_string(),
465-
);
466-
}
467-
None => print_row("Workspace", &"None".dark_grey().to_string()),
468-
}
469-
}
470-
Err(msg) => {
471-
eprintln!("{}", msg.red());
472-
std::process::exit(1);
473-
}
474-
}
426+
|app_url, code_challenge, state, port| {
427+
format!(
428+
"{app_url}/auth/cli-register/\
429+
?code_challenge={code_challenge}\
430+
&code_challenge_method=S256\
431+
&state={state}\
432+
&callback_port={port}\
433+
&method={method}"
434+
)
435+
},
436+
|code, code_verifier, _port| {
437+
crate::jwt::exchange_cli_register_code(&profile_config, code, code_verifier)
438+
},
439+
);
475440
}
476441

477442
/// Fetch workspaces with a freshly minted JWT and cache them in config.

0 commit comments

Comments
 (0)