From 8c4f4374fbc1f96e79296c9eb8e8ff4ee6a08989 Mon Sep 17 00:00:00 2001 From: Vincent Hatakeyama Date: Tue, 12 Nov 2024 11:05:00 +0100 Subject: [PATCH] [IMP] auth_saml: only write value that changes When using mapping, not writing the value systematically avoids getting security mail on login/email changes when there is no change. Also use SQL for blanking passwords avoids the security update mails. --- auth_saml/models/res_users.py | 41 +++++++++++++++++++++++++---------- auth_saml/readme/HISTORY.md | 10 +++++++-- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/auth_saml/models/res_users.py b/auth_saml/models/res_users.py index 412b5c6994..34bcb9b9d1 100644 --- a/auth_saml/models/res_users.py +++ b/auth_saml/models/res_users.py @@ -55,7 +55,15 @@ def _auth_saml_signin(self, provider: int, validation: dict, saml_response) -> s user_saml.with_env(new_env).write({"saml_access_token": saml_response}) if validation.get("mapped_attrs", {}): - user.write(validation.get("mapped_attrs", {})) + # Only write field that changes to avoid generating Security Update on users + # when login/email changes (from mail module) + vals = {} + for key, value in validation.get("mapped_attrs", {}).items(): + if not isinstance(value, str) or getattr(user, key) != value: + vals[key] = value + if vals: + # TODO handle login/password/email changes and mails + user.write(vals) return user.login @@ -158,27 +166,36 @@ def _set_password(self): # pylint: disable=protected-access super(ResUser, non_blank_password_users)._set_password() if blank_password_users: - # similar to what Odoo does in Users._set_encrypted_password - self.env.cr.execute( - "UPDATE res_users SET password = NULL WHERE id IN %s", - (tuple(blank_password_users.ids),), - ) - blank_password_users.invalidate_recordset(fnames=["password"]) + blank_password_users._set_password_blank() return + def _set_password_blank(self): + """Set the password to a value that prohibits logging.""" + # Use SQL to blank the password to avoid sending security messages (done in + # mail module) to end users. + _logger.debug( + "Removing password from %s user(s)", len(self.ids) + ) + # similar to what Odoo does in Users._set_encrypted_password + self.env.cr.execute( + "UPDATE res_users SET password = NULL WHERE id IN %s", + (tuple(self.ids),), + ) + self.invalidate_recordset(fnames=["password"]) + + def allow_saml_and_password_changed(self): """Called after the parameter is changed.""" if not self.allow_saml_and_password(): # sudo because the user doing the parameter change might not have the right # to search or write users - users_to_blank_password = self.sudo().search( + blank_password_users = self.sudo().search( [ + "&", "&", ("saml_ids", "!=", False), ("id", "not in", list(self._saml_allowed_user_ids())), + ("password", "!=", False) ] ) - _logger.debug( - "Removing password from %s user(s)", len(users_to_blank_password) - ) - users_to_blank_password.write({"password": False}) + blank_password_users._set_password_blank() diff --git a/auth_saml/readme/HISTORY.md b/auth_saml/readme/HISTORY.md index 89020f8c3c..27737662f0 100644 --- a/auth_saml/readme/HISTORY.md +++ b/auth_saml/readme/HISTORY.md @@ -1,3 +1,9 @@ -## 16.0.1.0.0 +## 17.0.1.1.0 -Initial migration for 16.0. +When using attribute mapping, only write value that changes. +No writing the value systematically avoids getting security mail on login/email +when there is no real change. + +## 17.0.1.0.0 + +Initial migration for 17.0.