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" 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..87c57a58 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. (`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, @@ -154,6 +161,31 @@ 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. + /// (`accept_client_passwords` in the config.) + #[serde(default, rename = "accept_client_passwords")] + pub accepts_clients: 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 +400,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 +616,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 +677,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 +856,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 +880,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).accepts_clients, + ))) + } 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 +944,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 +980,7 @@ impl RealmAuth { built_at: Instant::now(), realms, chain, - forwards_credentials: has_cred_forward, + client_chain, oidc, workload, workload_configured_issuers, @@ -949,12 +989,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 +1065,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.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. - 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 +1095,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 +1194,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 +1330,7 @@ pub async fn seed_auth_realms( auto_provision: None, capture_password_hash: false, captured_password_only: false, + 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(), @@ -1646,11 +1726,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); + } +}