From 5e3bd95992cee58ce2f7688b9c690c9569da2567 Mon Sep 17 00:00:00 2001 From: Alex Fournier Date: Mon, 20 Jul 2026 12:55:21 -0700 Subject: [PATCH] fix(cli): support fixed OIDC callback ports Signed-off-by: Alex Fournier --- crates/openshell-cli/src/oidc_auth.rs | 99 ++++++++++++++++++++++++++- docs/reference/gateway-auth.mdx | 2 + 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/crates/openshell-cli/src/oidc_auth.rs b/crates/openshell-cli/src/oidc_auth.rs index 379a531127..8fb840d5b3 100644 --- a/crates/openshell-cli/src/oidc_auth.rs +++ b/crates/openshell-cli/src/oidc_auth.rs @@ -29,6 +29,8 @@ use tokio::sync::oneshot; use tracing::debug; const AUTH_TIMEOUT: Duration = Duration::from_secs(120); +const DEFAULT_OIDC_CALLBACK_BIND: &str = "127.0.0.1:0"; +const OIDC_CALLBACK_PORT_ENV: &str = "OPENSHELL_OIDC_CALLBACK_PORT"; /// OIDC discovery document (subset of fields we need). #[derive(Debug, Deserialize)] @@ -95,6 +97,25 @@ fn build_ci_scopes(scopes: Option<&str>) -> Vec { .collect() } +fn oidc_callback_bind_address() -> Result { + match std::env::var(OIDC_CALLBACK_PORT_ENV) { + Ok(raw) => { + let port = raw.parse::().map_err(|_| { + miette::miette!( + "{OIDC_CALLBACK_PORT_ENV} must be a valid TCP port number, got '{raw}'" + ) + })?; + if port == 0 { + return Err(miette::miette!( + "{OIDC_CALLBACK_PORT_ENV} must be greater than 0" + )); + } + Ok(format!("127.0.0.1:{port}")) + } + Err(_) => Ok(DEFAULT_OIDC_CALLBACK_BIND.to_string()), + } +} + /// Run the OIDC Authorization Code + PKCE browser flow. /// /// Opens the user's browser to the Keycloak login page and waits for @@ -108,7 +129,9 @@ pub async fn oidc_browser_auth_flow( ) -> Result { let discovery = discover(issuer, insecure).await?; - let listener = TcpListener::bind("127.0.0.1:0").await.into_diagnostic()?; + let listener = TcpListener::bind(oidc_callback_bind_address()?) + .await + .into_diagnostic()?; let port = listener.local_addr().into_diagnostic()?.port(); let redirect_uri = format!("http://127.0.0.1:{port}/callback"); @@ -449,6 +472,39 @@ fn html_response(status: StatusCode, message: &str) -> Response> { #[cfg(test)] mod tests { use super::*; + use crate::TEST_ENV_LOCK as ENV_LOCK; + + struct EnvVarGuard { + key: &'static str, + original: Option, + } + + impl EnvVarGuard { + #[allow(unsafe_code)] + fn set(key: &'static str, value: &str) -> Self { + let original = std::env::var(key).ok(); + unsafe { std::env::set_var(key, value) }; + Self { key, original } + } + + #[allow(unsafe_code)] + fn remove(key: &'static str) -> Self { + let original = std::env::var(key).ok(); + unsafe { std::env::remove_var(key) }; + Self { key, original } + } + } + + impl Drop for EnvVarGuard { + #[allow(unsafe_code)] + fn drop(&mut self) { + if let Some(value) = &self.original { + unsafe { std::env::set_var(self.key, value) }; + } else { + unsafe { std::env::remove_var(self.key) }; + } + } + } #[test] fn http_client_secure_rejects_self_signed() { @@ -516,6 +572,47 @@ mod tests { assert!(scopes.is_empty()); } + #[test] + fn callback_bind_address_defaults_to_ephemeral_loopback() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvVarGuard::remove(OIDC_CALLBACK_PORT_ENV); + assert_eq!( + oidc_callback_bind_address().unwrap(), + DEFAULT_OIDC_CALLBACK_BIND + ); + } + + #[test] + fn callback_bind_address_uses_fixed_port_env() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvVarGuard::set(OIDC_CALLBACK_PORT_ENV, "8765"); + assert_eq!(oidc_callback_bind_address().unwrap(), "127.0.0.1:8765"); + } + + #[test] + fn callback_bind_address_rejects_invalid_port_env() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvVarGuard::set(OIDC_CALLBACK_PORT_ENV, "not-a-port"); + let err = oidc_callback_bind_address().unwrap_err(); + assert!(err.to_string().contains("valid TCP port")); + } + + #[test] + fn callback_bind_address_rejects_zero() { + let _lock = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = EnvVarGuard::set(OIDC_CALLBACK_PORT_ENV, "0"); + let err = oidc_callback_bind_address().unwrap_err(); + assert!(err.to_string().contains("greater than 0")); + } + #[test] fn bundle_from_response_sets_fields() { use oauth2::basic::BasicTokenResponse; diff --git a/docs/reference/gateway-auth.mdx b/docs/reference/gateway-auth.mdx index 94aca659ee..ec4bc89551 100644 --- a/docs/reference/gateway-auth.mdx +++ b/docs/reference/gateway-auth.mdx @@ -116,6 +116,8 @@ openshell gateway add https://gateway.example.com \ When you register or log in to an OIDC gateway, the CLI uses the Authorization Code flow with PKCE. It opens a browser, receives the authorization code on a localhost callback, exchanges the code for tokens, and stores the token bundle under the gateway credential directory. If `OPENSHELL_OIDC_CLIENT_SECRET` is set, the CLI uses the client credentials flow instead. Use that mode for CI and other non-interactive automation. +The CLI selects an ephemeral loopback port by default. When an identity provider requires an exact redirect URI, set `OPENSHELL_OIDC_CALLBACK_PORT` to a pre-registered port before `openshell gateway login`. For example, a client registered with `http://127.0.0.1:8765/callback` uses `OPENSHELL_OIDC_CALLBACK_PORT=8765 openshell gateway login production`. + The connection flow: 1. The CLI loads the stored OIDC token bundle.