Backend work to support minimum password complexity (#2965)

* config: Add minimum password complexity option

* PasswordManager: add function for checking if complexity is sufficient

* Enforce password complexity on registration, change and recovery

* cli: Use exit code 1 for weak passwords

This seems preferable to exit code 0, but ideally we should choose one
and document it.

* Expose minimum password complexity score over GraphQL

crates/handlers/src/graphql/model/site_config.rs

@@ -46,6 +46,11 @@ pub struct SiteConfig {
 
     /// Whether passwords are enabled and users can change their own passwords.
     password_change_allowed: bool,
+
+    /// Minimum password complexity, from 0 to 4, in terms of a zxcvbn score.
+    /// The exact scorer (including dictionaries and other data tables)
+    /// in use is <https://crates.io/crates/zxcvbn>.
+    minimum_password_complexity: u8,
 }
 
 #[ComplexObject]
@@ -69,6 +74,7 @@ impl SiteConfig {
             display_name_change_allowed: data_model.displayname_change_allowed,
             password_login_enabled: data_model.password_login_enabled,
             password_change_allowed: data_model.password_change_allowed,
+            minimum_password_complexity: data_model.minimum_password_complexity,
         }
     }
 }

crates/handlers/src/graphql/mutations/user.rs

@@ -480,6 +480,20 @@ impl UserMutations {
             });
         }
 
+        let password_manager = state.password_manager();
+
+        if !password_manager.is_enabled() {
+            return Ok(SetPasswordPayload {
+                status: SetPasswordStatus::PasswordChangesDisabled,
+            });
+        }
+
+        if !password_manager.is_password_complex_enough(&input.new_password)? {
+            return Ok(SetPasswordPayload {
+                status: SetPasswordStatus::InvalidNewPassword,
+            });
+        }
+
         let mut repo = state.repository().await?;
         let Some(user) = repo.user().lookup(user_id).await? else {
             return Ok(SetPasswordPayload {
@@ -487,13 +501,12 @@ impl UserMutations {
             });
         };
 
-        let password_manager = state.password_manager();
         if !requester.is_admin() {
             // If the user isn't an admin, we:
             // - check that password changes are enabled
             // - check that they know their current password
 
-            if !state.site_config().password_change_allowed || !password_manager.is_enabled() {
+            if !state.site_config().password_change_allowed {
                 return Ok(SetPasswordPayload {
                     status: SetPasswordStatus::PasswordChangesDisabled,
                 });

crates/handlers/src/passwords.rs

@@ -21,6 +21,7 @@ use pbkdf2::Pbkdf2;
 use rand::{CryptoRng, Rng, RngCore, SeedableRng};
 use thiserror::Error;
 use zeroize::Zeroizing;
+use zxcvbn::zxcvbn;
 
 pub type SchemeVersion = u16;
 
@@ -34,6 +35,9 @@ pub struct PasswordManager {
 }
 
 struct InnerPasswordManager {
+    /// Minimum complexity score of new passwords (between 0 and 4) as evaluated
+    /// by zxcvbn.
+    minimum_complexity: u8,
     current_hasher: Hasher,
     current_version: SchemeVersion,
 
@@ -42,7 +46,8 @@ struct InnerPasswordManager {
 }
 
 impl PasswordManager {
-    /// Creates a new [`PasswordManager`] from an iterator. The first item in
+    /// Creates a new [`PasswordManager`] from an iterator and a minimum allowed
+    /// complexity score between 0 and 4. The first item in
     /// the iterator will be the default hashing scheme.
     ///
     /// # Example
@@ -50,7 +55,7 @@ impl PasswordManager {
     /// ```rust
     /// pub use mas_handlers::passwords::{PasswordManager, Hasher};
     ///
-    /// PasswordManager::new([
+    /// PasswordManager::new(3, [
     ///     (3, Hasher::argon2id(Some(b"a-secret-pepper".to_vec()))),
     ///     (2, Hasher::argon2id(None)),
     ///     (1, Hasher::bcrypt(Some(10), None)),
@@ -61,6 +66,7 @@ impl PasswordManager {
     ///
     /// Returns an error if the iterator was empty
     pub fn new<I: IntoIterator<Item = (SchemeVersion, Hasher)>>(
+        minimum_complexity: u8,
         iter: I,
     ) -> Result<Self, anyhow::Error> {
         let mut iter = iter.into_iter();
@@ -75,6 +81,7 @@ impl PasswordManager {
 
         Ok(Self {
             inner: Some(Arc::new(InnerPasswordManager {
+                minimum_complexity,
                 current_hasher,
                 current_version,
                 other_hashers,
@@ -103,6 +110,18 @@ impl PasswordManager {
         self.inner.clone().ok_or(PasswordManagerDisabledError)
     }
 
+    /// Returns true if and only if the given password satisfies the minimum
+    /// complexity requirements.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the password manager is disabled
+    pub fn is_password_complex_enough(&self, password: &str) -> Result<bool, anyhow::Error> {
+        let inner = self.get_inner()?;
+        let score = zxcvbn(password, &[]);
+        Ok(u8::from(score.score()) >= inner.minimum_complexity)
+    }
+
     /// Hash a password with the default hashing scheme.
     /// Returns the version of the hashing scheme used and the hashed password.
     ///
@@ -461,13 +480,16 @@ mod tests {
         let password = Zeroizing::new(b"hunter2".to_vec());
         let wrong_password = Zeroizing::new(b"wrong-password".to_vec());
 
-        let manager = PasswordManager::new([
-            // Start with one hashing scheme: the one used by synapse, bcrypt + pepper
-            (
-                1,
-                Hasher::bcrypt(Some(10), Some(b"a-secret-pepper".to_vec())),
-            ),
-        ])
+        let manager = PasswordManager::new(
+            0,
+            [
+                // Start with one hashing scheme: the one used by synapse, bcrypt + pepper
+                (
+                    1,
+                    Hasher::bcrypt(Some(10), Some(b"a-secret-pepper".to_vec())),
+                ),
+            ],
+        )
         .unwrap();
 
         let (version, hash) = manager
@@ -510,13 +532,16 @@ mod tests {
             .await
             .expect_err("Verification should have failed");
 
-        let manager = PasswordManager::new([
-            (2, Hasher::argon2id(None)),
-            (
-                1,
-                Hasher::bcrypt(Some(10), Some(b"a-secret-pepper".to_vec())),
-            ),
-        ])
+        let manager = PasswordManager::new(
+            0,
+            [
+                (2, Hasher::argon2id(None)),
+                (
+                    1,
+                    Hasher::bcrypt(Some(10), Some(b"a-secret-pepper".to_vec())),
+                ),
+            ],
+        )
         .unwrap();
 
         // Verifying still works
@@ -563,14 +588,17 @@ mod tests {
             .await
             .expect_err("Verification should have failed");
 
-        let manager = PasswordManager::new([
-            (3, Hasher::argon2id(Some(b"a-secret-pepper".to_vec()))),
-            (2, Hasher::argon2id(None)),
-            (
-                1,
-                Hasher::bcrypt(Some(10), Some(b"a-secret-pepper".to_vec())),
-            ),
-        ])
+        let manager = PasswordManager::new(
+            0,
+            [
+                (3, Hasher::argon2id(Some(b"a-secret-pepper".to_vec()))),
+                (2, Hasher::argon2id(None)),
+                (
+                    1,
+                    Hasher::bcrypt(Some(10), Some(b"a-secret-pepper".to_vec())),
+                ),
+            ],
+        )
         .unwrap();
 
         // Verifying still works

crates/handlers/src/test_utils.rs

@@ -136,6 +136,7 @@ pub fn test_site_config() -> SiteConfig {
         password_change_allowed: true,
         account_recovery_allowed: true,
         captcha: None,
+        minimum_password_complexity: 1,
     }
 }
 
@@ -178,7 +179,10 @@ impl TestState {
         let metadata_cache = MetadataCache::new();
 
         let password_manager = if site_config.password_login_enabled {
-            PasswordManager::new([(1, Hasher::argon2id(None))])?
+            PasswordManager::new(
+                site_config.minimum_password_complexity,
+                [(1, Hasher::argon2id(None))],
+            )?
         } else {
             PasswordManager::disabled()
         };

crates/handlers/src/views/recovery/finish.rs

@@ -226,6 +226,18 @@ pub(crate) async fn post(
             );
     }
 
+    if !password_manager.is_password_complex_enough(&form.new_password)? {
+        // TODO This error should be localised,
+        //      but actually this entire form should be pushed down into the
+        //      React frontend so user gets real-time feedback.
+        form_state = form_state.with_error_on_field(
+            RecoveryFinishFormField::NewPassword,
+            FieldError::Policy {
+                message: "Password is too weak".to_owned(),
+            },
+        );
+    }
+
     let res = policy.evaluate_password(&form.new_password).await?;
 
     if !res.valid() {

crates/handlers/src/views/register.rs

@@ -198,6 +198,16 @@ pub(crate) async fn post(
             );
         }
 
+        if !password_manager.is_password_complex_enough(&form.password)? {
+            // TODO localise this error
+            state.add_error_on_field(
+                RegisterFormField::Password,
+                FieldError::Policy {
+                    message: "Password is too weak".to_owned(),
+                },
+            );
+        }
+
         // If the site has terms of service, the user must accept them
         if site_config.tos_uri.is_some() && form.accept_terms != "on" {
             state.add_error_on_field(RegisterFormField::AcceptTerms, FieldError::Required);
@@ -402,8 +412,8 @@ mod tests {
                 "csrf": csrf_token,
                 "username": "john",
                 "email": "john@example.com",
-                "password": "hunter2",
-                "password_confirm": "hunter2",
+                "password": "correcthorsebatterystaple",
+                "password_confirm": "correcthorsebatterystaple",
                 "accept_terms": "on",
             }),
         );