From 8f53cfa096a15c545444a3b64abfa19ff8c2eacd Mon Sep 17 00:00:00 2001 From: Adel Zaalouk Date: Fri, 22 May 2026 22:17:54 +0200 Subject: [PATCH] fix(cli): propagate --gateway-insecure to OIDC auth flows Thread the gateway_insecure flag through gateway_add(), gateway_login(), and all OIDC HTTP clients so that --gateway-insecure and OPENSHELL_GATEWAY_INSECURE apply to OIDC discovery, token exchange, and token refresh requests. Previously, the flag only affected gRPC connections to the gateway. OIDC HTTP clients (reqwest::get and http_client) always verified TLS certificates, causing gateway registration and login to fail when the OIDC issuer used a self-signed certificate (common on OpenShift with edge-terminated routes). Fixes #1534 Signed-off-by: Adel Zaalouk --- crates/openshell-cli/src/completers.rs | 2 +- crates/openshell-cli/src/main.rs | 10 +- crates/openshell-cli/src/oidc_auth.rs | 128 +++++++++++++++++++++---- crates/openshell-cli/src/run.rs | 27 +++++- 4 files changed, 142 insertions(+), 25 deletions(-) diff --git a/crates/openshell-cli/src/completers.rs b/crates/openshell-cli/src/completers.rs index acf23b135..a421b418a 100644 --- a/crates/openshell-cli/src/completers.rs +++ b/crates/openshell-cli/src/completers.rs @@ -98,7 +98,7 @@ async fn completion_grpc_client( Some("oidc") => { if let Some(bundle) = load_oidc_token(gateway_name) { if is_token_expired(&bundle) { - match oidc_refresh_token(&bundle).await { + match oidc_refresh_token(&bundle, tls_opts.gateway_insecure).await { Ok(refreshed) => { let _ = store_oidc_token(gateway_name, &refreshed); tls_opts.oidc_token = Some(refreshed.access_token); diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 7cb3391fe..3fa9c17d4 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -141,11 +141,14 @@ fn apply_auth(tls: &mut TlsOptions, gateway_name: &str) { return; }; if openshell_bootstrap::oidc_token::is_token_expired(&bundle) { + let insecure = std::env::var("OPENSHELL_GATEWAY_INSECURE") + .is_ok_and(|v| !v.is_empty() && v != "0" && v != "false"); // Try to refresh the token in-place using block_in_place // so the async refresh can run within the sync apply_auth call. match tokio::task::block_in_place(|| { - tokio::runtime::Handle::current() - .block_on(openshell_cli::oidc_auth::oidc_refresh_token(&bundle)) + tokio::runtime::Handle::current().block_on( + openshell_cli::oidc_auth::oidc_refresh_token(&bundle, insecure), + ) }) { Ok(refreshed) => { let _ = openshell_bootstrap::oidc_token::store_oidc_token( @@ -1917,6 +1920,7 @@ async fn main() -> Result<()> { &oidc_client_id, oidc_audience.as_deref(), oidc_scopes.as_deref(), + cli.gateway_insecure, ) .await?; } @@ -1942,7 +1946,7 @@ async fn main() -> Result<()> { Or set one with: openshell gateway select " ) })?; - run::gateway_login(&name).await?; + run::gateway_login(&name, cli.gateway_insecure).await?; } GatewayCommands::Logout { name } => { let name = name diff --git a/crates/openshell-cli/src/oidc_auth.rs b/crates/openshell-cli/src/oidc_auth.rs index fd742a418..379a53112 100644 --- a/crates/openshell-cli/src/oidc_auth.rs +++ b/crates/openshell-cli/src/oidc_auth.rs @@ -42,10 +42,13 @@ struct OidcDiscovery { /// /// Validates that the discovery document's `issuer` field matches the /// configured issuer URL to prevent SSRF or misdirection. -async fn discover(issuer: &str) -> Result { +async fn discover(issuer: &str, insecure: bool) -> Result { let normalized_issuer = issuer.trim_end_matches('/'); let url = format!("{normalized_issuer}/.well-known/openid-configuration"); - let resp: OidcDiscovery = reqwest::get(&url) + let client = http_client(insecure); + let resp: OidcDiscovery = client + .get(&url) + .send() .await .into_diagnostic()? .json() @@ -63,11 +66,12 @@ async fn discover(issuer: &str) -> Result { Ok(resp) } -fn http_client() -> reqwest::Client { - reqwest::ClientBuilder::new() - .redirect(reqwest::redirect::Policy::none()) - .build() - .expect("failed to build HTTP client") +fn http_client(insecure: bool) -> reqwest::Client { + let mut builder = reqwest::ClientBuilder::new().redirect(reqwest::redirect::Policy::none()); + if insecure { + builder = builder.danger_accept_invalid_certs(true); + } + builder.build().expect("failed to build HTTP client") } fn build_scopes(scopes: Option<&str>) -> Vec { @@ -100,8 +104,9 @@ pub async fn oidc_browser_auth_flow( client_id: &str, audience: Option<&str>, scopes: Option<&str>, + insecure: bool, ) -> Result { - let discovery = discover(issuer).await?; + let discovery = discover(issuer, insecure).await?; let listener = TcpListener::bind("127.0.0.1:0").await.into_diagnostic()?; let port = listener.local_addr().into_diagnostic()?.port(); @@ -161,7 +166,7 @@ pub async fn oidc_browser_auth_flow( server_handle.abort(); - let http = http_client(); + let http = http_client(insecure); let token_response = client .exchange_code(AuthorizationCode::new(code)) .set_pkce_verifier(pkce_verifier) @@ -184,6 +189,7 @@ pub async fn oidc_client_credentials_flow( client_id: &str, audience: Option<&str>, scopes: Option<&str>, + insecure: bool, ) -> Result { let client_secret = std::env::var("OPENSHELL_OIDC_CLIENT_SECRET").map_err(|_| { miette::miette!( @@ -191,7 +197,7 @@ pub async fn oidc_client_credentials_flow( ) })?; - let discovery = discover(issuer).await?; + let discovery = discover(issuer, insecure).await?; let client = BasicClient::new(ClientId::new(client_id.to_string())) .set_client_secret(ClientSecret::new(client_secret)) @@ -206,7 +212,7 @@ pub async fn oidc_client_credentials_flow( request = request.add_extra_param("audience", aud); } - let http = http_client(); + let http = http_client(insecure); let token_response = request .request_async(&http) .await @@ -223,19 +229,22 @@ pub async fn oidc_client_credentials_flow( /// /// Preserves the existing refresh token if the server does not return a new /// one (per OAuth 2.0 spec, the refresh response may omit `refresh_token`). -pub async fn oidc_refresh_token(bundle: &OidcTokenBundle) -> Result { +pub async fn oidc_refresh_token( + bundle: &OidcTokenBundle, + insecure: bool, +) -> Result { let refresh_token = bundle.refresh_token.as_deref().ok_or_else(|| { miette::miette!( "no refresh token available — re-authenticate with: openshell gateway login" ) })?; - let discovery = discover(&bundle.issuer).await?; + let discovery = discover(&bundle.issuer, insecure).await?; let client = BasicClient::new(ClientId::new(bundle.client_id.clone())) .set_token_uri(TokenUrl::new(discovery.token_endpoint).into_diagnostic()?); - let http = http_client(); + let http = http_client(insecure); let token_response = client .exchange_refresh_token(&RefreshToken::new(refresh_token.to_string())) .request_async(&http) @@ -253,7 +262,7 @@ pub async fn oidc_refresh_token(bundle: &OidcTokenBundle) -> Result Result { +pub async fn ensure_valid_oidc_token(gateway_name: &str, insecure: bool) -> Result { let bundle = openshell_bootstrap::oidc_token::load_oidc_token(gateway_name).ok_or_else(|| { miette::miette!( @@ -270,7 +279,7 @@ pub async fn ensure_valid_oidc_token(gateway_name: &str) -> Result { gateway = gateway_name, "OIDC token expired, attempting refresh" ); - let refreshed = oidc_refresh_token(&bundle).await?; + let refreshed = oidc_refresh_token(&bundle, insecure).await?; openshell_bootstrap::oidc_token::store_oidc_token(gateway_name, &refreshed)?; Ok(refreshed.access_token) } @@ -436,3 +445,90 @@ fn html_response(status: StatusCode, message: &str) -> Response> { .body(Full::new(Bytes::from(body))) .expect("response") } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn http_client_secure_rejects_self_signed() { + let client = http_client(false); + let rt = tokio::runtime::Runtime::new().unwrap(); + // A real self-signed server isn't available in unit tests, but we can + // verify the client is constructed and makes requests. The secure client + // should exist and function for valid endpoints. + let result = rt.block_on(async { client.get("https://127.0.0.1:1").send().await }); + assert!(result.is_err(), "connection to closed port should fail"); + } + + #[test] + fn http_client_insecure_builds_without_panic() { + let client = http_client(true); + // Verify the client is usable (doesn't panic on construction). + let rt = tokio::runtime::Runtime::new().unwrap(); + let result = rt.block_on(async { client.get("https://127.0.0.1:1").send().await }); + assert!(result.is_err(), "connection to closed port should fail"); + } + + #[test] + fn discover_validates_issuer_mismatch() { + let rt = tokio::runtime::Runtime::new().unwrap(); + // Discovery against a non-existent issuer should fail with a + // connection error, not silently succeed. + let result = rt.block_on(discover("http://127.0.0.1:1/realms/test", false)); + assert!(result.is_err()); + } + + #[test] + fn discover_insecure_passes_flag_through() { + let rt = tokio::runtime::Runtime::new().unwrap(); + // Same as above but with insecure=true. Should still fail on + // connection (no server) but must not panic. + let result = rt.block_on(discover("https://127.0.0.1:1/realms/test", true)); + assert!(result.is_err()); + } + + #[test] + fn percent_decode_basic() { + assert_eq!(percent_decode("hello%20world"), "hello world"); + assert_eq!(percent_decode("a%2Fb"), "a/b"); + assert_eq!(percent_decode("no+encoding+here"), "no encoding here"); + } + + #[test] + fn build_scopes_always_includes_openid() { + let scopes = build_scopes(None); + assert_eq!(scopes.len(), 1); + + let scopes = build_scopes(Some("profile email")); + assert_eq!(scopes.len(), 3); + } + + #[test] + fn build_scopes_deduplicates_openid() { + let scopes = build_scopes(Some("openid profile")); + assert_eq!(scopes.len(), 2); + } + + #[test] + fn build_ci_scopes_empty_on_none() { + let scopes = build_ci_scopes(None); + assert!(scopes.is_empty()); + } + + #[test] + fn bundle_from_response_sets_fields() { + use oauth2::basic::BasicTokenResponse; + + let token_response: BasicTokenResponse = serde_json::from_str( + r#"{"access_token":"test-access","token_type":"bearer","expires_in":300,"refresh_token":"test-refresh"}"#, + ) + .unwrap(); + let bundle = bundle_from_oauth2_response(&token_response, "https://issuer", "my-client"); + assert_eq!(bundle.access_token, "test-access"); + assert_eq!(bundle.refresh_token.as_deref(), Some("test-refresh")); + assert_eq!(bundle.issuer, "https://issuer"); + assert_eq!(bundle.client_id, "my-client"); + assert!(bundle.expires_at.is_some()); + } +} diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 4bb32eee8..f1a44ad31 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -858,6 +858,7 @@ pub async fn gateway_add( oidc_client_id: &str, oidc_audience: Option<&str>, oidc_scopes: Option<&str>, + gateway_insecure: bool, ) -> Result<()> { // If the endpoint starts with ssh://, parse it into an SSH destination // and a gateway endpoint automatically. The host is resolved via @@ -971,6 +972,7 @@ pub async fn gateway_add( oidc_client_id, oidc_audience, oidc_scopes, + gateway_insecure, ) .await { @@ -991,6 +993,7 @@ pub async fn gateway_add( oidc_client_id, oidc_audience, oidc_scopes, + gateway_insecure, ) .await { @@ -1164,7 +1167,7 @@ pub async fn gateway_add( /// Re-authenticate with an edge-authenticated or OIDC gateway. /// /// Dispatches to the appropriate auth flow based on `auth_mode`. -pub async fn gateway_login(name: &str) -> Result<()> { +pub async fn gateway_login(name: &str, gateway_insecure: bool) -> Result<()> { let metadata = openshell_bootstrap::load_gateway_metadata(name).map_err(|_| { miette::miette!( "Unknown gateway '{name}'.\n\ @@ -1190,11 +1193,23 @@ pub async fn gateway_login(name: &str) -> Result<()> { let scopes = metadata.oidc_scopes.as_deref(); let bundle = if std::env::var("OPENSHELL_OIDC_CLIENT_SECRET").is_ok() { - crate::oidc_auth::oidc_client_credentials_flow(issuer, client_id, audience, scopes) - .await? + crate::oidc_auth::oidc_client_credentials_flow( + issuer, + client_id, + audience, + scopes, + gateway_insecure, + ) + .await? } else { - crate::oidc_auth::oidc_browser_auth_flow(issuer, client_id, audience, scopes) - .await? + crate::oidc_auth::oidc_browser_auth_flow( + issuer, + client_id, + audience, + scopes, + gateway_insecure, + ) + .await? }; let username = jwt_preferred_username(&bundle.access_token); @@ -7927,6 +7942,7 @@ mod tests { "openshell-cli", None, None, + false, ) .await .expect("register plaintext gateway"); @@ -7958,6 +7974,7 @@ mod tests { "openshell-cli", None, None, + false, ) .await .expect("register plaintext gateway");