Enforce minimum and maximum password length #211

README.md

@@ -113,6 +113,7 @@ This releases include a security fix. If you are using an earlier version, you s
 
 * Use 'Secure' Attribute with Sensitive Cookie in HTTPS Session. [CVE-2022-3174](https://nvd.nist.gov/vuln/detail/CVE-2022-3174) #209
 * Avoid leakage of the stack trace in the default error page. [CVE-2022-3175](https://nvd.nist.gov/vuln/detail/CVE-2022-3175) #210
+* Enforce minimum and maximum password length #211
 
 ## 2.4.1 (2022-09-08)
 

doc/configuration.md

@@ -278,6 +278,20 @@ session even if the web server gets restarted, you may persist them on disk with
 | --- | --- | --- |
 | session-dir | location where to store user session information. When undefined, the user sessions are kept in memory. | /var/lib/rdiffweb/session |
 
+## Custom user's password length limits
+
+By default, Rdiffweb supports passwords with the following lengths:
+
+* Minimum: 8 characters
+* Maximum: 128 characters
+
+Changing the minimum or maximum length does not affect existing users' passwords. Existing users are not prompted to reset their passwords to meet the new limits. The new limit only applies when an existing user changes their password.
+
+| Option | Description | Example |
+| --- | --- | --- |
+| password-min-length | Minimum length of the user's password | 8 |
+| password-max-length | Maximum length of the user's password | 128 |
+
 ## Configure Rdiffweb appearance
 
 A number of options are available to customize the appearance of Rdiffweb to your

rdiffweb/controller/page_admin.py

@@ -168,7 +168,7 @@ class UserForm(CherryForm):
     userid = StringField(_('UserID'))
     username = StringField(_('Username'), validators=[validators.data_required()])
     email = EmailField(_('Email'), validators=[validators.optional()])
-    password = PasswordField(_('Password'))
+    password = PasswordField(_('Password'), validators=[validators.optional()])
     user_root = StringField(
         _('Root directory'), description=_("Absolute path defining the location of the repositories for this user.")
     )
@@ -190,6 +190,20 @@ class UserForm(CherryForm):
         _('Quota Used'), validators=[validators.optional()], description=_("Disk spaces (in bytes) used by this user.")
     )
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.password.validators += [
+            validators.length(
+                min=self.app.cfg.password_min_length,
+                max=self.app.cfg.password_max_length,
+                message=_('Password must have between %(min)d and %(max)d characters.'),
+            )
+        ]
+
+    @property
+    def app(self):
+        return cherrypy.request.app
+
     def validate_role(self, field):
         # Don't allow the user to changes it's "role" state.
         currentuser = cherrypy.request.currentuser

rdiffweb/controller/pref_general.py

@@ -25,7 +25,7 @@
 import cherrypy
 from wtforms.fields.html5 import EmailField
 from wtforms.fields.simple import PasswordField
-from wtforms.validators import DataRequired, EqualTo, InputRequired, Regexp
+from wtforms.validators import DataRequired, EqualTo, InputRequired, Length, Regexp
 
 from rdiffweb.controller import Controller, flash
 from rdiffweb.controller.cherrypy_wtf import CherryForm
@@ -54,6 +54,20 @@ class UserPasswordForm(CherryForm):
         _('Confirm new password'), validators=[InputRequired(_("Confirmation password is missing."))]
     )
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.new.validators += [
+            Length(
+                min=self.app.cfg.password_min_length,
+                max=self.app.cfg.password_max_length,
+                message=_('Password must have between %(min)d and %(max)d characters.'),
+            )
+        ]
+
+    @property
+    def app(self):
+        return cherrypy.request.app
+
 
 class PrefsGeneralPanelProvider(Controller):
     """

rdiffweb/controller/tests/test_page_admin.py

@@ -107,7 +107,7 @@ class AdminUsersAsAdminTest(AbstractAdminTest):
 
     def test_add_user_with_role_admin(self):
         # When trying to create a new user with role admin
-        self._add_user("admin_role", "admin_role@test.com", "test2", "/home/", ADMIN_ROLE)
+        self._add_user("admin_role", "admin_role@test.com", "password", "/home/", ADMIN_ROLE)
         # Then page return success
         self.assertStatus(200)
         # Then database is updated
@@ -117,18 +117,18 @@ def test_add_user_with_role_admin(self):
         self.listener.user_added.assert_called_once_with(userobj)
 
     def test_add_user_with_role_maintainer(self):
-        self._add_user("maintainer_role", "maintainer_role@test.com", "test2", "/home/", MAINTAINER_ROLE)
+        self._add_user("maintainer_role", "maintainer_role@test.com", "password", "/home/", MAINTAINER_ROLE)
         self.assertStatus(200)
         self.assertEqual(MAINTAINER_ROLE, self.app.store.get_user('maintainer_role').role)
 
     def test_add_user_with_role_user(self):
-        self._add_user("user_role", "user_role@test.com", "test2", "/home/", USER_ROLE)
+        self._add_user("user_role", "user_role@test.com", "password", "/home/", USER_ROLE)
         self.assertStatus(200)
         self.assertEqual(USER_ROLE, self.app.store.get_user('user_role').role)
 
     def test_add_user_with_invalid_role(self):
         # When trying to create a new user with an invalid role (admin instead of 0)
-        self._add_user("invalid", "invalid@test.com", "test2", "/home/", 'admin')
+        self._add_user("invalid", "invalid@test.com", "test1234", "/home/", 'admin')
         # Then an error message is displayed to the user
         self.assertStatus(200)
         self.assertInBody('role: Invalid Choice: could not coerce')
@@ -145,7 +145,7 @@ def test_add_user_with_invalid_role(self):
 
     def test_add_edit_delete(self):
         #  Add user to be listed
-        self._add_user("test2", "test2@test.com", "test2", "/home/", USER_ROLE)
+        self._add_user("test2", "test2@test.com", "test1234", "/home/", USER_ROLE)
         self.assertInBody("User added successfully.")
         self.assertInBody("test2")
         self.assertInBody("test2@test.com")
@@ -175,7 +175,7 @@ def test_add_edit_delete_user_with_encoding(self):
         """
         Check creation of user with non-ascii char.
         """
-        self._add_user("Éric", "éric@test.com", "Éric", "/home/", USER_ROLE)
+        self._add_user("Éric", "éric@test.com", "password", "/home/", USER_ROLE)
         self.assertInBody("User added successfully.")
         self.assertInBody("Éric")
         self.assertInBody("éric@test.com")
@@ -207,9 +207,9 @@ def test_add_user_with_existing_username(self):
         Verify failure trying to add the same user.
         """
         # Given a user named `test1`
-        self._add_user("test1", "test1@test.com", "test1", "/tmp/", USER_ROLE)
+        self._add_user("test1", "test1@test.com", "password", "/tmp/", USER_ROLE)
         # When trying to create a new user with the same name
-        self._add_user("test1", "test1@test.com", "test1", "/tmp/", USER_ROLE)
+        self._add_user("test1", "test1@test.com", "password", "/tmp/", USER_ROLE)
         # Then the user list is displayed with an error message.
         self.assertStatus(200)
         self.assertInBody("User test1 already exists.")
@@ -222,18 +222,18 @@ def test_add_user_with_invalid_root_directory(self):
             self._delete_user("test5")
         except Exception:
             pass
-        self._add_user("test5", "test1@test.com", "test5", "/var/invalid/", USER_ROLE)
+        self._add_user("test5", "test1@test.com", "password", "/var/invalid/", USER_ROLE)
         self.assertInBody("User added successfully.")
         self.assertInBody("User's root directory /var/invalid/ is not accessible!")
 
     def test_add_without_email(self):
         #  Add user to be listed
-        self._add_user("test2", None, "test2", "/tmp/", USER_ROLE)
+        self._add_user("test2", None, "password", "/tmp/", USER_ROLE)
         self.assertInBody("User added successfully.")
 
     def test_add_without_user_root(self):
         #  Add user to be listed
-        self._add_user("test6", None, "test6", None, USER_ROLE)
+        self._add_user("test6", None, "password", None, USER_ROLE)
         self.assertInBody("User added successfully.")
 
         user = self.app.store.get_user('test6')
@@ -267,6 +267,15 @@ def test_delete_user_admin(self):
         self.assertStatus(200)
         self.assertInBody("can't delete admin user")
 
+    def test_change_password_with_too_short(self):
+        self._edit_user(self.USERNAME, password='short')
+        self.assertInBody("Password must have between 8 and 128 characters.")
+
+    def test_change_password_with_too_long(self):
+        new_password = 'a' * 129
+        self._edit_user(self.USERNAME, password=new_password)
+        self.assertInBody("Password must have between 8 and 128 characters.")
+
     def test_change_admin_password(self):
         # Given rdiffweb is configured with admin-password option
         self.app.cfg.admin_password = 'hardcoded'
@@ -284,7 +293,7 @@ def test_edit_user_with_invalid_path(self):
         Verify failure trying to update user with invalid path.
         """
         self.app.store.add_user('test1')
-        self._edit_user("test1", "test1@test.com", "test", "/var/invalid/", USER_ROLE)
+        self._edit_user("test1", "test1@test.com", "password", "/var/invalid/", USER_ROLE)
         self.assertNotInBody("User added successfully.")
         self.assertInBody("User's root directory /var/invalid/ is not accessible!")
 

rdiffweb/controller/tests/test_page_prefs.py

@@ -86,22 +86,31 @@ def test_change_email_with_invalid_email(self):
 
     def test_change_password(self):
         # When udating user's password
-        self._set_password(self.PASSWORD, "newpass", "newpass")
+        self._set_password(self.PASSWORD, "newpassword", "newpassword")
         self.assertInBody("Password updated successfully.")
         # Then a notification is raised
         self.listener.user_password_changed.assert_called_once()
         # Change it back
-        self._set_password("newpass", self.PASSWORD, self.PASSWORD)
+        self._set_password("newpassword", self.PASSWORD, self.PASSWORD)
         self.assertInBody("Password updated successfully.")
 
     def test_change_password_with_wrong_confirmation(self):
         self._set_password(self.PASSWORD, "t", "a")
         self.assertInBody("The new password and its confirmation do not match.")
 
     def test_change_password_with_wrong_password(self):
-        self._set_password("oups", "t", "t")
+        self._set_password("oups", "newpassword", "newpassword")
         self.assertInBody("Wrong password")
 
+    def test_change_password_with_too_short(self):
+        self._set_password(self.PASSWORD, "short", "short")
+        self.assertInBody("Password must have between 8 and 128 characters.")
+
+    def test_change_password_with_too_long(self):
+        new_password = 'a' * 129
+        self._set_password(self.PASSWORD, new_password, new_password)
+        self.assertInBody("Password must have between 8 and 128 characters.")
+
     def test_invalid_pref(self):
         """
         Check if invalid prefs url is 404 Not Found.

rdiffweb/core/config.py

@@ -420,6 +420,20 @@ def get_parser():
         default=False,
     )
 
+    parser.add(
+        '--password-min-length',
+        type=int,
+        help="Minimum length of the user's password",
+        default=8,
+    )
+
+    parser.add(
+        '--password-max-length',
+        type=int,
+        help="Maximum length of the user's password",
+        default=128,
+    )
+
     parser.add_argument('--version', action='version', version='%(prog)s ' + VERSION)
 
     # Here we append a list of arguments for each locale.

rdiffweb/core/store.py

@@ -397,6 +397,9 @@ def set_password(self, password, old_password=None):
         assert old_password is None or isinstance(old_password, str)
         if not password:
             raise ValueError("password can't be empty")
+        cfg = self._store.app.cfg
+        if cfg.password_min_length > len(password) > cfg.password_max_length:
+            raise ValueError("invalid password length")
 
         # Cannot update admin-password if defined
         if self.username == self._store._admin_user and self._store._admin_password: