diff --git a/crates/prick-auth/src/callback.rs b/crates/prick-auth/src/callback.rs index a880604..970a486 100644 --- a/crates/prick-auth/src/callback.rs +++ b/crates/prick-auth/src/callback.rs @@ -60,6 +60,36 @@ pub fn redirect_uri(port: u16) -> String { format!("http://{CALLBACK_HOST}:{port}{CALLBACK_PATH}") } +/// Extracts the authorization response from what an operator pasted. +/// +/// Accepts the whole redirect -- `http://127.0.0.1:1234/callback?code=...` -- +/// because that is what the address bar holds, and also a bare query string for +/// anyone who trimmed it themselves. A fragment is dropped: no authorization +/// response this client asks for puts anything there, and a browser that +/// appends one must not turn a good paste into a failure. +/// +/// A bare authorization code is deliberately **not** accepted. `state` is the +/// only thing tying a redirect to the login that started it, so a spelling that +/// let the operator omit it would be a spelling with the forgery check turned +/// off -- and it would be the convenient one to reach for. +/// +/// # Errors +/// +/// [`AuthError::RedirectUnreadable`] if there is no authorization response in +/// it at all, which is what pasting the wrong line looks like. +pub fn parse_redirect(pasted: &str) -> Result, AuthError> { + let trimmed = pasted.trim(); + let after_query = trimmed.split_once('?').map_or(trimmed, |(_, query)| query); + let query = after_query.split_once('#').map_or(after_query, |(before, _)| before); + + let params = parse_query(query); + if params.iter().any(|(key, _)| key == "code" || key == "error") { + Ok(params) + } else { + Err(AuthError::RedirectUnreadable) + } +} + /// A parsed HTTP request line. #[derive(Debug, Clone, PartialEq, Eq)] pub struct RequestLine<'a> { @@ -127,6 +157,129 @@ fn decode_component(raw: &str) -> String { percent_decode_str(&plus_decoded).decode_utf8_lossy().into_owned() } +/// Which channel a redirect arrived on. +/// +/// Reported so the CLI can say how the login completed. The two are equally +/// valid: the same authorization response, carried by whichever route worked. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RedirectSource { + /// The browser reached the loopback listener. + Loopback, + /// The operator pasted the address the browser was redirected to. + Pasted, +} + +/// Reads a pasted redirect from stdin, ignoring blank lines. +/// +/// Blank lines are skipped rather than treated as an answer, because pressing +/// Enter while reading the instructions must not end the login. `Ok(None)` is +/// end of input -- Ctrl-D, or a terminal that went away -- and is not an error: +/// the loopback listener is still waiting, and reporting a failure here would +/// end a login that was about to succeed on the other channel. +/// +/// # Errors +/// +/// [`AuthError::RedirectUnreadable`] if a line arrives that carries no +/// authorization response, or an I/O failure reading the terminal. +fn read_pasted_redirect() -> Result>, AuthError> { + let stdin = std::io::stdin(); + let mut line = String::new(); + + loop { + line.clear(); + if stdin.lock().read_line(&mut line)? == 0 { + return Ok(None); + } + if !line.trim().is_empty() { + return parse_redirect(&line).map(Some); + } + } +} + +/// Waits for the authorization response on whichever channel delivers it. +/// +/// # Why race rather than choose +/// +/// Whether the browser can reach this machine's loopback is not knowable from +/// here. It depends on the network path between a browser that has not opened +/// yet, on a host this process cannot observe, and a port on this one. An +/// `ssh -L` tunnel is built entirely on the client side: the forwarded and +/// unforwarded cases are the same `bind` and the same `accept`, with no +/// syscall, environment variable or probe that separates them. +/// +/// Guessing therefore has to be wrong somewhere -- `SSH_CONNECTION` is unset +/// inside `tmux` and stripped by `sudo`, and WSL looks remote while its +/// loopback is shared with the browser's. So both channels are opened and the +/// first answer wins, which is correct in every topology without asking. +/// +/// # Threads, not tasks +/// +/// Both waits are detached OS threads rather than `spawn_blocking` tasks. A +/// blocked read on a terminal cannot be cancelled on any platform, and a tokio +/// runtime waits for its blocking tasks at shutdown -- so the losing channel +/// would hold the process open until its own deadline passed. A detached thread +/// ends with the process instead. +/// +/// # Errors +/// +/// Whatever the winning channel reported: [`AuthError::LoginTimeout`] if the +/// browser never arrived and nothing was pasted, or +/// [`AuthError::RedirectUnreadable`] for a paste that carried no response. +pub fn await_redirect( + listener: CallbackListener, + timeout: Duration, + accept_pasted: bool, +) -> Result<(Vec<(String, String)>, RedirectSource), AuthError> { + await_redirect_from(listener, timeout, accept_pasted.then_some(read_pasted_redirect)) +} + +/// [`await_redirect`] with the paste channel supplied rather than assumed. +/// +/// Exists so the race can be driven from both sides by a test: reading the real +/// stdin unconditionally would leave the paste channel untestable, because a +/// test harness owns stdin and has nothing to write to it. +fn await_redirect_from

( + listener: CallbackListener, + timeout: Duration, + paste: Option

, +) -> Result<(Vec<(String, String)>, RedirectSource), AuthError> +where + P: FnOnce() -> Result>, AuthError> + Send + 'static, +{ + let (sender, receiver) = std::sync::mpsc::channel(); + + let loopback = sender.clone(); + std::thread::spawn(move || { + let arrival = + listener.wait_for_callback(timeout).map(|params| (params, RedirectSource::Loopback)); + // A closed receiver means the other channel won. Nothing to report. + let _ = loopback.send(arrival); + }); + + match paste { + Some(paste) => { + std::thread::spawn(move || match paste() { + // End of input is not an answer, so it does not become one: the + // sender is dropped and the loopback keeps its full deadline. + Ok(None) => (), + Ok(Some(params)) => { + let _ = sender.send(Ok((params, RedirectSource::Pasted))); + } + Err(err) => { + let _ = sender.send(Err(err)); + } + }); + } + // Without a second sender the receiver ends as soon as the loopback + // thread finishes, which is what turns its timeout into this one. + None => drop(sender), + } + + // The first definitive answer, from either channel. Both send only once + // they have one, so there is nothing to filter here. + receiver.recv().unwrap_or(Err(AuthError::LoginTimeout { seconds: timeout.as_secs() })) +} + /// A single-shot loopback listener for the OAuth redirect. #[derive(Debug)] pub struct CallbackListener { @@ -466,4 +619,151 @@ mod tests { .expect("the real callback still arrives"); assert_eq!(params[0].1, "c"); } + + /// The lookup `oauth` does on the parsed pairs, spelled here so these tests + /// assert on values rather than on positions. + fn value<'a>(params: &'a [(String, String)], name: &str) -> Option<&'a str> { + params.iter().find(|(key, _)| key == name).map(|(_, found)| found.as_str()) + } + + #[test] + fn a_pasted_address_yields_what_a_delivered_callback_would() { + let params = + parse_redirect("http://127.0.0.1:54321/callback?code=abc&state=xyz").expect("parses"); + assert_eq!(value(¶ms, "code"), Some("abc")); + assert_eq!(value(¶ms, "state"), Some("xyz")); + } + + #[test] + fn the_newline_a_terminal_paste_carries_is_tolerated() { + let params = parse_redirect(" http://127.0.0.1:1/callback?code=a&state=b\r\n ") + .expect("a pasted line still has its line ending on it"); + assert_eq!(value(¶ms, "state"), Some("b")); + } + + #[test] + fn a_bare_query_string_is_accepted_for_anyone_who_trimmed_it_themselves() { + let params = parse_redirect("code=a&state=b").expect("parses"); + assert_eq!(value(¶ms, "code"), Some("a")); + } + + #[test] + fn a_fragment_a_browser_appended_is_not_mistaken_for_a_value() { + let params = + parse_redirect("http://127.0.0.1:1/callback?code=a&state=b#/").expect("parses"); + assert_eq!(value(¶ms, "state"), Some("b")); + } + + #[test] + fn an_error_redirect_is_carried_through_rather_than_rejected() { + // No `code`, but a real authorization response: the caller turns it into + // the server's own reason for refusing, which is more use than "that is + // not a redirect". + let params = parse_redirect("http://127.0.0.1:1/callback?error=access_denied&state=b") + .expect("an error response is still a response"); + assert_eq!(value(¶ms, "error"), Some("access_denied")); + } + + #[test] + fn a_bare_authorization_code_is_refused() { + // The security property. `state` is the only thing binding a redirect to + // the login that started it, so there must be no spelling that lets an + // operator hand over a code without one -- and "just paste the code" + // would be the convenient thing to reach for. + let err = parse_redirect("abc123").expect_err("a bare code is not a redirect"); + assert!(matches!(err, AuthError::RedirectUnreadable), "{err:?}"); + + assert!(parse_redirect("http://127.0.0.1:54321/callback").is_err()); + assert!(parse_redirect("").is_err()); + } + + #[test] + fn a_blank_line_is_not_an_answer() { + // Pressing Enter while reading the instructions must not end the login. + assert!(matches!(parse_redirect(" \r\n"), Err(AuthError::RedirectUnreadable))); + } + + #[test] + fn the_loopback_wins_when_the_browser_can_reach_it() { + let listener = CallbackListener::bind().expect("bind"); + let port = listener.port(); + + std::thread::spawn(move || { + let mut stream = TcpStream::connect((Ipv4Addr::LOCALHOST, port)).expect("connect"); + let _ = stream.write_all(b"GET /callback?code=loop&state=s HTTP/1.1\r\n\r\n"); + let mut response = String::new(); + let _ = stream.read_to_string(&mut response); + }); + + // With the paste channel off: stdin belongs to the test harness, so the + // race is driven from the side a test can actually supply. + let (params, source) = + await_redirect(listener, Duration::from_secs(15), false).expect("the callback arrives"); + assert_eq!(value(¶ms, "code"), Some("loop")); + assert_eq!(source, RedirectSource::Loopback); + } + + #[test] + fn a_login_nothing_completes_times_out_rather_than_hanging() { + let listener = CallbackListener::bind().expect("bind"); + let err = await_redirect(listener, Duration::from_millis(200), false) + .expect_err("nothing was going to arrive"); + assert!(matches!(err, AuthError::LoginTimeout { .. }), "{err:?}"); + } + + #[test] + fn a_paste_completes_a_login_the_loopback_never_receives() { + // The whole point of the feature: nothing ever connects to the listener, + // which is what a browser on another machine looks like from here. + let listener = CallbackListener::bind().expect("bind"); + + let (params, source) = await_redirect_from( + listener, + Duration::from_secs(30), + Some(|| parse_redirect("http://127.0.0.1:1/callback?code=pasted&state=s").map(Some)), + ) + .expect("the paste completes it"); + + assert_eq!(value(¶ms, "code"), Some("pasted")); + assert_eq!(source, RedirectSource::Pasted); + } + + #[test] + fn a_paste_that_never_comes_leaves_the_loopback_its_full_deadline() { + // End of input on the paste channel must not decide the login. The + // loopback still wins here, well after stdin has given up. + let listener = CallbackListener::bind().expect("bind"); + let port = listener.port(); + + std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(150)); + let mut stream = TcpStream::connect((Ipv4Addr::LOCALHOST, port)).expect("connect"); + let _ = stream.write_all(b"GET /callback?code=slow&state=s HTTP/1.1\r\n\r\n"); + let mut response = String::new(); + let _ = stream.read_to_string(&mut response); + }); + + let (params, source) = + await_redirect_from(listener, Duration::from_secs(30), Some(|| Ok(None))) + .expect("the browser still gets there"); + + assert_eq!(value(¶ms, "code"), Some("slow")); + assert_eq!(source, RedirectSource::Loopback); + } + + #[test] + fn a_bad_paste_ends_the_login_rather_than_being_swallowed() { + // Someone who pasted the wrong line is told so, instead of watching a + // listener nothing is going to reach sit there until it times out. + let listener = CallbackListener::bind().expect("bind"); + + let err = await_redirect_from( + listener, + Duration::from_secs(30), + Some(|| parse_redirect("not a redirect").map(Some)), + ) + .expect_err("a paste that carries no response is a failure"); + + assert!(matches!(err, AuthError::RedirectUnreadable), "{err:?}"); + } } diff --git a/crates/prick-auth/src/error.rs b/crates/prick-auth/src/error.rs index 16b6254..2a80cd4 100644 --- a/crates/prick-auth/src/error.rs +++ b/crates/prick-auth/src/error.rs @@ -71,6 +71,13 @@ pub enum AuthError { #[error("the login response did not match this login attempt and was discarded")] StateMismatch, + /// What was pasted in place of a redirect held no authorization response. + /// + /// Distinct from [`Self::StateMismatch`]: that one is a redirect belonging + /// to a different login, this one is not a redirect at all. + #[error("that does not carry an authorization response: no `code` or `error` in it")] + RedirectUnreadable, + /// The authorization server redirected back with an error. #[error("the authorization server refused the login: {error}")] Denied { @@ -150,6 +157,9 @@ impl AuthError { } Self::Discovery { .. } | Self::Registration { .. } => ErrorKind::NotPrick, Self::StateMismatch => ErrorKind::Forbidden, + // The operator pasted the wrong thing. A bad request from here, not a + // rejection from anywhere. + Self::RedirectUnreadable => ErrorKind::Validation, Self::LoginTimeout { .. } => ErrorKind::Timeout, Self::Api(err) => err.kind(), Self::Browser { .. } | Self::Store { .. } | Self::Io(_) => ErrorKind::Unknown, @@ -172,6 +182,7 @@ impl AuthError { Self::Discovery { .. } => "DISCOVERY_FAILED", Self::Registration { .. } => "REGISTRATION_FAILED", Self::StateMismatch => "STATE_MISMATCH", + Self::RedirectUnreadable => "REDIRECT_UNREADABLE", Self::Denied { .. } => "LOGIN_DENIED", Self::AuthExpired => "AUTH_EXPIRED", Self::NoCredential { .. } => "NO_CREDENTIAL", @@ -207,11 +218,18 @@ impl AuthError { previous attempt cannot be used.", ), Self::LoginTimeout { .. } => Some( - "Run `prk login ` again and complete the sign-in in the browser window it \ - opens.", + "Run `prk login ` again. Complete the sign-in in the browser, and if the \ + browser cannot reach this machine, paste the address it was redirected to when \ + asked.", + ), + Self::RedirectUnreadable => Some( + "Copy the whole address the browser was redirected to, including everything after \ + `?`, and paste that. The code on its own cannot be used: the `state` beside it is \ + what proves the redirect belongs to this login.", ), Self::Browser { .. } => Some( - "Open the printed URL manually. On a headless machine, use a service token \ + "Open the printed URL manually, in a browser anywhere, and paste the address it \ + is redirected to when asked. For an unattended machine use a service token \ instead: PRK_ACCESS_CLIENT_ID and PRK_ACCESS_CLIENT_SECRET.", ), Self::StorageUnavailable { .. } => { diff --git a/crates/prick-auth/src/lib.rs b/crates/prick-auth/src/lib.rs index 4cfdf3a..8907a85 100644 --- a/crates/prick-auth/src/lib.rs +++ b/crates/prick-auth/src/lib.rs @@ -55,6 +55,7 @@ pub mod oauth; pub mod session; pub mod store; +pub use callback::RedirectSource; pub use credential::{Credential, ServiceToken, TokenSource, service_token_from_env}; pub use discovery::Probe; pub use error::AuthError; diff --git a/crates/prick-auth/src/oauth.rs b/crates/prick-auth/src/oauth.rs index 61770cc..1907dc2 100644 --- a/crates/prick-auth/src/oauth.rs +++ b/crates/prick-auth/src/oauth.rs @@ -13,7 +13,10 @@ //! 4. **Register dynamically** for `http://127.0.0.1:/callback`. //! 5. **PKCE S256**, with the Cloudflare quirk handled -- see //! [`generate_pkce`]. -//! 6. **Open the browser**, then accept exactly one request on the listener. +//! 6. **Open the browser**, then take the redirect from whichever channel +//! delivers it -- one request on the listener, or an address pasted by an +//! operator whose browser cannot reach this machine. See +//! [`crate::callback::await_redirect`] for why both are open at once. //! 7. **Compare `state` in constant time.** //! 8. **Exchange** the code and store the tokens. //! @@ -36,7 +39,7 @@ use serde::Deserialize; use prick_api::{Body, Client}; use prick_core::pkce; -use crate::callback::CallbackListener; +use crate::callback::{self, CallbackListener, RedirectSource}; use crate::discovery::{self, AuthorizationServer, Probe}; use crate::error::AuthError; use crate::store::{StoredSession, Tokens}; @@ -360,11 +363,20 @@ fn single<'a>(params: &'a [(String, String)], name: &str) -> Option<&'a str> { pub struct LoginOptions { /// How long to wait for the browser round trip. pub timeout: Duration, + + /// Whether a redirect pasted on stdin may complete the login. + /// + /// The caller decides, because the caller knows whether there is a person + /// at a terminal: it belongs off when stdin is not a terminal or `--no-input` + /// was given, and on otherwise. When it is on, the paste races the loopback + /// listener and the first answer wins -- see [`crate::callback::await_redirect`] + /// for why that is a race rather than a decision. + pub accept_pasted_redirect: bool, } impl Default for LoginOptions { fn default() -> Self { - Self { timeout: Duration::from_secs(LOGIN_TIMEOUT_SECS) } + Self { timeout: Duration::from_secs(LOGIN_TIMEOUT_SECS), accept_pasted_redirect: false } } } @@ -375,6 +387,8 @@ pub struct LoginOutcome { pub session: StoredSession, /// What the unauthenticated probe revealed. pub probe: Probe, + /// Which channel the authorization response arrived on. + pub redirect_source: RedirectSource, } /// Runs the whole interactive login. @@ -462,11 +476,17 @@ where )?; open(&authorize)?; - // 7. One request, on a blocking thread so the reactor stays free. + // 7. The redirect, from whichever channel produces it: the loopback + // listener the browser was pointed at, or an operator pasting the + // address it was redirected to. On a blocking thread so the reactor + // stays free. let timeout = options.timeout; - let params = tokio::task::spawn_blocking(move || listener.wait_for_callback(timeout)) - .await - .map_err(|err| AuthError::Io(std::io::Error::other(err.to_string())))??; + let accept_pasted = options.accept_pasted_redirect; + let (params, redirect_source) = tokio::task::spawn_blocking(move || { + callback::await_redirect(listener, timeout, accept_pasted) + }) + .await + .map_err(|err| AuthError::Io(std::io::Error::other(err.to_string())))??; if let Some(error) = single(¶ms, "error") { return Err(AuthError::Denied { error: error.to_owned() }); @@ -504,6 +524,7 @@ where tokens, }, probe, + redirect_source, }) } diff --git a/crates/prick-auth/tests/login.rs b/crates/prick-auth/tests/login.rs index 72a06a3..5fd16af 100644 --- a/crates/prick-auth/tests/login.rs +++ b/crates/prick-auth/tests/login.rs @@ -164,7 +164,7 @@ async fn a_full_login_produces_a_storable_session() { let outcome = prick_auth::login( &client, &server.uri(), - &LoginOptions { timeout: Duration::from_secs(20) }, + &LoginOptions { timeout: Duration::from_secs(20), accept_pasted_redirect: false }, browser_answering(successful_redirect), ) .await @@ -201,7 +201,7 @@ async fn the_client_is_registered_for_the_loopback_port_that_was_just_bound() { prick_auth::login( &client, &server.uri(), - &LoginOptions { timeout: Duration::from_secs(20) }, + &LoginOptions { timeout: Duration::from_secs(20), accept_pasted_redirect: false }, browser_answering(successful_redirect), ) .await @@ -241,7 +241,7 @@ async fn the_token_request_carries_the_verifier_and_never_the_challenge() { prick_auth::login( &client, &server.uri(), - &LoginOptions { timeout: Duration::from_secs(20) }, + &LoginOptions { timeout: Duration::from_secs(20), accept_pasted_redirect: false }, browser_answering(successful_redirect), ) .await @@ -285,7 +285,7 @@ async fn every_request_names_the_resource_the_metadata_declared() { let outcome = prick_auth::login( &client, &server.uri(), - &LoginOptions { timeout: Duration::from_secs(20) }, + &LoginOptions { timeout: Duration::from_secs(20), accept_pasted_redirect: false }, browser_answering(move |authorize| { recorder.lock().expect("not poisoned").clone_from(&authorize.to_string()); successful_redirect(authorize) @@ -332,7 +332,7 @@ async fn the_authorization_request_uses_s256_with_an_acceptable_challenge() { prick_auth::login( &client, &server.uri(), - &LoginOptions { timeout: Duration::from_secs(20) }, + &LoginOptions { timeout: Duration::from_secs(20), accept_pasted_redirect: false }, browser_answering(move |authorize| { recorder.lock().expect("not poisoned").clone_from(&authorize.to_string()); successful_redirect(authorize) @@ -366,7 +366,7 @@ async fn a_redirect_carrying_the_wrong_state_is_discarded() { let err = prick_auth::login( &client, &server.uri(), - &LoginOptions { timeout: Duration::from_secs(20) }, + &LoginOptions { timeout: Duration::from_secs(20), accept_pasted_redirect: false }, browser_answering(|_| "code=forged&state=not-the-one-we-sent".to_owned()), ) .await @@ -391,7 +391,7 @@ async fn a_redirect_with_no_state_at_all_is_discarded() { let err = prick_auth::login( &client, &server.uri(), - &LoginOptions { timeout: Duration::from_secs(20) }, + &LoginOptions { timeout: Duration::from_secs(20), accept_pasted_redirect: false }, browser_answering(|_| "code=forged".to_owned()), ) .await @@ -409,7 +409,7 @@ async fn a_state_repeated_twice_is_discarded_rather_than_disambiguated() { let err = prick_auth::login( &client, &server.uri(), - &LoginOptions { timeout: Duration::from_secs(20) }, + &LoginOptions { timeout: Duration::from_secs(20), accept_pasted_redirect: false }, browser_answering(|authorize| { let real = successful_redirect(authorize); // The real state plus an attacker-chosen one. Picking whichever @@ -432,7 +432,7 @@ async fn a_denial_redirect_is_reported_as_a_denial() { let err = prick_auth::login( &client, &server.uri(), - &LoginOptions { timeout: Duration::from_secs(20) }, + &LoginOptions { timeout: Duration::from_secs(20), accept_pasted_redirect: false }, browser_answering(|authorize| { let state = authorize .query_pairs() @@ -460,7 +460,7 @@ async fn a_browser_that_never_answers_times_out_rather_than_hanging() { let err = prick_auth::login( &client, &server.uri(), - &LoginOptions { timeout: Duration::from_millis(250) }, + &LoginOptions { timeout: Duration::from_millis(250), accept_pasted_redirect: false }, |_url: &str| Ok(()), ) .await diff --git a/crates/prk/src/commands/auth.rs b/crates/prk/src/commands/auth.rs index 47c53fc..99974a4 100644 --- a/crates/prk/src/commands/auth.rs +++ b/crates/prk/src/commands/auth.rs @@ -1,8 +1,10 @@ //! `prk login`, `prk logout`, `prk whoami`. +use std::io::IsTerminal as _; + use clap::Args; -use prick_auth::{AuthError, Probe, StorageBackend, TokenStore, discovery}; +use prick_auth::{AuthError, Probe, RedirectSource, StorageBackend, TokenStore, discovery}; use crate::cli::GlobalArgs; use crate::commands::Context; @@ -27,10 +29,13 @@ pub struct LoginArgs { /// Print the authorization URL instead of opening a browser. /// - /// For a machine with no display: run this over SSH, open the URL on a - /// local browser, and the loopback listener still receives the redirect -- - /// provided the port is reachable, which it is when the SSH session - /// forwards it. + /// For a machine with no display. The login then completes by whichever + /// route works: the loopback listener, when the port is reachable from the + /// browser -- an SSH session forwarding it, or WSL sharing loopback with + /// Windows -- or by pasting the address the browser was redirected to. + /// + /// Detected already when there is no display to open a browser on, so this + /// flag is for the case where there is one and you want the URL anyway. #[arg(long)] pub no_browser: bool, } @@ -58,6 +63,16 @@ impl From for StorageBackend { } } +/// What to tell an operator whose browser cannot reach this machine. +/// +/// Printed for every interactive login rather than behind a flag, because +/// whether the browser can reach this machine's loopback is not knowable before +/// it tries -- see [`prick_auth::callback::await_redirect`]. Both channels are +/// open, so this is an offer, not an instruction: a login that completes in the +/// browser needs nothing from here. +const PASTE_PROMPT: &str = "If the browser cannot reach this machine, it will fail to load a 127.0.0.1 address.\n\ + That is expected. Paste that whole address here and press Enter:"; + /// Runs the interactive login. /// /// # Errors @@ -75,30 +90,40 @@ pub fn login(args: &LoginArgs, global: &GlobalArgs, out: Output) -> Result<(), C out.note(&format!("Signing in to {}", context.api_url())); let no_browser = args.no_browser || !prick_auth::browser::is_available(); + + // Whether there is anyone to paste. `--no-input` is a promise not to ask, + // and a stdin that is not a terminal is either a pipe carrying something + // else or a job with no operator -- reading either would consume input that + // was not an answer. + let accept_pasted = !global.no_input && std::io::stdin().is_terminal(); + + let options = prick_auth::LoginOptions { + accept_pasted_redirect: accept_pasted, + ..prick_auth::LoginOptions::default() + }; + let outcome = context.block_on(prick_auth::login( context.client(), context.api_url(), - &prick_auth::LoginOptions::default(), + &options, |authorize_url: &str| { if no_browser { // Not `data`: this is a diagnostic, and stdout belongs to the // answer. A login has no answer to print. out.note(&format!("Open this URL to continue:\n {authorize_url}")); - return Ok(()); - } - match prick_auth::browser::open(authorize_url) { - Ok(()) => { - out.note("Waiting for the browser to complete the sign-in..."); - Ok(()) - } + } else if let Err(err) = prick_auth::browser::open(authorize_url) { // Recoverable: the listener is already waiting, so printing the // URL is enough to finish the login by hand. - Err(err) => { - out.warn(&format!("{err}")); - out.note(&format!("Open this URL to continue:\n {authorize_url}")); - Ok(()) - } + out.warn(&format!("{err}")); + out.note(&format!("Open this URL to continue:\n {authorize_url}")); + } else { + out.note("Waiting for the browser to complete the sign-in..."); + } + + if accept_pasted { + out.note(PASTE_PROMPT); } + Ok(()) }, ))?; @@ -114,6 +139,7 @@ pub fn login(args: &LoginArgs, global: &GlobalArgs, out: Output) -> Result<(), C "issuer": outcome.session.issuer, "storage": StorageBackend::from(args.storage).as_str(), "expires_at": outcome.session.tokens.expires_at, + "redirect": redirect_label(outcome.redirect_source), })); } else { out.data(&format!("Signed in to {}", outcome.session.api_url)); @@ -122,6 +148,18 @@ pub fn login(args: &LoginArgs, global: &GlobalArgs, out: Output) -> Result<(), C Ok(()) } +/// Names the channel a redirect arrived on, for `--json`. +/// +/// Reported because it is the one part of a login an operator cannot otherwise +/// see, and it is what tells them whether loopback works from wherever they run +/// this -- which decides whether the next login needs a person at the terminal. +fn redirect_label(source: RedirectSource) -> &'static str { + match source { + RedirectSource::Loopback => "loopback", + RedirectSource::Pasted => "pasted", + } +} + /// Emits the warning for a server nothing is protecting. /// /// Through [`Output::warn`], which `--json` does not suppress. Every other diff --git a/docs/reference/cli/errors.md b/docs/reference/cli/errors.md index 14a0ea6..4b9e210 100644 --- a/docs/reference/cli/errors.md +++ b/docs/reference/cli/errors.md @@ -106,6 +106,7 @@ Codes the client raises itself, rather than reading off a response: | `UNREPRESENTABLE_OUTPUT` | 9 | A value contains a control character the chosen format cannot encode | | `TRUNCATED_OUTPUT` | 13 | stdout would not take the whole answer, and what it took carried secret material | | `INVALID_SCOPE` | 11 | A scope string could not be parsed | +| `REDIRECT_UNREADABLE` | 11 | What was pasted to complete a login carried no authorization response | | `UNSAFE_ENVIRONMENT` | 11 | A secret's name is one the loader interprets, and `--allow-unsafe-env` was not given | | `LAUNCH_FAILED` | 1, 126, 127 | `prk run` could not start the command — **127** not found, **126** found but not executable, **1** for anything else | @@ -288,6 +289,31 @@ neighbours are both about something else: exit 9 is a value that cannot be encoded, and exit 12 is a response too large to read — a size problem at the other end of the run, on the way in rather than on the way out. +### `REDIRECT_UNREADABLE` (exit 11) + +What was pasted to complete a [login your browser could not reach](/reference/cli/sign-in) +holds no authorization response — no `code` and no `error` in it. + +``` +error: that does not carry an authorization response: no `code` or `error` in it + help: Copy the whole address the browser was redirected to, including everything after `?`, and paste that. The code on its own cannot be used: the `state` beside it is what proves the redirect belongs to this login. +``` + +Usually the address was copied without its query string, or the code was copied +on its own. Paste the whole thing: + +``` +http://127.0.0.1:54321/callback?code=…&state=… +``` + +The authorization code alone is refused deliberately, and no flag relaxes it. +`state` is the only thing binding a redirect to the login that started it, so +accepting a bare code would be accepting a redirect nothing can check. + +Distinct from `STATE_MISMATCH`, which is a redirect that **is** an authorization +response but belongs to a different login — a stale browser tab, or a forgery. +Run `prk login` again for either. + ## Next steps - [`prk doctor`](/reference/cli/sign-in#prk-doctor) — check everything at once. diff --git a/docs/reference/cli/sign-in.md b/docs/reference/cli/sign-in.md index 910aaa6..8d369c0 100644 --- a/docs/reference/cli/sign-in.md +++ b/docs/reference/cli/sign-in.md @@ -40,7 +40,14 @@ Your browser opens, you complete the Cloudflare Access sign-in, and the token lands on disk. `prk login` also records **which server** it signed in to, so no later command needs `--api-url` or `PRK_API_URL`. -### Sign in on a machine with no browser +### Sign in on a machine your browser cannot reach + +This is the remote-shell and container case: you open the URL on your own +machine, and the redirect goes to a `127.0.0.1` address that means something +different there than it does on the machine you ran `prk login` on. + +You do not have to tell `prk` which situation you are in. Both routes are open +at once and the first one to produce the redirect completes the login. ```bash prk login https://prick.example.com --no-browser @@ -50,12 +57,56 @@ prk login https://prick.example.com --no-browser Signing in to https://prick.example.com Open this URL to continue: https://example.cloudflareaccess.com/cdn-cgi/access/sso/oidc/… +If the browser cannot reach this machine, it will fail to load a 127.0.0.1 address. +That is expected. Paste that whole address here and press Enter: +``` + +Open the URL in a browser anywhere. Then either: + +- **The browser reaches this machine** — over an SSH session forwarding the + port, or under WSL, which shares loopback with Windows. The login finishes on + its own and there is nothing to paste. +- **It does not** — the browser shows a connection error. That is the expected + outcome, and the address bar now holds the authorization response. Copy the + whole address and paste it at the prompt. + +``` +http://127.0.0.1:54321/callback?code=…&state=… +``` + +Paste the **whole** address, including everything after the `?`. The +authorization code on its own is refused: the `state` next to it is what proves +the redirect belongs to the login you just started, and a code without it cannot +be checked. Getting this wrong reports +[`REDIRECT_UNREADABLE`](/reference/cli/errors#redirect_unreadable-exit-11). + +The paste prompt appears whenever there is a terminal to answer it. With +`--no-input`, or with stdin coming from somewhere other than a terminal, only +the loopback route is used — so a scripted login behaves exactly as it did. + +`--json` reports which route completed it: + +```bash +prk login https://prick.example.com --json ``` -Open the URL on a machine that has a browser. The loopback listener on the -remote machine still receives the redirect, provided the port is reachable — -which it is over an SSH session that forwards it. `--no-browser` is also applied -automatically when no browser is available. +```json +{ "api_url": "https://prick.example.com", "redirect": "pasted", "…": "…" } +``` + +`--no-browser` only controls whether a browser is launched here; it is applied +automatically when there is no display to launch one on. + +#### Why it is not detected for you + +Whether a browser can reach this machine's loopback is not knowable before it +tries. An `ssh -L` tunnel is built entirely on the client side, so a forwarded +port and an unforwarded one are the same `bind` and the same `accept` from +inside `prk` — there is no environment variable or probe that separates them. +The signals that look promising are wrong in both directions: `SSH_CONNECTION` +is unset inside `tmux` and stripped by `sudo`, and WSL looks remote while its +loopback is shared with the browser's. Racing the two routes is correct in every +one of those cases without asking. ### Where the token is stored