diff --git a/README.md b/README.md index 85b66d52..3aa065e8 100644 --- a/README.md +++ b/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) diff --git a/doc/configuration.md b/doc/configuration.md index 0b0a3993..6e4556da 100644 --- a/doc/configuration.md +++ b/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 diff --git a/rdiffweb/controller/page_admin.py b/rdiffweb/controller/page_admin.py index 1c685de4..dc367fc3 100644 --- a/rdiffweb/controller/page_admin.py +++ b/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 diff --git a/rdiffweb/controller/pref_general.py b/rdiffweb/controller/pref_general.py index 804ac39a..2720b6b1 100644 --- a/rdiffweb/controller/pref_general.py +++ b/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): """ diff --git a/rdiffweb/controller/tests/test_page_admin.py b/rdiffweb/controller/tests/test_page_admin.py index 02134a46..76d1d8fd 100644 --- a/rdiffweb/controller/tests/test_page_admin.py +++ b/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!") diff --git a/rdiffweb/controller/tests/test_page_prefs.py b/rdiffweb/controller/tests/test_page_prefs.py index 59bfcad6..0f098073 100644 --- a/rdiffweb/controller/tests/test_page_prefs.py +++ b/rdiffweb/controller/tests/test_page_prefs.py @@ -86,12 +86,12 @@ 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): @@ -99,9 +99,18 @@ def test_change_password_with_wrong_confirmation(self): 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. diff --git a/rdiffweb/core/config.py b/rdiffweb/core/config.py index cd5fc086..183d0487 100644 --- a/rdiffweb/core/config.py +++ b/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. diff --git a/rdiffweb/core/store.py b/rdiffweb/core/store.py index 68569af8..6646c0ec 100644 --- a/rdiffweb/core/store.py +++ b/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: