diff --git a/README.md b/README.md index e6531be4..09c51a98 100644 --- a/README.md +++ b/README.md @@ -133,6 +133,7 @@ This next release focus on two-factor-authentication as a measure to increase se * Enforce permission on /etc/rdiffweb configuration folder * Enforce validation on fullname, username and email * Limit incorrect attempts to change the user's password to prevent brute force attacks #225 [CVE-2022-3273](https://nvd.nist.gov/vuln/detail/CVE-2022-3273) +* Enforce password policy new password cannot be set as new password [CVE-2022-3376](https://nvd.nist.gov/vuln/detail/CVE-2022-3376) Breaking changes: diff --git a/rdiffweb/controller/page_pref_general.py b/rdiffweb/controller/page_pref_general.py index 453378ba..9ae459e2 100644 --- a/rdiffweb/controller/page_pref_general.py +++ b/rdiffweb/controller/page_pref_general.py @@ -88,6 +88,13 @@ def is_submitted(self): # Validate only if action is set_profile_info return super().is_submitted() and self.action.data == 'set_password' + def validate_new(self, field): + """ + Make sure new password if not equals to old password. + """ + if self.new.data and self.new.data == self.current.data: + raise ValueError(_('The new password must be different from the current password.')) + def populate_obj(self, user): # Check if current password is "valid" if Not, rate limit the # number of attempts and logout user after too many invalid attempts. @@ -102,17 +109,19 @@ def populate_obj(self, user): level='warning', ) raise cherrypy.HTTPRedirect('/login/') - flash(_("Wrong current password."), level='warning') - else: - # Clear number of attempts - if CHANGE_PASSWORD_ATTEMPTS in cherrypy.session: - del cherrypy.session[CHANGE_PASSWORD_ATTEMPTS] - # If Valid, update password - try: - user.set_password(self.new.data) - flash(_("Password updated successfully."), level='success') - except ValueError as e: - flash(str(e), level='warning') + self.current.errors = [_("Wrong current password.")] + return False + + # Clear number of attempts + if CHANGE_PASSWORD_ATTEMPTS in cherrypy.session: + del cherrypy.session[CHANGE_PASSWORD_ATTEMPTS] + + try: + user.set_password(self.new.data) + return True + except ValueError as e: + self.new.errors = [str(e)] + return False class RefreshForm(CherryForm): @@ -151,11 +160,14 @@ def default(self, **kwargs): if profile_form.validate(): profile_form.populate_obj(self.app.currentuser) flash(_("Profile updated successfully."), level='success') + raise cherrypy.HTTPRedirect("") else: flash(profile_form.error_message, level='error') elif password_form.is_submitted(): if password_form.validate(): - password_form.populate_obj(self.app.currentuser) + if password_form.populate_obj(self.app.currentuser): + flash(_("Password updated successfully."), level='success') + raise cherrypy.HTTPRedirect("") else: flash(password_form.error_message, level='error') elif refresh_form.is_submitted(): diff --git a/rdiffweb/controller/tests/test_page_prefs_general.py b/rdiffweb/controller/tests/test_page_prefs_general.py index 15b8ee41..be60757c 100644 --- a/rdiffweb/controller/tests/test_page_prefs_general.py +++ b/rdiffweb/controller/tests/test_page_prefs_general.py @@ -82,7 +82,8 @@ def test_change_username_noop(self): method='POST', body={'action': 'set_profile_info', 'email': 'test@test.com', 'username': 'test'}, ) - self.assertStatus(200) + self.assertStatus(303) + self.getPage(self.PREFS) self.assertInBody("Profile updated successfully.") # Then database is updated with fullname user = UserObject.query.filter(UserObject.username == self.USERNAME).first() @@ -105,14 +106,16 @@ def test_change_fullname(self, new_fullname, expected_valid): # Given an authenticated user # When update the fullname self._set_profile_info("test@test.com", new_fullname) - self.assertStatus(200) if expected_valid: + self.assertStatus(303) + self.getPage(self.PREFS) self.assertInBody("Profile updated successfully.") # Then database is updated with fullname self.assertInBody(new_fullname) user = UserObject.query.filter(UserObject.username == self.USERNAME).first() self.assertEqual(new_fullname, user.fullname) else: + self.assertStatus(200) self.assertNotInBody("Profile updated successfully.") def test_change_fullname_method_get(self): @@ -139,7 +142,8 @@ def test_change_fullname_too_long(self): def test_change_email(self): self._set_profile_info("test@test.com") - self.assertStatus(200) + self.assertStatus(303) + self.getPage(self.PREFS) self.assertInBody("Profile updated successfully.") @parameterized.expand( @@ -156,11 +160,13 @@ def test_change_email(self): ) def test_change_email_with_invalid_email(self, new_email, expected_valid): self._set_profile_info(new_email) - self.assertStatus(200) if expected_valid: + self.assertStatus(303) + self.getPage(self.PREFS) self.assertInBody("Profile updated successfully.") self.assertNotInBody("Must be a valid email address.") else: + self.assertStatus(200) self.assertNotInBody("Profile updated successfully.") self.assertInBody("Must be a valid email address.") @@ -172,6 +178,10 @@ def test_change_password(self): self.listener.user_password_changed.reset_mock() # When udating user's password self._set_password(self.PASSWORD, "pr3j5Dwi", "pr3j5Dwi") + # Then user is redirect to same page + self.assertStatus(303) + # Then the page return success message. + self.getPage(self.PREFS) self.assertInBody("Password updated successfully.") # Then a notification is raised self.listener.user_password_changed.assert_called_once() @@ -208,6 +218,16 @@ def test_change_password_too_many_attemps(self): self.assertStatus(200) self.assertInBody('You were logged out because you entered the wrong password too many times.') + def test_change_password_with_same_value(self): + # Given a user with a password + self._set_password(self.PASSWORD, "pr3j5Dwi", "pr3j5Dwi") + self.assertStatus(303) + # When updating the pasword with the same password + self._set_password("pr3j5Dwi", "pr3j5Dwi", "pr3j5Dwi") + self.assertStatus(200) + # Then an error should be displayed + self.assertInBody("The new password must be different from the current password.") + def test_change_password_method_get(self): # Given an authenticated user # Trying to update password with GET method