Enforce password policy new password cannot be set as new password

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:
 

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():

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