Delete user's session on password change

* Revisit add, delete commit function
* Clean-up Access Token using a scheduled job

README.md

@@ -141,6 +141,7 @@ This next release focus on two-factor-authentication as a measure to increase se
 * Send email notification when enabling or disabling MFA [CVE-2022-3363](https://nvd.nist.gov/vuln/detail/CVE-2022-3363)
 * Use Argon2id to store password hash #231
 * Fixed plugin priorities to ensure that jobs are scheduled at each startup #232
+* Revoke previous user's sessions on password change [CVE-2022-3362](https://nvd.nist.gov/vuln/detail/CVE-2022-3362)
 
 Breaking changes:
 

rdiffweb/controller/api.py

@@ -60,7 +60,10 @@ def _checkpassword(realm, username, password):
     userobj = UserObject.get_user(username)
     if userobj is not None:
         # Verify if the password matches a token.
-        if userobj.validate_access_token(password):
+        access_token = userobj.validate_access_token(password)
+        if access_token:
+            access_token.accessed()
+            access_token.commit()
             return True
         # Disable password authentication for MFA
         if userobj.mfa == UserObject.ENABLED_MFA:
@@ -78,7 +81,8 @@ class ApiCurrentUser(Controller):
     @cherrypy.expose
     def default(self):
         u = self.app.currentuser
-        u.refresh_repos()
+        if u.refresh_repos():
+            u.commit()
         return {
             "email": u.email,
             "username": u.username,

rdiffweb/controller/page_admin_session.py

@@ -46,6 +46,7 @@ def default(self, action=None, **kwargs):
                     flash(_('You cannot revoke your current session.'), level='warning')
                 else:
                     session.delete()
+                    session.commit()
                     flash(_('The session was successfully revoked.'), level='success')
             else:
                 flash(form.error_message, level='error')

rdiffweb/controller/page_admin_users.py

@@ -182,6 +182,7 @@ def populate_obj(self, userobj):
             # Setting quota will silently fail. Check if quota was updated.
             if userobj.disk_quota != new_quota:
                 flash(_("Setting user's quota is not supported"), level='warning')
+        userobj.commit()
 
 
 class EditUserForm(UserForm):
@@ -214,6 +215,7 @@ def _delete_user(self, action, form):
                 user = UserObject.get_user(form.username.data)
                 if user:
                     user.delete()
+                    user.commit()
                     flash(_("User account removed."))
                 else:
                     flash(_("User doesn't exists!"), level='warning')

rdiffweb/controller/page_delete.py

@@ -40,6 +40,11 @@
 _logger = logging.getLogger(__name__)
 
 
+def delete_repo(repoobj, path):
+    repoobj.delete(path)
+    repoobj.commit()
+
+
 class DeleteRepoForm(CherryForm):
     confirm = StringField(_('Confirmation'), validators=[DataRequired()])
 
@@ -72,7 +77,7 @@ def default(self, path=b"", **kwargs):
         if form.is_submitted():
             if form.validate():
                 RepoObject.session.expunge(repo)
-                cherrypy.engine.publish('schedule_task', repo.delete, path)
+                cherrypy.engine.publish('schedule_task', delete_repo, repo, path)
                 # Redirect to parent folder or to root if repo get deleted
                 if path_obj.isroot:
                     raise cherrypy.HTTPRedirect(url_for('/'))

rdiffweb/controller/page_locations.py

@@ -34,7 +34,8 @@ class LocationsPage(Controller):
     @cherrypy.expose
     def index(self):
         # Get page params
-        self.app.currentuser.refresh_repos()
+        if self.app.currentuser.refresh_repos():
+            self.app.currentuser.commit()
         params = {
             "repos": self.app.currentuser.repo_objs,
             "disk_usage": self.app.currentuser.disk_usage,

rdiffweb/controller/page_pref_general.py

@@ -58,7 +58,7 @@ def is_submitted(self):
     def populate_obj(self, user):
         user.fullname = self.fullname.data
         user.email = self.email.data
-        user.add()
+        user.commit()
 
 
 class UserPasswordForm(CherryForm):
@@ -99,6 +99,7 @@ def populate_obj(self, user):
             return False
         try:
             user.set_password(self.new.data)
+            user.commit()
             return True
         except ValueError as e:
             self.new.errors = [str(e)]
@@ -120,7 +121,8 @@ def is_submitted(self):
 
     def populate_obj(self, user):
         try:
-            user.refresh_repos(delete=True)
+            if user.refresh_repos(delete=True):
+                user.commit()
             flash(_("Repositories successfully updated"), level='success')
         except ValueError as e:
             flash(str(e), level='warning')

rdiffweb/controller/page_pref_mfa.py

@@ -78,9 +78,11 @@ def populate_obj(self, userobj):
         # Enable or disable MFA only when a code is provided.
         if self.enable_mfa.data:
             userobj.mfa = UserObject.ENABLED_MFA
+            userobj.commit()
             flash(_("Two-Factor authentication enabled successfully."), level='success')
         elif self.disable_mfa.data:
             userobj.mfa = UserObject.DISABLED_MFA
+            userobj.commit()
             flash(_("Two-Factor authentication disabled successfully."), level='success')
 
     def validate_code(self, field):

rdiffweb/controller/page_pref_notification.py

@@ -76,6 +76,7 @@ def populate_obj(self, userobj):
             if repo.display_name in self:
                 # Update the maxage
                 repo.maxage = self[repo.display_name].data
+        userobj.commit()
 
 
 class PagePrefNotification(Controller):

rdiffweb/controller/page_pref_session.py

@@ -47,6 +47,7 @@ def default(self, action=None, **kwargs):
                     flash(_('You cannot revoke your current session.'), level='warning')
                 else:
                     session.delete()
+                    session.commit()
                     flash(_('The session was successfully revoked.'), level='success')
             else:
                 flash(form.error_message, level='error')

rdiffweb/controller/page_pref_sshkeys.py

@@ -72,7 +72,9 @@ class SshForm(CherryForm):
     def populate_obj(self, userobj):
         try:
             userobj.add_authorizedkey(key=self.key.data, comment=self.title.data)
+            userobj.commit()
         except DuplicateSSHKeyError as e:
+            userobj.rollback()
             flash(str(e), level='error')
         except Exception:
             flash(_("Unknown error while adding the SSH Key"), level='error')
@@ -86,6 +88,7 @@ def populate_obj(self, userobj):
         is_maintainer()
         try:
             userobj.delete_authorizedkey(self.fingerprint.data)
+            userobj.commit()
         except Exception:
             flash(_("Unknown error while removing the SSH Key"), level='error')
             _logger.warning("error removing ssh key", exc_info=1)

rdiffweb/controller/page_pref_tokens.py

@@ -62,6 +62,7 @@ def is_submitted(self):
     def populate_obj(self, userobj):
         try:
             token = userobj.add_access_token(self.name.data, self.expiration.data)
+            userobj.commit()
             flash(
                 _(
                     "Your new personal access token has been created.\n"

rdiffweb/controller/tests/test_api.py

@@ -101,6 +101,7 @@ def test_auth_with_access_token(self):
         # Given a user with an access token
         userobj = UserObject.get_user(self.USERNAME)
         token = userobj.add_access_token('test').encode('ascii')
+        userobj.commit()
         # When using this token to authenticated with /api
         self.getPage('/api/', headers=[("Authorization", "Basic " + b64encode(b"admin:" + token).decode('ascii'))])
         # Then authentication is successful
@@ -110,7 +111,7 @@ def test_auth_failed_with_mfa_enabled(self):
         # Given a user with MFA enabled
         userobj = UserObject.get_user(self.USERNAME)
         userobj.mfa = UserObject.ENABLED_MFA
-        userobj.add()
+        userobj.commit()
         # When authenticating with /api/
         self.getPage('/api/', headers=self.headers)
         # Then access is refused

rdiffweb/controller/tests/test_controller.py

@@ -178,7 +178,7 @@ def test_clean_up_session(self):
         # When this session get old
         data = SessionObject.query.filter(SessionObject.id == self.session_id).first()
         data.expiration_time = datetime.datetime.now() - datetime.timedelta(seconds=1)
-        data.add()
+        data.commit()
         session = DbSession(id=self.session_id)
         # Then the session get deleted by clean_up process
         session.clean_up()

rdiffweb/controller/tests/test_page_admin.py

@@ -23,7 +23,8 @@ class AdminPagesAsUser(rdiffweb.test.WebCase):
     def setUp(self):
         super().setUp()
         # Add test user
-        UserObject.add_user('test', 'test123')
+        userobj = UserObject.add_user('test', 'test123')
+        userobj.commit()
         self._login('test', 'test123')
 
     @parameterized.expand(

rdiffweb/controller/tests/test_page_admin_users.py

@@ -337,7 +337,8 @@ def test_delete_user_admin(self):
 
     def test_delete_user_method_get(self):
         # Given a user
-        UserObject.add_user('newuser')
+        user = UserObject.add_user('newuser')
+        user.commit()
         # When trying to delete this user using method GET
         self.getPage("/admin/users/?action=delete&username=newuser", method='GET')
         # Then page return without error
@@ -370,7 +371,8 @@ def test_edit_user_with_invalid_path(self):
         """
         Verify failure trying to update user with invalid path.
         """
-        UserObject.add_user('test1')
+        userobj = UserObject.add_user('test1')
+        userobj.commit()
         self._edit_user("test1", "test1@test.com", "pr3j5Dwi", "/var/invalid/", UserObject.USER_ROLE)
         self.assertNotInBody("User added successfully.")
         self.assertInBody("User's root directory /var/invalid/ is not accessible!")
@@ -397,18 +399,18 @@ def test_user_invalid_root(self):
         # Delete all user's
         for user in UserObject.query.all():
             if user.username != self.USERNAME:
-                user.delete()
+                user.delete().commit()
         # Change the user's root
         user = UserObject.get_user('admin')
         user.user_root = "/invalid"
-        user.add()
+        user.commit()
         self.getPage("/admin/users")
         self.assertInBody("Root directory not accessible!")
 
         # Query the page by default
         user = UserObject.get_user('admin')
         user.user_root = "/tmp/"
-        user.add()
+        user.commit()
         self.getPage("/admin/users")
         self.assertNotInBody("Root directory not accessible!")
 

rdiffweb/controller/tests/test_page_browse.py

@@ -49,8 +49,8 @@ def test_locations(self):
 
     def test_locations_with_broken_tree(self):
         userobj = UserObject.get_user(self.USERNAME)
-        RepoObject(userid=userobj.userid, repopath='testcases/broker-repo').add()
-        RepoObject(userid=userobj.userid, repopath='testcases/testcases').add()
+        RepoObject(userid=userobj.userid, repopath='testcases/broker-repo').add().commit()
+        RepoObject(userid=userobj.userid, repopath='testcases/testcases').add().commit()
         self.getPage("/")
 
     def test_WithRelativePath(self):
@@ -229,7 +229,7 @@ def test_with_single_repo(self):
         user = UserObject.get_user(self.USERNAME)
         user.user_root = os.path.join(self.testcases, 'testcases')
         user.refresh_repos()
-        user.add()
+        user.commit()
         self.assertEqual(['', 'broker-repo', 'testcases'], [r.name for r in user.repo_objs])
         # Check if listing locations is working
         self.getPage('/')
@@ -249,7 +249,7 @@ def test_browse_with_permissions(self):
         user_obj = UserObject.add_user('anotheruser', 'password')
         user_obj.user_root = self.testcases
         user_obj.refresh_repos()
-        user_obj.add()
+        user_obj.commit()
         self.getPage('/browse/admin')
         self.assertStatus('404 Not Found')
 
@@ -265,8 +265,8 @@ def test_browse_without_permissions(self):
         # Remove admin role.
         admin = UserObject.get_user('admin')
         admin.role = UserObject.USER_ROLE
-        admin.add()
         admin.refresh_repos()
+        admin.commit()
 
         # Browse other user's repos
         self.getPage('/browse/anotheruser/testcases')
@@ -278,7 +278,7 @@ def test_browser_with_failed_repo(self):
         # Given a failed repo
         admin = UserObject.get_user('admin')
         admin.user_root = 'invalid'
-        admin.add()
+        admin.commit()
         # When querying the logs
         self._browse(self.USERNAME, self.REPO, '')
         # Then the page is return with an error message

rdiffweb/controller/tests/test_page_delete.py

@@ -158,6 +158,7 @@ def test_delete_repo_as_admin(self):
         user_obj = UserObject.add_user('anotheruser', 'password')
         user_obj.user_root = self.testcases
         user_obj.refresh_repos()
+        user_obj.commit()
         self.assertEqual(['broker-repo', 'testcases'], [r.name for r in user_obj.repo_objs])
 
         self._delete('anotheruser', 'testcases', 'testcases')
@@ -178,6 +179,7 @@ def test_delete_repo_as_maintainer(self):
         user_obj.user_root = self.testcases
         user_obj.role = UserObject.MAINTAINER_ROLE
         user_obj.refresh_repos()
+        user_obj.commit()
         self.assertEqual(['broker-repo', 'testcases'], [r.name for r in user_obj.repo_objs])
 
         # Login as maintainer
@@ -200,6 +202,7 @@ def test_delete_repo_as_user(self):
         user_obj.user_root = self.testcases
         user_obj.role = UserObject.USER_ROLE
         user_obj.refresh_repos()
+        user_obj.commit()
         self.assertEqual(['broker-repo', 'testcases'], [r.name for r in user_obj.repo_objs])
 
         # Login as maintainer

rdiffweb/controller/tests/test_page_graphs.py

@@ -54,6 +54,7 @@ def test_as_another_user(self):
         user_obj = UserObject.add_user('anotheruser', 'password')
         user_obj.user_root = self.testcases
         user_obj.refresh_repos()
+        user_obj.commit()
 
         self.getPage("/graphs/activities/anotheruser/testcases")
         self.assertStatus('200 OK')
@@ -62,7 +63,7 @@ def test_as_another_user(self):
         # Remove admin role
         admin = UserObject.get_user('admin')
         admin.role = UserObject.USER_ROLE
-        admin.add()
+        admin.commit()
 
         # Browse admin's repos
         self.getPage("/graphs/activities/anotheruser/testcases")
@@ -90,7 +91,7 @@ def test_browser_with_failed_repo(self):
         # Given a failed repo
         admin = UserObject.get_user('admin')
         admin.user_root = 'invalid'
-        admin.add()
+        admin.commit()
         # When querying the logs
         self.getPage("/graphs/activities/" + self.USERNAME + "/" + self.REPO + "/")
         # Then the page is return with an error message

rdiffweb/controller/tests/test_page_history.py

@@ -75,13 +75,14 @@ def test_as_another_user(self):
         user_obj = UserObject.add_user('anotheruser', 'password')
         user_obj.user_root = self.testcases
         user_obj.refresh_repos()
+        user_obj.commit()
         self.getPage("/history/anotheruser/testcases")
         self.assertStatus('200 OK')
 
         # Remove admin right
         admin = UserObject.get_user('admin')
         admin.role = UserObject.USER_ROLE
-        admin.add()
+        admin.commit()
 
         # Browse admin's repos
         self.getPage("/history/anotheruser/testcases")
@@ -99,7 +100,7 @@ def test_browser_with_failed_repo(self):
         # Given a failed repo
         admin = UserObject.get_user('admin')
         admin.user_root = 'invalid'
-        admin.add()
+        admin.commit()
         # When querying the logs
         self.getPage("/history/" + self.USERNAME + "/" + self.REPO)
         # Then the page is return with an error message

rdiffweb/controller/tests/test_page_login.py

@@ -170,7 +170,8 @@ def test_login_twice(self):
         self.assertStatus(200)
         self.assertInBody(self.USERNAME)
         # Given another user
-        UserObject.add_user('otheruser', password='password')
+        userobj = UserObject.add_user('otheruser', password='password')
+        userobj.commit()
         # When trying to re-authenticated with login page
         self.getPage('/login/', method='POST', body={'login': 'otheruser', 'password': 'password'})
         # Then user is still authenticated with previous user

rdiffweb/controller/tests/test_page_logs.py

@@ -100,7 +100,7 @@ def test_browser_with_failed_repo(self):
         # Given a failed repo
         admin = UserObject.get_user('admin')
         admin.user_root = 'invalid'
-        admin.add()
+        admin.commit()
         # When querying the logs
         self._log(self.USERNAME, self.REPO)
         # Then the page is return with an error message