Skip to content

Commit f0f17bf

Browse files
authored
fix(cli): propagate --gateway-insecure to OIDC auth flows (#1535)
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 <azaalouk@redhat.com>
1 parent fbd580b commit f0f17bf

4 files changed

Lines changed: 142 additions & 25 deletions

File tree

crates/openshell-cli/src/completers.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ async fn completion_grpc_client(
9898
Some("oidc") => {
9999
if let Some(bundle) = load_oidc_token(gateway_name) {
100100
if is_token_expired(&bundle) {
101-
match oidc_refresh_token(&bundle).await {
101+
match oidc_refresh_token(&bundle, tls_opts.gateway_insecure).await {
102102
Ok(refreshed) => {
103103
let _ = store_oidc_token(gateway_name, &refreshed);
104104
tls_opts.oidc_token = Some(refreshed.access_token);

crates/openshell-cli/src/main.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,11 +141,14 @@ fn apply_auth(tls: &mut TlsOptions, gateway_name: &str) {
141141
return;
142142
};
143143
if openshell_bootstrap::oidc_token::is_token_expired(&bundle) {
144+
let insecure = std::env::var("OPENSHELL_GATEWAY_INSECURE")
145+
.is_ok_and(|v| !v.is_empty() && v != "0" && v != "false");
144146
// Try to refresh the token in-place using block_in_place
145147
// so the async refresh can run within the sync apply_auth call.
146148
match tokio::task::block_in_place(|| {
147-
tokio::runtime::Handle::current()
148-
.block_on(openshell_cli::oidc_auth::oidc_refresh_token(&bundle))
149+
tokio::runtime::Handle::current().block_on(
150+
openshell_cli::oidc_auth::oidc_refresh_token(&bundle, insecure),
151+
)
149152
}) {
150153
Ok(refreshed) => {
151154
let _ = openshell_bootstrap::oidc_token::store_oidc_token(
@@ -1917,6 +1920,7 @@ async fn main() -> Result<()> {
19171920
&oidc_client_id,
19181921
oidc_audience.as_deref(),
19191922
oidc_scopes.as_deref(),
1923+
cli.gateway_insecure,
19201924
)
19211925
.await?;
19221926
}
@@ -1942,7 +1946,7 @@ async fn main() -> Result<()> {
19421946
Or set one with: openshell gateway select <name>"
19431947
)
19441948
})?;
1945-
run::gateway_login(&name).await?;
1949+
run::gateway_login(&name, cli.gateway_insecure).await?;
19461950
}
19471951
GatewayCommands::Logout { name } => {
19481952
let name = name

crates/openshell-cli/src/oidc_auth.rs

Lines changed: 112 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,13 @@ struct OidcDiscovery {
4242
///
4343
/// Validates that the discovery document's `issuer` field matches the
4444
/// configured issuer URL to prevent SSRF or misdirection.
45-
async fn discover(issuer: &str) -> Result<OidcDiscovery> {
45+
async fn discover(issuer: &str, insecure: bool) -> Result<OidcDiscovery> {
4646
let normalized_issuer = issuer.trim_end_matches('/');
4747
let url = format!("{normalized_issuer}/.well-known/openid-configuration");
48-
let resp: OidcDiscovery = reqwest::get(&url)
48+
let client = http_client(insecure);
49+
let resp: OidcDiscovery = client
50+
.get(&url)
51+
.send()
4952
.await
5053
.into_diagnostic()?
5154
.json()
@@ -63,11 +66,12 @@ async fn discover(issuer: &str) -> Result<OidcDiscovery> {
6366
Ok(resp)
6467
}
6568

66-
fn http_client() -> reqwest::Client {
67-
reqwest::ClientBuilder::new()
68-
.redirect(reqwest::redirect::Policy::none())
69-
.build()
70-
.expect("failed to build HTTP client")
69+
fn http_client(insecure: bool) -> reqwest::Client {
70+
let mut builder = reqwest::ClientBuilder::new().redirect(reqwest::redirect::Policy::none());
71+
if insecure {
72+
builder = builder.danger_accept_invalid_certs(true);
73+
}
74+
builder.build().expect("failed to build HTTP client")
7175
}
7276

7377
fn build_scopes(scopes: Option<&str>) -> Vec<Scope> {
@@ -100,8 +104,9 @@ pub async fn oidc_browser_auth_flow(
100104
client_id: &str,
101105
audience: Option<&str>,
102106
scopes: Option<&str>,
107+
insecure: bool,
103108
) -> Result<OidcTokenBundle> {
104-
let discovery = discover(issuer).await?;
109+
let discovery = discover(issuer, insecure).await?;
105110

106111
let listener = TcpListener::bind("127.0.0.1:0").await.into_diagnostic()?;
107112
let port = listener.local_addr().into_diagnostic()?.port();
@@ -161,7 +166,7 @@ pub async fn oidc_browser_auth_flow(
161166

162167
server_handle.abort();
163168

164-
let http = http_client();
169+
let http = http_client(insecure);
165170
let token_response = client
166171
.exchange_code(AuthorizationCode::new(code))
167172
.set_pkce_verifier(pkce_verifier)
@@ -184,14 +189,15 @@ pub async fn oidc_client_credentials_flow(
184189
client_id: &str,
185190
audience: Option<&str>,
186191
scopes: Option<&str>,
192+
insecure: bool,
187193
) -> Result<OidcTokenBundle> {
188194
let client_secret = std::env::var("OPENSHELL_OIDC_CLIENT_SECRET").map_err(|_| {
189195
miette::miette!(
190196
"OPENSHELL_OIDC_CLIENT_SECRET environment variable is required for client credentials flow"
191197
)
192198
})?;
193199

194-
let discovery = discover(issuer).await?;
200+
let discovery = discover(issuer, insecure).await?;
195201

196202
let client = BasicClient::new(ClientId::new(client_id.to_string()))
197203
.set_client_secret(ClientSecret::new(client_secret))
@@ -206,7 +212,7 @@ pub async fn oidc_client_credentials_flow(
206212
request = request.add_extra_param("audience", aud);
207213
}
208214

209-
let http = http_client();
215+
let http = http_client(insecure);
210216
let token_response = request
211217
.request_async(&http)
212218
.await
@@ -223,19 +229,22 @@ pub async fn oidc_client_credentials_flow(
223229
///
224230
/// Preserves the existing refresh token if the server does not return a new
225231
/// one (per OAuth 2.0 spec, the refresh response may omit `refresh_token`).
226-
pub async fn oidc_refresh_token(bundle: &OidcTokenBundle) -> Result<OidcTokenBundle> {
232+
pub async fn oidc_refresh_token(
233+
bundle: &OidcTokenBundle,
234+
insecure: bool,
235+
) -> Result<OidcTokenBundle> {
227236
let refresh_token = bundle.refresh_token.as_deref().ok_or_else(|| {
228237
miette::miette!(
229238
"no refresh token available — re-authenticate with: openshell gateway login"
230239
)
231240
})?;
232241

233-
let discovery = discover(&bundle.issuer).await?;
242+
let discovery = discover(&bundle.issuer, insecure).await?;
234243

235244
let client = BasicClient::new(ClientId::new(bundle.client_id.clone()))
236245
.set_token_uri(TokenUrl::new(discovery.token_endpoint).into_diagnostic()?);
237246

238-
let http = http_client();
247+
let http = http_client(insecure);
239248
let token_response = client
240249
.exchange_refresh_token(&RefreshToken::new(refresh_token.to_string()))
241250
.request_async(&http)
@@ -253,7 +262,7 @@ pub async fn oidc_refresh_token(bundle: &OidcTokenBundle) -> Result<OidcTokenBun
253262
/// Ensure we have a valid OIDC token for the given gateway, refreshing if needed.
254263
///
255264
/// Returns the access token string.
256-
pub async fn ensure_valid_oidc_token(gateway_name: &str) -> Result<String> {
265+
pub async fn ensure_valid_oidc_token(gateway_name: &str, insecure: bool) -> Result<String> {
257266
let bundle =
258267
openshell_bootstrap::oidc_token::load_oidc_token(gateway_name).ok_or_else(|| {
259268
miette::miette!(
@@ -270,7 +279,7 @@ pub async fn ensure_valid_oidc_token(gateway_name: &str) -> Result<String> {
270279
gateway = gateway_name,
271280
"OIDC token expired, attempting refresh"
272281
);
273-
let refreshed = oidc_refresh_token(&bundle).await?;
282+
let refreshed = oidc_refresh_token(&bundle, insecure).await?;
274283
openshell_bootstrap::oidc_token::store_oidc_token(gateway_name, &refreshed)?;
275284
Ok(refreshed.access_token)
276285
}
@@ -436,3 +445,90 @@ fn html_response(status: StatusCode, message: &str) -> Response<Full<Bytes>> {
436445
.body(Full::new(Bytes::from(body)))
437446
.expect("response")
438447
}
448+
449+
#[cfg(test)]
450+
mod tests {
451+
use super::*;
452+
453+
#[test]
454+
fn http_client_secure_rejects_self_signed() {
455+
let client = http_client(false);
456+
let rt = tokio::runtime::Runtime::new().unwrap();
457+
// A real self-signed server isn't available in unit tests, but we can
458+
// verify the client is constructed and makes requests. The secure client
459+
// should exist and function for valid endpoints.
460+
let result = rt.block_on(async { client.get("https://127.0.0.1:1").send().await });
461+
assert!(result.is_err(), "connection to closed port should fail");
462+
}
463+
464+
#[test]
465+
fn http_client_insecure_builds_without_panic() {
466+
let client = http_client(true);
467+
// Verify the client is usable (doesn't panic on construction).
468+
let rt = tokio::runtime::Runtime::new().unwrap();
469+
let result = rt.block_on(async { client.get("https://127.0.0.1:1").send().await });
470+
assert!(result.is_err(), "connection to closed port should fail");
471+
}
472+
473+
#[test]
474+
fn discover_validates_issuer_mismatch() {
475+
let rt = tokio::runtime::Runtime::new().unwrap();
476+
// Discovery against a non-existent issuer should fail with a
477+
// connection error, not silently succeed.
478+
let result = rt.block_on(discover("http://127.0.0.1:1/realms/test", false));
479+
assert!(result.is_err());
480+
}
481+
482+
#[test]
483+
fn discover_insecure_passes_flag_through() {
484+
let rt = tokio::runtime::Runtime::new().unwrap();
485+
// Same as above but with insecure=true. Should still fail on
486+
// connection (no server) but must not panic.
487+
let result = rt.block_on(discover("https://127.0.0.1:1/realms/test", true));
488+
assert!(result.is_err());
489+
}
490+
491+
#[test]
492+
fn percent_decode_basic() {
493+
assert_eq!(percent_decode("hello%20world"), "hello world");
494+
assert_eq!(percent_decode("a%2Fb"), "a/b");
495+
assert_eq!(percent_decode("no+encoding+here"), "no encoding here");
496+
}
497+
498+
#[test]
499+
fn build_scopes_always_includes_openid() {
500+
let scopes = build_scopes(None);
501+
assert_eq!(scopes.len(), 1);
502+
503+
let scopes = build_scopes(Some("profile email"));
504+
assert_eq!(scopes.len(), 3);
505+
}
506+
507+
#[test]
508+
fn build_scopes_deduplicates_openid() {
509+
let scopes = build_scopes(Some("openid profile"));
510+
assert_eq!(scopes.len(), 2);
511+
}
512+
513+
#[test]
514+
fn build_ci_scopes_empty_on_none() {
515+
let scopes = build_ci_scopes(None);
516+
assert!(scopes.is_empty());
517+
}
518+
519+
#[test]
520+
fn bundle_from_response_sets_fields() {
521+
use oauth2::basic::BasicTokenResponse;
522+
523+
let token_response: BasicTokenResponse = serde_json::from_str(
524+
r#"{"access_token":"test-access","token_type":"bearer","expires_in":300,"refresh_token":"test-refresh"}"#,
525+
)
526+
.unwrap();
527+
let bundle = bundle_from_oauth2_response(&token_response, "https://issuer", "my-client");
528+
assert_eq!(bundle.access_token, "test-access");
529+
assert_eq!(bundle.refresh_token.as_deref(), Some("test-refresh"));
530+
assert_eq!(bundle.issuer, "https://issuer");
531+
assert_eq!(bundle.client_id, "my-client");
532+
assert!(bundle.expires_at.is_some());
533+
}
534+
}

crates/openshell-cli/src/run.rs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,7 @@ pub async fn gateway_add(
858858
oidc_client_id: &str,
859859
oidc_audience: Option<&str>,
860860
oidc_scopes: Option<&str>,
861+
gateway_insecure: bool,
861862
) -> Result<()> {
862863
// If the endpoint starts with ssh://, parse it into an SSH destination
863864
// and a gateway endpoint automatically. The host is resolved via
@@ -971,6 +972,7 @@ pub async fn gateway_add(
971972
oidc_client_id,
972973
oidc_audience,
973974
oidc_scopes,
975+
gateway_insecure,
974976
)
975977
.await
976978
{
@@ -991,6 +993,7 @@ pub async fn gateway_add(
991993
oidc_client_id,
992994
oidc_audience,
993995
oidc_scopes,
996+
gateway_insecure,
994997
)
995998
.await
996999
{
@@ -1164,7 +1167,7 @@ pub async fn gateway_add(
11641167
/// Re-authenticate with an edge-authenticated or OIDC gateway.
11651168
///
11661169
/// Dispatches to the appropriate auth flow based on `auth_mode`.
1167-
pub async fn gateway_login(name: &str) -> Result<()> {
1170+
pub async fn gateway_login(name: &str, gateway_insecure: bool) -> Result<()> {
11681171
let metadata = openshell_bootstrap::load_gateway_metadata(name).map_err(|_| {
11691172
miette::miette!(
11701173
"Unknown gateway '{name}'.\n\
@@ -1190,11 +1193,23 @@ pub async fn gateway_login(name: &str) -> Result<()> {
11901193
let scopes = metadata.oidc_scopes.as_deref();
11911194

11921195
let bundle = if std::env::var("OPENSHELL_OIDC_CLIENT_SECRET").is_ok() {
1193-
crate::oidc_auth::oidc_client_credentials_flow(issuer, client_id, audience, scopes)
1194-
.await?
1196+
crate::oidc_auth::oidc_client_credentials_flow(
1197+
issuer,
1198+
client_id,
1199+
audience,
1200+
scopes,
1201+
gateway_insecure,
1202+
)
1203+
.await?
11951204
} else {
1196-
crate::oidc_auth::oidc_browser_auth_flow(issuer, client_id, audience, scopes)
1197-
.await?
1205+
crate::oidc_auth::oidc_browser_auth_flow(
1206+
issuer,
1207+
client_id,
1208+
audience,
1209+
scopes,
1210+
gateway_insecure,
1211+
)
1212+
.await?
11981213
};
11991214

12001215
let username = jwt_preferred_username(&bundle.access_token);
@@ -7927,6 +7942,7 @@ mod tests {
79277942
"openshell-cli",
79287943
None,
79297944
None,
7945+
false,
79307946
)
79317947
.await
79327948
.expect("register plaintext gateway");
@@ -7958,6 +7974,7 @@ mod tests {
79587974
"openshell-cli",
79597975
None,
79607976
None,
7977+
false,
79617978
)
79627979
.await
79637980
.expect("register plaintext gateway");

0 commit comments

Comments
 (0)