diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e6a241..3c48b53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,13 +2,19 @@ ## Unreleased +### Added + +- `--redirect-uri`, `TEAMS_CLI_REDIRECT_URI` and the profile's `redirect_uri` select the browser login's redirect URI for `auth login`, `auth consent-url` and `auth doctor`. The login listens on the URI's host and port (both loopback addresses for `localhost`) for a request to the URI's path carrying the login's `state`; the flag is saved to the profile with the IDs. A redirect URI without a port gets a free port from the OS at each login, as MSAL does; the identity platform allows this because it ignores the port of a localhost redirect URI, so `--redirect-uri http://localhost` works for an app registered that way. + ### Changed +- The default redirect URI is now `http://localhost/callback` on a free port instead of `http://localhost:8400/callback`, so a busy port 8400 no longer blocks login. App registrations with `http://localhost:8400/callback` keep working because the port is ignored. - Updated Rust dependencies (`tokio` 1.54.1, `serde_json` 1.0.154, `uuid` 1.28.0, `webbrowser` 1.2.5) and the CI and release workflows' `Swatinem/rust-cache` pin to v2.9.3. `rust-version` in `Cargo.toml` is now 1.89, the oldest Rust the dependencies build with; it said 1.75. - Updated `sha2` to 0.11.1 (it computes the PKCE code challenge for browser login). The challenge is now unit-tested against the RFC 7636 example, so a hashing or encoding change cannot slip through as failed logins. ### Fixed +- The browser login keeps serving its loopback redirect URI until the callback arrives. It served one connection, so a browser's speculative connection or a second request could leave it waiting forever or failing with "Failed to receive auth code from callback". A redirect carrying the identity platform's `error` (a declined consent, say) now fails the login at once with `error` and `error_description` (exit code 3) instead of that generic message, requests to other paths or with another `state` are ignored, and the login gives up after 10 minutes with no callback. - Updated `rustls-webpki` to 0.103.16 and `webpki-roots` to 1.0.11 for RUSTSEC-2026-0382 and RUSTSEC-2026-0381, under which certificates issued by distrusted intermediates could still be accepted. The `tokio` update also clears RUSTSEC-2026-0386 (unsound `AsyncFd` constructors). ## v0.10.0 - 2026-10-15 diff --git a/README.md b/README.md index 9298321..a3ec034 100644 --- a/README.md +++ b/README.md @@ -124,7 +124,7 @@ The CLI has OSO's multi-tenant delegated public client app baked in: ```text Client ID: fba1b5d0-fdd0-4fe2-9729-9ccdc38f9595 Default authority: organizations -Loopback redirect URI: http://localhost:8400/callback +Loopback redirect URI: http://localhost/callback (any port) ``` Customer admin consent: @@ -200,11 +200,14 @@ teams auth login --device-code --scopes "User.Read ChannelMessage.Read.All offli # Browser-based login with a customer-owned app teams auth login --client-id --tenant-id +# Browser login with an app whose registered redirect URI is http://localhost (no path) +teams auth login --client-id --tenant-id --redirect-uri http://localhost + # Device code flow with a customer-owned app teams auth login --device-code --client-id --tenant-id ``` -A delegated login saves `--client-id` and `--tenant-id` to the profile once it +A delegated login saves `--client-id`, `--tenant-id` and `--redirect-uri` to the profile once it succeeds, so the profile's next `teams auth login` reuses that app registration instead of the built-in one. Environment variables and client credentials logins are never saved; see [docs/auth.md](docs/auth.md). @@ -254,6 +257,14 @@ Delegated scope resolution follows the same order: `--scopes` (or `TEAMS_CLI_SCOPES`), then the profile's `scopes` field, then the built-in default scope set. +The browser login's redirect URI follows it too: `--redirect-uri` (or +`TEAMS_CLI_REDIRECT_URI`), then the profile's `redirect_uri`, then +`http://localhost/callback`. The CLI listens on the URI's host and port for +the callback on the URI's path, and the URI must be registered on the app. +A URI without a port gets a free port from the OS at each login; the identity +platform ignores the port of a localhost redirect URI, so the registered URI +needs none. + When permissions are consented after login (for example an admin grants a new delegated scope), `teams auth refresh` silently redeems the stored refresh token for the resolved scopes — no browser or device code. Refresh tokens are @@ -716,6 +727,7 @@ auth_flow = "device-code" | `TEAMS_CLI_CLIENT_ID` | Azure AD application (client) ID | | `TEAMS_CLI_CLIENT_SECRET` | Azure AD client secret | | `TEAMS_CLI_TENANT_ID` | Azure AD tenant ID | +| `TEAMS_CLI_REDIRECT_URI` | Redirect URI for the browser login (same precedence as `--redirect-uri`; not saved to the profile) | | `TEAMS_CLI_PROFILE` | Named credential profile (overridden by `--profile`; an explicit value beats the config's `default.profile`) | | `TEAMS_CLI_SCOPES` | Delegated OAuth scopes for login (same precedence as `--scopes`; ignored by client credentials) | | `TEAMS_CLI_ACCESS_TOKEN` | Pre-obtained Microsoft Graph access token (skips login entirely; `--profile`/`TEAMS_CLI_PROFILE` do not affect which token is used) | diff --git a/docs/auth.md b/docs/auth.md index 50b6e26..4df1ded 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -33,7 +33,7 @@ Delegated login defaults to OSO's multi-tenant public client app: ```text Client ID: fba1b5d0-fdd0-4fe2-9729-9ccdc38f9595 Authority: organizations -Redirect URI: http://localhost:8400/callback +Redirect URI: http://localhost/callback (any port) ``` The default lets most users start with: @@ -210,11 +210,12 @@ teams auth login --device-code \ --tenant-id ``` -After a successful delegated login (browser or device code), a client ID or -tenant ID given with `--client-id` or `--tenant-id` is saved to the profile's -`client_id` and `tenant_id` in the config file. The next `teams auth login` for that profile signs in through the +After a successful delegated login (browser or device code), a client ID, +tenant ID or redirect URI given with `--client-id`, `--tenant-id` or +`--redirect-uri` is saved to the profile's `client_id`, `tenant_id` and +`redirect_uri` in the config file. The next `teams auth login` for that profile signs in through the same registration without being told again, instead of falling back to the -built-in app. Only those two values change; comments, layout and every other +built-in app. Only those values change; comments, layout and every other setting in the file are left as they were, and a symlinked config file stays a symlink. A login that fails saves nothing, and a value given again replaces the saved one. Saving is best effort: if the file cannot be written, the @@ -283,8 +284,9 @@ teams --profile customer auth login --device-code teams --profile customer auth doctor --output json ``` -Instead of editing the file, a delegated login given `--client-id` and -`--tenant-id` writes the profile's `client_id` and `tenant_id` once it +Instead of editing the file, a delegated login given `--client-id`, +`--tenant-id` and `--redirect-uri` writes the profile's `client_id`, +`tenant_id` and `redirect_uri` once it succeeds, leaving the rest of the file as it was (see "Customer-owned delegated app" above). @@ -292,7 +294,11 @@ BYO delegated app requirements: - Supported account type: usually single-tenant for locked-down enterprises. - Public client flows enabled. -- Redirect URI for browser flow: `http://localhost:8400/callback`. +- Redirect URI for browser flow: `http://localhost/callback` on a port the OS picks, by default. + `--redirect-uri`, `TEAMS_CLI_REDIRECT_URI` or the profile's `redirect_uri` + change it. Entra ignores the port on loopback redirect URIs but matches the + path exactly, so an app registered with `http://localhost` needs + `--redirect-uri http://localhost`. - Delegated Graph permissions required for the features the customer wants. - Admin consent granted where required. diff --git a/docs/man/teams-auth.7 b/docs/man/teams-auth.7 index f786ccc..1c21959 100644 --- a/docs/man/teams-auth.7 +++ b/docs/man/teams-auth.7 @@ -18,7 +18,7 @@ Delegated login defaults to the OSO multi-tenant public client app: .nf Client ID: fba1b5d0-fdd0-4fe2-9729-9ccdc38f9595 Authority: organizations -Redirect URI: http://localhost:8400/callback +Redirect URI: http://localhost/callback (any port) .fi .PP Common login commands: @@ -62,6 +62,14 @@ field replaces the default delegated scope string for that profile; is appended when missing. Use it when a BYO app has admin-consented permissions beyond the defaults. .PP +An app whose registered redirect URI has no path, such as +.B http://localhost, +needs +.B redirect_uri = "http://localhost" +in the profile or +.B --redirect-uri +at login. +.PP Then run: .PP .nf @@ -70,9 +78,10 @@ teams --profile customer auth doctor --output json .fi .PP Instead of editing the file, a delegated login given -.B --client-id -and +.B --client-id, .B --tenant-id +and +.B --redirect-uri saves them to the profile once it succeeds, leaving the rest of the file as it was. Values from .B TEAMS_CLI_CLIENT_ID diff --git a/docs/man/teams-config.5 b/docs/man/teams-config.5 index ed654e5..c50e964 100644 --- a/docs/man/teams-config.5 +++ b/docs/man/teams-config.5 @@ -144,6 +144,13 @@ Delegated OAuth scope string requested at login for this profile. Replaces the built-in default delegated scopes; .B offline_access is appended when missing. Ignored by client credentials login. +.TP +.B redirect_uri +Redirect URI for the browser login. The CLI listens on its host and port, +which must be localhost or a loopback address, and sends it to the identity +platform, where it must match the app registration. Default +http://localhost/callback. A URI without a port gets a free port, which the +identity platform allows for localhost. .SH RESOLUTION ORDER Profile resolution: .nf @@ -173,6 +180,13 @@ Delegated scope resolution: 3. Built-in default delegated scopes .fi .PP +Redirect URI resolution: +.nf +1. --redirect-uri flag or TEAMS_CLI_REDIRECT_URI +2. Selected config profile redirect_uri +3. http://localhost/callback +.fi +.PP Access token resolution for normal commands: .nf 1. TEAMS_CLI_ACCESS_TOKEN diff --git a/docs/man/teams.1 b/docs/man/teams.1 index e06cb96..0eb5a93 100644 --- a/docs/man/teams.1 +++ b/docs/man/teams.1 @@ -60,11 +60,11 @@ Set page size for paginated list commands. Follow Graph pagination and return all pages. .SH AUTHENTICATION .nf -teams auth login [--client-credentials] [--device-code] [--client-id ID] [--client-secret SECRET] [--tenant-id ID] [--scopes SCOPES] +teams auth login [--client-credentials] [--device-code] [--client-id ID] [--client-secret SECRET] [--tenant-id ID] [--redirect-uri URI] [--scopes SCOPES] teams auth refresh [--scopes SCOPES] teams auth status -teams auth consent-url [--client-id ID] [--tenant-id ID] [--scopes SCOPES] -teams auth doctor [--client-id ID] [--tenant-id ID] +teams auth consent-url [--client-id ID] [--tenant-id ID] [--redirect-uri URI] [--scopes SCOPES] +teams auth doctor [--client-id ID] [--tenant-id ID] [--redirect-uri URI] teams auth list teams auth switch NAME teams auth logout [--profile NAME] [--all] @@ -78,10 +78,13 @@ registrations can still be selected with .B --client-id and .B --tenant-id. -A successful delegated login saves those two flags to the profile's -.B client_id +A successful delegated login saves those two flags, and +.B --redirect-uri, +to the profile's +.B client_id, +.B tenant_id and -.B tenant_id, +.B redirect_uri, so the profile's next login reuses them; values from the environment, and a client credentials login, are never saved. Client credentials always require explicit customer app credentials. @@ -96,6 +99,15 @@ or then the profile .B scopes field, then the built-in default delegated scopes. +The browser login redirect URI is +.B --redirect-uri +or +.B TEAMS_CLI_REDIRECT_URI, +then the profile +.B redirect_uri +field, then +.B http://localhost/callback. +A URI without a port gets a free port from the operating system. .B teams auth refresh silently redeems the stored refresh token for the resolved scopes, picking up newly admin-consented permissions without a browser; without an explicit @@ -397,6 +409,10 @@ whose value shows in process listings and shell history. .B TEAMS_CLI_TENANT_ID Azure AD tenant ID. .TP +.B TEAMS_CLI_REDIRECT_URI +Redirect URI for the browser login. Same precedence as +.B --redirect-uri. +.TP .B TEAMS_CLI_PROFILE Named credential profile. Overridden by .B --profile. diff --git a/docs/quickstarts/enterprise-admin.md b/docs/quickstarts/enterprise-admin.md index 633a699..f58aae4 100644 --- a/docs/quickstarts/enterprise-admin.md +++ b/docs/quickstarts/enterprise-admin.md @@ -68,7 +68,7 @@ Confirm: For a customer-owned app: - Create an Entra app registration. -- Configure public client redirect URI `http://localhost:8400/callback`. +- Configure public client redirect URI `http://localhost/callback`. - Enable public client flows. - Add delegated Graph permissions required by your chosen command set. - Grant admin consent. diff --git a/docs/release-readiness.md b/docs/release-readiness.md index 233e0bf..044d6c0 100644 --- a/docs/release-readiness.md +++ b/docs/release-readiness.md @@ -34,7 +34,7 @@ Known live behavior: Entra app registration status as of 2026-05-27: - OSO public client app is multi-tenant (`AzureADMultipleOrgs`). -- Public client redirect URI is configured as `http://localhost:8400/callback`. +- Public client redirect URI is configured as `http://localhost/callback`. - Public client/device-code fallback is enabled. - Implicit grant access-token and ID-token issuance are disabled. - Home tenant admin consent has been granted for the current delegated scope bundle. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index bcb51d6..7303669 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -35,7 +35,7 @@ Check: - Tenant allows user consent or admin consent has been granted. - The OSO app is allowed by enterprise app policy. - BYO profile has `auth_app = "byo"` and a valid `client_id`. -- Browser PKCE redirect `http://localhost:8400/callback` is allowed. +- Browser PKCE redirect URI (default `http://localhost/callback`) is registered on the app. - Device-code login is used for SSH or remote terminals. Useful command: @@ -203,19 +203,27 @@ In headless Linux CI, prefer one of: ## Browser login callback fails -Browser login uses PKCE and the redirect URI: +Browser login uses PKCE and, by default, the redirect URI: ```text -http://localhost:8400/callback +http://localhost/callback ``` +on a free port the OS picks. `--redirect-uri` selects another registered one, +for example `--redirect-uri http://localhost` for an app registered with +`http://localhost`; a URI with a port fixes the port (for a firewall rule, say). + Check: -- Port `8400` is free. +- If the redirect URI names a port, that port is free. - Local firewall allows loopback. -- BYO app registration includes that redirect URI. +- BYO app registration includes that redirect URI. `AADSTS50011` means the URI is not registered on the app: register it, or pass the registered one. - Use `--device-code` if a browser callback is not possible. +If you decline consent or the identity platform refuses the sign-in, the +login fails at once with its `error` and `error_description` (exit code 3). +With no callback at all, it gives up after 10 minutes. + ## Rate limiting Exit code `6` means rate limited after retries. diff --git a/src/auth/auth_code_pkce.rs b/src/auth/auth_code_pkce.rs index 7287dec..66df1cd 100644 --- a/src/auth/auth_code_pkce.rs +++ b/src/auth/auth_code_pkce.rs @@ -1,16 +1,21 @@ use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine}; use bytes::Bytes; +use http_body_util::Full; +use hyper::{Response, StatusCode, Uri}; use reqwest::Client; use sha2::{Digest, Sha256}; +use std::collections::HashMap; use std::net::SocketAddr; -use std::sync::Arc; -use std::sync::Mutex; -use tokio::sync::oneshot; +use std::time::Duration; +use tokio::sync::mpsc; use super::token::MsTokenResponse; -use crate::config::{DEFAULT_DELEGATED_SCOPES, DEFAULT_REDIRECT_URI}; +use crate::config::DEFAULT_DELEGATED_SCOPES; use crate::error::{Result, TeamsError}; +/// How long the browser login waits for the redirect back to the CLI. +const CALLBACK_TIMEOUT: Duration = Duration::from_secs(10 * 60); + fn random_urlsafe_bytes(len: usize) -> Result { let mut bytes = vec![0u8; len]; getrandom::fill(&mut bytes) @@ -43,6 +48,7 @@ fn build_authorize_url( scopes: &str, state: &str, challenge: &str, + redirect_uri: &str, ) -> String { format!( "https://login.microsoftonline.com/{tenant_id}/oauth2/v2.0/authorize?\ @@ -54,109 +60,282 @@ fn build_authorize_url( &code_challenge={challenge}\ &code_challenge_method=S256\ &prompt=select_account", - urlencoding::encode(DEFAULT_REDIRECT_URI), + urlencoding::encode(redirect_uri), urlencoding::encode(scopes), ) } +/// The loopback socket addresses the redirect URI names. `localhost` can +/// resolve to ::1 or 127.0.0.1 depending on the browser, so both are +/// returned and the login listens on each. A URI without a port, or with +/// port 0 or 80, leaves the port to the OS (the url crate reports port 80 as +/// none), so those addresses carry port 0. +pub(crate) fn loopback_addrs(redirect_uri: &str) -> Result> { + let url = url::Url::parse(redirect_uri).map_err(|e| { + TeamsError::InvalidInput(format!("Invalid redirect URI '{redirect_uri}': {e}")) + })?; + let reject = || { + TeamsError::InvalidInput(format!( + "Redirect URI '{redirect_uri}' must use http and point at localhost or a loopback address" + )) + }; + // Only `localhost` and IP literals are resolved, so a typo never becomes a DNS lookup. + let host_ok = match url.host() { + Some(url::Host::Domain(name)) => name == "localhost", + Some(_) => true, + None => false, + }; + if url.scheme() != "http" || !host_ok { + return Err(reject()); + } + let port = url.port().unwrap_or(0); + let addrs: Vec = url + .socket_addrs(|| None) + .map_err(|e| { + TeamsError::InvalidInput(format!("Invalid redirect URI '{redirect_uri}': {e}")) + })? + .into_iter() + .filter(|addr| addr.ip().is_loopback()) + .map(|mut addr| { + addr.set_port(port); + addr + }) + .collect(); + if addrs.is_empty() { + return Err(reject()); + } + Ok(addrs) +} + +/// Listen on each address. A machine without IPv6 cannot bind `[::1]`, so one +/// bound address is enough, but a port held by another process is an error: +/// the browser could deliver the authorization code to that process. Port 0 +/// asks the OS for a free port, which the other addresses then share, so the +/// redirect URI names one port whichever address the browser connects to. +/// Returns the listeners and that port. +async fn bind_loopback(addrs: Vec) -> Result<(Vec, u16)> { + let mut listeners = Vec::new(); + let mut port = 0; + let mut skipped = None; + for mut addr in addrs { + if addr.port() == 0 { + addr.set_port(port); + } + match tokio::net::TcpListener::bind(addr).await { + Ok(listener) => { + port = listener + .local_addr() + .map_err(|e| { + TeamsError::AuthError(format!( + "Failed to bind loopback server on {addr}: {e}" + )) + })? + .port(); + listeners.push(listener); + } + Err(e) if e.kind() == std::io::ErrorKind::AddrInUse => { + return Err(TeamsError::AuthError(format!( + "Failed to bind loopback server on {addr}: {e}" + ))) + } + Err(e) => { + tracing::debug!("Not listening on {addr}: {e}"); + skipped = Some((addr, e)); + } + } + } + if listeners.is_empty() { + let (addr, e) = skipped + .ok_or_else(|| TeamsError::AuthError("No loopback address to listen on".into()))?; + return Err(TeamsError::AuthError(format!( + "Failed to bind loopback server on {addr}: {e}" + ))); + } + Ok((listeners, port)) +} + +/// The redirect URI with the listener's port in it, when the URI left the +/// port to the OS. A URI that names a port is sent exactly as given. +fn redirect_uri_with_port(redirect_uri: &str, port: u16) -> Result { + let mut url = url::Url::parse(redirect_uri).map_err(|e| { + TeamsError::InvalidInput(format!("Invalid redirect URI '{redirect_uri}': {e}")) + })?; + if url.port().unwrap_or(0) != 0 { + return Ok(redirect_uri.to_string()); + } + url.set_port(Some(port)) + .map_err(|()| TeamsError::AuthError("Cannot set the redirect URI port".into()))?; + Ok(url.to_string()) +} + +/// What the redirect back to the loopback server carried. +enum Callback { + Code(String), + Error(String), +} + +/// Serve the redirect URI on `listeners` until a request to `path` carries +/// `state` and either an authorization code or an error, and return the code. +/// +/// Every connection on every listener is served, and anything else is +/// answered and ignored: a browser's speculative connection, a favicon +/// request, or a callback with another state cannot end the wait. An error +/// from the identity platform fails the login with its description, and so +/// does `timeout` passing without a callback. +async fn receive_callback( + listeners: Vec, + path: String, + state: String, + timeout: Duration, +) -> Result { + use hyper::service::service_fn; + use hyper_util::rt::TokioIo; + + let (tx, mut rx) = mpsc::channel::(1); + let mut accept_loops = tokio::task::JoinSet::new(); + for listener in listeners { + let (tx, path, state) = (tx.clone(), path.clone(), state.clone()); + accept_loops.spawn(async move { + loop { + let stream = match listener.accept().await { + Ok((stream, _)) => stream, + Err(e) => { + tracing::debug!("Login callback accept failed: {e}"); + tokio::time::sleep(Duration::from_millis(50)).await; + continue; + } + }; + let (tx, path, state) = (tx.clone(), path.clone(), state.clone()); + tokio::spawn(async move { + let service = service_fn(move |req: hyper::Request| { + let reply = answer_callback(req.uri(), &path, &state, &tx); + async move { Ok::<_, std::convert::Infallible>(reply) } + }); + let _ = hyper::server::conn::http1::Builder::new() + .serve_connection(TokioIo::new(stream), service) + .await; + }); + } + }); + } + drop(tx); + + let received = tokio::time::timeout(timeout, rx.recv()).await; + accept_loops.abort_all(); + match received { + Ok(Some(Callback::Code(code))) => Ok(code), + Ok(Some(Callback::Error(message))) => Err(TeamsError::AuthError(message)), + Ok(None) => Err(TeamsError::AuthError( + "The login callback server stopped before the browser returned".into(), + )), + Err(_) => { + let secs = timeout.as_secs(); + let waited = if secs >= 60 { + format!("{} minutes", secs / 60) + } else { + format!("{secs} seconds") + }; + Err(TeamsError::AuthError(format!( + "No sign-in came back from the browser within {waited}. Run \ + `teams auth login` again, or use `teams auth login --device-code`." + ))) + } + } +} + +/// The loopback server's reply to one request. A request to the callback +/// path with this login's state sends its code or error on `tx`. +fn answer_callback( + uri: &Uri, + path: &str, + state: &str, + tx: &mpsc::Sender, +) -> Response> { + if uri.path() != path { + return callback_page(StatusCode::NOT_FOUND, "Not found."); + } + let params: HashMap = + url::form_urlencoded::parse(uri.query().unwrap_or("").as_bytes()) + .into_owned() + .collect(); + if params.get("state").map(String::as_str) != Some(state) { + return callback_page( + StatusCode::BAD_REQUEST, + "This sign-in response is not for the login in progress.", + ); + } + if let Some(code) = params.get("code") { + let _ = tx.try_send(Callback::Code(code.clone())); + return callback_page( + StatusCode::OK, + "Authentication successful! You can close this tab.", + ); + } + if let Some(error) = params.get("error") { + // Entra's descriptions span lines (trace and correlation IDs). + let message = match params.get("error_description") { + Some(description) => format!( + "Sign-in failed: {error}: {}", + description.split_whitespace().collect::>().join(" ") + ), + None => format!("Sign-in failed: {error}"), + }; + let page = format!("{message}\n\nReturn to the terminal."); + let _ = tx.try_send(Callback::Error(message)); + return callback_page(StatusCode::OK, &page); + } + callback_page(StatusCode::BAD_REQUEST, "Missing code or state parameter.") +} + +fn callback_page(status: StatusCode, text: &str) -> Response> { + Response::builder() + .status(status) + .header("Content-Type", "text/plain; charset=utf-8") + .header("Connection", "close") + .body(Full::new(Bytes::from(text.to_string()))) + .expect("static response parts are valid") +} + /// Authenticate using authorization code flow with PKCE. /// Opens a browser and starts a loopback HTTP server to receive the callback. pub async fn authenticate( client_id: &str, tenant_id: &str, scopes: Option<&str>, + redirect_uri: &str, ) -> Result { + let addrs = loopback_addrs(redirect_uri)?; let scopes = scopes.unwrap_or(DEFAULT_DELEGATED_SCOPES); let (verifier, challenge) = generate_pkce()?; let state = random_urlsafe_bytes(16)?; - let auth_url = build_authorize_url(client_id, tenant_id, scopes, &state, &challenge); - - // Start loopback server - let (tx, rx) = oneshot::channel::(); - let expected_state = state.clone(); - - let addr: SocketAddr = "127.0.0.1:8400".parse().unwrap(); - let listener = tokio::net::TcpListener::bind(addr) - .await - .map_err(|e| TeamsError::AuthError(format!("Failed to bind loopback server: {e}")))?; - - let server_handle = tokio::spawn(async move { - use http_body_util::Full; - use hyper::body::Incoming; - use hyper::service::service_fn; - use hyper::{Request, Response}; - use hyper_util::rt::TokioIo; - - let tx = Arc::new(Mutex::new(Some(tx))); - - if let Ok((stream, _)) = listener.accept().await { - let io = TokioIo::new(stream); - let expected = expected_state.clone(); - let tx = Arc::clone(&tx); - - let _ = hyper::server::conn::http1::Builder::new() - .serve_connection( - io, - service_fn(move |req: Request| { - let expected = expected.clone(); - let tx = Arc::clone(&tx); - async move { - let query = req.uri().query().unwrap_or(""); - let params: Vec<(String, String)> = - url::form_urlencoded::parse(query.as_bytes()) - .into_owned() - .collect(); - - let code = params - .iter() - .find(|(k, _)| k == "code") - .map(|(_, v)| v.clone()); - let state = params - .iter() - .find(|(k, _)| k == "state") - .map(|(_, v)| v.clone()); - - if let (Some(code), Some(state)) = (code, state) { - if state == expected { - if let Some(tx) = tx.lock().unwrap().take() { - let _ = tx.send(code); - } - Ok::<_, hyper::Error>(Response::new(Full::new(Bytes::from( - "Authentication successful! You can close this tab.", - )))) - } else { - Ok(Response::new(Full::new(Bytes::from( - "State mismatch error.", - )))) - } - } else { - Ok(Response::new(Full::new(Bytes::from( - "Missing code or state parameter.", - )))) - } - } - }), - ) - .await - .ok(); - } - }); - - // Open browser + let (listeners, port) = bind_loopback(addrs).await?; + let redirect_uri = redirect_uri_with_port(redirect_uri, port)?; + let callback_path = url::Url::parse(&redirect_uri) + .map_err(|e| { + TeamsError::InvalidInput(format!("Invalid redirect URI '{redirect_uri}': {e}")) + })? + .path() + .to_string(); + let auth_url = build_authorize_url( + client_id, + tenant_id, + scopes, + &state, + &challenge, + &redirect_uri, + ); + + // The listeners are bound, so a fast redirect waits in their backlog + // until the callback server below accepts it. eprintln!("Opening browser for authentication..."); if webbrowser::open(&auth_url).is_err() { eprintln!("Could not open browser. Please visit this URL manually:"); eprintln!("{auth_url}"); } - // Wait for callback - let code = rx - .await - .map_err(|_| TeamsError::AuthError("Failed to receive auth code from callback".into()))?; - - server_handle.abort(); + let code = receive_callback(listeners, callback_path, state, CALLBACK_TIMEOUT).await?; // Exchange code for token let token_url = format!("https://login.microsoftonline.com/{tenant_id}/oauth2/v2.0/token"); @@ -168,7 +347,7 @@ pub async fn authenticate( ("grant_type", "authorization_code"), ("client_id", client_id), ("code", &code), - ("redirect_uri", DEFAULT_REDIRECT_URI), + ("redirect_uri", redirect_uri.as_str()), ("code_verifier", &verifier), ("scope", scopes), ]) @@ -203,6 +382,7 @@ mod tests { "User.Read offline_access", "st4te", "ch4llenge", + "http://localhost:8400", ); assert!( @@ -210,6 +390,7 @@ mod tests { ); assert!(url.contains("client_id=client-1")); assert!(url.contains("&response_type=code")); + assert!(url.contains("&redirect_uri=http%3A%2F%2Flocalhost%3A8400")); assert!(url.contains("&scope=User.Read%20offline_access")); assert!(url.contains("&state=st4te")); assert!(url.contains("&code_challenge=ch4llenge")); @@ -238,8 +419,193 @@ mod tests { #[test] fn authorize_url_forces_account_picker() { - let url = build_authorize_url("client-1", "tenant-1", "User.Read", "s", "c"); + let url = build_authorize_url( + "client-1", + "tenant-1", + "User.Read", + "s", + "c", + "http://localhost:8400", + ); assert!(url.contains("&prompt=select_account")); } + + /// One HTTP/1.1 GET to `addr`; returns the status and body. + async fn get(addr: SocketAddr, target: &str) -> (u16, String) { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + let mut stream = tokio::net::TcpStream::connect(addr).await.unwrap(); + let request = + format!("GET {target} HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n"); + stream.write_all(request.as_bytes()).await.unwrap(); + let mut buf = Vec::new(); + stream.read_to_end(&mut buf).await.unwrap(); + let text = String::from_utf8_lossy(&buf).into_owned(); + let status = text.split(' ').nth(1).unwrap().parse().unwrap(); + let body = text.split("\r\n\r\n").nth(1).unwrap_or("").to_string(); + (status, body) + } + + /// A callback server for path `/callback` and state `st4te`. + async fn callback_server( + timeout: Duration, + ) -> (SocketAddr, tokio::task::JoinHandle>) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let login = tokio::spawn(receive_callback( + vec![listener], + "/callback".into(), + "st4te".into(), + timeout, + )); + (addr, login) + } + + #[tokio::test] + async fn callback_waits_past_stray_connections_and_other_requests() { + let (addr, login) = callback_server(Duration::from_secs(30)).await; + + // A speculative connection that never sends a request. + let _idle = tokio::net::TcpStream::connect(addr).await.unwrap(); + assert_eq!(get(addr, "/favicon.ico").await.0, 404); + assert_eq!(get(addr, "/callback?code=forged&state=other").await.0, 400); + assert_eq!(get(addr, "/callback?state=st4te").await.0, 400); + assert!(!login.is_finished()); + + let (status, body) = get(addr, "/callback?code=c0de&state=st4te").await; + assert_eq!(status, 200); + assert!(body.contains("successful"), "{body}"); + assert_eq!(login.await.unwrap().unwrap(), "c0de"); + } + + #[tokio::test] + async fn an_error_redirect_fails_the_login_with_its_description() { + let (addr, login) = callback_server(Duration::from_secs(30)).await; + + // Another login's error is not this one's. + let other = "/callback?error=access_denied&state=other"; + assert_eq!(get(addr, other).await.0, 400); + assert!(!login.is_finished()); + + let target = "/callback?error=access_denied&error_description=\ + AADSTS65004%3A+User+declined+to+consent.%0D%0ATrace+ID%3A+abc&state=st4te"; + let (status, body) = get(addr, target).await; + assert_eq!(status, 200); + assert!(body.contains("access_denied"), "{body}"); + + let err = login.await.unwrap().unwrap_err(); + assert_eq!(err.exit_code(), 3); + let message = err.to_string(); + assert!( + message.contains("access_denied: AADSTS65004: User declined to consent. Trace ID: abc"), + "{message}" + ); + } + + #[tokio::test] + async fn waiting_for_the_callback_times_out() { + let (_addr, login) = callback_server(Duration::from_millis(200)).await; + let err = login.await.unwrap().unwrap_err(); + assert_eq!(err.exit_code(), 3); + assert!(err.to_string().contains("teams auth login"), "{err}"); + } + + #[test] + fn loopback_addrs_follow_the_redirect_uri() { + let addrs = loopback_addrs("http://localhost:8400/callback").unwrap(); + assert!(!addrs.is_empty()); + assert!(addrs + .iter() + .all(|a| a.ip().is_loopback() && a.port() == 8400)); + + assert_eq!( + loopback_addrs("http://127.0.0.1:9400").unwrap(), + vec!["127.0.0.1:9400".parse::().unwrap()] + ); + assert_eq!( + loopback_addrs("http://[::1]:8400").unwrap(), + vec!["[::1]:8400".parse::().unwrap()] + ); + for uri in [ + "http://localhost", + "http://localhost:80/callback", + "http://localhost:0", + ] { + assert!( + loopback_addrs(uri).unwrap().iter().all(|a| a.port() == 0), + "{uri}" + ); + } + } + + #[test] + fn loopback_addrs_reject_other_uris() { + for uri in [ + "https://localhost:8400", + "http://10.0.0.1:8400", + "localhost:8400", + "not a url", + "http://example.invalid:8400", + ] { + assert_eq!(loopback_addrs(uri).unwrap_err().exit_code(), 2, "{uri}"); + } + } + + /// A busy port is an error, because another process would get the + /// authorization code; an address this machine does not have, as `[::1]` + /// on a machine without IPv6, is skipped. 192.0.2.1 (TEST-NET-1) is never + /// a local address. + #[tokio::test] + async fn bind_loopback_skips_an_unavailable_address_but_not_a_busy_port() { + let held = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let busy = held.local_addr().unwrap(); + let err = bind_loopback(vec![busy]).await.unwrap_err(); + assert_eq!(err.exit_code(), 3, "{err}"); + assert!(err.to_string().contains(&busy.to_string()), "{err}"); + + let unavailable: SocketAddr = "192.0.2.1:0".parse().unwrap(); + let free: SocketAddr = "127.0.0.1:0".parse().unwrap(); + assert_eq!( + bind_loopback(vec![unavailable, free]) + .await + .unwrap() + .0 + .len(), + 1 + ); + assert!(bind_loopback(vec![unavailable]).await.is_err()); + } + + /// Port 0 gets a free port from the OS, and every other address gets the + /// same one, so the redirect URI names one port. [::1] may be skipped on a + /// machine without IPv6, which is why only the bound ones are checked. + #[tokio::test] + async fn bind_loopback_shares_one_free_port() { + let addrs: Vec = + vec!["127.0.0.1:0".parse().unwrap(), "[::1]:0".parse().unwrap()]; + let (listeners, port) = bind_loopback(addrs).await.unwrap(); + assert_ne!(port, 0); + assert!(!listeners.is_empty()); + assert!(listeners + .iter() + .all(|l| l.local_addr().unwrap().port() == port)); + } + + #[test] + fn redirect_uri_gets_the_listener_port_only_when_it_names_none() { + for (uri, want) in [ + ("http://localhost", "http://localhost:54321/"), + ( + "http://localhost/callback", + "http://localhost:54321/callback", + ), + ( + "http://localhost:8400/callback", + "http://localhost:8400/callback", + ), + ("http://localhost:0", "http://localhost:54321/"), + ] { + assert_eq!(redirect_uri_with_port(uri, 54321).unwrap(), want, "{uri}"); + } + } } diff --git a/src/cli/auth.rs b/src/cli/auth.rs index f4b8185..9a13a22 100644 --- a/src/cli/auth.rs +++ b/src/cli/auth.rs @@ -33,6 +33,15 @@ pub struct LoginArgs { #[arg(long)] tenant_id: Option, + /// Redirect URI for the browser login: the CLI listens on its host and + /// port (localhost or a loopback address) and sends it to the identity + /// platform, where it must match the app registration. Without a port, + /// a free port is used. Saved to the profile after a successful delegated + /// login. TEAMS_CLI_REDIRECT_URI is used when this is absent, but is not + /// saved [default: http://localhost/callback] + #[arg(long)] + redirect_uri: Option, + /// OAuth scopes (space-separated, for delegated flows) #[arg(long, env = "TEAMS_CLI_SCOPES")] scopes: Option, @@ -60,6 +69,10 @@ pub enum AuthCommand { #[arg(long, env = "TEAMS_CLI_TENANT_ID")] tenant_id: Option, + /// Redirect URI registered on the app for the browser login + #[arg(long, env = "TEAMS_CLI_REDIRECT_URI")] + redirect_uri: Option, + /// OAuth scopes (space-separated) to include in the consent URL #[arg(long, env = "TEAMS_CLI_SCOPES")] scopes: Option, @@ -73,6 +86,10 @@ pub enum AuthCommand { /// Azure AD tenant ID or domain #[arg(long, env = "TEAMS_CLI_TENANT_ID")] tenant_id: Option, + + /// Redirect URI registered on the app for the browser login + #[arg(long, env = "TEAMS_CLI_REDIRECT_URI")] + redirect_uri: Option, }, /// List stored profiles and the account each one holds List, @@ -211,12 +228,17 @@ fn profile_row(summary: &serde_json::Value, active: &str) -> Vec { ] } -fn delegated_admin_consent_url(client_id: &str, tenant_id: &str, delegated_scopes: &str) -> String { +fn delegated_admin_consent_url( + client_id: &str, + tenant_id: &str, + delegated_scopes: &str, + redirect_uri: &str, +) -> String { let scopes = graph_admin_consent_scopes(delegated_scopes); format!( "https://login.microsoftonline.com/{tenant_id}/v2.0/adminconsent?client_id={client_id}&scope={}&redirect_uri={}", urlencoding::encode(&scopes), - urlencoding::encode(config::DEFAULT_REDIRECT_URI) + urlencoding::encode(redirect_uri) ) } @@ -250,8 +272,8 @@ fn login_registration( Ok((client_id, tenant_id)) } -/// Save the IDs given on the command line to the profile, editing only those -/// two keys in the config file. The file is read again here rather than +/// Save the IDs and redirect URI given on the command line to the profile, +/// editing only those three keys in the config file. The file is read again here rather than /// reused from startup, because an interactive login can take minutes and /// the file may have been edited meanwhile. /// @@ -282,6 +304,7 @@ fn save_registration(args: &LoginArgs, config_path: Option<&str>, profile: &str) profile, args.client_id.as_deref(), args.tenant_id.as_deref(), + args.redirect_uri.as_deref(), )?; match updated { Some(updated) => config::write_config_text(&path, &updated).map(|()| true), @@ -294,7 +317,7 @@ fn save_registration(args: &LoginArgs, config_path: Option<&str>, profile: &str) fn save_registration_or_warn(args: &LoginArgs, config_path: Option<&str>, profile: &str) -> bool { save_registration(args, config_path, profile).unwrap_or_else(|e| { eprintln!( - "Warning: signed in, but could not save the client and tenant IDs to profile \ + "Warning: signed in, but could not save the client ID, tenant ID and redirect URI to profile \ '{profile}': {e}. Pass them again at the next login." ); false @@ -338,6 +361,11 @@ async fn login( let start = Instant::now(); config::check_new_profile_name(profile, config)?; let (client_id, tenant_id) = login_registration(&args, config, profile)?; + // A given redirect URI is saved to the profile after any delegated login, + // so it is checked here even when the device code flow does not use it. + if let Some(uri) = args.redirect_uri.as_deref() { + auth::auth_code_pkce::loopback_addrs(uri)?; + } let client_secret = if args.client_credentials { Some( config::resolve_client_secret(args.client_secret.as_deref()).ok_or_else(|| { @@ -358,7 +386,10 @@ async fn login( if args.device_code { auth::device_code::authenticate(&client_id, &tenant_id, Some(&scopes)).await? } else { - auth::auth_code_pkce::authenticate(&client_id, &tenant_id, Some(&scopes)).await? + let redirect_uri = + config::resolve_redirect_uri(args.redirect_uri.as_deref(), profile, config); + auth::auth_code_pkce::authenticate(&client_id, &tenant_id, Some(&scopes), &redirect_uri) + .await? } }; @@ -416,6 +447,7 @@ pub async fn run( AuthCommand::ConsentUrl { client_id, tenant_id, + redirect_uri, scopes, } => { let start = Instant::now(); @@ -423,14 +455,16 @@ pub async fn run( config::resolve_delegated_client_id(client_id.as_deref(), profile, config)?; let tenant_id = config::resolve_delegated_tenant_id(tenant_id.as_deref(), profile, config); + let redirect_uri = + config::resolve_redirect_uri(redirect_uri.as_deref(), profile, config); let scopes = config::resolve_delegated_scopes(scopes.as_deref(), profile, config); - let url = delegated_admin_consent_url(&client_id, &tenant_id, &scopes); + let url = delegated_admin_consent_url(&client_id, &tenant_id, &scopes, &redirect_uri); let msg = serde_json::json!({ "admin_consent_url": url, "client_id": client_id, "tenant_id": tenant_id, "scope": scopes, - "redirect_uri": config::DEFAULT_REDIRECT_URI, + "redirect_uri": redirect_uri, }); output::print_success(format, &msg, start); Ok(()) @@ -439,12 +473,15 @@ pub async fn run( AuthCommand::Doctor { client_id, tenant_id, + redirect_uri, } => { let start = Instant::now(); let client_id = config::resolve_delegated_client_id(client_id.as_deref(), profile, config)?; let tenant_id = config::resolve_delegated_tenant_id(tenant_id.as_deref(), profile, config); + let redirect_uri = + config::resolve_redirect_uri(redirect_uri.as_deref(), profile, config); let auth_app = if client_id == config::OSO_PUBLIC_CLIENT_ID { "oso" } else { @@ -455,8 +492,12 @@ pub async fn run( let claims = token.as_ref().and_then(|t| t.unverified_claims()); let warnings = token_warnings(claims.as_ref()); let resolved_scopes = config::resolve_delegated_scopes(None, profile, config); - let admin_consent_url = - delegated_admin_consent_url(&client_id, &tenant_id, &resolved_scopes); + let admin_consent_url = delegated_admin_consent_url( + &client_id, + &tenant_id, + &resolved_scopes, + &redirect_uri, + ); let msg = serde_json::json!({ "profile": profile, "auth_app": auth_app, @@ -465,7 +506,7 @@ pub async fn run( "admin_consent_url": admin_consent_url, "default_delegated_scopes": config::DEFAULT_DELEGATED_SCOPES, "resolved_delegated_scopes": resolved_scopes, - "redirect_uri": config::DEFAULT_REDIRECT_URI, + "redirect_uri": redirect_uri, "authenticated": token.is_some(), "warnings": warnings, "token": token.as_ref().map(|t| serde_json::json!({ @@ -763,6 +804,7 @@ mod tests { client_id: client_id.map(str::to_string), client_secret: None, tenant_id: client_id.map(|_| "tenant-1".to_string()), + redirect_uri: None, scopes: None, } } @@ -784,6 +826,25 @@ mod tests { assert!(!save_registration(&args, Some(path), "work").unwrap()); } + #[test] + fn a_delegated_login_saves_the_redirect_uri_it_was_given() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + let path = path.to_str().unwrap(); + let mut args = login_args(false, Some("app-1")); + assert!(save_registration(&args, Some(path), "work").unwrap()); + let saved = config::load_config(Some(path)).unwrap(); + assert_eq!(saved.profiles["work"].redirect_uri, None); + + args.redirect_uri = Some("http://localhost:8400".to_string()); + assert!(save_registration(&args, Some(path), "work").unwrap()); + let saved = config::load_config(Some(path)).unwrap(); + assert_eq!( + saved.profiles["work"].redirect_uri.as_deref(), + Some("http://localhost:8400") + ); + } + /// A client credentials login saves nothing: the profile's next delegated /// login would otherwise sign in through the confidential application. #[test] diff --git a/src/config.rs b/src/config.rs index 3aa023a..ef6a575 100644 --- a/src/config.rs +++ b/src/config.rs @@ -10,7 +10,8 @@ use crate::error::{Result, TeamsError}; pub const OSO_PUBLIC_CLIENT_ID: &str = "fba1b5d0-fdd0-4fe2-9729-9ccdc38f9595"; pub const DEFAULT_DELEGATED_TENANT_ID: &str = "organizations"; pub const DEFAULT_DELEGATED_SCOPES: &str = "User.Read Team.ReadBasic.All Channel.ReadBasic.All ChannelMessage.Send Chat.ReadWrite ChatMessage.Send ChatMessage.Read User.ReadBasic.All Presence.Read.All Presence.ReadWrite offline_access"; -pub const DEFAULT_REDIRECT_URI: &str = "http://localhost:8400/callback"; +// The port is left to the OS at login, which the identity platform allows because it ignores the port of a localhost redirect URI. +pub const DEFAULT_REDIRECT_URI: &str = "http://localhost/callback"; /// Default storage namespace of release builds. pub const RELEASE_NAMESPACE: &str = "teams-cli"; @@ -159,6 +160,7 @@ pub struct ProfileConfig { pub tenant_id: Option, pub auth_flow: Option, pub scopes: Option, + pub redirect_uri: Option, } fn default_format() -> String { @@ -356,6 +358,22 @@ pub fn resolve_delegated_tenant_id( .unwrap_or_else(|| DEFAULT_DELEGATED_TENANT_ID.to_string()) } +/// The redirect URI for the browser login: flag, `TEAMS_CLI_REDIRECT_URI`, +/// the profile, then the built-in default. +pub fn resolve_redirect_uri(cli_flag: Option<&str>, profile: &str, config: &ConfigFile) -> String { + if let Some(v) = cli_flag { + return v.to_string(); + } + if let Ok(v) = std::env::var("TEAMS_CLI_REDIRECT_URI") { + return v; + } + config + .profiles + .get(profile) + .and_then(|p| p.redirect_uri.clone()) + .unwrap_or_else(|| DEFAULT_REDIRECT_URI.to_string()) +} + /// Where login found the ID it signs in with, for the notice it prints. The /// order is the resolution order. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -393,9 +411,10 @@ impl IdSource { } } -/// Set a profile's `client_id` and `tenant_id` in the text of a config file, -/// so that the next `auth login` for the profile signs in through the same -/// application registration without being told again. Only these two keys +/// Set a profile's `client_id`, `tenant_id` and `redirect_uri` in the text of +/// a config file, so that the next `auth login` for the profile signs in +/// through the same application registration without being told again. Only +/// these three keys /// change; comments, formatting, other profiles and keys this version does /// not know are left as they were. Blank values are ignored. Returns the new /// text, or `None` when the profile already holds these values. @@ -404,6 +423,7 @@ pub fn remember_registration( profile: &str, client_id: Option<&str>, tenant_id: Option<&str>, + redirect_uri: Option<&str>, ) -> Result> { let mut document: toml_edit::DocumentMut = text .parse() @@ -427,7 +447,11 @@ pub fn remember_registration( .ok_or_else(|| not_a_table(&format!("profiles.{profile}")))?; let mut changed = false; - for (key, value) in [("client_id", client_id), ("tenant_id", tenant_id)] { + for (key, value) in [ + ("client_id", client_id), + ("tenant_id", tenant_id), + ("redirect_uri", redirect_uri), + ] { let Some(value) = value.map(str::trim).filter(|v| !v.is_empty()) else { continue; }; @@ -761,6 +785,7 @@ scopes = "User.Read People.Read offline_access" tenant_id: None, auth_flow: None, scopes: None, + redirect_uri: None, }, ); @@ -794,6 +819,30 @@ scopes = "User.Read People.Read offline_access" ); } + #[test] + fn resolve_redirect_uri_priority() { + let mut config = ConfigFile::default(); + assert_eq!( + resolve_redirect_uri(None, "work", &config), + DEFAULT_REDIRECT_URI + ); + config.profiles.insert( + "work".to_string(), + ProfileConfig { + redirect_uri: Some("http://localhost:9400".to_string()), + ..Default::default() + }, + ); + assert_eq!( + resolve_redirect_uri(None, "work", &config), + "http://localhost:9400" + ); + assert_eq!( + resolve_redirect_uri(Some("http://localhost:9500"), "work", &config), + "http://localhost:9500" + ); + } + #[test] fn default_delegated_scopes_avoid_admin_required_channel_read() { assert!(DEFAULT_DELEGATED_SCOPES.contains("ChatMessage.Send")); @@ -900,6 +949,7 @@ scopes = "User.Read People.Read offline_access" tenant_id: Some("tenant".into()), auth_flow: Some("device-code".into()), scopes: None, + redirect_uri: None, }, ); @@ -1010,8 +1060,14 @@ scopes = "User.Read People.Read offline_access" ); } - fn remember(text: &str, profile: &str, client: Option<&str>, tenant: Option<&str>) -> String { - remember_registration(text, profile, client, tenant) + fn remember( + text: &str, + profile: &str, + client: Option<&str>, + tenant: Option<&str>, + redirect: Option<&str>, + ) -> String { + remember_registration(text, profile, client, tenant, redirect) .unwrap() .expect("the registration should change the file") } @@ -1022,10 +1078,16 @@ scopes = "User.Read People.Read offline_access" #[test] fn remember_registration_creates_the_file_and_the_profile() { - let text = remember("", "benbot", Some("app-1"), Some("tenant-1")); + let text = remember( + "", + "benbot", + Some("app-1"), + Some("tenant-1"), + Some("http://localhost:8400"), + ); assert_eq!( text, - "[profiles.benbot]\nclient_id = \"app-1\"\ntenant_id = \"tenant-1\"\n" + "[profiles.benbot]\nclient_id = \"app-1\"\ntenant_id = \"tenant-1\"\nredirect_uri = \"http://localhost:8400\"\n" ); } @@ -1050,7 +1112,7 @@ scopes = \"User.Read\" [profiles.other] client_id = \"other-app\" "; - let text = remember(original, "work", Some("new-app"), None); + let text = remember(original, "work", Some("new-app"), None, None); assert_eq!( text, original.replace("client_id = \"old-app\"", "client_id = \"new-app\"") @@ -1060,7 +1122,7 @@ client_id = \"other-app\" #[test] fn remember_registration_keeps_a_replaced_value_s_comment_and_spacing() { let original = "[profiles.work]\nclient_id = \"old-app\" # customer app\n"; - let text = remember(original, "work", Some("new-app"), None); + let text = remember(original, "work", Some("new-app"), None, None); assert_eq!( text, "[profiles.work]\nclient_id = \"new-app\" # customer app\n" @@ -1071,12 +1133,12 @@ client_id = \"other-app\" fn remember_registration_edits_dotted_and_inline_profiles() { let dotted = "profiles.work.client_id = \"old-app\"\n"; assert_eq!( - remember(dotted, "work", Some("new-app"), None), + remember(dotted, "work", Some("new-app"), None, None), "profiles.work.client_id = \"new-app\"\n" ); let inline = "[profiles]\nwork = { client_id = \"old-app\" }\n"; assert_eq!( - remember(inline, "work", Some("new-app"), None), + remember(inline, "work", Some("new-app"), None, None), "[profiles]\nwork = { client_id = \"new-app\" }\n" ); } @@ -1084,7 +1146,7 @@ client_id = \"other-app\" #[test] fn remember_registration_adds_a_profile_beside_existing_ones() { let original = "[profiles.work]\nclient_id = \"app-1\"\n"; - let text = remember(original, "benbot", Some("app-2"), None); + let text = remember(original, "benbot", Some("app-2"), None, None); let config = parse(&text); assert_eq!(config.profiles["work"].client_id.as_deref(), Some("app-1")); assert_eq!( @@ -1103,17 +1165,26 @@ client_id = \"other-app\" (Some(" "), Some("")), ] { assert_eq!( - remember_registration(original, "work", client, tenant).unwrap(), + remember_registration(original, "work", client, tenant, None).unwrap(), None ); } - assert_eq!(remember_registration("", "new", None, None).unwrap(), None); + assert_eq!( + remember_registration("", "new", None, None, None).unwrap(), + None + ); } #[test] fn remember_registration_trims_values_and_ignores_blank_ones() { let original = "[profiles.work]\nclient_id = \"app-1\"\ntenant_id = \"tenant-1\"\n"; - let config = parse(&remember(original, "work", Some(" app-2 "), Some(" "))); + let config = parse(&remember( + original, + "work", + Some(" app-2 "), + Some(" "), + None, + )); assert_eq!(config.profiles["work"].client_id.as_deref(), Some("app-2")); assert_eq!( config.profiles["work"].tenant_id.as_deref(), @@ -1123,8 +1194,10 @@ client_id = \"other-app\" #[test] fn remember_registration_refuses_a_file_it_cannot_edit_safely() { - assert!(remember_registration("profiles = \"x\"\n", "work", Some("a"), None).is_err()); - assert!(remember_registration("[profiles\n", "work", Some("a"), None).is_err()); + assert!( + remember_registration("profiles = \"x\"\n", "work", Some("a"), None, None).is_err() + ); + assert!(remember_registration("[profiles\n", "work", Some("a"), None, None).is_err()); } /// The saved IDs are what the next login resolves when it is given none, @@ -1136,6 +1209,7 @@ client_id = \"other-app\" "work", Some("app-1"), Some("tenant-1"), + None, ); let config = parse(&text); assert_eq!( diff --git a/tests/cli.rs b/tests/cli.rs index be54f7d..8a4474a 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -19,6 +19,7 @@ fn teams_process() -> std::process::Command { cmd.env_remove("TEAMS_CLI_CLIENT_ID"); cmd.env_remove("TEAMS_CLI_CLIENT_SECRET"); cmd.env_remove("TEAMS_CLI_TENANT_ID"); + cmd.env_remove("TEAMS_CLI_REDIRECT_URI"); cmd.env_remove("TEAMS_CLI_ACCESS_TOKEN"); cmd.env_remove("TEAMS_CLI_TOKEN_STORE"); cmd @@ -224,6 +225,160 @@ fn auth_login_help_documents_scopes_flag_and_env() { ); } +#[test] +fn auth_login_help_documents_redirect_uri_flag_and_env() { + teams() + .args(["auth", "login", "--help"]) + .assert() + .success() + .stdout( + predicate::str::contains("--redirect-uri ") + .and(predicate::str::contains("TEAMS_CLI_REDIRECT_URI")), + ); +} + +#[test] +fn auth_consent_url_uses_the_default_redirect_uri() { + teams() + .args(["auth", "consent-url", "--output", "json"]) + .assert() + .success() + .stdout( + predicate::str::contains(r#""redirect_uri": "http://localhost/callback""#).and( + predicate::str::contains("redirect_uri=http%3A%2F%2Flocalhost%2Fcallback"), + ), + ); +} + +#[test] +fn auth_consent_url_uses_redirect_uri_flag() { + teams() + .args([ + "auth", + "consent-url", + "--redirect-uri", + "http://localhost:8400", + "--output", + "json", + ]) + .assert() + .success() + .stdout( + predicate::str::contains(r#""redirect_uri": "http://localhost:8400""#) + .and(predicate::str::contains( + "redirect_uri=http%3A%2F%2Flocalhost%3A8400", + )) + .and(predicate::str::contains("%2Fcallback").not()), + ); +} + +#[test] +fn auth_consent_url_uses_redirect_uri_env() { + let mut cmd = teams(); + cmd.env("TEAMS_CLI_REDIRECT_URI", "http://127.0.0.1:9400"); + cmd.args(["auth", "consent-url", "--output", "json"]) + .assert() + .success() + .stdout(predicate::str::contains( + r#""redirect_uri": "http://127.0.0.1:9400""#, + )); +} + +#[test] +fn auth_consent_url_uses_profile_redirect_uri() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + fs::write( + &path, + r#" +[profiles.customer] +auth_app = "byo" +client_id = "11111111-1111-1111-1111-111111111111" +tenant_id = "22222222-2222-2222-2222-222222222222" +redirect_uri = "http://localhost:9400" +"#, + ) + .unwrap(); + + teams() + .args([ + "--config", + path.to_str().unwrap(), + "--profile", + "customer", + "auth", + "consent-url", + "--output", + "json", + ]) + .assert() + .success() + .stdout(predicate::str::contains( + r#""redirect_uri": "http://localhost:9400""#, + )); + + teams() + .args([ + "--config", + path.to_str().unwrap(), + "--profile", + "customer", + "auth", + "consent-url", + "--redirect-uri", + "http://localhost:9500", + "--output", + "json", + ]) + .assert() + .success() + .stdout(predicate::str::contains( + r#""redirect_uri": "http://localhost:9500""#, + )); +} + +#[test] +fn auth_doctor_reports_redirect_uri() { + teams() + .args([ + "auth", + "doctor", + "--redirect-uri", + "http://localhost:8400", + "--output", + "json", + ]) + .assert() + .success() + .stdout(predicate::str::contains( + r#""redirect_uri": "http://localhost:8400""#, + )); +} + +#[test] +fn auth_login_rejects_a_redirect_uri_that_is_not_loopback() { + teams() + .args(["auth", "login", "--redirect-uri", "http://10.0.0.1:8400"]) + .assert() + .code(2) + .stdout(predicate::str::contains("localhost")); + teams() + .args(["auth", "login", "--redirect-uri", "https://localhost:8400"]) + .assert() + .code(2); + teams() + .args([ + "auth", + "login", + "--device-code", + "--redirect-uri", + "http://10.0.0.1:8400", + ]) + .assert() + .code(2) + .stdout(predicate::str::contains("localhost")); +} + #[test] fn auth_help_shows_refresh_subcommand() { teams()