feat(cli): complete a login by pasting the redirect when loopback is unreachable

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<Vec<(String, String)>, 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<Option<Vec<(String, String)>>, 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<P>(
+    listener: CallbackListener,
+    timeout: Duration,
+    paste: Option<P>,
+) -> Result<(Vec<(String, String)>, RedirectSource), AuthError>
+where
+    P: FnOnce() -> Result<Option<Vec<(String, String)>>, 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(&params, "code"), Some("abc"));
+        assert_eq!(value(&params, "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(&params, "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(&params, "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(&params, "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(&params, "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(&params, "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(&params, "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(&params, "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:?}");
+    }
 }

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 <url>` again and complete the sign-in in the browser window it \
-                 opens.",
+                "Run `prk login <url>` 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 { .. } => {

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;