From 3824eaa7f2cb34d0d72398b659d2134d46006916 Mon Sep 17 00:00:00 2001 From: Ken Simon Date: Tue, 14 Jul 2026 09:54:30 -0400 Subject: [PATCH 1/3] Re-introduce basic auth, generate password if not set A recent security finding pointed out that the default configuration of nico-core, with the helm chart at its defaults, is very insecure, since there is no authentication on the web frontend. We could easily say this behaves correctly, since "auth=none" should mean no auth, but I think there's a point to be made that the defaults should be secure. Defaulting to oauth2 is untenable, because it requires too many other parameters (client ID, client secret, endpoint, etc), so we can't just have a working "default" oauth2 setup. Instead, this follows prior art from projects like Grafana and ArgoCD, by defaulting to basic auth, and requiring a password to be set. If no password is set in the values.yaml of the helm chart, the chart will create an initial randomly-generated admin password via kubernetes secrets. If oauth2 is specified (like in other gitops deployments that aren't using the helm chart), nothing needs to be done: using oauth does not require an admin password to be set. If the helm chart is not used, and no auth is configured, nico-api will generate one at startup and use it for the lifetime of the process. The password is logged as a warning message, and the basic auth prompt explains to look in the logs for the generated password. This is not expected to be seen in production: the helm chart will generate a secret and configure it. Logging a password would only happen if somebody is running nico-api without the helm chart, without a password env var, and via a deployment that isn't already configured for auth=none or auth=oauth2. While we're at it, make auth a little easier to specify from values.yaml by creating a toplevel values.yaml config for `webAuth.mode` in nico-api, rather than having to use `extraEnv` and configuring it via environment variables. --- Cargo.lock | 2 + book/src/configuration/configurability.md | 36 +- crates/api-web/Cargo.toml | 2 + crates/api-web/src/lib.rs | 327 ++++++++++++++++-- crates/api-web/src/tests/mod.rs | 6 +- crates/utils/src/lib.rs | 1 + crates/utils/src/none_if_empty.rs | 211 +++++++++++ deploy/nico-base/api/deployment.yaml | 7 +- dev/deployment/devspace/values.base.yaml | 4 + dev/mac-local-dev/README.md | 2 +- dev/mac-local-dev/run-nico-api.sh | 1 + dev/webdev-env/run-env.sh | 5 +- docs/operations/debug_webui.md | 42 ++- helm/README.md | 27 +- helm/charts/nico-api/templates/NOTES.txt | 7 + helm/charts/nico-api/templates/_helpers.tpl | 74 ++++ .../charts/nico-api/templates/deployment.yaml | 11 + .../templates/web-basic-auth-secret.yaml | 18 + .../tests/web_auth_deployment_test.yaml | 115 ++++++ .../nico-api/tests/web_auth_secret_test.yaml | 56 +++ helm/charts/nico-api/values.yaml | 12 + helm/examples/values-full.yaml | 19 +- 22 files changed, 921 insertions(+), 64 deletions(-) create mode 100644 crates/utils/src/none_if_empty.rs create mode 100644 helm/charts/nico-api/templates/NOTES.txt create mode 100644 helm/charts/nico-api/templates/web-basic-auth-secret.yaml create mode 100644 helm/charts/nico-api/tests/web_auth_deployment_test.yaml create mode 100644 helm/charts/nico-api/tests/web_auth_secret_test.yaml diff --git a/Cargo.lock b/Cargo.lock index 3e2b9a12ac..a96eabba58 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1493,6 +1493,7 @@ dependencies = [ "carbide-sqlx-testing", "carbide-test-harness", "carbide-test-support", + "carbide-utils", "carbide-uuid", "carbide-version", "chrono", @@ -1507,6 +1508,7 @@ dependencies = [ "lazy_static", "mac_address", "oauth2", + "rand 0.10.1", "regex", "reqwest 0.13.4", "serde", diff --git a/book/src/configuration/configurability.md b/book/src/configuration/configurability.md index 999ab5e3a6..00c013f368 100644 --- a/book/src/configuration/configurability.md +++ b/book/src/configuration/configurability.md @@ -399,8 +399,40 @@ oauth2_client_id = "nico-api" allowed_access_groups = ["nico-operators", "nico-admins"] ``` -The chart supports overriding any `[auth.web]` field via -`nico-api.extraEnv` (see [`helm/README.md` → OAuth2 / SSO Setup](../../../helm/README.md#oauth2--sso-setup)). +The deployed WebUI authentication mode is configured with +`nico-api.webAuth.mode` (`basic`, `oauth2`, or `none`) and defaults to Basic +Auth with a generated password Secret. When `webAuth.mode: oauth2` is selected, +provide the endpoints, client credentials, and allowed groups through +`nico-api.extraEnv`: + +```yaml +nico-api: + webAuth: + mode: oauth2 + extraEnv: + - name: CARBIDE_WEB_OAUTH2_AUTH_ENDPOINT + value: "https://keycloak.example.com/realms/nico/protocol/openid-connect/auth" + - name: CARBIDE_WEB_OAUTH2_TOKEN_ENDPOINT + value: "https://keycloak.example.com/realms/nico/protocol/openid-connect/token" + - name: CARBIDE_WEB_OAUTH2_CLIENT_ID + value: "nico-api" + - name: CARBIDE_WEB_OAUTH2_CLIENT_SECRET + valueFrom: + secretKeyRef: + name: nico-web-oauth2-client + key: client_secret + - name: CARBIDE_WEB_ALLOWED_ACCESS_GROUPS + value: "nico-operators,nico-admins" + - name: CARBIDE_WEB_ALLOWED_ACCESS_GROUPS_ID_LIST + value: "," +``` + +`extraEnv` accepts normal Kubernetes `env` entries, including `valueFrom` +references. An existing `CARBIDE_WEB_AUTH_TYPE` entry there takes precedence +over `webAuth.mode` for backward compatibility, but new configurations should +use `webAuth.mode` to select the mode. See +[`helm/README.md` → OAuth2 / SSO Setup](../../../helm/README.md#oauth2--sso-setup) +for the complete Helm guidance. `[auth.acls]` defines per-principal HTTP method+path allow/deny rules (used by `nico-bmc-proxy` and other authenticating proxies). The example diff --git a/crates/api-web/Cargo.toml b/crates/api-web/Cargo.toml index 8c7011a5cf..3f74d0bcf9 100644 --- a/crates/api-web/Cargo.toml +++ b/crates/api-web/Cargo.toml @@ -35,6 +35,7 @@ carbide-health-report = { path = "../health-report" } carbide-measured-boot = { path = "../measured-boot", features = ["sqlx"] } carbide-rpc = { path = "../rpc", features = ["sqlx", "model"] } carbide-rpc-utils = { path = "../rpc-utils" } +carbide-utils = { path = "../utils" } carbide-uuid = { path = "../uuid", features = ["sqlx"] } carbide-version = { path = "../version" } config-version = { path = "../config-version", features = ["sqlx"] } @@ -61,6 +62,7 @@ itertools = { workspace = true } lazy_static = { workspace = true } mac_address = { workspace = true } oauth2 = { workspace = true, default-features = false } +rand = { workspace = true } regex = { workspace = true } reqwest = { workspace = true, default-features = false, features = [ "rustls", diff --git a/crates/api-web/src/lib.rs b/crates/api-web/src/lib.rs index a902f69b6c..f942af76b4 100644 --- a/crates/api-web/src/lib.rs +++ b/crates/api-web/src/lib.rs @@ -17,6 +17,7 @@ use std::collections::HashMap; use std::env; +use std::str::FromStr; use std::sync::Arc; use std::sync::atomic::Ordering; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -28,10 +29,12 @@ use axum::middleware::Next; use axum::response::{Html, IntoResponse, Redirect, Response}; use axum::routing::{Router, get, post}; use axum_extra::extract::cookie::{Cookie, Key, PrivateCookieJar}; +use base64::Engine as _; +use base64::prelude::BASE64_STANDARD; use carbide_api_core::cfg::file::ToolLink; use carbide_api_core::{Api, AuthContext, CarbideError, DefaultCredential}; use carbide_authn::middleware::Principal; -use http::header::CONTENT_TYPE; +use http::header::{AUTHORIZATION, CONTENT_TYPE, WWW_AUTHENTICATE}; use http::{HeaderMap, Request, StatusCode}; use itertools::Itertools; use oauth2::basic::{ @@ -42,6 +45,7 @@ use oauth2::{ AuthUrl, Client, ClientId, ClientSecret, CsrfToken, EndpointNotSet, EndpointSet, PkceCodeChallenge, RedirectUrl, Scope, StandardRevocableToken, TokenUrl, }; +use rand::RngExt as _; use rpc::forge::forge_server::Forge; use rpc::forge::{self as forgerpc}; use tonic::service::AxumBody; @@ -226,6 +230,8 @@ fn format_state_sla(sla: Option<&forgerpc::StateSla>) -> String { // `fixtures(...)`; use explicit setup helpers instead. #[cfg(test)] pub(crate) use carbide_macros::sqlx_test; +use carbide_utils::none_if_empty::NoneIfEmpty; + #[cfg(test)] mod tests; @@ -282,6 +288,11 @@ mod ufm_browser; mod vpc; const AUTH_TYPE_ENV: &str = "CARBIDE_WEB_AUTH_TYPE"; +const BASIC_AUTH_PASSWORD_ENV: &str = "CARBIDE_WEB_BASIC_AUTH_PASSWORD"; +const BASIC_AUTH_USERNAME: &str = "admin"; +const BASIC_AUTH_REALM: &str = "Basic realm=\"NICo\""; +const TEMPORARY_BASIC_AUTH_REALM: &str = + "Basic realm=\"NICo - check service logs for the random admin password\""; const AUTH_CALLBACK_ROOT: &str = "auth-callback"; // Details https://entra.microsoft.com/#view/Microsoft_AAD_RegisteredApps/ApplicationMenuBlade/~/Overview/appId/5ae5fa35-be8e-44cc-be7b-01ff76af5315/isMSAApp~/false @@ -329,15 +340,79 @@ pub(crate) struct Oauth2Layer { allowed_access_groups_ids_to_name: HashMap, } +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +enum WebAuthMode { + #[default] + Basic, + OAuth2, + None, +} + +impl FromStr for WebAuthMode { + type Err = eyre::Error; + + fn from_str(value: &str) -> Result { + match value.to_ascii_lowercase().as_str() { + "basic" => Ok(Self::Basic), + "oauth2" => Ok(Self::OAuth2), + "none" => Ok(Self::None), + other => Err(eyre::eyre!( + "unknown {AUTH_TYPE_ENV}={other:?}: expected \"basic\", \"oauth2\", or \"none\"" + )), + } + } +} + +#[derive(Clone)] +enum WebAuth { + Basic { + password: String, + challenge: &'static str, + }, + OAuth2(Box), + None, +} + +impl WebAuth { + fn basic(configured_password: Option) -> Self { + match configured_password { + Some(password) => WebAuth::Basic { + password, + challenge: BASIC_AUTH_REALM, + }, + None => { + let password: String = rand::rng() + .sample_iter(&rand::distr::Alphanumeric) + .take(32) + .map(char::from) + .collect(); + tracing::warn!( + username = BASIC_AUTH_USERNAME, + password = %password, + "{BASIC_AUTH_PASSWORD_ENV} is not configured; generated a temporary WebUI Basic Auth password for this process. This password is not persisted and changes on every launch. Configure a permanent password by setting {BASIC_AUTH_PASSWORD_ENV} to a non-empty value." + ); + WebAuth::Basic { + password, + challenge: TEMPORARY_BASIC_AUTH_REALM, + } + } + } + } +} + /// All the URLs in the admin interface. Nested under /admin in api.rs. pub fn routes(api: Arc) -> eyre::Result> { - // `CARBIDE_WEB_AUTH_TYPE`: `none` (default) = no in-process auth — protect the admin UI with - // network policy, or a reverse proxy (OAuth2 Proxy, etc.). `oauth2` = Entra / OIDC via env. - let auth_type = env::var(AUTH_TYPE_ENV) - .unwrap_or_else(|_| "none".to_string()) - .to_lowercase(); - let oauth_extension_layer = match auth_type.as_str() { - "oauth2" => { + let auth_mode = + env::var(AUTH_TYPE_ENV).map_or(Ok(WebAuthMode::default()), |value| value.parse())?; + routes_with_auth_mode(api, auth_mode) +} + +fn routes_with_auth_mode( + api: Arc, + auth_mode: WebAuthMode, +) -> eyre::Result> { + let web_auth = match auth_mode { + WebAuthMode::OAuth2 => { // Get our cookiejar key so we can add it as an extension. let private_cookiejar_key = Key::try_from( env::var(CARBIDE_WEB_PRIVATE_COOKIEJAR_KEY_ENV) @@ -398,33 +473,31 @@ pub fn routes(api: Arc) -> eyre::Result> { builder.build()? }; - Some(Oauth2Layer { + WebAuth::OAuth2(Box::new(Oauth2Layer { client, private_cookiejar_key, allowed_access_groups_filter, allowed_access_groups_ids_to_name, http_client, - }) + })) } - "none" | "" => { + WebAuthMode::None => { tracing::warn!( "{}: admin web UI has no in-process authentication; restrict access with network policy, a private network, or an authenticating reverse proxy (for example OAuth2 Proxy)", AUTH_TYPE_ENV ); - None + WebAuth::None } - "basic" => { - return Err(eyre::eyre!( - "{AUTH_TYPE_ENV}=basic is not supported. Use \"none\" (default; secure the UI with network controls or an auth proxy) or \"oauth2\" (SSO via Entra)." - )); - } - other => { - return Err(eyre::eyre!( - "unknown {AUTH_TYPE_ENV}={other:?}: expected \"none\" or \"oauth2\"" - )); + WebAuthMode::Basic => { + WebAuth::basic(env::var(BASIC_AUTH_PASSWORD_ENV).ok().none_if_empty()) } }; + let oauth_extension_layer = match &web_auth { + WebAuth::OAuth2(layer) => Some(layer.as_ref().clone()), + WebAuth::Basic { .. } | WebAuth::None => None, + }; + Ok(NormalizePath::trim_trailing_slash( Router::new() .route("/", get(root)) @@ -820,28 +893,34 @@ pub fn routes(api: Arc) -> eyre::Result> { .route("/logs", get(logs::page)) .route("/logs/{source}/stream", get(logs::stream)) .route("/logs/{source}/history", get(logs::history)) - .layer(axum::middleware::from_fn(auth_oauth2)) + .layer(axum::middleware::from_fn(web_auth_middleware_fn)) + .layer(Extension(web_auth)) .layer(Extension(oauth_extension_layer)) .with_state(api), )) } -pub async fn auth_oauth2( +pub async fn web_auth_middleware_fn( headers: HeaderMap, mut req: Request, next: Next, ) -> Result { - let oauth_extension_layer = match req.extensions().get::>() { + let oauth_extension_layer = match req.extensions().get::().cloned() { None => { - tracing::error!("failed to find oauth2 extension layer"); + tracing::error!("failed to find web authentication extension layer"); return Err(StatusCode::INTERNAL_SERVER_ERROR); } - Some(o) => match o { - None => { + Some(WebAuth::None) => return Ok(next.run(req).await), + Some(WebAuth::Basic { + password, + challenge, + }) => { + if basic_credentials_are_valid(&headers, &password) { return Ok(next.run(req).await); } - Some(oa) => oa.to_owned(), - }, + return Ok((StatusCode::UNAUTHORIZED, [(WWW_AUTHENTICATE, challenge)]).into_response()); + } + Some(WebAuth::OAuth2(layer)) => *layer, }; // /auth-callback should pass through because that's @@ -944,6 +1023,196 @@ pub async fn auth_oauth2( .into_response()) } +fn basic_credentials_are_valid(headers: &HeaderMap, configured_password: &str) -> bool { + let Some((scheme, encoded)) = headers + .get(AUTHORIZATION) + .and_then(|value| value.to_str().ok()) + .and_then(|value| value.split_once(' ')) + else { + return false; + }; + if !scheme.eq_ignore_ascii_case("basic") || encoded.is_empty() { + return false; + } + let Ok(credentials) = BASE64_STANDARD.decode(encoded) else { + return false; + }; + let Ok(credentials) = String::from_utf8(credentials) else { + return false; + }; + let Some((username, password)) = credentials.split_once(':') else { + return false; + }; + + username == BASIC_AUTH_USERNAME && password == configured_password +} + +#[cfg(test)] +mod web_auth_tests { + use axum::body::Body; + use carbide_test_support::{Case, Outcome, check_cases, value_scenarios}; + use http::Request; + use tower::ServiceExt as _; + + use super::*; + + #[test] + fn auth_mode_parsing() { + check_cases( + [ + Case { + scenario: "basic", + input: "basic", + expect: Outcome::Yields(WebAuthMode::Basic), + }, + Case { + scenario: "oauth2", + input: "oauth2", + expect: Outcome::Yields(WebAuthMode::OAuth2), + }, + Case { + scenario: "none", + input: "none", + expect: Outcome::Yields(WebAuthMode::None), + }, + Case { + scenario: "case insensitive", + input: "BASIC", + expect: Outcome::Yields(WebAuthMode::Basic), + }, + Case { + scenario: "empty", + input: "", + expect: Outcome::Fails, + }, + Case { + scenario: "unknown", + input: "saml", + expect: Outcome::Fails, + }, + ], + |value| value.parse::().map_err(drop), + ); + assert_eq!(WebAuthMode::default(), WebAuthMode::Basic); + } + + #[test] + fn basic_credentials() { + value_scenarios!(run = |header: Option<&str>| { + let mut headers = HeaderMap::new(); + if let Some(header) = header { + headers.insert(AUTHORIZATION, header.parse().unwrap()); + } + basic_credentials_are_valid(&headers, "correct-password") + }; + "accepted" { + Some("Basic YWRtaW46Y29ycmVjdC1wYXNzd29yZA==") => true, + Some("basic YWRtaW46Y29ycmVjdC1wYXNzd29yZA==") => true, + } + "rejected" { + Some("Basic dXNlcjpjb3JyZWN0LXBhc3N3b3Jk") => false, + Some("Basic YWRtaW46d3Jvbmc=") => false, + None => false, + Some("Bearer token") => false, + Some("Basic not-base64") => false, + Some("Basic YWRtaW4=") => false, + } + ); + assert!(!basic_credentials_are_valid(&HeaderMap::new(), "")); + } + + fn auth_test_router(auth: WebAuth) -> Router { + Router::new() + .route("/", get(|| async { StatusCode::NO_CONTENT })) + .layer(axum::middleware::from_fn(web_auth_middleware_fn)) + .layer(Extension(auth)) + } + + #[tokio::test] + async fn valid_basic_credentials_reach_the_ui() { + let response = auth_test_router(WebAuth::basic(Some("secret".to_string()))) + .oneshot( + Request::builder() + .uri("/") + .header(AUTHORIZATION, "Basic YWRtaW46c2VjcmV0") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + async fn invalid_basic_credentials_receive_a_challenge() { + let response = auth_test_router(WebAuth::basic(Some("secret".to_string()))) + .oneshot(Request::builder().uri("/").body(Body::empty()).unwrap()) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert_eq!(response.headers()[WWW_AUTHENTICATE], BASIC_AUTH_REALM); + } + + #[tokio::test] + async fn empty_basic_configuration_generates_a_usable_temporary_password() { + let auth = WebAuth::basic(None); + let WebAuth::Basic { + password, + challenge, + } = &auth + else { + panic!("basic_auth must return Basic authentication"); + }; + assert_eq!(password.len(), 32); + assert!( + password + .chars() + .all(|character| character.is_ascii_alphanumeric()) + ); + assert_eq!(*challenge, TEMPORARY_BASIC_AUTH_REALM); + + let credentials = BASE64_STANDARD.encode(format!("{BASIC_AUTH_USERNAME}:{password}")); + let response = auth_test_router(auth) + .oneshot( + Request::builder() + .uri("/") + .header(AUTHORIZATION, format!("Basic {credentials}")) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + async fn temporary_password_challenge_points_to_the_logs() { + let response = auth_test_router(WebAuth::basic(None)) + .oneshot(Request::builder().uri("/").body(Body::empty()).unwrap()) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert_eq!( + response.headers()[WWW_AUTHENTICATE], + TEMPORARY_BASIC_AUTH_REALM + ); + } + + #[tokio::test] + async fn explicit_none_reaches_the_ui() { + let response = auth_test_router(WebAuth::None) + .oneshot(Request::builder().uri("/").body(Body::empty()).unwrap()) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NO_CONTENT); + } +} + #[derive(Template)] #[template(path = "index.html")] struct Index { diff --git a/crates/api-web/src/tests/mod.rs b/crates/api-web/src/tests/mod.rs index 07f91d6137..0755c2225e 100644 --- a/crates/api-web/src/tests/mod.rs +++ b/crates/api-web/src/tests/mod.rs @@ -19,7 +19,7 @@ use carbide_test_harness::prelude::TestHarness; use hyper::http::Request; use hyper::http::request::Builder; -use crate::routes; +use crate::{WebAuthMode, routes_with_auth_mode}; mod env; mod explored_endpoint; @@ -29,11 +29,11 @@ mod managed_host; mod vpc; fn make_test_app(test_harness: &TestHarness) -> Router { - let r = routes(test_harness.api_arc()).unwrap(); + let r = routes_with_auth_mode(test_harness.api_arc(), WebAuthMode::None).unwrap(); Router::new().nest_service("/admin", r) } -/// Builder for admin UI requests (in-process auth defaults to none in tests). +/// Builder for admin UI requests (tests explicitly disable in-process auth). fn web_request_builder() -> Builder { Request::builder().header("Host", "with.the.most") } diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index 0ad1a5d1c6..03d7f84d66 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -25,6 +25,7 @@ pub mod config; mod host_port_pair; pub mod managed_loop; pub mod metrics; +pub mod none_if_empty; pub mod periodic_timer; pub mod redfish; pub mod sku; diff --git a/crates/utils/src/none_if_empty.rs b/crates/utils/src/none_if_empty.rs new file mode 100644 index 0000000000..1291c8a686 --- /dev/null +++ b/crates/utils/src/none_if_empty.rs @@ -0,0 +1,211 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +use std::collections::HashMap; + +/// Converts an empty value, or an optional empty value, to [`None`]. +pub trait NoneIfEmpty { + type Val; + + fn none_if_empty(self) -> Option; +} + +impl NoneIfEmpty for Option { + type Val = String; + + fn none_if_empty(self) -> Option { + self.filter(|value| !value.is_empty()) + } +} + +impl NoneIfEmpty for String { + type Val = String; + + fn none_if_empty(self) -> Option { + if self.is_empty() { None } else { Some(self) } + } +} + +impl NoneIfEmpty for Option> { + type Val = Vec; + fn none_if_empty(self) -> Option { + match self { + None => None, + Some(v) if v.is_empty() => None, + Some(v) => Some(v), + } + } +} + +impl NoneIfEmpty for Vec { + type Val = Vec; + fn none_if_empty(self) -> Option { + if self.is_empty() { None } else { Some(self) } + } +} + +impl NoneIfEmpty for Option> { + type Val = HashMap; + fn none_if_empty(self) -> Option { + match self { + None => None, + Some(h) if h.is_empty() => None, + Some(h) => Some(h), + } + } +} + +impl NoneIfEmpty for HashMap { + type Val = HashMap; + fn none_if_empty(self) -> Option { + if self.is_empty() { None } else { Some(self) } + } +} + +impl<'a> NoneIfEmpty for &'a str { + type Val = &'a str; + fn none_if_empty(self) -> Option { + if self.is_empty() { None } else { Some(self) } + } +} + +impl<'a> NoneIfEmpty for Option<&'a str> { + type Val = &'a str; + fn none_if_empty(self) -> Option { + self.filter(|value| !value.is_empty()) + } +} + +impl<'a, T> NoneIfEmpty for &'a [T] { + type Val = &'a [T]; + fn none_if_empty(self) -> Option { + if self.is_empty() { None } else { Some(self) } + } +} + +impl<'a, T> NoneIfEmpty for Option<&'a [T]> { + type Val = &'a [T]; + fn none_if_empty(self) -> Option { + self.filter(|value| !value.is_empty()) + } +} + +#[cfg(test)] +mod tests { + use carbide_test_support::value_scenarios; + + use super::*; + + #[test] + fn filters_empty_strings() { + value_scenarios!( + run = |value| value.none_if_empty(); + "option string" { + None:: => None, + Some(String::new()) => None, + Some("value".to_string()) => Some("value".to_string()), + } + ); + + value_scenarios!( + run = |value: String| value.none_if_empty(); + "string" { + String::new() => None, + "value".to_string() => Some("value".to_string()), + } + ); + } + + #[test] + fn filters_empty_borrowed_strings() { + value_scenarios!( + run = |value| value.none_if_empty(); + "optional borrowed string" { + None::<&str> => None, + Some("") => None, + Some("value") => Some("value"), + } + ); + + value_scenarios!( + run = |value: &str| value.none_if_empty(); + "borrowed string" { + "" => None, + "value" => Some("value"), + } + ); + } + + #[test] + fn filters_empty_vectors() { + value_scenarios!( + run = |value| value.none_if_empty(); + "option vector" { + None::> => None, + Some(Vec::new()) => None, + Some(vec![1, 2]) => Some(vec![1, 2]), + } + ); + + value_scenarios!( + run = |value: Vec| value.none_if_empty(); + "vector" { + Vec::new() => None, + vec![1, 2] => Some(vec![1, 2]), + } + ); + } + + #[test] + fn filters_empty_borrowed_slices() { + value_scenarios!( + run = |value| value.none_if_empty(); + "optional borrowed slice" { + None::<&[u8]> => None, + Some(&[] as &[u8]) => None, + Some(&[1_u8, 2] as &[u8]) => Some(&[1_u8, 2] as &[u8]), + } + ); + + value_scenarios!( + run = |value: &[u8]| value.none_if_empty(); + "borrowed slice" { + &[] as &[u8] => None, + &[1_u8, 2] as &[u8] => Some(&[1_u8, 2] as &[u8]), + } + ); + } + + #[test] + fn filters_empty_hash_maps() { + value_scenarios!( + run = |value| value.none_if_empty(); + "option hash map" { + None::> => None, + Some(HashMap::new()) => None, + Some(HashMap::from([("key", 1)])) => Some(HashMap::from([("key", 1)])), + } + ); + + value_scenarios!( + run = |value: HashMap<&str, u8>| value.none_if_empty(); + "hash map" { + HashMap::new() => None, + HashMap::from([("key", 1)]) => Some(HashMap::from([("key", 1)])), + } + ); + } +} diff --git a/deploy/nico-base/api/deployment.yaml b/deploy/nico-base/api/deployment.yaml index 0980876858..9a8f16a314 100644 --- a/deploy/nico-base/api/deployment.yaml +++ b/deploy/nico-base/api/deployment.yaml @@ -122,9 +122,10 @@ spec: # configMapKeyRef: # name: nico-web-api-hostname # key: hostname - # Admin UI: omit or NICO_WEB_AUTH_TYPE=none (no in-process auth; use network/proxy). - # For Entra SSO: NICO_WEB_AUTH_TYPE=oauth2 plus OAuth env vars below. - # - name: NICO_WEB_AUTH_TYPE + # Admin UI defaults to Basic Auth. Set CARBIDE_WEB_BASIC_AUTH_PASSWORD + # from a Secret, or select Entra SSO with the OAuth env vars below. + # Without one, nico-api generates a temporary password and logs it at startup. + # - name: CARBIDE_WEB_AUTH_TYPE # value: oauth2 - name: BITNAMI_DEBUG value: "false" diff --git a/dev/deployment/devspace/values.base.yaml b/dev/deployment/devspace/values.base.yaml index cba862e532..f37f6e50ea 100644 --- a/dev/deployment/devspace/values.base.yaml +++ b/dev/deployment/devspace/values.base.yaml @@ -6,6 +6,10 @@ nico-api: requests: cpu: "1" memory: "1Gi" + legacyAlias: + enabled: false + webAuth: + mode: "none" siteConfig: enabled: true nicoApiSiteConfig: | diff --git a/dev/mac-local-dev/README.md b/dev/mac-local-dev/README.md index 84764e38c9..15a50ab7cb 100644 --- a/dev/mac-local-dev/README.md +++ b/dev/mac-local-dev/README.md @@ -225,7 +225,7 @@ this ensures Vault and Postgres are initialised and the token file exists. Retrieve the environment variables for the run configuration: ```bash -echo "NICO_WEB_AUTH_TYPE=basic" +echo "CARBIDE_WEB_AUTH_TYPE=none # local development only" echo "DATABASE_URL=postgresql://postgres:admin@localhost" echo "VAULT_ADDR=http://localhost:8201" echo "VAULT_KV_MOUNT_LOCATION=secrets" diff --git a/dev/mac-local-dev/run-nico-api.sh b/dev/mac-local-dev/run-nico-api.sh index 9b64194f78..7f50e4b156 100755 --- a/dev/mac-local-dev/run-nico-api.sh +++ b/dev/mac-local-dev/run-nico-api.sh @@ -136,6 +136,7 @@ fi # ----------------------------------------------------------------------------- # Environment # ----------------------------------------------------------------------------- +# Local development explicitly disables auth; production/Helm defaults to Basic. # api-web currently reads CARBIDE_WEB_AUTH_TYPE; keep both vars in sync during rename. WEB_AUTH_TYPE="${CARBIDE_WEB_AUTH_TYPE:-${NICO_WEB_AUTH_TYPE:-none}}" export NICO_WEB_AUTH_TYPE="$WEB_AUTH_TYPE" diff --git a/dev/webdev-env/run-env.sh b/dev/webdev-env/run-env.sh index a6c36c5d38..1dee5e5018 100755 --- a/dev/webdev-env/run-env.sh +++ b/dev/webdev-env/run-env.sh @@ -81,6 +81,7 @@ export VAULT_KV_MOUNT_LOCATION="secrets" export VAULT_PKI_MOUNT_LOCATION="certs" export VAULT_PKI_ROLE_NAME="role" export CARBIDE_WEB_AUTH_TYPE="none" +# This local-only environment explicitly disables auth. Deployed WebUIs default to Basic. # Run SQL migrations echo "Running database migrations..." @@ -92,8 +93,8 @@ menu() { ┌─────────────────────────────────────────┐ │ http://localhost:1079/admin/ │ - │ No in-process auth │ - │ (recommended to set oauth2) │ + │ No auth (local development only) │ + │ Deployed WebUIs default to Basic │ │ │ │ Templates: crates/api/templates/ │ │ │ diff --git a/docs/operations/debug_webui.md b/docs/operations/debug_webui.md index b9116f8c17..8782de109b 100644 --- a/docs/operations/debug_webui.md +++ b/docs/operations/debug_webui.md @@ -8,29 +8,47 @@ description: "Overview of the NICo administrative web interface, authentication NICo includes a built-in administrative web interface intended for operational debugging and inspection. It is served at the `/admin` path of the NICo API server and provides read-oriented views of infrastructure state alongside a limited set of administrative actions. -**Hardcoded Basic Auth has been removed as of NICo v0.7.0.** - -Previously, if SSO was not configured, the WebUI defaulted to basic authentication with a hardcoded credential. This fallback has been removed to resolve a P0 security vulnerability. - -- **Production / secure deployments**: Enable OIDC/SSO (`CARBIDE_WEB_AUTH_TYPE=oauth2`). See [Azure OIDC for Infra Controller Web UI](../playbooks/nico_web_oauth2.md) for setup instructions. -- **Development / lab environments**: The WebUI operates with no authentication by default. Bind the service to localhost or restrict access using a network ACL or auth proxy (for example, `mod_proxy`). - -If your current workflows rely on the default basic auth credentials, transition to an OIDC provider or proxy-based authentication before updating to v0.7.0 or later. +The WebUI defaults to Basic authentication. Helm installations generate and +persist a password in `nico-api-web-basic-auth`. As a last-resort safeguard for +non-Helm or older deployment manifests, if `CARBIDE_WEB_BASIC_AUTH_PASSWORD` is +unset or empty, NICo generates a temporary 32-character password for that +process and logs it at warning level. That fallback password is not persisted +and changes on every process launch. Use the WebUI only through the +TLS-protected endpoint. ## Authentication -Authentication mode is controlled by the `CARBIDE_WEB_AUTH_TYPE` environment variable. +For Helm installations, configure `nico-api.webAuth.mode`. The default is +`basic`; `oauth2` and `none` are explicit alternatives. A +`CARBIDE_WEB_AUTH_TYPE` entry in `nico-api.extraEnv` takes precedence as a +backward-compatibility contract. | Value | Behavior | |-------|----------| -| *(unset)* or `none` | No authentication. A warning is logged at startup. Restrict access using network controls or a reverse proxy. | +| *(unset)* or `basic` | HTTP Basic Auth with fixed username `admin`. Uses `CARBIDE_WEB_BASIC_AUTH_PASSWORD`, or a temporary password reported in the service logs when unset or empty. | | `oauth2` | Microsoft Entra (Azure AD) OIDC via PKCE flow. Group-based access enforcement via MS Graph API. | -| `basic` | **Not supported.** The service returns an error on startup if this value is set. | +| `none` | No in-process authentication. A warning is logged at startup; restrict access using network controls or an authenticating proxy. | + +For a default Helm installation, retrieve the generated password with the +`kubectl` command printed in the release notes. To use an operator-managed +credential instead: + +```yaml +nico-api: + webAuth: + mode: basic + basic: + existingSecret: + name: nico-web-password + key: password +``` ### OAuth2 (Entra) Configuration -When `CARBIDE_WEB_AUTH_TYPE=oauth2`, the following environment variables are required: +When Helm's `nico-api.webAuth.mode` is `oauth2` (or the legacy +`CARBIDE_WEB_AUTH_TYPE=oauth2` override is used), provide the following +provider settings through `nico-api.extraEnv`: | Variable | Description | |----------|-------------| diff --git a/helm/README.md b/helm/README.md index 57166a7758..d677105fed 100644 --- a/helm/README.md +++ b/helm/README.md @@ -154,15 +154,29 @@ The `global.image.repository` and `global.image.tag` values **must** be set -- t | `unbound` | `unbound.image.repository` / `.tag` | `""` (must be set) | | `unbound` (exporter) | `unbound.exporterImage.repository` / `.tag` | `""` (must be set) | +### WebUI Authentication + +The `/admin` WebUI defaults to HTTP Basic Auth with username `admin`. By +default, Helm creates `nico-api-web-basic-auth` with a generated password and +preserves that password across direct Helm upgrades. Release notes show a +`kubectl` command for retrieving it without printing it during installation. + +For an operator-managed password, set +`nico-api.webAuth.basic.existingSecret.name` and `.key`. Set +`nico-api.webAuth.mode` to `oauth2` or `none` to select another mode; those +modes do not create or reference the Basic password Secret. If a non-Helm or +older deployment does not supply `CARBIDE_WEB_BASIC_AUTH_PASSWORD`, nico-api +falls back to a temporary per-process password reported in its startup logs. + ### OAuth2 / SSO Setup To enable OAuth2 authentication (for example, Azure AD or Okta), configure the `nico-api.extraEnv` values: ```yaml nico-api: + webAuth: + mode: oauth2 extraEnv: - - name: CARBIDE_WEB_AUTH_TYPE - value: "oauth2" - name: CARBIDE_WEB_OAUTH2_AUTH_ENDPOINT value: "https://your-idp/authorize" - name: CARBIDE_WEB_OAUTH2_TOKEN_ENDPOINT @@ -171,6 +185,8 @@ nico-api: value: "your-client-id" - name: CARBIDE_WEB_ALLOWED_ACCESS_GROUPS value: "group1,group2" + - name: CARBIDE_WEB_ALLOWED_ACCESS_GROUPS_ID_LIST + value: "," - name: CARBIDE_WEB_OAUTH2_CLIENT_SECRET valueFrom: secretKeyRef: @@ -178,7 +194,12 @@ nico-api: key: client_secret ``` -The `extraEnv` array supports any Kubernetes `env` spec, including `valueFrom` references to Secrets and ConfigMaps. +The `extraEnv` array supports any Kubernetes `env` spec, including `valueFrom` +references to Secrets and ConfigMaps. For backward compatibility, a +`CARBIDE_WEB_AUTH_TYPE` entry in `extraEnv` takes precedence over +`webAuth.mode`, and the chart does not emit a duplicate mode variable. +Password variables in `extraEnv` remain supported, but +`webAuth.basic.existingSecret` is preferred. ### External LoadBalancer Services diff --git a/helm/charts/nico-api/templates/NOTES.txt b/helm/charts/nico-api/templates/NOTES.txt new file mode 100644 index 0000000000..8a52be2e24 --- /dev/null +++ b/helm/charts/nico-api/templates/NOTES.txt @@ -0,0 +1,7 @@ +{{- if and (eq (include "nico-api.webAuth.renderBasicSecret" .) "true") (eq (include "nico-api.webAuth.hasExistingBasicSecret" .) "false") }} +The NICo WebUI is configured for HTTP Basic authentication. + +Retrieve the generated password (username: admin) with: + + kubectl -n {{ include "nico-api.namespace" . }} get secret nico-api-web-basic-auth -o jsonpath='{.data.password}' | base64 -d +{{- end }} diff --git a/helm/charts/nico-api/templates/_helpers.tpl b/helm/charts/nico-api/templates/_helpers.tpl index c4d9d111b1..862331a254 100644 --- a/helm/charts/nico-api/templates/_helpers.tpl +++ b/helm/charts/nico-api/templates/_helpers.tpl @@ -45,6 +45,80 @@ Global image reference {{ .Values.global.image.repository }}:{{ .Values.global.image.tag }} {{- end }} +{{/* Validate and return the configured WebUI authentication mode. */}} +{{- define "nico-api.webAuth.configuredMode" -}} +{{- $mode := default "basic" .Values.webAuth.mode -}} +{{- if not (has $mode (list "basic" "oauth2" "none")) -}} +{{- fail (printf "nico-api.webAuth.mode must be one of basic, oauth2, or none; got %q" $mode) -}} +{{- end -}} +{{- $mode -}} +{{- end -}} + +{{/* Whether extraEnv contains the legacy mode variable, including valueFrom entries. */}} +{{- define "nico-api.webAuth.hasModeOverride" -}} +{{- $found := false -}} +{{- range .Values.extraEnv -}} + {{- if eq .name "CARBIDE_WEB_AUTH_TYPE" -}} + {{- $found = true -}} + {{- end -}} +{{- end -}} +{{- $found -}} +{{- end -}} + +{{/* A literal legacy override, or an empty string when absent/valueFrom. */}} +{{- define "nico-api.webAuth.literalModeOverride" -}} +{{- range .Values.extraEnv -}} + {{- if and (eq .name "CARBIDE_WEB_AUTH_TYPE") (hasKey . "value") -}} + {{- .value -}} + {{- end -}} +{{- end -}} +{{- end -}} + +{{- define "nico-api.webAuth.hasLiteralModeOverride" -}} +{{- $found := false -}} +{{- range .Values.extraEnv -}} + {{- if and (eq .name "CARBIDE_WEB_AUTH_TYPE") (hasKey . "value") -}} + {{- $found = true -}} + {{- end -}} +{{- end -}} +{{- $found -}} +{{- end -}} + +{{/* Statically known effective mode. "unknown" means extraEnv uses valueFrom. */}} +{{- define "nico-api.webAuth.effectiveMode" -}} +{{- $configured := include "nico-api.webAuth.configuredMode" . -}} +{{- $override := include "nico-api.webAuth.literalModeOverride" . -}} +{{- if eq (include "nico-api.webAuth.hasLiteralModeOverride" .) "true" -}} + {{- if not (has $override (list "basic" "oauth2" "none")) -}} + {{- fail (printf "literal CARBIDE_WEB_AUTH_TYPE in nico-api.extraEnv must be one of basic, oauth2, or none; got %q" $override) -}} + {{- end -}} + {{- $override -}} +{{- else if eq (include "nico-api.webAuth.hasModeOverride" .) "true" -}} +unknown +{{- else -}} +{{- $configured -}} +{{- end -}} +{{- end -}} + +{{/* Render a Basic password Secret for known basic mode, or conservative valueFrom mode. */}} +{{- define "nico-api.webAuth.renderBasicSecret" -}} +{{- $effective := include "nico-api.webAuth.effectiveMode" . -}} +{{- $configured := include "nico-api.webAuth.configuredMode" . -}} +{{- if or (eq $effective "basic") (and (eq $effective "unknown") (eq $configured "basic")) -}}true{{- else -}}false{{- end -}} +{{- end -}} + +{{- define "nico-api.webAuth.basicSecretName" -}} +{{- default "nico-api-web-basic-auth" .Values.webAuth.basic.existingSecret.name -}} +{{- end -}} + +{{- define "nico-api.webAuth.basicSecretKey" -}} +{{- default "password" .Values.webAuth.basic.existingSecret.key -}} +{{- end -}} + +{{- define "nico-api.webAuth.hasExistingBasicSecret" -}} +{{- if .Values.webAuth.basic.existingSecret.name -}}true{{- else -}}false{{- end -}} +{{- end -}} + {{/* Certificate spec Usage: {{ include "nico-api.certificateSpec" (dict "name" "{{ include "nico-api.name" . }}-certificate" "cert" .Values.certificate "global" .Values.global "namespace" (include "nico-api.namespace" .)) }} diff --git a/helm/charts/nico-api/templates/deployment.yaml b/helm/charts/nico-api/templates/deployment.yaml index a091306a21..9fdeb18582 100644 --- a/helm/charts/nico-api/templates/deployment.yaml +++ b/helm/charts/nico-api/templates/deployment.yaml @@ -140,6 +140,17 @@ spec: configMapKeyRef: name: {{ .Values.envFrom.nicoWebHostname.configMapName }} key: hostname + {{- if ne (include "nico-api.webAuth.hasModeOverride" .) "true" }} + - name: CARBIDE_WEB_AUTH_TYPE + value: {{ include "nico-api.webAuth.configuredMode" . | quote }} + {{- end }} + {{- if eq (include "nico-api.webAuth.effectiveMode" .) "basic" }} + - name: CARBIDE_WEB_BASIC_AUTH_PASSWORD + valueFrom: + secretKeyRef: + name: {{ include "nico-api.webAuth.basicSecretName" . }} + key: {{ include "nico-api.webAuth.basicSecretKey" . }} + {{- end }} - name: RUST_BACKTRACE value: {{ .Values.env.RUST_BACKTRACE | quote }} - name: RUST_LIB_BACKTRACE diff --git a/helm/charts/nico-api/templates/web-basic-auth-secret.yaml b/helm/charts/nico-api/templates/web-basic-auth-secret.yaml new file mode 100644 index 0000000000..6c26a12f7e --- /dev/null +++ b/helm/charts/nico-api/templates/web-basic-auth-secret.yaml @@ -0,0 +1,18 @@ +{{- if and (eq (include "nico-api.webAuth.renderBasicSecret" .) "true") (eq (include "nico-api.webAuth.hasExistingBasicSecret" .) "false") }} +{{- $secretName := "nico-api-web-basic-auth" }} +{{- $password := randAlphaNum 32 | b64enc }} +{{- $existing := lookup "v1" "Secret" (include "nico-api.namespace" .) $secretName }} +{{- if and $existing $existing.data (hasKey $existing.data "password") }} +{{- $password = index $existing.data "password" }} +{{- end }} +apiVersion: v1 +kind: Secret +metadata: + name: {{ $secretName }} + namespace: {{ include "nico-api.namespace" . }} + labels: + {{- include "nico-api.labels" . | nindent 4 }} +type: Opaque +data: + password: {{ $password | quote }} +{{- end }} diff --git a/helm/charts/nico-api/tests/web_auth_deployment_test.yaml b/helm/charts/nico-api/tests/web_auth_deployment_test.yaml new file mode 100644 index 0000000000..6cdf56ad92 --- /dev/null +++ b/helm/charts/nico-api/tests/web_auth_deployment_test.yaml @@ -0,0 +1,115 @@ +suite: WebUI authentication deployment configuration +templates: + - deployment.yaml +tests: + - it: defaults to Basic and injects the generated password Secret + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_AUTH_TYPE + value: basic + - contains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_BASIC_AUTH_PASSWORD + valueFrom: + secretKeyRef: + name: nico-api-web-basic-auth + key: password + + - it: injects an existing Basic password Secret + set: + webAuth.basic.existingSecret.name: operator-web-password + webAuth.basic.existingSecret.key: credential + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_BASIC_AUTH_PASSWORD + valueFrom: + secretKeyRef: + name: operator-web-password + key: credential + + - it: oauth2 mode does not inject a Basic password + set: + webAuth.mode: oauth2 + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_AUTH_TYPE + value: oauth2 + - notContains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_BASIC_AUTH_PASSWORD + + - it: none mode does not inject a Basic password + set: + webAuth.mode: none + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_AUTH_TYPE + value: none + - notContains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_BASIC_AUTH_PASSWORD + + - it: literal legacy mode takes precedence without a duplicate built-in mode + set: + extraEnv: + - name: CARBIDE_WEB_AUTH_TYPE + value: oauth2 + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_AUTH_TYPE + value: oauth2 + - notContains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_BASIC_AUTH_PASSWORD + + - it: preserves a valueFrom legacy mode without adding a built-in mode or password + set: + extraEnv: + - name: CARBIDE_WEB_AUTH_TYPE + valueFrom: + configMapKeyRef: + name: web-auth-mode + key: mode + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_AUTH_TYPE + valueFrom: + configMapKeyRef: + name: web-auth-mode + key: mode + - notContains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_BASIC_AUTH_PASSWORD + + - it: rejects an invalid configured mode + set: + webAuth.mode: saml + asserts: + - failedTemplate: + errorMessage: 'nico-api.webAuth.mode must be one of basic, oauth2, or none; got "saml"' + + - it: rejects an invalid literal legacy mode + set: + extraEnv: + - name: CARBIDE_WEB_AUTH_TYPE + value: saml + asserts: + - failedTemplate: + errorMessage: 'literal CARBIDE_WEB_AUTH_TYPE in nico-api.extraEnv must be one of basic, oauth2, or none; got "saml"' diff --git a/helm/charts/nico-api/tests/web_auth_secret_test.yaml b/helm/charts/nico-api/tests/web_auth_secret_test.yaml new file mode 100644 index 0000000000..80ac5847fe --- /dev/null +++ b/helm/charts/nico-api/tests/web_auth_secret_test.yaml @@ -0,0 +1,56 @@ +suite: WebUI Basic authentication Secret +templates: + - web-basic-auth-secret.yaml +tests: + - it: generates a 32-character password in the default mode + asserts: + - isKind: + of: Secret + - equal: + path: metadata.name + value: nico-api-web-basic-auth + - matchRegex: + path: data.password + pattern: '^[A-Za-z0-9+/]{42}[A-Za-z0-9+/=]{2}$' + + - it: does not generate a Secret when an existing one is configured + set: + webAuth.basic.existingSecret.name: operator-web-password + asserts: + - hasDocuments: + count: 0 + + - it: does not generate a Secret for oauth2 + set: + webAuth.mode: oauth2 + asserts: + - hasDocuments: + count: 0 + + - it: does not generate a Secret for none + set: + webAuth.mode: none + asserts: + - hasDocuments: + count: 0 + + - it: does not generate a Secret for a literal legacy oauth2 override + set: + extraEnv: + - name: CARBIDE_WEB_AUTH_TYPE + value: oauth2 + asserts: + - hasDocuments: + count: 0 + + - it: conservatively generates a Secret for a valueFrom mode override + set: + extraEnv: + - name: CARBIDE_WEB_AUTH_TYPE + valueFrom: + configMapKeyRef: + name: web-auth-mode + key: mode + asserts: + - isKind: + of: Secret diff --git a/helm/charts/nico-api/values.yaml b/helm/charts/nico-api/values.yaml index cd7cdbdea8..be62359e91 100644 --- a/helm/charts/nico-api/values.yaml +++ b/helm/charts/nico-api/values.yaml @@ -150,6 +150,18 @@ env: extraEnv: [] +## Authentication for the /admin WebUI. CARBIDE_WEB_AUTH_TYPE in extraEnv +## takes precedence for compatibility with existing installations. +webAuth: + ## Allowed values: basic, oauth2, none. + mode: basic + basic: + ## Use an operator-managed password Secret instead of generating one. + ## Leave name empty to create nico-api-web-basic-auth with a generated password. + existingSecret: + name: "" + key: password + ## External secret/configmap references mounted into the pod envFrom: vaultApproleTokens: diff --git a/helm/examples/values-full.yaml b/helm/examples/values-full.yaml index 08537d741b..51706176d6 100644 --- a/helm/examples/values-full.yaml +++ b/helm/examples/values-full.yaml @@ -73,23 +73,24 @@ nico-api: RUST_BACKTRACE: "full" RUST_LIB_BACKTRACE: "0" - ## OAuth2/SSO authentication — configure via extraEnv - ## These are not built into the chart to keep it auth-provider agnostic. + ## WebUI authentication defaults to Basic with a generated password Secret. + ## Select OAuth2 here; provider-specific settings remain in extraEnv. + webAuth: + mode: oauth2 + extraEnv: - - name: NICO_WEB_AUTH_TYPE - value: "oauth2" - - name: NICO_WEB_OAUTH2_AUTH_ENDPOINT + - name: CARBIDE_WEB_OAUTH2_AUTH_ENDPOINT value: "https://login.example.com/oauth2/v2.0/authorize" - - name: NICO_WEB_OAUTH2_TOKEN_ENDPOINT + - name: CARBIDE_WEB_OAUTH2_TOKEN_ENDPOINT value: "https://login.example.com/oauth2/v2.0/token" - - name: NICO_WEB_OAUTH2_CLIENT_ID + - name: CARBIDE_WEB_OAUTH2_CLIENT_ID value: "your-oauth2-client-id" - - name: NICO_WEB_OAUTH2_CLIENT_SECRET + - name: CARBIDE_WEB_OAUTH2_CLIENT_SECRET valueFrom: secretKeyRef: name: azure-sso-nico-web-client-secret key: client_secret - - name: NICO_WEB_ALLOWED_ACCESS_GROUPS + - name: CARBIDE_WEB_ALLOWED_ACCESS_GROUPS value: "nico-admins,nico-operators" ## NICo Web UI hostname From e718662e597ba1a304757c25a22179d4a2b39d4c Mon Sep 17 00:00:00 2001 From: Ken Simon Date: Tue, 14 Jul 2026 16:25:39 -0400 Subject: [PATCH 2/3] coderabbit feedback --- crates/api-web/src/auth.rs | 3 +- crates/api-web/src/lib.rs | 24 ++++++------ crates/api-web/src/redfish_actions.rs | 7 ++-- crates/api-web/src/redfish_browser.rs | 7 ++-- helm/charts/nico-api/templates/NOTES.txt | 2 +- helm/charts/nico-api/templates/_helpers.tpl | 12 ++++-- .../charts/nico-api/templates/deployment.yaml | 2 +- .../templates/web-basic-auth-secret.yaml | 9 +++-- .../tests/web_auth_deployment_test.yaml | 38 ++++++++++++++++++- .../nico-api/tests/web_auth_secret_test.yaml | 23 +++++++++++ helm/examples/values-full.yaml | 3 ++ 11 files changed, 98 insertions(+), 32 deletions(-) diff --git a/crates/api-web/src/auth.rs b/crates/api-web/src/auth.rs index 446814af6d..51d292063b 100644 --- a/crates/api-web/src/auth.rs +++ b/crates/api-web/src/auth.rs @@ -58,7 +58,7 @@ pub async fn callback( AxumState(_state): AxumState>, request_headers: HeaderMap, Query(query): Query, - Extension(oauth2_layer): Extension>, + Extension(oauth2_layer): Extension>>, ) -> AuthCallbackResponse { use AuthCallbackError::*; let Some(oauth2_layer) = oauth2_layer else { @@ -85,6 +85,7 @@ pub async fn callback( // which includes an access token. let _ = match oauth2_layer .client + .clone() .set_client_secret(ClientSecret::new(client_secret.to_string())) .exchange_client_credentials() .add_scope(Scope::new(format!("{client_id}/.default"))) diff --git a/crates/api-web/src/lib.rs b/crates/api-web/src/lib.rs index f942af76b4..40ba4f8887 100644 --- a/crates/api-web/src/lib.rs +++ b/crates/api-web/src/lib.rs @@ -331,7 +331,6 @@ pub(crate) type Oauth2ClientWithPropertiesSet = Client< EndpointSet, >; -#[derive(Clone)] pub(crate) struct Oauth2Layer { client: Oauth2ClientWithPropertiesSet, http_client: reqwest::Client, @@ -363,13 +362,12 @@ impl FromStr for WebAuthMode { } } -#[derive(Clone)] enum WebAuth { Basic { password: String, challenge: &'static str, }, - OAuth2(Box), + OAuth2(Arc), None, } @@ -473,7 +471,7 @@ fn routes_with_auth_mode( builder.build()? }; - WebAuth::OAuth2(Box::new(Oauth2Layer { + WebAuth::OAuth2(Arc::new(Oauth2Layer { client, private_cookiejar_key, allowed_access_groups_filter, @@ -483,8 +481,8 @@ fn routes_with_auth_mode( } WebAuthMode::None => { tracing::warn!( - "{}: admin web UI has no in-process authentication; restrict access with network policy, a private network, or an authenticating reverse proxy (for example OAuth2 Proxy)", - AUTH_TYPE_ENV + auth_type_env = AUTH_TYPE_ENV, + "admin web UI has no in-process authentication; restrict access with network policy, a private network, or an authenticating reverse proxy (for example OAuth2 Proxy)" ); WebAuth::None } @@ -494,7 +492,7 @@ fn routes_with_auth_mode( }; let oauth_extension_layer = match &web_auth { - WebAuth::OAuth2(layer) => Some(layer.as_ref().clone()), + WebAuth::OAuth2(layer) => Some(Arc::clone(layer)), WebAuth::Basic { .. } | WebAuth::None => None, }; @@ -894,7 +892,7 @@ fn routes_with_auth_mode( .route("/logs/{source}/stream", get(logs::stream)) .route("/logs/{source}/history", get(logs::history)) .layer(axum::middleware::from_fn(web_auth_middleware_fn)) - .layer(Extension(web_auth)) + .layer(Extension(Arc::new(web_auth))) .layer(Extension(oauth_extension_layer)) .with_state(api), )) @@ -905,7 +903,7 @@ pub async fn web_auth_middleware_fn( mut req: Request, next: Next, ) -> Result { - let oauth_extension_layer = match req.extensions().get::().cloned() { + let oauth_extension_layer = match req.extensions().get::>().map(AsRef::as_ref) { None => { tracing::error!("failed to find web authentication extension layer"); return Err(StatusCode::INTERNAL_SERVER_ERROR); @@ -915,12 +913,12 @@ pub async fn web_auth_middleware_fn( password, challenge, }) => { - if basic_credentials_are_valid(&headers, &password) { + if basic_credentials_are_valid(&headers, password) { return Ok(next.run(req).await); } - return Ok((StatusCode::UNAUTHORIZED, [(WWW_AUTHENTICATE, challenge)]).into_response()); + return Ok((StatusCode::UNAUTHORIZED, [(WWW_AUTHENTICATE, *challenge)]).into_response()); } - Some(WebAuth::OAuth2(layer)) => *layer, + Some(WebAuth::OAuth2(layer)) => Arc::clone(layer), }; // /auth-callback should pass through because that's @@ -1125,7 +1123,7 @@ mod web_auth_tests { Router::new() .route("/", get(|| async { StatusCode::NO_CONTENT })) .layer(axum::middleware::from_fn(web_auth_middleware_fn)) - .layer(Extension(auth)) + .layer(Extension(Arc::new(auth))) } #[tokio::test] diff --git a/crates/api-web/src/redfish_actions.rs b/crates/api-web/src/redfish_actions.rs index 240e688d8c..dbdaa34869 100644 --- a/crates/api-web/src/redfish_actions.rs +++ b/crates/api-web/src/redfish_actions.rs @@ -51,11 +51,12 @@ pub struct RedfishActionsTable { /// and displays the result pub async fn query( AxumState(state): AxumState>, - Extension(oauth2_layer): Extension>, + Extension(oauth2_layer): Extension>>, request_headers: HeaderMap, ) -> Response { - let cookiejar = oauth2_layer - .map(|layer| PrivateCookieJar::from_headers(&request_headers, layer.private_cookiejar_key)); + let cookiejar = oauth2_layer.map(|layer| { + PrivateCookieJar::from_headers(&request_headers, layer.private_cookiejar_key.clone()) + }); let mut browser = RedfishBrowser { actions: RedfishActionsTable { diff --git a/crates/api-web/src/redfish_browser.rs b/crates/api-web/src/redfish_browser.rs index 68c4a62bf2..b01a74237a 100644 --- a/crates/api-web/src/redfish_browser.rs +++ b/crates/api-web/src/redfish_browser.rs @@ -62,11 +62,12 @@ pub struct QueryParams { pub async fn query( AxumState(state): AxumState>, AxumQuery(query): AxumQuery, - Extension(oauth2_layer): Extension>, + Extension(oauth2_layer): Extension>>, request_headers: HeaderMap, ) -> Response { - let cookiejar = oauth2_layer - .map(|layer| PrivateCookieJar::from_headers(&request_headers, layer.private_cookiejar_key)); + let cookiejar = oauth2_layer.map(|layer| { + PrivateCookieJar::from_headers(&request_headers, layer.private_cookiejar_key.clone()) + }); let mut browser = RedfishBrowser { url: query.url.clone().unwrap_or_default(), diff --git a/helm/charts/nico-api/templates/NOTES.txt b/helm/charts/nico-api/templates/NOTES.txt index 8a52be2e24..acaad63c9b 100644 --- a/helm/charts/nico-api/templates/NOTES.txt +++ b/helm/charts/nico-api/templates/NOTES.txt @@ -3,5 +3,5 @@ The NICo WebUI is configured for HTTP Basic authentication. Retrieve the generated password (username: admin) with: - kubectl -n {{ include "nico-api.namespace" . }} get secret nico-api-web-basic-auth -o jsonpath='{.data.password}' | base64 -d + kubectl -n {{ include "nico-api.namespace" . }} get secret {{ include "nico-api.webAuth.basicSecretName" . }} -o jsonpath='{.data.{{ include "nico-api.webAuth.basicSecretKey" . }}}' | base64 -d {{- end }} diff --git a/helm/charts/nico-api/templates/_helpers.tpl b/helm/charts/nico-api/templates/_helpers.tpl index 862331a254..e6ea47c930 100644 --- a/helm/charts/nico-api/templates/_helpers.tpl +++ b/helm/charts/nico-api/templates/_helpers.tpl @@ -86,7 +86,6 @@ Global image reference {{/* Statically known effective mode. "unknown" means extraEnv uses valueFrom. */}} {{- define "nico-api.webAuth.effectiveMode" -}} -{{- $configured := include "nico-api.webAuth.configuredMode" . -}} {{- $override := include "nico-api.webAuth.literalModeOverride" . -}} {{- if eq (include "nico-api.webAuth.hasLiteralModeOverride" .) "true" -}} {{- if not (has $override (list "basic" "oauth2" "none")) -}} @@ -96,15 +95,20 @@ Global image reference {{- else if eq (include "nico-api.webAuth.hasModeOverride" .) "true" -}} unknown {{- else -}} -{{- $configured -}} +{{- include "nico-api.webAuth.configuredMode" . -}} {{- end -}} {{- end -}} {{/* Render a Basic password Secret for known basic mode, or conservative valueFrom mode. */}} {{- define "nico-api.webAuth.renderBasicSecret" -}} {{- $effective := include "nico-api.webAuth.effectiveMode" . -}} -{{- $configured := include "nico-api.webAuth.configuredMode" . -}} -{{- if or (eq $effective "basic") (and (eq $effective "unknown") (eq $configured "basic")) -}}true{{- else -}}false{{- end -}} +{{- if eq $effective "basic" -}} +true +{{- else if eq $effective "unknown" -}} + {{- if eq (include "nico-api.webAuth.configuredMode" .) "basic" -}}true{{- else -}}false{{- end -}} +{{- else -}} +false +{{- end -}} {{- end -}} {{- define "nico-api.webAuth.basicSecretName" -}} diff --git a/helm/charts/nico-api/templates/deployment.yaml b/helm/charts/nico-api/templates/deployment.yaml index 9fdeb18582..1d66f4e57d 100644 --- a/helm/charts/nico-api/templates/deployment.yaml +++ b/helm/charts/nico-api/templates/deployment.yaml @@ -144,7 +144,7 @@ spec: - name: CARBIDE_WEB_AUTH_TYPE value: {{ include "nico-api.webAuth.configuredMode" . | quote }} {{- end }} - {{- if eq (include "nico-api.webAuth.effectiveMode" .) "basic" }} + {{- if eq (include "nico-api.webAuth.renderBasicSecret" .) "true" }} - name: CARBIDE_WEB_BASIC_AUTH_PASSWORD valueFrom: secretKeyRef: diff --git a/helm/charts/nico-api/templates/web-basic-auth-secret.yaml b/helm/charts/nico-api/templates/web-basic-auth-secret.yaml index 6c26a12f7e..fac4d0eb48 100644 --- a/helm/charts/nico-api/templates/web-basic-auth-secret.yaml +++ b/helm/charts/nico-api/templates/web-basic-auth-secret.yaml @@ -1,9 +1,10 @@ {{- if and (eq (include "nico-api.webAuth.renderBasicSecret" .) "true") (eq (include "nico-api.webAuth.hasExistingBasicSecret" .) "false") }} -{{- $secretName := "nico-api-web-basic-auth" }} +{{- $secretName := include "nico-api.webAuth.basicSecretName" . }} +{{- $secretKey := include "nico-api.webAuth.basicSecretKey" . }} {{- $password := randAlphaNum 32 | b64enc }} {{- $existing := lookup "v1" "Secret" (include "nico-api.namespace" .) $secretName }} -{{- if and $existing $existing.data (hasKey $existing.data "password") }} -{{- $password = index $existing.data "password" }} +{{- if and $existing $existing.data (hasKey $existing.data $secretKey) }} +{{- $password = index $existing.data $secretKey }} {{- end }} apiVersion: v1 kind: Secret @@ -14,5 +15,5 @@ metadata: {{- include "nico-api.labels" . | nindent 4 }} type: Opaque data: - password: {{ $password | quote }} + {{ $secretKey }}: {{ $password | quote }} {{- end }} diff --git a/helm/charts/nico-api/tests/web_auth_deployment_test.yaml b/helm/charts/nico-api/tests/web_auth_deployment_test.yaml index 6cdf56ad92..8ae70a1d87 100644 --- a/helm/charts/nico-api/tests/web_auth_deployment_test.yaml +++ b/helm/charts/nico-api/tests/web_auth_deployment_test.yaml @@ -32,6 +32,19 @@ tests: name: operator-web-password key: credential + - it: generates the default Secret with a custom key + set: + webAuth.basic.existingSecret.key: credential + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_BASIC_AUTH_PASSWORD + valueFrom: + secretKeyRef: + name: nico-api-web-basic-auth + key: credential + - it: oauth2 mode does not inject a Basic password set: webAuth.mode: oauth2 @@ -76,7 +89,24 @@ tests: content: name: CARBIDE_WEB_BASIC_AUTH_PASSWORD - - it: preserves a valueFrom legacy mode without adding a built-in mode or password + - it: literal legacy mode takes precedence over an invalid configured mode + set: + webAuth.mode: saml + extraEnv: + - name: CARBIDE_WEB_AUTH_TYPE + value: oauth2 + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_AUTH_TYPE + value: oauth2 + - notContains: + path: spec.template.spec.containers[0].env + content: + name: CARBIDE_WEB_BASIC_AUTH_PASSWORD + + - it: preserves a valueFrom legacy mode and conservatively injects a password set: extraEnv: - name: CARBIDE_WEB_AUTH_TYPE @@ -93,10 +123,14 @@ tests: configMapKeyRef: name: web-auth-mode key: mode - - notContains: + - contains: path: spec.template.spec.containers[0].env content: name: CARBIDE_WEB_BASIC_AUTH_PASSWORD + valueFrom: + secretKeyRef: + name: nico-api-web-basic-auth + key: password - it: rejects an invalid configured mode set: diff --git a/helm/charts/nico-api/tests/web_auth_secret_test.yaml b/helm/charts/nico-api/tests/web_auth_secret_test.yaml index 80ac5847fe..4c540da87b 100644 --- a/helm/charts/nico-api/tests/web_auth_secret_test.yaml +++ b/helm/charts/nico-api/tests/web_auth_secret_test.yaml @@ -13,6 +13,19 @@ tests: path: data.password pattern: '^[A-Za-z0-9+/]{42}[A-Za-z0-9+/=]{2}$' + - it: generates the default Secret with a custom key + set: + webAuth.basic.existingSecret.key: credential + asserts: + - equal: + path: metadata.name + value: nico-api-web-basic-auth + - matchRegex: + path: data.credential + pattern: '^[A-Za-z0-9+/]{42}[A-Za-z0-9+/=]{2}$' + - notExists: + path: data.password + - it: does not generate a Secret when an existing one is configured set: webAuth.basic.existingSecret.name: operator-web-password @@ -43,6 +56,16 @@ tests: - hasDocuments: count: 0 + - it: literal legacy mode takes precedence over an invalid configured mode + set: + webAuth.mode: saml + extraEnv: + - name: CARBIDE_WEB_AUTH_TYPE + value: oauth2 + asserts: + - hasDocuments: + count: 0 + - it: conservatively generates a Secret for a valueFrom mode override set: extraEnv: diff --git a/helm/examples/values-full.yaml b/helm/examples/values-full.yaml index 51706176d6..8e7dfb9f8b 100644 --- a/helm/examples/values-full.yaml +++ b/helm/examples/values-full.yaml @@ -92,6 +92,9 @@ nico-api: key: client_secret - name: CARBIDE_WEB_ALLOWED_ACCESS_GROUPS value: "nico-admins,nico-operators" + ## Fail-closed examples: replace with the corresponding group object IDs. + - name: CARBIDE_WEB_ALLOWED_ACCESS_GROUPS_ID_LIST + value: "00000000-0000-0000-0000-000000000001,00000000-0000-0000-0000-000000000002" ## NICo Web UI hostname hostname: "nico.example.com" From 034ffbae61451aa4ab252a56c6f94f19fbeeebc9 Mon Sep 17 00:00:00 2001 From: Ken Simon Date: Wed, 15 Jul 2026 09:10:49 -0400 Subject: [PATCH 3/3] Update helm/charts/nico-api/templates/NOTES.txt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Ken Simon --- helm/charts/nico-api/templates/NOTES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/charts/nico-api/templates/NOTES.txt b/helm/charts/nico-api/templates/NOTES.txt index acaad63c9b..99f527d058 100644 --- a/helm/charts/nico-api/templates/NOTES.txt +++ b/helm/charts/nico-api/templates/NOTES.txt @@ -3,5 +3,5 @@ The NICo WebUI is configured for HTTP Basic authentication. Retrieve the generated password (username: admin) with: - kubectl -n {{ include "nico-api.namespace" . }} get secret {{ include "nico-api.webAuth.basicSecretName" . }} -o jsonpath='{.data.{{ include "nico-api.webAuth.basicSecretKey" . }}}' | base64 -d + kubectl -n {{ include "nico-api.namespace" . }} get secret {{ include "nico-api.webAuth.basicSecretName" . }} -o jsonpath="{.data['{{ include "nico-api.webAuth.basicSecretKey" . }}']}" | base64 -d {{- end }}