From 0e76568c06b142c6edef2c6ab5095eab61d06ccb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 07:32:39 +0000 Subject: [PATCH 1/3] Auth realms: accept client passwords over HTTP Basic per realm Whether a client may sign in with a password (HTTP Basic on the feeds and /v2/token, npm login --auth-type=legacy) was one install-wide setting, allow_password_basic, and an enabled credential-forwarding realm turned it on for the whole chain: every local password then worked from every client, whatever the setting said. It is now each password realm's own accept_client_passwords: on by default for credential_forwarding realms (the Nexus case), off for the local realm. The realm snapshot builds a second chain for clients from the realms that accept them. A local realm that does not stays in it reject-only (LocalPasswordProvider::reject_only): a password a client sends for a local account is refused like a wrong one and never forwarded upstream. With no realm accepting client passwords the path is off. The browser login keeps the full chain, and an API token works as before. The upgrade (0006) copies allow_password_basic onto the local realm; the setting leaves the settings document and Admin -> Settings, and a PUT that still names it is refused with where it went. The realm form gets the checkbox on the local and credential-forwarding realms. The test harnesses, the Playwright setup and cargo xtask seed set the local realm's flag instead. Fixes #647 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012e6DJXRhLYxFjzrWiwKFKc --- DEVELOPMENT.md | 23 +- README.md | 6 +- crates/core/src/auth/local_password.rs | 16 +- crates/core/src/auth/provider.rs | 17 + crates/core/src/auth/token.rs | 5 +- crates/core/tests/local_password.rs | 44 +- crates/e2e/src/stack.rs | 17 +- crates/server/src/app.rs | 4 +- crates/server/src/auth/mod.rs | 58 ++- crates/server/src/auth/realms.rs | 229 ++++++--- crates/server/src/auth/settings.rs | 52 +- crates/server/src/cli_auth.rs | 6 +- crates/server/src/config.rs | 17 +- crates/server/tests/auth_settings.rs | 39 +- crates/server/tests/client_passwords.rs | 457 ++++++++++++++++++ crates/server/tests/common/mod.rs | 47 +- crates/server/tests/service_accounts.rs | 3 +- docs/changes/2.2.0/647.feature.md | 12 + docs/changes/2.2.0/647.upgrade.md | 11 + docs/deployment.md | 40 +- docs/docker.md | 9 +- docs/npm.md | 8 +- migrations/0006_realm_client_passwords.sql | 26 + web/e2e/harness.ts | 23 +- web/src/api/realms.ts | 18 +- web/src/api/settings.ts | 2 - web/src/components/RealmForm.test.ts | 39 ++ web/src/components/RealmForm.tsx | 68 ++- web/src/routes/admin/realms/Edit.tsx | 11 +- web/src/routes/admin/realms/realms.test.tsx | 56 ++- web/src/routes/admin/settings/Settings.tsx | 40 +- .../routes/admin/settings/settings.test.tsx | 20 +- xtask/src/main.rs | 41 +- 33 files changed, 1189 insertions(+), 275 deletions(-) create mode 100644 crates/server/tests/client_passwords.rs create mode 100644 docs/changes/2.2.0/647.feature.md create mode 100644 docs/changes/2.2.0/647.upgrade.md create mode 100644 migrations/0006_realm_client_passwords.sql diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 1d63c4b5..2528592a 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -76,8 +76,8 @@ With the server running, in a second terminal: cargo xtask seed ``` -This turns anonymous reads and password Basic on (**Admin → Settings**), -creates an anonymous-readable hosted repo `nuget`, mints a push token, and +This turns anonymous reads on (**Admin → Settings**), has the local realm +accept passwords over HTTP Basic (**Admin → Authentication**), creates an anonymous-readable hosted repo `nuget`, mints a push token, and pushes the bundled sample package. It prints the feed URL and the available versions. The feed is then browsable without auth at `http://localhost:8421/repository/nuget/index.json`. @@ -309,8 +309,8 @@ connections, drains in-flight requests, then stops the background workers. ### Access and sign-in settings -Anonymous access, the login screen, passwords over HTTP Basic, the session -lifetimes and the login lockout are **not** in `forge.toml`: they are auth +Anonymous access, the login screen, the session lifetimes and the login +lockout are **not** in `forge.toml`: they are auth settings kept in the database, set under **Admin → Settings** (`/manage/settings`) or through `GET`/`PUT /admin/settings/auth`, and every instance applies a change without a restart (the others within @@ -323,15 +323,19 @@ to set them. See | `anonymous_mode` | `off` (default) / `all`. Makes unauthenticated requests an *anonymous principal*. | | repo `anonymous_read` | **Also required** for anonymous reads — set it per repository. | | `login_mode` | `identifier_first` (default) or `combined`. | -| `allow_password_basic` | Accept a user's password over HTTP Basic on the feeds (default off; the admin API never takes one). | | `session_idle_ttl_seconds` / `session_absolute_ttl_seconds` | Browser-session lifetimes (default 8 h / 7 d). | | `login_max_failures` / `login_failure_window_seconds` | Password-failure lockout per client IP + username for `/auth/login` and Basic password auth (default 10 per 300 s; `0` disables). | > **Anonymous reads need both** the anonymous mode *and* the repo's > `anonymous_read=true` (authz = authenticated user **or** > anonymous-and-repo-allows). For zero-auth local browsing, `cargo xtask seed` -> sets the anonymous mode to `all` (and turns password Basic on) and creates -> an `anonymous_read` repository. +> sets the anonymous mode to `all` (and has the local realm accept passwords +> over HTTP Basic) and creates an `anonymous_read` repository. +> +> Whether a client may send a password over HTTP Basic is a realm's setting +> (`accept_client_passwords`, **Admin → Authentication**): off for the local +> realm, on for a credential-forwarding one, until set. See +> [docs/deployment.md](./docs/deployment.md#passwords-from-clients). ## Authentication realms @@ -851,8 +855,9 @@ http://localhost:8421. **Runtime configuration.** The baked `forge.toml` carries non-secret defaults (`bind 0.0.0.0:8421`, SPA at `/app/web/dist`, no bootstrap admin — that comes only from `FORGE_AUTH__BOOTSTRAP_ADMIN__*` env), and no S3 keys. -Anonymous access and password Basic start off and are set under **Admin → -Settings** (`cargo xtask seed` turns both on for a dev stack). Placeholder +Anonymous access and the local realm's passwords from clients start off and +are set under **Admin → Settings** and **Admin → Authentication** (`cargo +xtask seed` turns both on for a dev stack). Placeholder credentials (`minioadmin` S3 keys, a `changeme` password) are **refused at startup**; only the dev compose stack sets `FORGE_SERVER__ALLOW_INSECURE_DEFAULTS=true` (plus admin@example.com/changeme). diff --git a/README.md b/README.md index 46f6defc..680d9650 100644 --- a/README.md +++ b/README.md @@ -152,9 +152,9 @@ identity (IRSA, EKS Pod Identity, STS web identity; see bakes no bootstrap admin: `FORGE_AUTH__BOOTSTRAP_ADMIN__PASSWORD` creates one on the first start, named by `FORGE_AUTH__BOOTSTRAP_ADMIN__USERNAME` (a UPN, default `admin@example.com`), and once it is removed the image simply starts without one. A new install has -no anonymous access and accepts no passwords over HTTP Basic (clients use API -tokens); an administrator changes both in the web UI under **Admin → -Settings** (see +no anonymous access and its local realm accepts no passwords over HTTP Basic +(clients use API tokens); an administrator changes them in the web UI under +**Admin → Settings** and **Admin → Authentication** (see [docs/deployment.md](docs/deployment.md#access-and-sign-in-settings)). ### Point a `dotnet` client at it diff --git a/crates/core/src/auth/local_password.rs b/crates/core/src/auth/local_password.rs index 7b835065..2ceb3fac 100644 --- a/crates/core/src/auth/local_password.rs +++ b/crates/core/src/auth/local_password.rs @@ -203,6 +203,8 @@ pub struct LocalPasswordProvider { /// A credential-forwarding provider follows this one in the chain; see /// [`Self::forwarding_follows`]. forwarding_follows: bool, + /// Whether a password is checked at all; see [`Self::reject_only`]. + verifies: bool, } impl LocalPasswordProvider { @@ -222,6 +224,7 @@ impl LocalPasswordProvider { pool, limiter, forwarding_follows: false, + verifies: true, } } @@ -238,6 +241,17 @@ impl LocalPasswordProvider { self } + /// Check no password, but keep the local accounts' names: a name the + /// local store owns is refused (after a dummy verification, so it answers + /// like a wrong password), and only a foreign one goes on to the next + /// provider. The chain of a local realm that accepts no passwords from + /// clients holds the provider this way, so that such a password is never + /// forwarded to a credential-forwarding realm behind it. + pub fn reject_only(mut self) -> Self { + self.verifies = false; + self + } + /// `Skip` for a name that is not a local password account: at once when /// forwarding follows, after a dummy verification otherwise. async fn skip_foreign(&self, candidate: &str) -> AuthOutcome { @@ -330,7 +344,7 @@ impl AuthProvider for LocalPasswordProvider { // Skip: neither the owner's password nor a guess may reach a later // (upstream) realm. It costs a (dummy) verification and answers like a // wrong password, so it does not show that the account is deactivated. - if !user.active { + if !user.active || !self.verifies { return self .after_dummy_verify(cred.password, AuthOutcome::Rejected) .await; diff --git a/crates/core/src/auth/provider.rs b/crates/core/src/auth/provider.rs index 7b600c0d..9ca14f7d 100644 --- a/crates/core/src/auth/provider.rs +++ b/crates/core/src/auth/provider.rs @@ -214,6 +214,23 @@ pub trait AuthProvider: Send + Sync { async fn authenticate(&self, cred: &PasswordCredential<'_>) -> AuthOutcome; } +/// A provider shared by several chains (a realm's, in the chain of the +/// browser login and in the one of the clients' passwords). +#[async_trait::async_trait] +impl AuthProvider for std::sync::Arc

{ + fn id(&self) -> &str { + (**self).id() + } + + fn source(&self) -> AuthSource { + (**self).source() + } + + async fn authenticate(&self, cred: &PasswordCredential<'_>) -> AuthOutcome { + (**self).authenticate(cred).await + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/core/src/auth/token.rs b/crates/core/src/auth/token.rs index 7733b51b..ddc0960e 100644 --- a/crates/core/src/auth/token.rs +++ b/crates/core/src/auth/token.rs @@ -289,9 +289,8 @@ impl PrincipalSource for TokenHeaderSource { /// username, run through the [`crate::auth::Chain`] of /// [`crate::auth::AuthProvider`]s: /// * token-as-basic always wins first; -/// * the password path is gated — when no chain is configured (neither -/// `allow_password_basic` nor an enabled credential-forwarding realm) the -/// password path is skipped entirely; +/// * the password path is gated — when no chain is configured (no realm +/// accepts passwords from clients) the password path is skipped entirely; /// * a present-but-wrong / unknown password yields /// [`SourceOutcome::Invalid`] (→ 401, never downgraded to anonymous). /// diff --git a/crates/core/tests/local_password.rs b/crates/core/tests/local_password.rs index 677c7c53..44913f61 100644 --- a/crates/core/tests/local_password.rs +++ b/crates/core/tests/local_password.rs @@ -3,8 +3,8 @@ //! //! The argon2id password check is the [`LocalPasswordProvider`], invoked //! through the [`Chain`]. [`BasicAuthSource`] tries a token presented as the -//! basic secret first, then falls back to the password chain only when -//! password-basic is enabled; invalid credentials → 401. These tests assert +//! basic secret first, then falls back to the password chain only when a +//! realm accepts passwords from clients; invalid credentials → 401. These tests assert //! both the provider's outcomes directly and the `BasicAuthSource` semantics //! end-to-end. //! @@ -169,6 +169,41 @@ async fn local_provider_rejects_inactive_user() { } } +#[tokio::test] +async fn a_reject_only_provider_refuses_its_names_and_skips_the_others() { + // #647: a local realm that accepts no passwords from clients keeps its + // names in the clients' chain: the right password of an active local + // account is refused like a wrong one (never Skip, which would forward it + // upstream), and only a name the local store does not own goes on. + let pool = common::pool().await; + let username = insert_local(&pool, "pw", true).await; + let unknown = format!("nobody-{}@example.com", common::unique()); + for forwarding_follows in [false, true] { + let provider = LocalPasswordProvider::new(pool.clone()) + .forwarding_follows(forwarding_follows) + .reject_only(); + for password in ["pw", "a-guess"] { + assert_eq!( + provider.authenticate(&cred(&username, password)).await, + AuthOutcome::Rejected, + "{password} (forwarding follows: {forwarding_follows})" + ); + } + assert_eq!( + provider.authenticate(&cred(&unknown, "pw")).await, + AuthOutcome::Skip, + "forwarding follows: {forwarding_follows}" + ); + } + // The same account signs in through a provider that checks passwords. + assert!(matches!( + LocalPasswordProvider::new(pool.clone()) + .authenticate(&cred(&username, "pw")) + .await, + AuthOutcome::Authenticated(_) + )); +} + #[tokio::test] async fn local_provider_rejects_an_invited_account() { // #322: an invited local account has no password until its user chooses @@ -340,8 +375,9 @@ async fn basic_password_authenticates_when_enabled() { #[tokio::test] async fn basic_password_disabled_does_not_authenticate() { - // allow_password_basic = false → password path is entirely off; a valid - // password is treated as an invalid basic credential. + // No realm accepts passwords from clients → the password path is + // entirely off; a valid password is treated as an invalid basic + // credential. let pool = common::pool().await; let username = insert_local(&pool, "pw", true).await; let src = basic_source(&pool, false); diff --git a/crates/e2e/src/stack.rs b/crates/e2e/src/stack.rs index 574838dc..b9907cac 100644 --- a/crates/e2e/src/stack.rs +++ b/crates/e2e/src/stack.rs @@ -540,6 +540,15 @@ impl StackBuilder { forge_server::app::ensure_auth_realms(&config, &pool, &envelope) .await .expect("seed auth realms"); + // Tests sign in over Basic: the local realm accepts passwords from + // clients. A peer on the same database sets the same. + sqlx::query( + "UPDATE auth_realm SET config = config || '{\"accept_client_passwords\": true}' \ + WHERE kind = 'local'", + ) + .execute(&pool) + .await + .expect("seed the local realm's client passwords"); let cache = match &redis { Some(url) => Cache::with_redis(config.cache.l1_max_capacity, url) @@ -552,10 +561,10 @@ impl StackBuilder { .expect("registry token key"); let state = AppState::new(pool.clone(), store, cache, config, envelope) .with_registry_keys(registry_keys); - // The access policy is stored in the database: password Basic on - // (tests sign in over Basic), the stack's anonymous mode, and any - // further settings. A peer on the same database stores the same. - let mut update = serde_json::json!({"allow_password_basic": true}); + // The access policy is stored in the database: the stack's anonymous + // mode, and any further settings. A peer on the same database stores + // the same. + let mut update = serde_json::json!({}); for extra in [Some(self.anon.settings()), self.auth_settings.clone()] .into_iter() .flatten() diff --git a/crates/server/src/app.rs b/crates/server/src/app.rs index dac8076e..2ed7e519 100644 --- a/crates/server/src/app.rs +++ b/crates/server/src/app.rs @@ -43,8 +43,8 @@ use crate::web_ui::spa_service; /// /// The Basic source's password path asks [`crate::auth::RealmAuth`] for the /// chain on every attempt, so admin realm edits apply without a restart -/// (and the wire path stays disabled unless the `allow_password_basic` auth -/// setting or an enabled credential-forwarding realm allows it). The +/// (the wire path holds only the realms that accept passwords from clients, +/// `accept_client_passwords`, and stays disabled without one). The /// anonymous policy comes from the auth settings on every request too /// ([`AnonSource::Settings`]). The session source is /// always mounted — local password logins mint sessions too — and every diff --git a/crates/server/src/auth/mod.rs b/crates/server/src/auth/mod.rs index 5be7de71..ea45da6d 100644 --- a/crates/server/src/auth/mod.rs +++ b/crates/server/src/auth/mod.rs @@ -1037,24 +1037,42 @@ impl IntoResponse for LoginRefusal { } } -/// Check a username and password as an interactive login does: through the -/// realm chain (local and credential-forwarding realms, in priority order), +/// Who sends the password [`verify_password_login`] checks, which picks the +/// realm chain that checks it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum PasswordFrom { + /// The browser login: every password-capable realm + /// ([`RealmSnapshot::chain`]). + Browser, + /// A client (`npm login --auth-type=legacy`): only the realms that accept + /// passwords from clients ([`RealmSnapshot::client_chain`]). + Client, +} + +/// Check a username and password as a login does: through the realm chain +/// `from` picks (local and credential-forwarding realms, in priority order), /// under the login lockout, and never for a secret that is a token Forge -/// issued. Not gated by `allow_password_basic`, which governs passwords on -/// the feeds only. Returns the signed-in user and the canonical username. -/// `POST /auth/login` and `npm login` sign in through it. +/// issued. Returns the signed-in user and the canonical username. +/// `POST /auth/login` signs in through it as [`PasswordFrom::Browser`], +/// `npm login --auth-type=legacy` as [`PasswordFrom::Client`]. pub(crate) async fn verify_password_login( state: &AppState, client_ip: IpAddr, username: &str, password: &str, + from: PasswordFrom, ) -> Result<(forge_core::auth::principal::UserPrincipal, String), LoginRefusal> { let snap = snapshot_or_resp(state) .await .map_err(|_| LoginRefusal::Unavailable)?; - let Some(chain) = snap.chain.clone() else { - // No enabled password-capable realm. Uniform denial (the UI learns - // whether to even offer the form from /auth/login-options). + let chain = match from { + PasswordFrom::Browser => snap.chain.clone(), + PasswordFrom::Client => snap.client_chain.clone(), + }; + let Some(chain) = chain else { + // No enabled password-capable realm (for a client: none that accepts + // passwords from clients). Uniform denial (the UI learns whether to + // even offer the form from /auth/login-options). return Err(LoginRefusal::Invalid); }; if username.len() > MAX_USERNAME_BYTES || password.is_empty() { @@ -1162,9 +1180,10 @@ pub(crate) async fn verify_password_login( /// chain (local + credential-forwarding realms, in priority order) and mint a /// browser session. /// -/// This is the form-login counterpart of the wire Basic path: it runs the SAME -/// chain with the same fail-closed semantics, but is **not** gated by -/// `auth.allow_password_basic` (that flag governs API Basic auth only). The +/// This is the form-login counterpart of the wire Basic path, with the same +/// fail-closed semantics, but it runs the chain of **every** password-capable +/// realm: a realm's `accept_client_passwords` governs only the passwords +/// clients send (it is how a local user gets a token). The /// response mirrors `/auth/me` so the SPA can bootstrap immediately; failures /// are a uniform 401 (no username-exists oracle). A username over /// [`MAX_USERNAME_BYTES`], or one containing `:`, is refused the same way @@ -1177,11 +1196,18 @@ async fn password_login( Json(body): Json, ) -> Response { let client_ip = login_client_ip(&state, connect.map(|Extension(ci)| ci.0), &headers); - let (principal, username) = - match verify_password_login(&state, client_ip, &body.username, &body.password).await { - Ok(verified) => verified, - Err(refusal) => return refusal.into_response(), - }; + let (principal, username) = match verify_password_login( + &state, + client_ip, + &body.username, + &body.password, + PasswordFrom::Browser, + ) + .await + { + Ok(verified) => verified, + Err(refusal) => return refusal.into_response(), + }; // Fresh session per login (fixation-proof), same as the OIDC callback. let issued = match state diff --git a/crates/server/src/auth/realms.rs b/crates/server/src/auth/realms.rs index a1f9ae9b..3126a8d7 100644 --- a/crates/server/src/auth/realms.rs +++ b/crates/server/src/auth/realms.rs @@ -142,6 +142,13 @@ pub struct CredForwardRealmConfig { /// keeps the hashes from being deleted under it. #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub captured_password_only: bool, + /// Accept the realm's passwords from clients over HTTP Basic (the feeds, + /// `docker login`, `npm login --auth-type=legacy`), as a Nexus that + /// checks them does. On by default; off, the realm's users sign their + /// clients in with tokens, and only the browser login takes their + /// passwords. + #[serde(default = "default_true")] + pub accept_client_passwords: bool, /// How long a *good* credential is trusted without re-forwarding (seconds). #[serde(default = "default_positive_ttl")] pub positive_ttl_secs: u64, @@ -154,6 +161,30 @@ pub struct CredForwardRealmConfig { pub key_pepper: String, } +/// `config` shape for the built-in `local` realm. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct LocalRealmConfig { + /// Accept local accounts' passwords from clients over HTTP Basic. Off by + /// default: local users sign their clients in with tokens (API tokens, + /// `npm login --auth-type=web`), and only the browser login takes their + /// passwords. A password a client sends for a local account is then + /// refused, never forwarded to a credential-forwarding realm. + #[serde(default)] + pub accept_client_passwords: bool, +} + +impl LocalRealmConfig { + /// The stored config of the local realm; one that does not parse accepts + /// no passwords from clients (fail closed). + fn of(realm: &AuthRealm, config: &JsonValue) -> Self { + serde_json::from_value(config.clone()).unwrap_or_else(|e| { + tracing::error!(realm = %realm.name, error = %e, "local realm config unreadable; it accepts no passwords from clients"); + Self::default() + }) + } +} + impl std::fmt::Debug for CredForwardRealmConfig { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("CredForwardRealmConfig") @@ -368,14 +399,17 @@ pub fn secret_keys(kind: &str) -> &'static [&'static str] { } /// Parse + validate a realm `config` document for `kind`. Returns a normalized -/// document (kind-specific defaults filled) or a human-readable error. `local` -/// accepts only an empty/no-op config; every other kind refuses unknown keys -/// (the `_set` markers of [`redact_config`], which a client may echo -/// back, are dropped first). +/// document (kind-specific defaults filled) or a human-readable error. Every +/// kind refuses unknown keys (the `_set` markers of [`redact_config`], +/// which a client may echo back, are dropped first). pub fn validate_config(kind: &str, config: &JsonValue) -> Result { let config = &without_secret_markers(kind, config); match kind { - KIND_LOCAL => Ok(JsonValue::Object(Default::default())), + KIND_LOCAL => { + let cfg: LocalRealmConfig = serde_json::from_value(config.clone()) + .map_err(|e| format!("invalid config: {e}"))?; + Ok(serde_json::to_value(&cfg).expect("serializable")) + } KIND_CRED_FORWARD => { let cfg: CredForwardRealmConfig = serde_json::from_value(config.clone()) .map_err(|e| format!("invalid config: {e}"))?; @@ -581,12 +615,17 @@ pub struct RealmSnapshot { /// All realm rows, in try-order (priority ascending). pub realms: Vec, /// The password chain over the enabled password-capable realms (`local`, - /// `credential_forwarding`), or `None` when none are enabled. + /// `credential_forwarding`), or `None` when none are enabled: the + /// browser login's (`POST /auth/login`). pub chain: Option>, - /// Whether an enabled credential-forwarding realm was built. It turns the - /// **wire** Basic password path on by itself; otherwise the - /// `allow_password_basic` setting decides ([`super::AuthSettings`]). - pub forwards_credentials: bool, + /// The chain of the passwords clients send (HTTP Basic on the feeds and + /// `/v2/token`, `npm login --auth-type=legacy`): the realms that accept + /// them (`accept_client_passwords`), and a local realm that does not as a + /// reject-only guard of its names + /// ([`LocalPasswordProvider::reject_only`]). `None` when no realm accepts + /// passwords from clients: the path is off, and a client signs in with a + /// token. + pub client_chain: Option>, /// The (at most one — admin API invariant) enabled OIDC realm's runtime. pub oidc: Option, /// Enabled `workload_identity` (WIF) realms, each with its issuer-scoped @@ -637,8 +676,7 @@ pub struct RealmAuth { pool: PgPool, cache: Cache, config: Arc, - /// The auth settings: whether passwords are accepted over HTTP Basic, - /// and the session service an OIDC login signs in with. + /// The auth settings: the session service an OIDC login signs in with. settings: Arc, ttl: Duration, snapshot: tokio::sync::RwLock>>, @@ -817,21 +855,17 @@ impl RealmAuth { async fn build(&self) -> anyhow::Result { let realms = auth_realm_dao::list_realms(&self.pool).await?; - let mut providers: Vec> = Vec::new(); - let mut has_cred_forward = false; + // The credential-forwarding providers built, in order, each with + // whether its realm accepts passwords from clients. + let mut forwarding: Vec<(Arc, bool)> = Vec::new(); let mut oidc: Option = None; let mut workload: Vec = Vec::new(); let mut workload_configured_issuers: Vec = Vec::new(); let mut trusted_header: Vec = Vec::new(); - // Where the local provider goes, and whether a credential-forwarding - // provider was built after it. Then names the local store does not own - // go upstream next, and the local provider need not equalize their - // timing with a dummy hash. Decided from the providers actually built: - // a forwarding realm that is skipped (fail closed) makes no upstream - // call, so the local provider must pay the dummy hash itself. - let mut local_at: Option = None; - let mut forwarding_follows_local = false; + // Where the local provider goes (the number of forwarding providers + // ahead of it), and whether its realm accepts passwords from clients. + let mut local_at: Option<(usize, bool)> = None; for realm in realms.iter().filter(|r| r.enabled) { // The logical config (encrypted secrets merged in). A realm whose @@ -845,13 +879,14 @@ impl RealmAuth { }; match realm.kind.as_str() { // Inserted once the loop knows what follows it. - KIND_LOCAL => local_at = local_at.or(Some(providers.len())), + KIND_LOCAL => { + local_at = local_at.or(Some(( + forwarding.len(), + LocalRealmConfig::of(realm, &config).accept_client_passwords, + ))) + } KIND_CRED_FORWARD => match self.build_cred_forward(realm, &config) { - Ok(p) => { - has_cred_forward = true; - forwarding_follows_local |= local_at.is_some(); - providers.push(p); - } + Ok(built) => forwarding.push(built), // One malformed row must not take down the other realms; // its own logins fail (closed) until an admin fixes it. Err(e) => { @@ -908,23 +943,27 @@ impl RealmAuth { } } - if let Some(at) = local_at { - providers.insert( - at, - Box::new( - LocalPasswordProvider::with_limiter( - self.pool.clone(), - self.password_limiter.clone(), - ) - .forwarding_follows(forwarding_follows_local), - ), - ); - } - - let chain = if providers.is_empty() { - None + let chain = self.password_chain(local_at.map(|(at, _)| (at, true)), &forwarding); + // The clients' chain: the realms that accept their passwords, and a + // local realm that does not as a guard of its names. Without any + // realm that accepts them, the path is off. + let accepted = local_at.is_some_and(|(_, accepts)| accepts) + || forwarding.iter().any(|(_, accepts)| *accepts); + let client_forwarding: Vec<(Arc, bool)> = forwarding + .iter() + .filter(|(_, accepts)| *accepts) + .cloned() + .collect(); + let client_chain = if accepted { + // The local provider keeps its place among the forwarding + // providers that stay. + let client_local = local_at.map(|(at, accepts)| { + let ahead = forwarding[..at].iter().filter(|(_, a)| *a).count(); + (ahead, accepts) + }); + self.password_chain(client_local, &client_forwarding) } else { - Some(Arc::new(Chain::new(providers))) + None }; workload_configured_issuers.sort(); let workload_ambiguous_issuers = workload_configured_issuers @@ -940,7 +979,7 @@ impl RealmAuth { built_at: Instant::now(), realms, chain, - forwards_credentials: has_cred_forward, + client_chain, oidc, workload, workload_configured_issuers, @@ -949,12 +988,48 @@ impl RealmAuth { }) } - /// Build the per-realm credential-forwarding provider. + /// A password chain of the `forwarding` providers, with the local provider + /// inserted after the first `at` of them when `local` is `Some((at, + /// verifies))` (checking passwords, or [`LocalPasswordProvider::reject_only`]). + /// `None` when it holds no provider. + fn password_chain( + &self, + local: Option<(usize, bool)>, + forwarding: &[(Arc, bool)], + ) -> Option> { + let mut providers: Vec> = forwarding + .iter() + .map(|(provider, _)| Box::new(provider.clone()) as Box) + .collect(); + if let Some((at, verifies)) = local { + // With a forwarding provider after it, names the local store does + // not own go upstream next, and the local provider need not + // equalize their timing with a dummy hash. Decided from the + // providers actually built: a forwarding realm that is skipped + // (fail closed) makes no upstream call, so the local provider must + // pay the dummy hash itself. + let provider = LocalPasswordProvider::with_limiter( + self.pool.clone(), + self.password_limiter.clone(), + ) + .forwarding_follows(at < forwarding.len()); + let provider = if verifies { + provider + } else { + provider.reject_only() + }; + providers.insert(at, Box::new(provider)); + } + (!providers.is_empty()).then(|| Arc::new(Chain::new(providers))) + } + + /// Build the per-realm credential-forwarding provider, with whether the + /// realm accepts passwords from clients. fn build_cred_forward( &self, realm: &AuthRealm, config: &JsonValue, - ) -> anyhow::Result> { + ) -> anyhow::Result<(Arc, bool)> { let cfg: CredForwardRealmConfig = serde_json::from_value(config.clone()).map_err(|e| anyhow::anyhow!("config: {e}"))?; // Also for rows stored before the https rule: never forward passwords @@ -989,16 +1064,20 @@ impl RealmAuth { // The request's own timeout normally fires first (and is logged as // such); the cache's deadline is the backstop its lease is sized by. .with_forward_timeout(request_timeout + Duration::from_secs(1)); + let accepts_clients = cfg.accept_client_passwords; if cfg.captured_password_only { // The upstream is no longer asked; the accounts' kept hashes // decide, through the same (re-keyed) verdict cache. - return Ok(Box::new(CapturedPasswordProvider::new( - provider_id, - realm.id, - self.pool.clone(), - cred_cache, - self.password_limiter.clone(), - ))); + return Ok(( + Arc::new(CapturedPasswordProvider::new( + provider_id, + realm.id, + self.pool.clone(), + cred_cache, + self.password_limiter.clone(), + )), + accepts_clients, + )); } let core = forge_core::auth::UpstreamBasicConfig { url: cfg.url, @@ -1015,9 +1094,12 @@ impl RealmAuth { realm_id: Some(realm.id), capture_password_hash: cfg.capture_password_hash, }; - Ok(Box::new( - UpstreamBasicProvider::new(core, client, cred_cache) - .with_hash_limiter(self.password_limiter.clone()), + Ok(( + Arc::new( + UpstreamBasicProvider::new(core, client, cred_cache) + .with_hash_limiter(self.password_limiter.clone()), + ), + accepts_clients, )) } @@ -1111,11 +1193,7 @@ impl RealmAuth { impl ChainSource for RealmAuth { async fn password_chain(&self) -> Result>, ChainUnavailable> { match self.snapshot().await { - Ok(s) if s.forwards_credentials => Ok(s.chain.clone()), - Ok(s) if self.settings.current().await.settings.allow_password_basic => { - Ok(s.chain.clone()) - } - Ok(_) => Ok(None), + Ok(s) => Ok(s.client_chain.clone()), Err(e) => { tracing::error!(error = %e, "realm snapshot failed; password path denied"); Err(ChainUnavailable) @@ -1251,6 +1329,7 @@ pub async fn seed_auth_realms( auto_provision: None, capture_password_hash: false, captured_password_only: false, + accept_client_passwords: true, positive_ttl_secs: ub.cache.positive_ttl_secs, negative_ttl_secs: ub.cache.negative_ttl_secs, key_pepper: ub.cache.key_pepper.clone(), @@ -1646,11 +1725,27 @@ mod tests { } #[test] - fn local_config_is_empty_and_workload_identity_validates() { - assert_eq!( - validate_config(KIND_LOCAL, &json!({"anything": 1})).unwrap(), - json!({}) - ); + fn local_config_takes_its_flag_and_workload_identity_validates() { + // The local realm's config is whether it accepts passwords from + // clients (off until set); any other key is refused. + for (config, accepts) in [ + (json!({}), false), + (json!({"accept_client_passwords": true}), true), + (json!({"accept_client_passwords": false}), false), + ] { + assert_eq!( + validate_config(KIND_LOCAL, &config).unwrap(), + json!({ "accept_client_passwords": accepts }) + ); + } + assert!(validate_config(KIND_LOCAL, &json!({"anything": 1})).is_err()); + // A credential-forwarding realm accepts them until set. + let forwarding = validate_config( + KIND_CRED_FORWARD, + &json!({"url": "https://nexus.example/status", "key_pepper": "p"}), + ) + .unwrap(); + assert_eq!(forwarding["accept_client_passwords"], true); // workload_identity validates through the WIF module: // an empty config is rejected (no issuer/audiences/rules)… assert!(validate_config(KIND_WORKLOAD_IDENTITY, &json!({})).is_err()); diff --git a/crates/server/src/auth/settings.rs b/crates/server/src/auth/settings.rs index c904d512..ec523270 100644 --- a/crates/server/src/auth/settings.rs +++ b/crates/server/src/auth/settings.rs @@ -1,8 +1,7 @@ //! Sign-in and anonymous-access policy, set at runtime (#294). //! //! The anonymous mode, the login screen's mode and its remembered accounts, -//! password Basic authentication, the session lifetimes and the password -//! lockout are policy +//! the session lifetimes and the password lockout are policy //! an administrator changes while Forge runs, and every instance must //! apply the same values. They are kept in `forge_setting` under //! [`AUTH_SETTINGS_KEY`] as one JSON document ([`AuthSettings`]), changed @@ -22,8 +21,9 @@ //! //! An install that never saved them uses [`AuthSettings::default`]: no //! anonymous access, the identifier-first login screen remembering accounts, -//! no password Basic authentication, 8 h / 7 d sessions and 10 failures per 5 -//! minutes. When the +//! 8 h / 7 d sessions and 10 failures per 5 minutes. (Whether a realm accepts +//! passwords from clients over HTTP Basic is the realm's own setting, +//! `accept_client_passwords`.) When the //! row cannot be read, an instance keeps the settings it has (the defaults //! before the first read) and tries again after the TTL. @@ -124,10 +124,6 @@ pub struct AuthSettings { pub anonymous_mode: AnonymousMode, /// What the login screen asks for first. pub login_mode: LoginMode, - /// Accept a user's password over HTTP Basic on the feeds (an API token - /// always works). An enabled credential-forwarding realm accepts its - /// passwords there regardless. - pub allow_password_basic: bool, /// A browser session ends this long (seconds) after its last use. pub session_idle_ttl_seconds: u64, /// A browser session ends this long (seconds) after sign-in, used or not. @@ -150,7 +146,6 @@ impl Default for AuthSettings { AuthSettings { anonymous_mode: AnonymousMode::Off, login_mode: LoginMode::IdentifierFirst, - allow_password_basic: false, session_idle_ttl_seconds: 8 * 60 * 60, session_absolute_ttl_seconds: 7 * 24 * 60 * 60, login_max_failures: 10, @@ -203,10 +198,6 @@ impl AuthSettings { self.anonymous_mode != other.anonymous_mode, ); diff("login_mode", self.login_mode != other.login_mode); - diff( - "allow_password_basic", - self.allow_password_basic != other.allow_password_basic, - ); diff( "session_idle_ttl_seconds", self.session_idle_ttl_seconds != other.session_idle_ttl_seconds, @@ -239,7 +230,10 @@ impl AuthSettings { pub struct AuthSettingsUpdate { pub anonymous_mode: Option, pub login_mode: Option, - pub allow_password_basic: Option, + /// Gone in 2.2: whether passwords are accepted from clients is set per + /// realm (`accept_client_passwords`). Refused with where it went. + #[serde(default, rename = "allow_password_basic")] + removed_allow_password_basic: Option, pub session_idle_ttl_seconds: Option, pub session_absolute_ttl_seconds: Option, pub login_max_failures: Option, @@ -250,6 +244,9 @@ pub struct AuthSettingsUpdate { impl AuthSettingsUpdate { /// `current` with this update's values applied, validated. pub fn apply_to(self, current: &AuthSettings) -> Result { + if self.removed_allow_password_basic.is_some() { + return Err(ALLOW_PASSWORD_BASIC_MOVED.to_string()); + } let mut next = current.clone(); if let Some(mode) = self.anonymous_mode { next.anonymous_mode = mode.parse()?; @@ -257,9 +254,6 @@ impl AuthSettingsUpdate { if let Some(mode) = self.login_mode { next.login_mode = mode.parse()?; } - if let Some(allow) = self.allow_password_basic { - next.allow_password_basic = allow; - } if let Some(ttl) = self.session_idle_ttl_seconds { next.session_idle_ttl_seconds = ttl; } @@ -280,6 +274,12 @@ impl AuthSettingsUpdate { } } +/// The answer to a settings update that still sets `allow_password_basic`. +const ALLOW_PASSWORD_BASIC_MOVED: &str = + "allow_password_basic is gone: whether passwords are accepted from clients over HTTP \ + Basic is set per realm, as accept_client_passwords in the config of the local and \ + credential-forwarding realms (Admin → Authentication, PATCH /admin/realms/{id})"; + /// The settings stored for the install, [`AuthSettings::default`] when none /// were saved. A stored document that does not validate is an error. pub async fn load(pool: &PgPool) -> anyhow::Result { @@ -509,7 +509,6 @@ mod tests { fn defaults_are_closed_and_valid() { let settings = AuthSettings::default(); assert_eq!(settings.validate().unwrap(), AnonPolicy::Off); - assert!(!settings.allow_password_basic); assert_eq!(settings.login_mode, LoginMode::IdentifierFirst); assert!(settings.remember_accounts); // An empty or partial stored document takes the defaults, so a row @@ -530,7 +529,13 @@ mod tests { ) .unwrap(); assert_eq!(stored.anonymous_mode, AnonymousMode::Off); - assert!(stored.allow_password_basic); + assert!( + serde_json::to_value(&stored) + .unwrap() + .get("allow_password_basic") + .is_none(), + "the moved setting is read past, and not written back" + ); assert_eq!(stored.validate().unwrap(), AnonPolicy::Off); let off: AuthSettings = serde_json::from_str(r#"{"anonymous_mode":"off"}"#).unwrap(); assert_eq!(off.anonymous_mode, AnonymousMode::Off); @@ -543,7 +548,7 @@ mod tests { #[test] fn an_update_keeps_what_it_leaves_out() { let current = AuthSettings { - allow_password_basic: true, + login_max_failures: 3, ..AuthSettings::default() }; let update: AuthSettingsUpdate = serde_json::from_value(serde_json::json!({ @@ -560,7 +565,7 @@ mod tests { current.changed_keys(&next).last(), Some(&"remember_accounts") ); - assert!(next.allow_password_basic, "left out, kept"); + assert_eq!(next.login_max_failures, 3, "left out, kept"); assert_eq!(next.validate().unwrap(), AnonPolicy::All); } @@ -597,5 +602,10 @@ mod tests { ] { assert!(serde_json::from_value::(body).is_err()); } + // The moved setting is refused with where it went, whatever its value. + for allow in [true, false] { + assert!(refused(serde_json::json!({"allow_password_basic": allow})) + .contains("accept_client_passwords"),); + } } } diff --git a/crates/server/src/cli_auth.rs b/crates/server/src/cli_auth.rs index 3c19a8fc..a5c95ebd 100644 --- a/crates/server/src/cli_auth.rs +++ b/crates/server/src/cli_auth.rs @@ -32,7 +32,7 @@ use forge_db::token_dao::{self, NewToken}; use forge_db::user_dao; use forge_plugin::api::{AuthOutcome, AuthRequest}; -use crate::auth::{verify_password_login, LoginRefusal}; +use crate::auth::{verify_password_login, LoginRefusal, PasswordFrom}; use crate::state::AppState; /// The longest host name a web login keeps. @@ -135,7 +135,9 @@ async fn password_login( username: &str, password: &str, ) -> Result { - let (principal, _) = match verify_password_login(state, client_ip, username, password).await { + let verified = + verify_password_login(state, client_ip, username, password, PasswordFrom::Client).await; + let (principal, _) = match verified { Ok(verified) => verified, Err(refusal) => { tracing::info!( diff --git a/crates/server/src/config.rs b/crates/server/src/config.rs index c2c598b0..4ecb3c11 100644 --- a/crates/server/src/config.rs +++ b/crates/server/src/config.rs @@ -541,7 +541,8 @@ impl S3WebIdentityConfig { /// The access and sign-in policy that used to live here (the anonymous mode /// and its allowlist, the login mode, password Basic, the session lifetimes /// and the lockout) is set in the web UI and kept in the database -/// ([`crate::auth::AuthSettings`]); its keys are refused ([`REMOVED_AUTH_KEYS`]). +/// ([`crate::auth::AuthSettings`], and the password realms' +/// `accept_client_passwords`); its keys are refused ([`REMOVED_AUTH_KEYS`]). #[derive(Debug, Clone, Deserialize)] pub struct AuthConfig { /// CIDRs of proxies whose `X-Forwarded-For` we trust, and whose headers @@ -619,7 +620,8 @@ pub struct AuthConfig { /// The `[auth]` keys whose settings moved to the database (#294): the web UI's /// **Admin → Settings** and `/admin/settings/auth` set them for every -/// instance. A configuration that still sets one is refused at startup. +/// instance (`allow_password_basic` is each password realm's own since 2.2, +/// #647). A configuration that still sets one is refused at startup. pub const REMOVED_AUTH_KEYS: [&str; 8] = [ "anonymous_mode", "anonymous_ip_allowlist", @@ -2592,12 +2594,13 @@ impl Config { return Ok(()); } Err(ConfigError::Invalid(format!( - "{} {} removed: anonymous access, the login mode, password Basic \ - authentication, the session lifetimes and the login lockout are set in the \ - web UI (Admin → Settings) or through /admin/settings/auth, for every \ - instance at once. Remove {} from the configuration (FORGE_AUTH__* and \ + "{} {} removed: anonymous access, the login mode, the session lifetimes and \ + the login lockout are set in the web UI (Admin → Settings) or through \ + /admin/settings/auth, and whether a realm accepts passwords from clients over \ + HTTP Basic on the realm (Admin → Authentication, accept_client_passwords), for \ + every instance at once. Remove {} from the configuration (FORGE_AUTH__* and \ FORGE_OIDC__* variables included); an install starts with anonymous access \ - off and password Basic authentication off", + off and its local realm accepting no passwords from clients", keys.join(", "), if keys.len() == 1 { "was" } else { "were" }, if keys.len() == 1 { "it" } else { "them" }, diff --git a/crates/server/tests/auth_settings.rs b/crates/server/tests/auth_settings.rs index d23d793a..50ab4683 100644 --- a/crates/server/tests/auth_settings.rs +++ b/crates/server/tests/auth_settings.rs @@ -113,14 +113,12 @@ async fn login_mode(srv: &TestServer) -> String { #[tokio::test] async fn admins_read_and_change_the_settings_and_wrong_values_are_refused() { let srv = start(ServerOpts::default()).await; - // The harness's database starts with password Basic on, the rest at - // the defaults. + // The harness's database starts with the defaults. assert_eq!( get_settings(&srv).await, json!({ "anonymous_mode": "off", "login_mode": "identifier_first", - "allow_password_basic": true, "session_idle_ttl_seconds": 28800, "session_absolute_ttl_seconds": 604800, "login_max_failures": 10, @@ -150,7 +148,7 @@ async fn admins_read_and_change_the_settings_and_wrong_values_are_refused() { assert_eq!(resp.status(), 200); let saved: Value = resp.json().await.unwrap(); assert_eq!(saved["login_mode"], "combined"); - assert_eq!(saved["allow_password_basic"], true, "left out, kept"); + assert_eq!(saved["anonymous_mode"], "off", "left out, kept"); assert_eq!(get_settings(&srv).await, saved); assert_eq!(login_mode(&srv).await, "combined", "applied at once"); @@ -241,29 +239,20 @@ async fn anonymous_access_follows_the_setting_without_a_restart() { assert_eq!(read_index(&srv, &public, user).await, 403); } -/// Password Basic, the session lifetimes and the lockout apply to the next -/// request after a change. +/// The session lifetimes and the lockout apply to the next request after a +/// change. Password Basic is no setting any more (#647): it moved to the +/// realms, and an update that still names it is refused with where it went. #[tokio::test] -async fn password_basic_sessions_and_lockout_follow_the_settings() { +async fn sessions_and_lockout_follow_the_settings() { let srv = start(ServerOpts::default()).await; - let private = create_repo(&srv, false).await; - let (admin, admin_password) = srv.admin_password_basic(); - let (_, admin_token) = srv.admin_basic(); - - // Password Basic: on in the harness, then off. A token keeps working. - assert_eq!( - read_index(&srv, &private, Some((&admin, &admin_password))).await, - 200 - ); - put_settings(&srv, json!({"allow_password_basic": false})).await; - assert_eq!( - read_index(&srv, &private, Some((&admin, &admin_password))).await, - 401 - ); - assert_eq!( - read_index(&srv, &private, Some((&admin, &admin_token))).await, - 200 - ); + let before = get_settings(&srv).await; + for allow in [true, false] { + let resp = put_settings(&srv, json!({"allow_password_basic": allow})).await; + assert_eq!(resp.status(), 400); + let message = resp.text().await.unwrap(); + assert!(message.contains("accept_client_passwords"), "{message}"); + } + assert_eq!(get_settings(&srv).await, before, "nothing changed"); // Session lifetimes: a sign-in after the change gets them. put_settings( diff --git a/crates/server/tests/client_passwords.rs b/crates/server/tests/client_passwords.rs new file mode 100644 index 00000000..09af87c8 --- /dev/null +++ b/crates/server/tests/client_passwords.rs @@ -0,0 +1,457 @@ +//! Passwords from clients are accepted per realm (#647): a realm's +//! `accept_client_passwords` decides whether its users may send their +//! password from a client (HTTP Basic on the feeds and `/v2/token`, +//! `npm login --auth-type=legacy`). The browser login takes every realm's +//! passwords, and an API token works whatever the realms say. Requires +//! Docker. Each test has a database of its own: the realms are install-wide. + +mod common; + +use base64::Engine; +use common::{ServerOpts, TestServer}; +use serde_json::{json, Value}; +use wiremock::{matchers, Mock, MockServer, ResponseTemplate}; + +/// The local part of the credential-forwarding realm's account. +const FORWARDED: &str = "fwd"; + +/// The password the mock upstream accepts for [`FORWARDED`]. +const UPSTREAM_PASSWORD: &str = "upstream-pw-1"; + +/// The file every test reads from the raw repository. +const RAW_FILE: &str = "hello.txt"; + +/// An admin API request, with the admin's token over Basic. +fn admin(srv: &TestServer, method: reqwest::Method, path: &str) -> reqwest::RequestBuilder { + let (user, token) = srv.admin_basic(); + srv.client() + .request(method, format!("{}{path}", srv.base_url)) + .basic_auth(user, Some(token)) +} + +/// `user:password` of a Basic `authorization` header. +fn basic_credentials(request: &wiremock::Request) -> Option { + let encoded = request + .headers + .get("authorization")? + .to_str() + .ok()? + .strip_prefix("Basic ")? + .to_string(); + let decoded = base64::engine::general_purpose::STANDARD + .decode(encoded) + .ok()?; + String::from_utf8(decoded).ok() +} + +/// A server on a database of its own, with a credential-forwarding realm +/// `nexus` over a mock upstream that accepts [`FORWARDED`] / +/// [`UPSTREAM_PASSWORD`], that realm's account `fwd@example.com` (a writer), +/// and hosted NuGet, raw and npm repositories nobody reads anonymously. The +/// local realm accepts no passwords from clients, its default. +struct Setup { + srv: TestServer, + upstream: MockServer, + forwarded: String, + nuget: String, + raw: String, + npm: String, +} + +impl Setup { + async fn new() -> Setup { + let upstream = MockServer::start().await; + let accepted = format!( + "Basic {}", + base64::engine::general_purpose::STANDARD + .encode(format!("{FORWARDED}:{UPSTREAM_PASSWORD}")) + ); + Mock::given(matchers::header("authorization", accepted.as_str())) + .respond_with(ResponseTemplate::new(200)) + .mount(&upstream) + .await; + Mock::given(matchers::any()) + .respond_with(ResponseTemplate::new(401)) + .mount(&upstream) + .await; + + let srv = common::start_with( + common::fresh_pool().await, + ServerOpts { + bootstrap_admin: Some(( + format!("admin{}@example.com", common::unique()), + "adminpw".into(), + )), + ..Default::default() + }, + ) + .await; + srv.set_local_client_passwords(false).await; + let resp = admin(&srv, reqwest::Method::POST, "/admin/realms") + .json(&json!({ + "kind": "credential_forwarding", + "name": "nexus", + "config": {"url": format!("{}/status", upstream.uri()), "key_pepper": "pepper"}, + })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 201, "{}", resp.text().await.unwrap()); + let forwarded = format!("{FORWARDED}@example.com"); + let resp = admin(&srv, reqwest::Method::POST, "/admin/users") + .json(&json!({"username": forwarded, "role": "writer", "realm": "nexus"})) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 201, "{}", resp.text().await.unwrap()); + + let suffix = common::unique(); + let (nuget, raw, npm) = ( + format!("nuget{suffix}"), + format!("raw{suffix}"), + format!("npm{suffix}"), + ); + for (name, format) in [(&nuget, "nuget"), (&raw, "raw"), (&npm, "npm")] { + let resp = admin(&srv, reqwest::Method::POST, "/admin/repositories") + .json(&json!({"name": name, "kind": "hosted", "format": format})) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 201, "{}", resp.text().await.unwrap()); + } + let resp = admin( + &srv, + reqwest::Method::PUT, + &format!("/repository/{raw}/{RAW_FILE}"), + ) + .body("hello") + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 201, "{}", resp.text().await.unwrap()); + Setup { + srv, + upstream, + forwarded, + nuget, + raw, + npm, + } + } + + /// The status and body of every client sign-in with `username` and + /// `secret`: a NuGet and a raw read with Basic, `docker login`'s token + /// request, and `npm login --auth-type=legacy`. + async fn clients(&self, username: &str, secret: &str) -> Vec<(&'static str, u16, String)> { + let srv = &self.srv; + let client = srv.client(); + let basic = |url: String| client.get(url).basic_auth(username, Some(secret)); + let requests = [ + ( + "nuget", + basic(format!( + "{}/repository/{}/index.json", + srv.base_url, self.nuget + )), + ), + ( + "raw", + basic(format!( + "{}/repository/{}/{RAW_FILE}", + srv.base_url, self.raw + )), + ), + ( + "docker login", + basic(format!( + "{}/v2/token?account={username}&service=test.local", + srv.base_url + )), + ), + ( + "npm login", + client + .put(format!( + "{}/repository/{}/-/user/org.couchdb.user:{}", + srv.base_url, + self.npm, + username.replace('@', "%40") + )) + .json(&json!({"name": username, "password": secret, "type": "user"})), + ), + ]; + let mut answers = Vec::new(); + for (what, request) in requests { + let resp = request.send().await.unwrap(); + let status = resp.status().as_u16(); + answers.push((what, status, resp.text().await.unwrap())); + } + answers + } + + /// Every client sign-in with `username` and `secret` is let in, or none. + async fn assert_clients(&self, username: &str, secret: &str, accepted: bool) { + for (what, status, body) in self.clients(username, secret).await { + let expected = match (accepted, what) { + (false, _) => 401, + (true, "npm login") => 201, + (true, _) => 200, + }; + assert_eq!(status, expected, "{username} with {what}: {body}"); + } + } + + /// How many upstream requests named the account `local_part`. + async fn forwarded_for(&self, local_part: &str) -> usize { + let prefix = format!("{local_part}:"); + self.upstream + .received_requests() + .await + .unwrap_or_default() + .iter() + .filter_map(basic_credentials) + .filter(|credentials| credentials.starts_with(&prefix)) + .count() + } + + /// The id of the realm `name`. + async fn realm_id(&self, name: &str) -> i64 { + forge_db::auth_realm_dao::find_realm_by_name(&self.srv.pool, name) + .await + .unwrap() + .expect("the realm") + .id + } + + /// The config of realm `id`, as the admin API shows it. + async fn realm_config(&self, id: i64) -> Value { + let realm: Value = admin( + &self.srv, + reqwest::Method::GET, + &format!("/admin/realms/{id}"), + ) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + realm["config"].clone() + } + + /// Set `accept_client_passwords` on realm `name` through the admin API + /// (Admin → Authentication), keeping the rest of its config. + async fn set_flag(&self, name: &str, accept: bool) { + let id = self.realm_id(name).await; + let mut config = self.realm_config(id).await; + config["accept_client_passwords"] = json!(accept); + let resp = admin( + &self.srv, + reqwest::Method::PATCH, + &format!("/admin/realms/{id}"), + ) + .json(&json!({ "config": config })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200, "{}", resp.text().await.unwrap()); + } +} + +/// The issue's case: a credential-forwarding realm accepts its users' +/// passwords from every client, the local realm (closed by default) none of +/// its users', whose token keeps working and whose password never reaches +/// the upstream. The browser login takes both. +#[tokio::test] +async fn a_forwarding_realm_takes_client_passwords_and_a_closed_local_realm_none() { + let setup = Setup::new().await; + let srv = &setup.srv; + let (admin_user, admin_password) = srv.admin_password_basic(); + let (_, admin_token) = srv.admin_basic(); + let admin_local = admin_user.split('@').next().unwrap().to_string(); + + setup + .assert_clients(&setup.forwarded, UPSTREAM_PASSWORD, true) + .await; + setup + .assert_clients(&setup.forwarded, "not-the-password", false) + .await; + + // The local user's password is refused by every client, with the answer + // a wrong one gets, and is never sent upstream. + let right = setup.clients(&admin_user, &admin_password).await; + let wrong = setup.clients(&admin_user, "not-the-password").await; + assert_eq!( + right, wrong, + "a closed realm's password answers like a wrong one" + ); + for (what, status, _) in &right { + assert_eq!(*status, 401, "{what}"); + } + assert_eq!( + setup.forwarded_for(&admin_local).await, + 0, + "no local password reaches the upstream" + ); + + // Its API token works on the feeds and for `docker login`. (`npm login` + // takes a password only: npm sends the token itself.) + for (what, status, body) in setup.clients(&admin_user, &admin_token).await { + if what != "npm login" { + assert_eq!(status, 200, "{what}: {body}"); + } + } + let resp = srv + .client() + .get(format!( + "{}/repository/{}/-/whoami", + srv.base_url, setup.npm + )) + .bearer_auth(&admin_token) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200, "npm with the token"); + + // The browser login takes both realms' passwords. + srv.login(&admin_user, &admin_password).await; + srv.login(&setup.forwarded, UPSTREAM_PASSWORD).await; +} + +/// Each realm's flag follows its admin edits at once: the local realm opened +/// takes its users' passwords from clients, the forwarding realm closed +/// takes none (its users then sign their clients in with tokens, which the +/// browser login gets them), and with no realm open the path is off. +#[tokio::test] +async fn each_realm_opens_and_closes_its_own_passwords() { + let setup = Setup::new().await; + let srv = &setup.srv; + let (admin_user, admin_password) = srv.admin_password_basic(); + + setup.set_flag("local", true).await; + setup + .assert_clients(&admin_user, &admin_password, true) + .await; + setup + .assert_clients(&setup.forwarded, UPSTREAM_PASSWORD, true) + .await; + + setup.set_flag("nexus", false).await; + setup + .assert_clients(&admin_user, &admin_password, true) + .await; + let before = setup.forwarded_for(FORWARDED).await; + setup + .assert_clients(&setup.forwarded, UPSTREAM_PASSWORD, false) + .await; + assert_eq!( + setup.forwarded_for(FORWARDED).await, + before, + "a closed forwarding realm forwards no client's password" + ); + srv.login(&setup.forwarded, UPSTREAM_PASSWORD).await; + + setup.set_flag("local", false).await; + setup + .assert_clients(&admin_user, &admin_password, false) + .await; + srv.login(&admin_user, &admin_password).await; +} + +/// The admin API shows and validates the flag: the local realm's is off and +/// a credential-forwarding realm's on until set, and the local realm takes +/// no other config. +#[tokio::test] +async fn the_admin_api_takes_the_flag_on_the_password_realms() { + let setup = Setup::new().await; + let local = setup.realm_id("local").await; + let nexus = setup.realm_id("nexus").await; + assert_eq!( + setup.realm_config(local).await["accept_client_passwords"], + false + ); + assert_eq!( + setup.realm_config(nexus).await["accept_client_passwords"], + true + ); + + for body in [ + json!({"config": {"accept_client_passwords": "yes"}}), + json!({"config": {"accept_client_password": true}}), + ] { + let resp = admin( + &setup.srv, + reqwest::Method::PATCH, + &format!("/admin/realms/{local}"), + ) + .json(&body) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 400, "{body}"); + } + assert_eq!( + setup.realm_config(local).await["accept_client_passwords"], + false + ); +} + +/// The upgrade (`0006_realm_client_passwords.sql`) gives the local realm the +/// install-wide `allow_password_basic` it had, and leaves a forwarding +/// realm's config as it was (it accepts client passwords by default). +#[tokio::test] +async fn the_upgrade_carries_allow_password_basic_onto_the_local_realm() { + const MIGRATION: &str = include_str!("../../../migrations/0006_realm_client_passwords.sql"); + let pool = common::fresh_pool().await; + let forwarding = json!({"url": "https://nexus.example/status", "method": "GET"}); + sqlx::query( + "INSERT INTO auth_realm (kind, name, priority, config) \ + VALUES ('credential_forwarding', 'nexus', 50, $1)", + ) + .bind(&forwarding) + .execute(&pool) + .await + .unwrap(); + for (stored, accepts) in [ + ( + Some(r#"{"anonymous_mode":"off","allow_password_basic":true}"#), + true, + ), + ( + Some(r#"{"anonymous_mode":"off","allow_password_basic":false}"#), + false, + ), + (Some(r#"{"anonymous_mode":"all"}"#), false), + (None, false), + ] { + sqlx::query("UPDATE auth_realm SET config = '{}' WHERE kind = 'local'") + .execute(&pool) + .await + .unwrap(); + sqlx::query("DELETE FROM forge_setting WHERE key = 'auth_settings'") + .execute(&pool) + .await + .unwrap(); + if let Some(stored) = stored { + forge_db::authz_dao::set_setting(&pool, "auth_settings", stored) + .await + .unwrap(); + } + sqlx::raw_sql(MIGRATION).execute(&pool).await.unwrap(); + let (local, nexus): (Value, Value) = sqlx::query_as( + "SELECT (SELECT config FROM auth_realm WHERE kind = 'local'), \ + (SELECT config FROM auth_realm WHERE name = 'nexus')", + ) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!( + local, + json!({"accept_client_passwords": accepts}), + "{stored:?}" + ); + assert_eq!(nexus, forwarding, "{stored:?}"); + // The settings document still loads. + forge_server::auth::load_auth_settings(&pool).await.unwrap(); + } +} diff --git a/crates/server/tests/common/mod.rs b/crates/server/tests/common/mod.rs index 682247c1..14ad6e6a 100644 --- a/crates/server/tests/common/mod.rs +++ b/crates/server/tests/common/mod.rs @@ -119,30 +119,27 @@ pub async fn pool() -> PgPool { let _guard = INIT_LOCK.lock().await; if MIGRATED.get().is_none() { forge_db::migrate(&pool).await.expect("migrate"); - seed_auth_settings(&pool).await; + seed_client_passwords(&pool).await; MIGRATED.set(()).ok(); } } pool } -/// The auth settings every test database starts with: password Basic on -/// (tests sign in over Basic), everything else at its default (anonymous -/// access off). A test that needs others changes them on a database of its -/// own ([`fresh_pool`], [`TestServer::set_auth_settings`]): the shared one -/// serves every test of the binary at once. -async fn seed_auth_settings(pool: &PgPool) { - let settings = forge_server::auth::AuthSettings { - allow_password_basic: true, - ..Default::default() - }; - forge_db::authz_dao::set_setting( - pool, - forge_server::auth::AUTH_SETTINGS_KEY, - &serde_json::to_string(&settings).expect("settings serialize"), +/// Every test database starts with its local realm accepting passwords from +/// clients (tests sign in over Basic), and the auth settings at their +/// defaults (anonymous access off). A test that needs others changes them on +/// a database of its own ([`fresh_pool`], [`TestServer::set_auth_settings`], +/// [`TestServer::set_local_client_passwords`]): the shared one serves every +/// test of the binary at once. +async fn seed_client_passwords(pool: &PgPool) { + sqlx::query( + "UPDATE auth_realm SET config = config || '{\"accept_client_passwords\": true}' \ + WHERE kind = 'local'", ) + .execute(pool) .await - .expect("seed auth settings"); + .expect("seed the local realm's client passwords"); } /// A migrated pool over a fresh, uniquely named database on the same Postgres @@ -167,7 +164,7 @@ pub async fn fresh_pool() -> PgPool { .await .expect("connect fresh database"); forge_db::migrate(&pool).await.expect("migrate"); - seed_auth_settings(&pool).await; + seed_client_passwords(&pool).await; pool } @@ -603,6 +600,22 @@ impl TestServer { .clone() } + /// Have the local realm accept passwords from clients, or not + /// (`accept_client_passwords`), as of the next request. For a server on a + /// database of its own ([`fresh_pool`]). + pub async fn set_local_client_passwords(&self, accept: bool) { + sqlx::query( + "UPDATE auth_realm \ + SET config = config || jsonb_build_object('accept_client_passwords', $1::boolean) \ + WHERE kind = 'local'", + ) + .bind(accept) + .execute(&self.pool) + .await + .expect("set the local realm's client passwords"); + self.state.realms.invalidate().await; + } + /// Sign `username` in through `/auth/login` (password realms). pub async fn login(&self, username: &str, password: &str) -> BrowserSession { let resp = self diff --git a/crates/server/tests/service_accounts.rs b/crates/server/tests/service_accounts.rs index c052a1bf..0d818ba1 100644 --- a/crates/server/tests/service_accounts.rs +++ b/crates/server/tests/service_accounts.rs @@ -382,8 +382,7 @@ async fn a_patch_applies_all_of_its_fields_or_none() { #[tokio::test] async fn nobody_signs_in_to_a_service_account_but_its_tokens_work() { let srv = server().await; - srv.set_auth_settings(json!({"allow_password_basic": true})) - .await; + srv.set_local_client_passwords(true).await; let name = format!("ci{}@example.com", common::unique()); let id = create_service_account(&srv, &name, None).await["id"] .as_i64() diff --git a/docs/changes/2.2.0/647.feature.md b/docs/changes/2.2.0/647.feature.md new file mode 100644 index 00000000..80990e53 --- /dev/null +++ b/docs/changes/2.2.0/647.feature.md @@ -0,0 +1,12 @@ +Whether a client may sign in with a password over HTTP Basic (a NuGet, raw +or Helm feed, `docker login`, `npm login --auth-type=legacy`) is set per +realm: **Accept passwords from clients (HTTP Basic)** on the local and +credential-forwarding realms under **Admin → Authentication** +(`accept_client_passwords`). A credential-forwarding realm accepts its +users' passwords by default, as a Nexus that checks them does; the local +realm accepts none by default, and its users sign their clients in with +tokens. An enabled credential-forwarding realm no longer opens the local +realm's passwords to every client as well, and a password a client sends +for a local account while the local realm is closed is refused like a wrong +one and never forwarded upstream. The browser login takes every realm's +passwords, and an API token works as before. diff --git a/docs/changes/2.2.0/647.upgrade.md b/docs/changes/2.2.0/647.upgrade.md new file mode 100644 index 00000000..71c662d0 --- /dev/null +++ b/docs/changes/2.2.0/647.upgrade.md @@ -0,0 +1,11 @@ +The **Accept passwords over HTTP Basic on the feeds** setting +(`allow_password_basic` in `/admin/settings/auth`) is gone: the upgrade +copies its value onto the local realm's new **Accept passwords from clients +(HTTP Basic)** (`accept_client_passwords`, **Admin → Authentication**), and +`PUT /admin/settings/auth` refuses the key. Credential-forwarding realms +accept their users' passwords from clients, as before. With +`allow_password_basic` off, the local realm's passwords were accepted over +Basic anyway while a credential-forwarding realm was enabled; now they are +not, and local users sign their clients in with tokens unless the local +realm is set to accept passwords. Scripts that set `allow_password_basic` +set the realm instead (`PATCH /admin/realms/{id}`). diff --git a/docs/deployment.md b/docs/deployment.md index 0d069373..2dd36125 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -582,7 +582,6 @@ wrong. | `anonymous_mode` | `off` | `off` or `all`: who is served without credentials. A repository is read anonymously only when it also allows it (**Allow anonymous read**, or a grant of the `anonymous` role). Under `all`, signed-in users also read what anonymous callers may. To let a network read without credentials, have the load balancer name a service account through a [trusted-header realm](#trusted-header-realms). | | `login_mode` | `identifier_first` | The login screen: the username first (then a password prompt or the identity provider whose login domains match), or `combined` (username and password, plus the OIDC button). | | `remember_accounts` | `true` | The login screen offers the accounts signed in on the device before, so signing in again is one click (see below). `false` for kiosks and shared computers. | -| `allow_password_basic` | `false` | Accept a user's password over HTTP Basic on the feeds (see below). | | `session_idle_ttl_seconds` | `28800` (8 h) | A browser session ends this long after its last use. | | `session_absolute_ttl_seconds` | `604800` (7 d) | A browser session ends this long after sign-in. New lifetimes apply to sessions signed in afterwards; an open session's idle window moves to the new idle timeout on its next use. | | `login_max_failures` | `10` | Failed passwords per client network and username before the lockout; `0` turns it off. Each instance counts its own. | @@ -612,17 +611,34 @@ username. With `remember_accounts` off (**Admin → Settings → Remember accoun on the login screen**), the login screen keeps none and forgets those a browser kept, the next time it is opened there. -#### Password Basic auth - -`allow_password_basic` is off by default: the web UI signs in through its -login form (not affected by this setting), and NuGet clients and scripts -authenticate with API tokens (a token in the Basic password field is always -accepted). An enabled credential-forwarding realm turns the Basic password -path on by itself, because its users have no tokens until they first sign in -— and that path runs the whole password chain, so **local** users' passwords -are then accepted over Basic too, whatever this setting says. Turn it on only -if a client must send a local user's password over Basic without such a -realm. +#### Passwords from clients + +Whether a client may send a user's password over HTTP Basic (a NuGet, raw or +Helm feed, `docker login` through the registry's token service, `npm login +--auth-type=legacy`) is set **per realm**: **Accept passwords from clients +(HTTP Basic)** on the realm's page under **Admin → Authentication** +(`accept_client_passwords` in the config of the `local` realm or a +`credential_forwarding` one, `PATCH /admin/realms/{id}`). + +- **The local realm: off by default.** Local users sign their clients in + with tokens: an API token from their **Tokens** page (also the password of + `docker login`), or `npm login --auth-type=web`. Turn it on only if a + client must send a local user's password. +- **A credential-forwarding realm: on by default,** as against a Nexus that + checks the passwords: the realm's users have no token until they first + sign in. Turned off, they sign in in the browser and use tokens. +- **The browser login takes every realm's passwords,** whatever the flag: + it is how a user gets a token. An API token sent as the Basic password + always works. +- **A closed local realm still guards its names.** A password a client + sends for a local account is refused with the 401 of a wrong password (the + lockout counts it), and is never forwarded to a credential-forwarding + realm behind it. + +Before 2.2 this was one setting, `allow_password_basic` under **Admin → +Settings**, and an enabled credential-forwarding realm turned it on for every +realm. The upgrade copies its value onto the local realm, and `PUT +/admin/settings/auth` refuses the key with where it went. A password sent over Basic is a **feed** credential only: the admin API (`/admin/*`) and token minting refuse it with 403, even for an administrator, diff --git a/docs/docker.md b/docs/docker.md index 1ddc28b7..9fb7e1f8 100644 --- a/docs/docker.md +++ b/docs/docker.md @@ -64,10 +64,11 @@ docker push forge.example.com/docker-hosted/team/app:1.3 - **API tokens** (created on the Tokens page, or `POST /admin/me/tokens`) are the password to use. A token's repository scope applies. -- **Passwords** work where Basic passwords are allowed at all: the - `allow_password_basic` setting (Admin → Settings), or an enabled - credential-forwarding realm (so - users of an existing Nexus or LDAP sign in with their upstream password). +- **Passwords** work for the users of a realm that accepts passwords from + clients ([Admin → Authentication](deployment.md#passwords-from-clients)): + a credential-forwarding realm by default (so users of an existing Nexus or + LDAP sign in with their upstream password), the local realm once it is set + to. The password goes to the token endpoint only; the registry itself sees the short-lived token. - **CI workloads** with a workload-identity realm pass their OIDC token, or diff --git a/docs/npm.md b/docs/npm.md index c23ceac1..16736e72 100644 --- a/docs/npm.md +++ b/docs/npm.md @@ -104,9 +104,11 @@ URL to open elsewhere. A login waits for its approval for 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 -login does (local passwords, credential forwarding, the lockout after -failed attempts) and mints a token. No account is ever created by a login. +--auth-type=legacy` send them to Forge, which checks them through the realms +that accept passwords from clients ([Admin → +Authentication](deployment.md#passwords-from-clients): a credential-forwarding +realm by default, the local realm once it is set to), with the lockout after +failed attempts, and mints a token. No account is ever created by a login. npm's legacy login refuses a username containing an `@`, which every Forge account has (an e-mail address): use the browser login, Yarn's, or a token from the web UI. diff --git a/migrations/0006_realm_client_passwords.sql b/migrations/0006_realm_client_passwords.sql new file mode 100644 index 00000000..9fe24835 --- /dev/null +++ b/migrations/0006_realm_client_passwords.sql @@ -0,0 +1,26 @@ +-- --------------------------------------------------------------------------- +-- Passwords from clients are accepted per realm (#647) +-- +-- Whether a client (a feed's HTTP Basic, `docker login`, `npm login +-- --auth-type=legacy`) may sign in with a password is the realm's own +-- `accept_client_passwords`, on the `local` and `credential_forwarding` +-- realms. It was one install-wide setting, `allow_password_basic` in the +-- `auth_settings` document (off by default), which an enabled +-- credential-forwarding realm turned on for every realm. +-- +-- The local realm takes the old setting's value. A credential-forwarding +-- realm accepts its passwords from clients by default, as it did; its config +-- is left as it is, so the fingerprint of a realm seeded from +-- `[auth.upstream_basic]` still matches. The old key stays in the settings +-- document, which reads past it, until the next save there drops it. +-- --------------------------------------------------------------------------- +UPDATE auth_realm +SET config = config || jsonb_build_object( + 'accept_client_passwords', + coalesce( + (SELECT (value::jsonb ->> 'allow_password_basic')::boolean + FROM forge_setting + WHERE key = 'auth_settings' + AND jsonb_typeof(value::jsonb -> 'allow_password_basic') = 'boolean'), + false)) +WHERE kind = 'local'; diff --git a/web/e2e/harness.ts b/web/e2e/harness.ts index b83eb270..525a0abc 100644 --- a/web/e2e/harness.ts +++ b/web/e2e/harness.ts @@ -190,17 +190,30 @@ async function signInAdmin(): Promise { } /** - * Open anonymous reads (so the Browse screen works without a token) and - * password Basic (so specs can use the bootstrap admin's password on the - * feeds): auth settings, changed through the admin API. + * Open anonymous reads (so the Browse screen works without a token), an auth + * setting, and have the local realm accept passwords over Basic (so specs can + * use the bootstrap admin's password on the feeds): both through the admin API. */ async function openAccess({ cookie, csrf }: AdminSession): Promise { + const headers = { cookie, 'x-csrf-token': csrf, 'content-type': 'application/json' } const res = await fetch(`${BASE_URL}/admin/settings/auth`, { method: 'PUT', - headers: { cookie, 'x-csrf-token': csrf, 'content-type': 'application/json' }, - body: JSON.stringify({ anonymous_mode: 'all', allow_password_basic: true }), + headers, + body: JSON.stringify({ anonymous_mode: 'all' }), }) if (!res.ok) throw new Error(`PUT /admin/settings/auth -> ${res.status}`) + const realms = await fetch(`${BASE_URL}/admin/realms`, { headers }) + if (!realms.ok) throw new Error(`GET /admin/realms -> ${realms.status}`) + const local = ((await realms.json()) as { id: number; kind: string }[]).find( + (realm) => realm.kind === 'local', + ) + if (!local) throw new Error('no local realm') + const patched = await fetch(`${BASE_URL}/admin/realms/${local.id}`, { + method: 'PATCH', + headers, + body: JSON.stringify({ config: { accept_client_passwords: true } }), + }) + if (!patched.ok) throw new Error(`PATCH /admin/realms/${local.id} -> ${patched.status}`) } /** Mint an admin-scoped API token for the bootstrap admin from its session. */ diff --git a/web/src/api/realms.ts b/web/src/api/realms.ts index 9b31df17..2ae5c515 100644 --- a/web/src/api/realms.ts +++ b/web/src/api/realms.ts @@ -84,6 +84,11 @@ export interface CredentialForwardingConfig { capture_password_hash?: boolean /** Stop forwarding: sign in with the kept hashes (needs `capture_password_hash`). */ captured_password_only?: boolean + /** + * Accept the realm's passwords from clients over HTTP Basic (feeds, + * `docker login`, `npm login`). Absent means on. + */ + accept_client_passwords?: boolean positive_ttl_secs?: number negative_ttl_secs?: number /** SECRET — write-only; required on create, omit to keep on update. */ @@ -153,13 +158,22 @@ export interface TrustedHeaderConfig { auto_provision?: boolean } -/** Kind-specific config; `local` has none. */ +/** The built-in `local` realm's config. */ +export interface LocalRealmConfig { + /** + * Accept local accounts' passwords from clients over HTTP Basic (feeds, + * `docker login`, `npm login`). Absent means off. + */ + accept_client_passwords?: boolean +} + +/** Kind-specific config. */ export type RealmConfig = | OidcRealmConfig | CredentialForwardingConfig | WorkloadIdentityConfig | TrustedHeaderConfig - | Record + | LocalRealmConfig /** * What the IdP's federated credential must name for an OIDC realm that diff --git a/web/src/api/settings.ts b/web/src/api/settings.ts index 0ed5d13a..14bc1ed4 100644 --- a/web/src/api/settings.ts +++ b/web/src/api/settings.ts @@ -23,8 +23,6 @@ export interface AuthSettings { login_mode: LoginMode /** The login screen offers the accounts signed in on the device before. */ remember_accounts: boolean - /** Accept a user's password over HTTP Basic on the feeds. */ - allow_password_basic: boolean session_idle_ttl_seconds: number session_absolute_ttl_seconds: number /** `0` turns the lockout off. */ diff --git a/web/src/components/RealmForm.test.ts b/web/src/components/RealmForm.test.ts index 4f9445bb..435e14ac 100644 --- a/web/src/components/RealmForm.test.ts +++ b/web/src/components/RealmForm.test.ts @@ -374,6 +374,45 @@ describe('OIDC federated credential', () => { }) }) +describe('passwords from clients', () => { + const stored = (kind: RealmView['kind'], config: RealmView['config']): RealmView => ({ + id: 1, + kind, + name: kind, + enabled: true, + priority: 1, + config, + created_at: '2026-01-01T00:00:00Z', + updated_at: '2026-01-01T00:00:00Z', + }) + + it('the local realm sends its flag, off until set', () => { + expect(toRealmConfig(seedRealmFormValues(stored('local', {})))).toEqual({ + accept_client_passwords: false, + }) + for (const accept of [true, false]) { + const config = { accept_client_passwords: accept } + expect(toRealmConfig(seedRealmFormValues(stored('local', config)))).toEqual(config) + } + }) + + it('a forwarding realm sends it only when off, its default being on', () => { + const base = { url: 'https://nexus.example.com/status' } + expect(seedRealmFormValues(stored('credential_forwarding', base)).accept_client_passwords).toBe( + true, + ) + const closed = { ...base, accept_client_passwords: false } + expect(toRealmConfig(seedRealmFormValues(stored('credential_forwarding', closed)))).toEqual( + closed, + ) + expect( + toRealmConfig( + seedRealmFormValues(stored('credential_forwarding', { ...base, accept_client_passwords: true })), + ), + ).toEqual(base) + }) +}) + describe('kept password options', () => { const stored = (config: RealmView['config']): RealmView => ({ id: 1, diff --git a/web/src/components/RealmForm.tsx b/web/src/components/RealmForm.tsx index bbaa9584..abcfba8f 100644 --- a/web/src/components/RealmForm.tsx +++ b/web/src/components/RealmForm.tsx @@ -3,6 +3,7 @@ import { useEffect, useRef, useState, type ReactNode } from 'react' import type { ClientAuthentication, CredentialForwardingConfig, + LocalRealmConfig, OidcRealmConfig, RealmConfig, RealmKind, @@ -45,8 +46,8 @@ import { * enter in the IdP's app registration: the redirect URI and, for a * federated credential, the issuer, subject and audience it must name. * - **credential_forwarding** → upstream URL / acceptance criteria / - * write-only key pepper / groups / timeouts and caching TTLs / kept - * password hashes + * write-only key pepper / passwords from clients (on by default) / groups / + * timeouts and caching TTLs / kept password hashes * - **workload_identity** (WIF) → public OIDC issuer / allowed audiences / * optional authorized parties / bounded token lifetime / ordered * claim→role rules (no secrets, no redaction). **Edit as JSON** shows the @@ -57,7 +58,8 @@ import { * write-only secret the proxy proves itself with (a random one can be * generated and copied into the proxy's configuration before saving), * account provisioning; the side column says what the proxy must do - * - **local** → no config; only name + enabled are editable (built-in realm) + * - **local** → name + enabled, and whether clients may send its passwords + * over HTTP Basic (built-in realm; off by default) * * On **create** the kind is chosen from cards (local is built-in and cannot * be created); on **edit** it is fixed and shown read-only, and saving a realm @@ -128,6 +130,8 @@ export interface RealmFormValues { capture_password_hash: boolean /** Credential forwarding: stop forwarding, sign in with the kept hashes. */ captured_password_only: boolean + /** Local and credential forwarding: accept the passwords clients send over Basic. */ + accept_client_passwords: boolean admin_group: string positive_ttl_secs: string negative_ttl_secs: string @@ -202,6 +206,35 @@ export function disableRealmWarning(kind: RealmKind, name: string): string | nul } } +/** Whether a password realm accepts its passwords from clients over Basic. */ +function ClientPasswordsPanel({ + checked, + onChange, + description, +}: { + checked: boolean + onChange: (checked: boolean) => void + description: string +}) { + return ( + +

+ +
+ + ) +} + /** A string list as one entry per line (entries may contain commas). */ function joinLines(list?: string[]): string { return (list ?? []).join('\n') @@ -250,6 +283,7 @@ export function seedRealmFormValues(existing?: RealmView): RealmFormValues { const header = ( existing?.kind === 'trusted_header' ? existing.config : {} ) as TrustedHeaderConfig + const local = (existing?.kind === 'local' ? existing.config : {}) as LocalRealmConfig return { kind: existing?.kind ?? 'oidc', name: existing?.name ?? '', @@ -296,6 +330,11 @@ export function seedRealmFormValues(existing?: RealmView): RealmFormValues { allow_http: fwd.allow_http ?? false, capture_password_hash: fwd.capture_password_hash ?? false, captured_password_only: fwd.captured_password_only ?? false, + // The local realm's is off until set, a forwarding realm's on. + accept_client_passwords: + existing?.kind === 'local' + ? (local.accept_client_passwords ?? false) + : (fwd.accept_client_passwords ?? true), admin_group: fwd.admin_group ?? '', positive_ttl_secs: fwd.positive_ttl_secs != null ? String(fwd.positive_ttl_secs) : '', negative_ttl_secs: fwd.negative_ttl_secs != null ? String(fwd.negative_ttl_secs) : '', @@ -903,7 +942,7 @@ export function RealmForm({ title="General" description={ isLocal - ? 'The local realm is built in (argon2 passwords); it has no configuration beyond its name and enabled flag.' + ? 'The local realm is built in (argon2 passwords). Besides its name and enabled flag, it sets only whether clients may send its passwords.' : undefined } > @@ -943,6 +982,14 @@ export function RealmForm({ /> + {isLocal && ( + set('accept_client_passwords', c)} + description="Lets a client (NuGet, raw and Helm feeds, docker login, npm login --auth-type=legacy) send a local user's password over HTTP Basic. Off, local users sign their clients in with tokens: an API token from their tokens page, or npm login --auth-type=web. The browser login takes their passwords either way, and a password a client sends while this is off is refused, never passed on to a credential-forwarding realm." + /> + )} + {isOidc && ( <> + set('accept_client_passwords', c)} + description="Lets the realm's users sign a client (NuGet, raw and Helm feeds, docker login, npm login --auth-type=legacy) in with the password the upstream checks, as they did against a Nexus. Off, they sign their clients in with tokens, which the browser login gets them; the browser login takes their passwords either way." + /> + toast({ tone: 'ok', title: 'Realm saved' }), diff --git a/web/src/routes/admin/realms/realms.test.tsx b/web/src/routes/admin/realms/realms.test.tsx index c8389c10..c343f7db 100644 --- a/web/src/routes/admin/realms/realms.test.tsx +++ b/web/src/routes/admin/realms/realms.test.tsx @@ -1286,10 +1286,64 @@ describe('admin realms edit', { timeout: 20_000 }, () => { await u.click(screen.getByRole('button', { name: 'Disable and save' })) await waitFor(() => - expect(patches).toEqual([{ name: 'local', enabled: false }]), + expect(patches).toEqual([ + { name: 'local', enabled: false, config: { accept_client_passwords: false } }, + ]), ) }) + it('the local realm accepts no client passwords until checked', async () => { + const local = realm({ id: 1, kind: 'local', name: 'local' }) + const patches: unknown[] = [] + mockApi({ + realms: [local], + onPatch: (_id, body) => { + patches.push(body) + return json(local) + }, + }) + + const u = userEvent.setup() + await renderAt('/manage/realms/1') + + const accept = await screen.findByRole('checkbox', { + name: 'Accept passwords from clients (HTTP Basic)', + }) + expect(accept).not.toBeChecked() + // Users of a closed realm sign their clients in with tokens. + expect(screen.getByText(/local users sign their clients in with tokens/)).toBeInTheDocument() + await u.click(accept) + await u.click(screen.getByRole('button', { name: 'Save changes' })) + await waitFor(() => + expect(patches).toEqual([ + { name: 'local', enabled: true, config: { accept_client_passwords: true } }, + ]), + ) + }) + + it('a forwarding realm keeps its client passwords until unchecked', async () => { + let patched: { config?: Record } | null = null + mockApi({ + realms: [nexus], + onPatch: (_id, body) => { + patched = body as typeof patched + return json(nexus) + }, + }) + + const u = userEvent.setup() + await renderAt('/manage/realms/3') + const accept = await screen.findByRole('checkbox', { + name: 'Accept passwords from clients (HTTP Basic)', + }) + // A realm stored before the setting existed accepts them, and a new one. + expect(accept).toBeChecked() + await u.click(accept) + await u.click(screen.getByRole('button', { name: 'Save changes' })) + await waitFor(() => expect(patched).not.toBeNull()) + expect(patched!.config).toMatchObject({ accept_client_passwords: false }) + }) + it('asks before saving an OIDC realm disabled', async () => { let patched: unknown = null mockApi({ diff --git a/web/src/routes/admin/settings/Settings.tsx b/web/src/routes/admin/settings/Settings.tsx index a051010b..0306b728 100644 --- a/web/src/routes/admin/settings/Settings.tsx +++ b/web/src/routes/admin/settings/Settings.tsx @@ -7,7 +7,6 @@ import type { AuthSettingsUpdate, LoginMode, } from '~/api/settings' -import { useRealms } from '~/queries/useRealms' import { useRepositories } from '~/queries/useRepositories' import { useAuthSettings, useUpdateAuthSettings } from '~/queries/useSettings' import { @@ -214,7 +213,6 @@ function AnonymousRepositories() { interface SignInDraft { login_mode: LoginMode remember_accounts: boolean - allow_password_basic: boolean session_idle_ttl_seconds: string session_absolute_ttl_seconds: string login_max_failures: string @@ -243,7 +241,6 @@ function validateSignIn( body: { login_mode: draft.login_mode, remember_accounts: draft.remember_accounts, - allow_password_basic: draft.allow_password_basic, session_idle_ttl_seconds: idle!, session_absolute_ttl_seconds: absolute!, login_max_failures: failures!, @@ -259,16 +256,15 @@ function durationHint(raw: string): string | null { } /** - * The login screen and its remembered accounts, password Basic, session - * lifetimes and the lockout. + * The login screen and its remembered accounts, session lifetimes and the + * lockout. (Passwords from clients are accepted per realm, under + * Authentication.) */ function SignInForm({ settings }: { settings: AuthSettings }) { const update = useUpdateAuthSettings() - const realms = useRealms() const [draft, setDraft] = useState({ login_mode: settings.login_mode, remember_accounts: settings.remember_accounts, - allow_password_basic: settings.allow_password_basic, session_idle_ttl_seconds: String(settings.session_idle_ttl_seconds), session_absolute_ttl_seconds: String(settings.session_absolute_ttl_seconds), login_max_failures: String(settings.login_max_failures), @@ -291,9 +287,6 @@ function SignInForm({ settings }: { settings: AuthSettings }) { update.mutate(checked.body) } - const forwarding = (realms.data ?? []).filter( - (r) => r.enabled && r.kind === 'credential_forwarding', - ) const serverError = update.isError ? update.error.body || update.error.message : null return ( @@ -334,24 +327,15 @@ function SignInForm({ settings }: { settings: AuthSettings }) { description="After a sign-in, the browser keeps the account (its username, name and how it signs in, never a password or a session) and the login screen offers it next time, so signing in again is one click. Turn it off for kiosks and shared computers: their login screens then forget the accounts they kept." /> - set('allow_password_basic', checked)} - description={ - <> - Lets a NuGet client or script send a user's password instead of - an API token, which always works. The admin API never accepts a - password over Basic. - {forwarding.length > 0 && - ` The enabled credential-forwarding realm${forwarding.length === 1 ? '' : 's'} ${forwarding - .map((r) => r.name) - .join(', ')} accept${forwarding.length === 1 ? 's' : ''} passwords there whatever this says.`} - - } - /> +

+ Whether a client (a NuGet feed, docker login,{' '} + npm login) may send a user's password over HTTP Basic is + set per realm, under{' '} + + Authentication + + . An API token always works there. +

{ it('shows the stored settings and the repositories allowing anonymous reads', async () => { mockApi({ - settings: { ...stored, login_mode: 'combined', allow_password_basic: true }, + settings: { ...stored, login_mode: 'combined' }, repositories: [ repo({ id: 1, name: 'public-feed', anonymous_read: true }), repo({ id: 2, name: 'private-feed' }), @@ -163,18 +162,13 @@ describe('admin settings', () => { expect(within(signIn).getByRole('combobox', { name: 'Login screen' })).toHaveValue( 'combined', ) - expect( - within(signIn).getByRole('checkbox', { - name: 'Accept passwords over HTTP Basic on the feeds', - }), - ).toBeChecked() + // Passwords from clients are a realm's setting, not one here. + expect(within(signIn).queryByRole('checkbox', { name: /HTTP Basic/ })).toBeNull() + expect(within(signIn).getByText(/over HTTP Basic is set per realm/)).toBeInTheDocument() expect( within(signIn).getByRole('textbox', { name: 'Session idle timeout (seconds)' }), ).toHaveValue('28800') expect(within(signIn).getByText(/\(= 8 hours\)/)).toBeInTheDocument() - expect( - await within(signIn).findByText(/credential-forwarding realm nexus accepts passwords/), - ).toBeInTheDocument() }) it('saving anonymous access PUTs its own fields only', async () => { @@ -219,11 +213,6 @@ describe('admin settings', () => { within(signIn).getByRole('combobox', { name: 'Login screen' }), 'combined', ) - await u.click( - within(signIn).getByRole('checkbox', { - name: 'Accept passwords over HTTP Basic on the feeds', - }), - ) const remember = within(signIn).getByRole('checkbox', { name: 'Remember accounts on the login screen', }) @@ -235,7 +224,6 @@ describe('admin settings', () => { { login_mode: 'combined', remember_accounts: false, - allow_password_basic: true, session_idle_ttl_seconds: 600, session_absolute_ttl_seconds: 604800, login_max_failures: 10, diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 5ba47b03..cedba7f5 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -553,15 +553,19 @@ fn seed() -> Result<(), String> { ]) }; - // A dev stack reads anonymously and takes passwords over Basic (so - // `curl -u admin@example.com:changeme` works on the feeds). These are - // auth settings now, set in the web UI (Admin → Settings) rather than in - // forge.toml. - println!("Opening anonymous reads and password Basic (Admin → Settings)…"); + // A dev stack reads anonymously (an auth setting, Admin → Settings) and + // its local realm takes passwords over Basic (Admin → Authentication), so + // `curl -u admin@example.com:changeme` works on the feeds. + println!("Opening anonymous reads (Admin → Settings)…"); + session("PUT", "/admin/settings/auth", r#"{"anonymous_mode":"all"}"#).output_string()?; + println!("Accepting the local realm's passwords over Basic (Admin → Authentication)…"); + let realms = session("GET", "/admin/realms", "").output_string()?; + let local = local_realm_id(&realms) + .ok_or_else(|| "could not find the local realm in /admin/realms".to_string())?; session( - "PUT", - "/admin/settings/auth", - r#"{"anonymous_mode":"all","allow_password_basic":true}"#, + "PATCH", + &format!("/admin/realms/{local}"), + r#"{"config":{"accept_client_passwords":true}}"#, ) .output_string()?; @@ -612,6 +616,15 @@ fn seed() -> Result<(), String> { Ok(()) } +/// The id of the `local` realm in a `GET /admin/realms` answer, whose +/// objects start with `"id":,"kind":""` (the runner has no JSON +/// dependency). +fn local_realm_id(json: &str) -> Option { + let kind = json.find(r#""kind":"local""#)?; + let id = json[..kind].rfind(r#""id":"#)? + r#""id":"#.len(); + json[id..kind].trim_end_matches(',').parse().ok() +} + /// Escape `"` and `\\` for a JSON string literal (the runner has no JSON /// dependency). fn json_escape(raw: &str) -> String { @@ -819,3 +832,15 @@ fn compose_app(sub: &[&str]) -> Cmd { ]) .args(sub) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn local_realm_id_reads_the_local_realm_of_a_realm_list() { + let realms = r#"[{"id":7,"kind":"oidc","name":"entra"},{"id":12,"kind":"local","name":"local","config":{}}]"#; + assert_eq!(local_realm_id(realms), Some(12)); + assert_eq!(local_realm_id(r#"[{"id":3,"kind":"oidc"}]"#), None); + } +} From 8c8f2c3a835730eab75c452b51b50920ab704792 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 09:00:06 +0000 Subject: [PATCH 2/3] Realm configs: name the client-password flag accepts_clients in Rust CodeQL's cleartext-logging query took the realm flag for a secret by its name: the tuple holding the local provider's position and `accept_client_passwords` was tainted, and the position reached `Vec::insert`, whose panic message goes to stderr. The flag is a yes or no, not a password. Its Rust field is `accepts_clients` now, still `accept_client_passwords` in the realm's config. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012e6DJXRhLYxFjzrWiwKFKc --- crates/server/src/auth/realms.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/server/src/auth/realms.rs b/crates/server/src/auth/realms.rs index 3126a8d7..87c57a58 100644 --- a/crates/server/src/auth/realms.rs +++ b/crates/server/src/auth/realms.rs @@ -146,9 +146,9 @@ pub struct CredForwardRealmConfig { /// `docker login`, `npm login --auth-type=legacy`), as a Nexus that /// checks them does. On by default; off, the realm's users sign their /// clients in with tokens, and only the browser login takes their - /// passwords. - #[serde(default = "default_true")] - pub accept_client_passwords: bool, + /// passwords. (`accept_client_passwords` in the config.) + #[serde(default = "default_true", rename = "accept_client_passwords")] + pub accepts_clients: bool, /// How long a *good* credential is trusted without re-forwarding (seconds). #[serde(default = "default_positive_ttl")] pub positive_ttl_secs: u64, @@ -170,8 +170,9 @@ pub struct LocalRealmConfig { /// `npm login --auth-type=web`), and only the browser login takes their /// passwords. A password a client sends for a local account is then /// refused, never forwarded to a credential-forwarding realm. - #[serde(default)] - pub accept_client_passwords: bool, + /// (`accept_client_passwords` in the config.) + #[serde(default, rename = "accept_client_passwords")] + pub accepts_clients: bool, } impl LocalRealmConfig { @@ -882,7 +883,7 @@ impl RealmAuth { KIND_LOCAL => { local_at = local_at.or(Some(( forwarding.len(), - LocalRealmConfig::of(realm, &config).accept_client_passwords, + LocalRealmConfig::of(realm, &config).accepts_clients, ))) } KIND_CRED_FORWARD => match self.build_cred_forward(realm, &config) { @@ -1064,7 +1065,7 @@ impl RealmAuth { // The request's own timeout normally fires first (and is logged as // such); the cache's deadline is the backstop its lease is sized by. .with_forward_timeout(request_timeout + Duration::from_secs(1)); - let accepts_clients = cfg.accept_client_passwords; + let accepts_clients = cfg.accepts_clients; if cfg.captured_password_only { // The upstream is no longer asked; the accounts' kept hashes // decide, through the same (re-keyed) verdict cache. @@ -1329,7 +1330,7 @@ pub async fn seed_auth_realms( auto_provision: None, capture_password_hash: false, captured_password_only: false, - accept_client_passwords: true, + accepts_clients: true, positive_ttl_secs: ub.cache.positive_ttl_secs, negative_ttl_secs: ub.cache.negative_ttl_secs, key_pepper: ub.cache.key_pepper.clone(), From a0dcc2c8532cb3e36e2ce85931086725dc85c9fa Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 09:07:07 +0000 Subject: [PATCH 3/3] npm-e2e: let the local realm accept client passwords before Yarn's login Yarn's legacy login sends the admin's local password, and the local realm now takes passwords from clients only once it accepts them. The step turns that on through the realm's config, as the Rust and web e2e harnesses do. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012e6DJXRhLYxFjzrWiwKFKc --- .github/scripts/npm-e2e.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/scripts/npm-e2e.sh b/.github/scripts/npm-e2e.sh index 50b65b2f..c0b982db 100755 --- a/.github/scripts/npm-e2e.sh +++ b/.github/scripts/npm-e2e.sh @@ -298,6 +298,11 @@ done [ "$(npm whoami --registry "$(reg npm-hosted)")" = "$admin_user" ] || fail "npm whoami (hosted)" step "legacy login with Yarn, and logout" +# Yarn sends the admin's password: the local realm takes passwords from +# clients only once it accepts them. +local_realm=$(admin GET /admin/realms | jq -r '.[] | select(.kind == "local") | .id') +[ -n "$local_realm" ] || fail "no local realm" +admin PATCH "/admin/realms/$local_realm" '{"config":{"accept_client_passwords":true}}' >/dev/null yarn_js="${YARN_BERRY:-}" if [ -z "$yarn_js" ]; then mkdir -p "$work/yarn"