Client passwords: a closed local realm's refusal no longer counts towards the login lockout

crates/core/src/auth/chain.rs

@@ -18,6 +18,10 @@
 //!    user's typed password would leak upstream. A rejection
 //!    from a non-local provider is remembered but does **not** stop the chain
 //!    (a username may exist in a later provider). On its own it never grants.
+//!    [`AuthOutcome::NotAccepted`] (a local account whose realm takes no
+//!    password on this chain) is terminal the same way, and folds into
+//!    [`ChainOutcome::NotAccepted`]: denied like a rejection, but not counted
+//!    towards the sign-in lockout.
 //! 4. [`AuthOutcome::Unavailable`] is remembered but does **not** grant.
 //!    **Fail closed:** an unreachable upstream can never open a hole — if no
 //!    provider authenticates and an `Unavailable` was seen, the chain reports
@@ -61,6 +65,12 @@ pub enum ChainOutcome {
     /// No provider authenticated and at least one definitively **rejected** the
     /// credential. The caller returns 401 (bad credentials), never anonymous.
     Rejected,
+    /// A local provider owns the username but takes no password for it on
+    /// this chain ([`AuthOutcome::NotAccepted`]): the chain stopped there. The
+    /// caller denies exactly as for [`ChainOutcome::Rejected`] (same status,
+    /// body and timing), but no password was checked, so the attempt does not
+    /// count towards the sign-in lockout.
+    NotAccepted,
     /// No provider authenticated and at least one was **unavailable** (and none
     /// rejected). Fail-closed: the caller denies. Surfaced distinctly from
     /// [`ChainOutcome::Rejected`] so the caller can log/metric upstream health,
@@ -145,14 +155,22 @@ impl Chain {
                 AuthOutcome::Rejected if provider.source() == AuthSource::Local => {
                     return (ChainOutcome::Rejected, source);
                 }
+                // A local account whose realm takes no password here: final
+                // like a wrong password (never forwarded), but nothing was
+                // checked.
+                AuthOutcome::NotAccepted if provider.source() == AuthSource::Local => {
+                    return (ChainOutcome::NotAccepted, source);
+                }
                 // The local store could not decide (DB error, argon2 overload):
                 // it may still own this username, so do not forward (rule 4).
                 AuthOutcome::Unavailable if provider.source() == AuthSource::Local => {
                     return (ChainOutcome::Unavailable, source);
                 }
                 // The database failed: no later provider can decide either.
                 AuthOutcome::Failed => return (ChainOutcome::Failed, source),
-                AuthOutcome::Rejected => saw_rejected = saw_rejected.or(source),
+                AuthOutcome::Rejected | AuthOutcome::NotAccepted => {
+                    saw_rejected = saw_rejected.or(source)
+                }
                 AuthOutcome::Unavailable => saw_unavailable = saw_unavailable.or(source),
                 AuthOutcome::Skip => {}
             }
@@ -184,7 +202,9 @@ fn record_sign_in(outcome: &ChainOutcome, decided_by: Option<AuthSource>) {
     };
     let outcome = match outcome {
         ChainOutcome::Authenticated { .. } => SignInOutcome::Success,
-        ChainOutcome::Rejected | ChainOutcome::NoCredentials => SignInOutcome::Rejected,
+        ChainOutcome::Rejected | ChainOutcome::NotAccepted | ChainOutcome::NoCredentials => {
+            SignInOutcome::Rejected
+        }
         ChainOutcome::Unavailable => SignInOutcome::Unavailable,
         ChainOutcome::Failed => SignInOutcome::Error,
     };

crates/core/src/auth/local_password.rs

@@ -242,8 +242,9 @@ impl LocalPasswordProvider {
     }
 
     /// 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
+    /// local store owns is refused as `NotAccepted` (after a dummy
+    /// verification, so it answers like a wrong password, but does not count
+    /// towards the sign-in lockout), 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.
@@ -252,6 +253,21 @@ impl LocalPasswordProvider {
         self
     }
 
+    /// How a name the local store owns is refused without checking its
+    /// password: `Rejected`, or `NotAccepted` when this provider checks no
+    /// password at all ([`Self::reject_only`]). Nothing is verified then, so
+    /// the refusal is no guess and does not count towards the sign-in
+    /// lockout: a client still sending an account's password to a realm that
+    /// takes none from clients must not lock its owner out of the browser
+    /// login. It answers the same either way.
+    fn refused(&self) -> AuthOutcome {
+        if self.verifies {
+            AuthOutcome::Rejected
+        } else {
+            AuthOutcome::NotAccepted
+        }
+    }
+
     /// `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 {
@@ -332,9 +348,7 @@ impl AuthProvider for LocalPasswordProvider {
         // and no upstream realm is asked about it.
         let Some(stored) = user.password_hash.clone() else {
             if user.auth_source == "local" || user.is_service_account() {
-                return self
-                    .after_dummy_verify(cred.password, AuthOutcome::Rejected)
-                    .await;
+                return self.after_dummy_verify(cred.password, self.refused()).await;
             }
             return self.skip_foreign(cred.password).await;
         };
@@ -345,9 +359,7 @@ impl AuthProvider for LocalPasswordProvider {
         // (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 || !self.verifies {
-            return self
-                .after_dummy_verify(cred.password, AuthOutcome::Rejected)
-                .await;
+            return self.after_dummy_verify(cred.password, self.refused()).await;
         }
 
         match self.limiter.verify(stored, cred.password.to_string()).await {

crates/core/src/auth/provider.rs

@@ -179,6 +179,13 @@ pub enum AuthOutcome {
     /// wrong local password). Never grants; terminal when the provider's source
     /// is [`AuthSource::Local`], otherwise the chain continues.
     Rejected,
+    /// The provider owns this username but takes no password for it here
+    /// (a local realm that accepts no passwords from clients, see
+    /// `LocalPasswordProvider::reject_only`): refused like a wrong password,
+    /// but nothing was checked, so it is no guess and does not count towards
+    /// the sign-in lockout. Terminal when the provider's source is
+    /// [`AuthSource::Local`], otherwise treated like [`Self::Rejected`].
+    NotAccepted,
     /// The provider can't decide — typically "this username isn't mine".
     /// The chain advances to the next provider.
     Skip,

crates/core/src/auth/token.rs

@@ -489,6 +489,14 @@ impl PrincipalSource for BasicAuthSource {
                     attempt.failed();
                     return SourceOutcome::Invalid;
                 }
+                // A local account whose realm takes no password from clients:
+                // denied like a wrong password, but nothing was checked, so it
+                // does not count towards the lockout (an old client still
+                // sending it must not lock its owner out of the browser login).
+                ChainOutcome::NotAccepted => {
+                    attempt.released();
+                    return SourceOutcome::Invalid;
+                }
                 // A fail-closed outage (or an argon2 overload) also denies, but
                 // is not the caller's fault, so it does not count towards the
                 // lockout.

crates/core/tests/local_password.rs

@@ -174,7 +174,9 @@ 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.
+    // upstream), and only a name the local store does not own goes on. #795:
+    // nothing is checked, so the refusal is `NotAccepted`, not a `Rejected`
+    // guess.
     let pool = common::pool().await;
     let username = insert_local(&pool, "pw", true).await;
     let unknown = format!("nobody-{}@example.com", common::unique());
@@ -185,7 +187,7 @@ async fn a_reject_only_provider_refuses_its_names_and_skips_the_others() {
         for password in ["pw", "a-guess"] {
             assert_eq!(
                 provider.authenticate(&cred(&username, password)).await,
-                AuthOutcome::Rejected,
+                AuthOutcome::NotAccepted,
                 "{password} (forwarding follows: {forwarding_follows})"
             );
         }
@@ -204,6 +206,66 @@ async fn a_reject_only_provider_refuses_its_names_and_skips_the_others() {
     ));
 }
 
+/// #795: a password a reject-only local provider refuses was never checked,
+/// so it is no guess. Over Basic it answers 401 like a wrong password, but it
+/// does not count towards the (network, username) lockout the browser login
+/// shares; a wrong password through a provider that checks still counts.
+#[tokio::test]
+async fn a_reject_only_refusal_is_not_counted_towards_the_lockout() {
+    let pool = common::pool().await;
+    let username = insert_local(&pool, "pw", true).await;
+    let (upstream, upstream_calls) = CountingProvider::new(
+        AuthSource::UpstreamBasic,
+        AuthOutcome::Rejected,
+        Duration::ZERO,
+    );
+    let chain = Arc::new(Chain::new(vec![
+        Box::new(
+            LocalPasswordProvider::new(pool.clone())
+                .forwarding_follows(true)
+                .reject_only(),
+        ),
+        Box::new(upstream),
+    ]));
+    assert_eq!(
+        chain.authenticate(&cred(&username, "pw")).await,
+        ChainOutcome::NotAccepted
+    );
+
+    let throttle = Arc::new(LoginThrottle::new(5, Duration::from_secs(60)));
+    let src =
+        BasicAuthSource::with_chain(pool.clone(), Some(chain)).with_throttle(throttle.clone());
+    for password in ["pw", "a-guess"] {
+        assert_eq!(
+            src.resolve(&basic_header(&username, password), ip()).await,
+            SourceOutcome::Invalid,
+            "{password}"
+        );
+    }
+    assert_eq!(
+        throttle.tracked(),
+        0,
+        "a refusal without a check is no failure"
+    );
+    assert_eq!(
+        upstream_calls.load(Ordering::SeqCst),
+        0,
+        "a local account's password never reaches the upstream"
+    );
+
+    // A provider that checks passwords still counts a wrong one.
+    let checking = Arc::new(Chain::new(vec![
+        Box::new(LocalPasswordProvider::new(pool.clone())) as Box<dyn AuthProvider>,
+    ]));
+    let src =
+        BasicAuthSource::with_chain(pool.clone(), Some(checking)).with_throttle(throttle.clone());
+    assert_eq!(
+        src.resolve(&basic_header(&username, "a-guess"), ip()).await,
+        SourceOutcome::Invalid
+    );
+    assert_eq!(throttle.tracked(), 1, "a wrong password is counted");
+}
+
 #[tokio::test]
 async fn local_provider_rejects_an_invited_account() {
     // #322: an invited local account has no password until its user chooses

crates/server/src/auth/mod.rs

@@ -1189,6 +1189,12 @@ pub(crate) async fn verify_password_login(
             attempt.released();
             return Err(LoginRefusal::Invalid);
         }
+        // A local account whose realm takes no password from clients: no
+        // password was checked, so it is no guess and does not count either.
+        ChainOutcome::NotAccepted => {
+            attempt.released();
+            return Err(LoginRefusal::Invalid);
+        }
         // Forge's own database failed: not a verdict on the credentials.
         ChainOutcome::Failed => {
             attempt.released();

crates/server/tests/client_passwords.rs

@@ -318,6 +318,78 @@ async fn a_forwarding_realm_takes_client_passwords_and_a_closed_local_realm_none
     srv.login(&setup.forwarded, UPSTREAM_PASSWORD).await;
 }
 
+/// #795: a closed local realm's password sent by a client was never checked,
+/// so it is no guess: refused like a wrong one, but not counted towards the
+/// (network, username) lockout the browser login shares. An old client still
+/// sending it does not lock its owner out of the sign-in that gets them a
+/// token. A wrong password through a realm that checks it still counts.
+#[tokio::test]
+async fn a_closed_local_realms_password_from_clients_does_not_lock_the_browser_login() {
+    let setup = Setup::new().await;
+    let srv = &setup.srv;
+    let (admin_user, admin_password) = srv.admin_password_basic();
+    let settings = srv.state.auth_settings.current().await.settings.clone();
+    let attempts = 15;
+    assert!(attempts > settings.login_max_failures);
+
+    let raw = format!("{}/repository/{}/{RAW_FILE}", srv.base_url, setup.raw);
+    for _ in 0..attempts {
+        let resp = srv
+            .client()
+            .get(&raw)
+            .basic_auth(&admin_user, Some(&admin_password))
+            .send()
+            .await
+            .unwrap();
+        assert_eq!(resp.status(), 401, "Basic with a closed realm's password");
+    }
+    // `npm login --auth-type=legacy` refuses it the same way.
+    let npm_login = format!(
+        "{}/repository/{}/-/user/org.couchdb.user:{}",
+        srv.base_url,
+        setup.npm,
+        admin_user.replace('@', "%40")
+    );
+    for _ in 0..attempts {
+        let resp = srv
+            .client()
+            .put(&npm_login)
+            .json(&json!({"name": admin_user, "password": admin_password, "type": "user"}))
+            .send()
+            .await
+            .unwrap();
+        assert_eq!(
+            resp.status(),
+            401,
+            "npm login with a closed realm's password"
+        );
+    }
+    // From the same address, the browser login still takes the password.
+    srv.login(&admin_user, &admin_password).await;
+
+    // Wrong passwords still count: the browser login checks them, and so
+    // locks the pair once the budget is spent.
+    let login = format!("{}/auth/login", srv.base_url);
+    for _ in 0..settings.login_max_failures {
+        let resp = srv
+            .client()
+            .post(&login)
+            .json(&json!({"username": admin_user, "password": "not-the-password"}))
+            .send()
+            .await
+            .unwrap();
+        assert_eq!(resp.status(), 401);
+    }
+    let resp = srv
+        .client()
+        .post(&login)
+        .json(&json!({"username": admin_user, "password": admin_password}))
+        .send()
+        .await
+        .unwrap();
+    assert_eq!(resp.status(), 429, "wrong passwords still lock the pair");
+}
+
 /// 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

docs/changes/2.2.0/795.fixed.md

@@ -0,0 +1,8 @@
+A local realm that accepts no passwords from clients counted each password
+a client still sent for one of its accounts (HTTP Basic on the feeds and
+`/v2/token`, `npm login --auth-type=legacy`) as a failed sign-in. A NuGet or
+docker client configured with the password before the upgrade therefore
+locked its owner out of the browser login (429) from the same network, the
+sign-in that gets them a token. Such a password is still refused like a
+wrong one, but no longer counts towards the lockout: nothing is checked on
+that path. Wrong passwords for realms that check them count as before.