Skip to content

Commit

Permalink
[IMP] auth_saml: only write value that changes
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
vincent-hatakeyama committed Nov 12, 2024
1 parent def9106 commit b1dc23b
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 14 deletions.
38 changes: 26 additions & 12 deletions auth_saml/models/res_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}

Check warning on line 60 in auth_saml/models/res_users.py

View check run for this annotation

Codecov / codecov/patch

auth_saml/models/res_users.py#L60

Added line #L60 was not covered by tests
for key, value in validation.get("mapped_attrs", {}).items():
if not isinstance(value, str) or getattr(user, key) != value:
vals[key] = value

Check warning on line 63 in auth_saml/models/res_users.py

View check run for this annotation

Codecov / codecov/patch

auth_saml/models/res_users.py#L63

Added line #L63 was not covered by tests
if vals:
# TODO handle login/password/email changes and mails
user.write(vals)

Check warning on line 66 in auth_saml/models/res_users.py

View check run for this annotation

Codecov / codecov/patch

auth_saml/models/res_users.py#L66

Added line #L66 was not covered by tests

return user.login

Expand Down Expand Up @@ -158,27 +166,33 @@ 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()
10 changes: 8 additions & 2 deletions auth_saml/readme/HISTORY.md
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit b1dc23b

Please sign in to comment.