From 4a91c1e02050d6cc9b921648b086ea1fc703e6ec Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 23:07:53 +0000 Subject: [PATCH] Always serve the web UI: remove the web.enabled switch `[web] enabled = false` turned the web UI off, and with it the administration screens, the page on which an invited user chooses their password and the page that approves an `npm login` in the browser (which then answered 404). Forge is administered from the browser, so the switch only added a configuration in which features quietly went missing. The SPA fallback is now always mounted, and the web login of `npm login` no longer has a "no web UI" 404. `web.enabled` is kept in `WebConfig` only to refuse `false` at startup (`FORGE_WEB__ENABLED` included), naming the setting to remove, so a headless deployment learns of the change instead of quietly getting a UI; a leftover `true` is accepted. `web.assets_dir` stays. The sample configurations, the CI e2e scripts, the Playwright harness and the server's test harness no longer set it: the test servers start with the web UI mounted, from a `web/dist` that holds no built UI in a test, so the fallback answers 404 as before. Fixes #652 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012e6DJZDaP6c2gEFqFeJ8ZP --- .env.example | 3 +- .github/scripts/npm-e2e.sh | 1 - .github/scripts/oci-e2e.sh | 2 - .github/scripts/raw-e2e.sh | 2 - crates/server/src/app.rs | 13 ++--- crates/server/src/cli_auth.rs | 10 +--- crates/server/src/config.rs | 79 ++++++++++++++++++++++++------ crates/server/src/web_ui.rs | 11 ++--- crates/server/tests/common/mod.rs | 15 +++--- crates/server/tests/npm.rs | 3 -- crates/server/tests/oidc_routes.rs | 2 +- crates/server/tests/telemetry.rs | 4 -- crates/server/tests/web_ui.rs | 4 +- docker/forge.docker.toml | 1 - docs/changes/2.2.0/652.upgrade.md | 6 +++ docs/npm.md | 3 +- forge.toml | 7 ++- forge.toml.example | 2 +- web/README.md | 5 +- web/e2e/harness.ts | 1 - 20 files changed, 98 insertions(+), 76 deletions(-) create mode 100644 docs/changes/2.2.0/652.upgrade.md diff --git a/.env.example b/.env.example index 7e0680fa..aea33f43 100644 --- a/.env.example +++ b/.env.example @@ -180,8 +180,7 @@ FORGE_RETENTION__LEASE_SECONDS=300 # --------------------------------------------------------------------------- # Static web UI (SPA) serving # --------------------------------------------------------------------------- -# Serve the built TanStack Start SPA from the Rust server (single origin). -FORGE_WEB__ENABLED=true +# The Rust server always serves the built TanStack Start SPA (single origin). # Directory holding the built SPA (index.html + assets/). Defaults to web/dist. FORGE_WEB__ASSETS_DIR=web/dist diff --git a/.github/scripts/npm-e2e.sh b/.github/scripts/npm-e2e.sh index cd444097..50b65b2f 100755 --- a/.github/scripts/npm-e2e.sh +++ b/.github/scripts/npm-e2e.sh @@ -242,7 +242,6 @@ password = "$admin_password" grace_seconds = 0 poll_interval_seconds = 2 [web] -enabled = true assets_dir = "$work/web" EOF FORGE_CONFIG="$work/forge.toml" RUST_LOG=info "$server_bin" > "$work/server.log" 2>&1 & diff --git a/.github/scripts/oci-e2e.sh b/.github/scripts/oci-e2e.sh index 74882e16..3ad5b02b 100755 --- a/.github/scripts/oci-e2e.sh +++ b/.github/scripts/oci-e2e.sh @@ -76,8 +76,6 @@ password = "$admin_password" [outbox] grace_seconds = 0 poll_interval_seconds = 2 -[web] -enabled = false EOF FORGE_CONFIG="$work/forge.toml" RUST_LOG=info "$server_bin" > "$work/server.log" 2>&1 & server_pid=$! diff --git a/.github/scripts/raw-e2e.sh b/.github/scripts/raw-e2e.sh index 10488119..fb6fd4b1 100755 --- a/.github/scripts/raw-e2e.sh +++ b/.github/scripts/raw-e2e.sh @@ -81,8 +81,6 @@ password = "$admin_password" [outbox] grace_seconds = 0 poll_interval_seconds = 2 -[web] -enabled = false EOF FORGE_CONFIG="$work/forge.toml" RUST_LOG=info "$server_bin" > "$work/server.log" 2>&1 & server_pid=$! diff --git a/crates/server/src/app.rs b/crates/server/src/app.rs index 43f60ba0..dac8076e 100644 --- a/crates/server/src/app.rs +++ b/crates/server/src/app.rs @@ -114,8 +114,8 @@ pub fn principal_layer(state: &AppState) -> anyhow::Result { /// read authz enforced inside the handlers). /// * `/repository/{repo}/...` — repositories of every non-Docker format, /// each served by its format's plugin ([`crate::plugins`]). -/// * everything else — the SPA (when `web.enabled`), whose admin screens live -/// under `/manage/*` (see [`crate::web_ui`]). +/// * everything else — the SPA, whose admin screens live under `/manage/*` +/// (see [`crate::web_ui`]). /// /// All branches run behind the principal middleware so every handler sees a /// resolved [`forge_core::auth::principal::Principal`] extension. @@ -173,12 +173,9 @@ pub fn build_router(state: AppState, layer: PrincipalLayer) -> Router { .route("/repository/{repo}/{*rest}", any(crate::plugins::dispatch)); // Static SPA fallback, installed LAST so the API routers above always win - // and only genuine UI paths fall through to `index.html`. Skipped for a - // headless deployment (`web.enabled = false`). The admin UI lives under - // `/manage/*`; `/admin/*` is the admin API's. - if state.config.web.enabled { - app = app.fallback_service(spa_service(&state.config.web)); - } + // and only genuine UI paths fall through to `index.html`. The admin UI + // lives under `/manage/*`; `/admin/*` is the admin API's. + let app = app.fallback_service(spa_service(&state.config.web)); let hsts = state.cookie_secure; let public_base_url = state.config.public_base_url(); diff --git a/crates/server/src/cli_auth.rs b/crates/server/src/cli_auth.rs index fd6a79ee..3c19a8fc 100644 --- a/crates/server/src/cli_auth.rs +++ b/crates/server/src/cli_auth.rs @@ -162,21 +162,13 @@ async fn password_login( .await } -/// Without the web UI there is no page to approve a web login on: 404, -/// and npm falls back to asking for a password. +/// Start a web login: the page that approves it and the secret to poll with. async fn start_web_login( state: &AppState, repo: &Repository, client_ip: IpAddr, hostname: Option, ) -> Result { - if !state.config.web.enabled { - return Err(( - StatusCode::NOT_FOUND, - "logins approved in the browser need the web UI", - ) - .into_response()); - } // Starting a login is free for anyone: bounded per client network, as // OIDC logins are. if !state.oidc_login_starts.allow(client_ip) { diff --git a/crates/server/src/config.rs b/crates/server/src/config.rs index d44038b6..c2c598b0 100644 --- a/crates/server/src/config.rs +++ b/crates/server/src/config.rs @@ -1684,31 +1684,34 @@ fn default_nuget_max_nuspec_compression_ratio() -> u64 { /// `web.*` — static SPA serving. /// -/// When `enabled`, the server serves the built TanStack Start SPA from -/// `assets_dir` (a directory containing `index.html` + `assets/*`), mounted as -/// the router's fallback **after** the `/admin`, `/repository`, and `/browse` API -/// routers so API paths always win. Unknown paths fall back to `index.html` so +/// The server always serves the built TanStack Start SPA from `assets_dir` (a +/// directory containing `index.html` + `assets/*`), mounted as the router's +/// fallback **after** the `/admin`, `/repository`, and `/browse` API routers so +/// API paths always win. Unknown paths fall back to `index.html` so /// client-side (TanStack Router) deep links resolve. /// /// `assets_dir` defaults to the relative path `web/dist`, matching the /// repository layout (`web/` is built by Vite into `web/dist/`). Container -/// images set `FORGE_WEB__ASSETS_DIR` to the in-image asset path. A headless -/// deployment sets `FORGE_WEB__ENABLED=false`. +/// images set `FORGE_WEB__ASSETS_DIR` to the in-image asset path. #[derive(Debug, Clone, Deserialize)] pub struct WebConfig { - /// Serve the SPA. When false, only the API routers are mounted. - #[serde(default = "default_true")] - pub enabled: bool, /// Directory holding the built SPA (`index.html` + `assets/`). #[serde(default = "default_web_assets_dir")] pub assets_dir: String, + /// The removed `web.enabled`, kept only to refuse `false`: the + /// administration screens, the invitation page and the page approving a + /// browser login of `npm login` are the web UI's, so Forge always serves + /// it. `WebConfig` accepts unknown keys, so without this a headless + /// deployment would quietly get a UI. A leftover `true` changes nothing. + #[serde(default, rename = "enabled")] + removed_enabled: Option, } impl Default for WebConfig { fn default() -> Self { WebConfig { - enabled: default_true(), assets_dir: default_web_assets_dir(), + removed_enabled: None, } } } @@ -2420,6 +2423,15 @@ impl Config { .into(), )); } + if self.web.removed_enabled == Some(false) { + return Err(ConfigError::Invalid( + "web.enabled = false was removed: Forge always serves the web UI, which \ + holds the administration screens, the invitation page and the page that \ + approves an npm login in the browser. Remove web.enabled from the \ + configuration (FORGE_WEB__ENABLED included)" + .into(), + )); + } if self.blob_cache.path.is_some() && self.blob_cache.max_bytes == BlobCacheMaxBytes::Bytes(0) { @@ -3278,20 +3290,57 @@ mod tests { assert_eq!(cfg.outbox.processed_retention_seconds, 7 * 24 * 3600); assert_eq!(cfg.retention.poll_interval_seconds, 30); assert_eq!(cfg.retention.lease_seconds, 300); - // Web UI: enabled by default, served from `web/dist`. - assert!(cfg.web.enabled); + // Web UI: served from `web/dist`. assert_eq!(cfg.web.assets_dir, "web/dist"); } #[test] fn web_block_overrides_defaults() { + let toml = MINIMAL.replace("[auth]", "[auth]\n[web]\nassets_dir = \"/srv/ui\""); + let cfg = Config::from_figment(fig_from_toml(&toml)).expect("valid"); + assert_eq!(cfg.web.assets_dir, "/srv/ui"); + } + + /// The web UI cannot be turned off any more: `web.enabled = false` is + /// refused, naming the setting to remove, and a leftover `true` changes + /// nothing. + #[test] + fn web_enabled_false_is_refused() { + let toml = MINIMAL.replace("[auth]", "[auth]\n[web]\nenabled = false"); + let err = Config::from_figment(fig_from_toml(&toml)) + .unwrap_err() + .to_string(); + assert!( + err.contains("web.enabled = false was removed") + && err.contains("Remove web.enabled") + && err.contains("FORGE_WEB__ENABLED"), + "{err}" + ); let toml = MINIMAL.replace( "[auth]", - "[auth]\n[web]\nenabled = false\nassets_dir = \"/srv/ui\"", + "[auth]\n[web]\nenabled = true\nassets_dir = \"/srv/ui\"", ); let cfg = Config::from_figment(fig_from_toml(&toml)).expect("valid"); - assert!(!cfg.web.enabled); assert_eq!(cfg.web.assets_dir, "/srv/ui"); + // No `[web]` section at all. + Config::from_figment(fig_from_toml(MINIMAL)).expect("valid"); + } + + #[test] + #[allow(clippy::result_large_err)] // figment::Jail's closure returns figment::Error. + fn web_enabled_false_variable_is_refused() { + figment::Jail::expect_with(|jail| { + jail.create_file("forge.toml", MINIMAL)?; + jail.set_env("FORGE_WEB__ENABLED", "false"); + let err = Config::load(Some(std::path::Path::new("forge.toml"))).unwrap_err(); + assert!( + err.to_string().contains("web.enabled = false was removed"), + "{err}" + ); + jail.set_env("FORGE_WEB__ENABLED", "true"); + Config::load(Some(std::path::Path::new("forge.toml"))).expect("valid"); + Ok(()) + }); } #[test] @@ -3376,7 +3425,6 @@ mod tests { jail.set_env("FORGE_OUTBOX__POLL_INTERVAL_SECONDS", "3"); jail.set_env("FORGE_RETENTION__POLL_INTERVAL_SECONDS", "11"); jail.set_env("FORGE_RETENTION__LEASE_SECONDS", "120"); - jail.set_env("FORGE_WEB__ENABLED", "false"); jail.set_env("FORGE_WEB__ASSETS_DIR", "/srv/ui"); // OIDC section (redirect host must match public_base_url = https://forge.test). jail.set_env("FORGE_OIDC__ISSUER", "https://idp.test/v2.0"); @@ -3424,7 +3472,6 @@ mod tests { assert_eq!(cfg.outbox.poll_interval_seconds, 3); assert_eq!(cfg.retention.poll_interval_seconds, 11); assert_eq!(cfg.retention.lease_seconds, 120); - assert!(!cfg.web.enabled); assert_eq!(cfg.web.assets_dir, "/srv/ui"); let oidc = cfg.oidc.as_ref().expect("oidc set via env"); assert_eq!(oidc.issuer, "https://idp.test/v2.0"); diff --git a/crates/server/src/web_ui.rs b/crates/server/src/web_ui.rs index 93f2bb28..1c5d2e6e 100644 --- a/crates/server/src/web_ui.rs +++ b/crates/server/src/web_ui.rs @@ -114,9 +114,8 @@ struct Spa { /// whereas `not_found_service` forces the status to `404` — wrong for an SPA /// history fallback where unknown client routes must return `200 index.html`. /// -/// The caller is responsible for only installing this when `web.enabled` is -/// true. If `index.html` is missing (UI not built), the fallback yields a 404 — -/// the API still works. +/// If `index.html` is missing (UI not built), the fallback yields a 404 — the +/// API still works. pub fn spa_service(cfg: &WebConfig) -> Router { let assets_dir = PathBuf::from(&cfg.assets_dir); let index = assets_dir.join("index.html"); @@ -231,10 +230,8 @@ mod tests { fs::write(dir.path().join("index.html"), "INDEX").unwrap(); fs::create_dir_all(dir.path().join("assets")).unwrap(); fs::write(dir.path().join("assets").join("a.js"), "ASSET").unwrap(); - let cfg = WebConfig { - enabled: true, - assets_dir: dir.path().to_string_lossy().to_string(), - }; + let mut cfg = WebConfig::default(); + cfg.assets_dir = dir.path().to_string_lossy().to_string(); let svc = spa_service(&cfg); (dir, svc) } diff --git a/crates/server/tests/common/mod.rs b/crates/server/tests/common/mod.rs index 06c282b7..938aa82a 100644 --- a/crates/server/tests/common/mod.rs +++ b/crates/server/tests/common/mod.rs @@ -251,12 +251,13 @@ fn config_toml(db_url: &str, s3_endpoint: &str, opts: &ServerOpts) -> String { Some((u, p)) => format!("[auth.bootstrap_admin]\nusername = \"{u}\"\npassword = \"{p}\"\n"), None => String::new(), }; - // Default to a disabled web UI in tests so a missing `web/dist` never - // affects API behaviour; tests that exercise SPA serving pass an explicit - // assets dir. + // The web UI is always mounted. Without an explicit assets dir the + // default `web/dist` resolves against this crate's directory, which holds + // no built UI, so the fallback answers 404; tests that exercise SPA + // serving pass an assets dir. let web = match web_assets_dir { - Some(dir) => format!("[web]\nenabled = true\nassets_dir = {dir:?}\n"), - None => "[web]\nenabled = false\n".to_string(), + Some(dir) => format!("[web]\nassets_dir = {dir:?}\n"), + None => String::new(), }; format!( r#" @@ -374,8 +375,8 @@ pub async fn envelope(pool: &PgPool) -> std::sync::Arc, pub spawn_workers: bool, - /// When set, the static SPA is served from this directory; otherwise the - /// web UI is disabled for the test. + /// When set, the static SPA is served from this directory; otherwise + /// from the default `web/dist`, which holds no built UI in a test. pub web_assets_dir: Option, /// When set, this raw TOML is appended as the `[oidc]` section so the server /// builds an `OidcRuntime` + mounts the `/auth/oidc/*` routes. diff --git a/crates/server/tests/npm.rs b/crates/server/tests/npm.rs index 3099c809..f71e413b 100644 --- a/crates/server/tests/npm.rs +++ b/crates/server/tests/npm.rs @@ -908,14 +908,11 @@ async fn a_selector_on_a_write_grant_scopes_publishes_and_reads() { #[tokio::test] async fn npm_login_legacy_and_web_and_logout() { - let assets = tempfile::tempdir().unwrap(); - std::fs::write(assets.path().join("index.html"), "").unwrap(); let admin_user = format!("admin{}@example.com", unique()); let srv = common::start_with( common::pool().await, common::ServerOpts { bootstrap_admin: Some((admin_user.clone(), "adminpw".into())), - web_assets_dir: Some(assets.path().display().to_string()), ..Default::default() }, ) diff --git a/crates/server/tests/oidc_routes.rs b/crates/server/tests/oidc_routes.rs index c8cfff85..4cf5a11f 100644 --- a/crates/server/tests/oidc_routes.rs +++ b/crates/server/tests/oidc_routes.rs @@ -1179,7 +1179,7 @@ async fn auth_routes_absent_when_oidc_disabled() { .send() .await .unwrap(); - // Web UI is disabled in the default harness, so an unmounted route 404s. + // An unmounted route under `/auth` is a 404, not the SPA's `index.html`. assert_eq!(resp.status(), 404); } diff --git a/crates/server/tests/telemetry.rs b/crates/server/tests/telemetry.rs index 18bd1466..4faf47a9 100644 --- a/crates/server/tests/telemetry.rs +++ b/crates/server/tests/telemetry.rs @@ -923,12 +923,8 @@ async fn a_request_joins_the_callers_trace_down_to_the_upstream_and_s3() { /// one that collects the user's token. No span holds either. #[tokio::test] async fn no_span_holds_the_secrets_of_a_web_login() { - // A web login is approved in the web UI: without it, none starts. - let assets = tempfile::tempdir().unwrap(); - std::fs::write(assets.path().join("index.html"), "").unwrap(); let srv = start_with(common::ServerOpts { bootstrap_admin: Some((format!("admin{}@example.com", unique()), "adminpw".into())), - web_assets_dir: Some(assets.path().display().to_string()), ..Default::default() }) .await; diff --git a/crates/server/tests/web_ui.rs b/crates/server/tests/web_ui.rs index f6dcaea0..7cc87c8a 100644 --- a/crates/server/tests/web_ui.rs +++ b/crates/server/tests/web_ui.rs @@ -139,8 +139,8 @@ async fn spa_serving_and_api_precedence() { assert!(!body.contains("forge-spa"), "{path}: {body}"); } } - // Removed feed prefixes are no longer API routes: when the UI is enabled, - // they fall through to its SPA fallback rather than reaching a feed. + // Removed feed prefixes are no longer API routes: they fall through to + // the SPA fallback rather than reaching a feed. for path in ["/v3/nuget/index.json", "/raw/binaries/file"] { let res = client .get(format!("{}{path}", server.base_url)) diff --git a/docker/forge.docker.toml b/docker/forge.docker.toml index 9469f963..650d122b 100644 --- a/docker/forge.docker.toml +++ b/docker/forge.docker.toml @@ -65,5 +65,4 @@ force_path_style = true [web] # The server serves the built SPA from here (also set via FORGE_WEB__ASSETS_DIR # in the Dockerfile). Matches where stage 1's web/dist is copied in the image. -enabled = true assets_dir = "/app/web/dist" diff --git a/docs/changes/2.2.0/652.upgrade.md b/docs/changes/2.2.0/652.upgrade.md new file mode 100644 index 00000000..a0d5fc77 --- /dev/null +++ b/docs/changes/2.2.0/652.upgrade.md @@ -0,0 +1,6 @@ +Forge always serves the web UI: `web.enabled` was removed. A configuration +that sets `enabled = false` under `[web]`, or `FORGE_WEB__ENABLED=false`, +refuses to start: remove the setting. A leftover `enabled = true` changes +nothing. A deployment without the web UI lacked the administration screens, +the page on which an invited user chooses their password and the page that +approves an `npm login` in the browser. diff --git a/docs/npm.md b/docs/npm.md index a21404a4..c9a58bc0 100644 --- a/docs/npm.md +++ b/docs/npm.md @@ -101,8 +101,7 @@ npm whoami --registry https://forge.example.com/repository/npm-all/ Without a browser on the machine (`--browser=false`), npm prints the page's URL to open elsewhere. A login waits for its approval for `cli_login.web_login_ttl_seconds` (10 minutes), and the page answers only -the signed-in user who opens it. Logins in the browser need the web UI -(`[web] enabled`); without it npm falls back to asking for a password. +the signed-in user who opens it. **A username and a password**: `yarn npm login` (Yarn 2+) and `npm login --auth-type=legacy` send them to Forge, which checks them like the web diff --git a/forge.toml b/forge.toml index f3e0c990..4cefc957 100644 --- a/forge.toml +++ b/forge.toml @@ -69,9 +69,8 @@ password = "changeme" # grace_seconds = 3600 # delay before a dereferenced blob is deleted from S3 # poll_interval_seconds = 10 -# Static SPA serving. The Rust server serves the built TanStack Start UI from -# `assets_dir` (single origin, no Node proxy). Enabled by default; `assets_dir` -# defaults to `web/dist`. Set `enabled = false` for a headless deployment. +# Static SPA serving. The Rust server always serves the built TanStack Start UI +# from `assets_dir` (single origin, no Node proxy); `assets_dir` defaults to +# `web/dist`. # [web] -# enabled = true # assets_dir = "web/dist" diff --git a/forge.toml.example b/forge.toml.example index 746dee39..9be36188 100644 --- a/forge.toml.example +++ b/forge.toml.example @@ -215,7 +215,7 @@ max_unpacked_bytes = 1073741824 # (default: 1 GiB) the most a published or prox [cli_login] # Logins of command-line clients (`npm login`, `yarn npm login`), with a -# password or approved in the browser (which needs the web UI). +# password or approved in the browser. token_lifetime_days = 90 # (default: 90; 0: never expires) the API token a login mints web_login_ttl_seconds = 600 # (default: 600; 60-3600) how long a login approved in the browser waits diff --git a/web/README.md b/web/README.md index 1e253831..326c2d0f 100644 --- a/web/README.md +++ b/web/README.md @@ -35,9 +35,8 @@ pnpm install --frozen-lockfile pnpm run build # tsc --noEmit + vite build -> dist/ ``` -The Rust server serves `dist/` when `web.enabled = true` (config) with -`web.assets_dir` pointing at it (default `web/dist`). See -`crates/server/src/web_ui.rs`. +The Rust server serves `dist/` from `web.assets_dir` (config, default +`web/dist`). See `crates/server/src/web_ui.rs`. ## Test diff --git a/web/e2e/harness.ts b/web/e2e/harness.ts index 9a732305..b83eb270 100644 --- a/web/e2e/harness.ts +++ b/web/e2e/harness.ts @@ -144,7 +144,6 @@ function startServer() { FORGE_AUTH__BOOTSTRAP_ADMIN__USERNAME: ADMIN_USER, FORGE_AUTH__BOOTSTRAP_ADMIN__PASSWORD: ADMIN_PASS, // web UI: serve the freshly built dist (single origin). - FORGE_WEB__ENABLED: 'true', FORGE_WEB__ASSETS_DIR: path.join(webDir, 'dist'), // outbox: drain quickly so a pushed package becomes visible fast. FORGE_OUTBOX__POLL_INTERVAL_SECONDS: '1',