diff --git a/README.md b/README.md index 429a605cb2..4ff2affd0f 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ addon | version | maintainers | summary [auth_oauth_multi_token](auth_oauth_multi_token/) | 18.0.2.0.0 | | Allow multiple connection with the same OAuth account [auth_oidc](auth_oidc/) | 18.0.1.0.0 | sbidoul | Allow users to login through OpenID Connect Provider [auth_oidc_environment](auth_oidc_environment/) | 18.0.1.0.0 | | This module allows to use server env for OIDC configuration -[auth_saml](auth_saml/) | 18.0.1.0.2 | vincent-hatakeyama | SAML2 Authentication +[auth_saml](auth_saml/) | 18.0.1.0.3 | vincent-hatakeyama | SAML2 Authentication [auth_session_timeout](auth_session_timeout/) | 18.0.1.0.0 | | This module disable all inactive sessions since a given delay [auth_signup_verify_email](auth_signup_verify_email/) | 18.0.1.0.0 | | Force uninvited users to use a good email for signup [auth_user_case_insensitive](auth_user_case_insensitive/) | 18.0.1.0.0 | | Makes the user login field case insensitive diff --git a/auth_saml/README.rst b/auth_saml/README.rst index fbfa634e8d..fa34def2bb 100644 --- a/auth_saml/README.rst +++ b/auth_saml/README.rst @@ -1,3 +1,7 @@ +.. image:: https://odoo-community.org/readme-banner-image + :target: https://odoo-community.org/get-involved?utm_source=readme + :alt: Odoo Community Association + ==================== SAML2 Authentication ==================== @@ -7,13 +11,13 @@ SAML2 Authentication !! This file is generated by oca-gen-addon-readme !! !! changes will be overwritten. !! !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! - !! source digest: sha256:9713ed18925a56b7a75da4faf0e627f8b34a316a93ec46c53ac72d14bcc03d53 + !! source digest: sha256:6c12eb5e5c1c80ee3a898dfd7985b6f518a5e0dbad26db680d94dd9759a57699 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! .. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png :target: https://odoo-community.org/page/development-status :alt: Beta -.. |badge2| image:: https://img.shields.io/badge/licence-AGPL--3-blue.png +.. |badge2| image:: https://img.shields.io/badge/license-AGPL--3-blue.png :target: http://www.gnu.org/licenses/agpl-3.0-standalone.html :alt: License: AGPL-3 .. |badge3| image:: https://img.shields.io/badge/github-OCA%2Fserver--auth-lightgray.png?logo=github @@ -96,6 +100,16 @@ Known issues / Roadmap Changelog ========= +18.0.1.0.3 (2025-09-11) +----------------------- + +Features +~~~~~~~~ + +- When using attribute mapping, only write value that changes. Not + writing the value systematically avoids getting security mail on + login/email when there is no real change. + 18.0.1.0.2 (2025-05-13) ----------------------- @@ -104,10 +118,10 @@ Bugfixes - Avoid redirecting when there is a SAML error. -16.0.1.0.0 +18.0.1.0.0 ---------- -Initial migration for 16.0. +Initial migration for 18.0. Bug Tracker =========== diff --git a/auth_saml/__manifest__.py b/auth_saml/__manifest__.py index 4e63c913d6..86c79c7f9b 100644 --- a/auth_saml/__manifest__.py +++ b/auth_saml/__manifest__.py @@ -4,7 +4,7 @@ { "name": "SAML2 Authentication", - "version": "18.0.1.0.2", + "version": "18.0.1.0.3", "category": "Tools", "author": "XCG Consulting, Odoo Community Association (OCA)", "maintainers": ["vincent-hatakeyama"], diff --git a/auth_saml/models/res_users.py b/auth_saml/models/res_users.py index 90b4eb222a..fe869a3d26 100644 --- a/auth_saml/models/res_users.py +++ b/auth_saml/models/res_users.py @@ -55,7 +55,20 @@ 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 the value or the field value is not a str, + # avoid comparison and write anyway + if ( + not isinstance(value, str) + or not isinstance(user[key], str) + or user[key] != value + ): + vals[key] = value + if vals: + user.write(vals) return user.login @@ -163,27 +176,31 @@ 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())), ] ) - _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 c707ac5f6a..e9060eb5ac 100644 --- a/auth_saml/readme/HISTORY.md +++ b/auth_saml/readme/HISTORY.md @@ -1,3 +1,12 @@ +## 18.0.1.0.3 (2025-09-11) + +### Features + +- When using attribute mapping, only write value that changes. + Not writing the value systematically avoids getting security mail on login/email + when there is no real change. + + ## 18.0.1.0.2 (2025-05-13) ### Bugfixes @@ -5,6 +14,6 @@ - Avoid redirecting when there is a SAML error. -## 16.0.1.0.0 +## 18.0.1.0.0 -Initial migration for 16.0. +Initial migration for 18.0. diff --git a/auth_saml/tests/test_pysaml.py b/auth_saml/tests/test_pysaml.py index a666a1496f..54e9a3fbb4 100644 --- a/auth_saml/tests/test_pysaml.py +++ b/auth_saml/tests/test_pysaml.py @@ -140,17 +140,11 @@ def test__compute_sp_metadata_url__provider_has_sp_baseurl(self): self.assertEqual(self.saml_provider.sp_metadata_url, expected_url) self.saml_provider.sp_baseurl = temp - def test__hook_validate_auth_response(self): - # Create a fake response with attributes - fake_response = DummyResponse(200, "fake_data") - fake_response.set_identity( - {"email": "new_user@example.com", "first_name": "New", "last_name": "User"} - ) - - # Add attribute mappings to the provider + def _add_mapping_to_provider(self): + """Add mapping to the provider""" self.saml_provider.attribute_mapping_ids = [ - (0, 0, {"attribute_name": "email", "field_name": "login"}), - (0, 0, {"attribute_name": "first_name", "field_name": "name"}), + (0, 0, {"attribute_name": "mail", "field_name": "login"}), + (0, 0, {"attribute_name": "givenName", "field_name": "name"}), ( 0, 0, @@ -158,6 +152,13 @@ def test__hook_validate_auth_response(self): ), # This attribute is not in attrs ] + def test__hook_validate_auth_response(self): + # Create a fake response with attributes + fake_response = DummyResponse(200, "fake_data") + fake_response.set_identity( + {"mail": "new_user@example.com", "givenName": "New", "last_name": "User"} + ) + self._add_mapping_to_provider() # Call the method result = self.saml_provider._hook_validate_auth_response( fake_response, "test@example.com" @@ -274,6 +275,17 @@ def test_login_with_saml(self): # User should now be able to log in with the token self.authenticate(user="test@example.com", password=token) + def test_login_with_saml_mapping_attributes(self): + """Test login with SAML on a provider with mapping attributes""" + self.assertEqual(self.user.name, "User") + self.assertEqual(self.user.login, "test@example.com") + self._add_mapping_to_provider() + self.test_login_with_saml() + # Changed due to mapping and FakeIDP returning another value + self.assertEqual(self.user.name, "Test") + # Not changed + self.assertEqual(self.user.login, "test@example.com") + def test_disallow_user_password_when_changing_ir_config_parameter(self): """Test that disabling users from having both a password and SAML ids remove users password."""