diff --git a/README.md b/README.md index 7d93cbafd0..977ded7b48 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ addon | version | maintainers | summary [auth_jwt](auth_jwt/) | 17.0.1.0.0 | [![sbidoul](https://github.com/sbidoul.png?size=30px)](https://github.com/sbidoul) | JWT bearer token authentication. [auth_ldaps](auth_ldaps/) | 17.0.1.0.0 | | Allows to use LDAP over SSL authentication [auth_oidc](auth_oidc/) | 17.0.1.1.0 | [![sbidoul](https://github.com/sbidoul.png?size=30px)](https://github.com/sbidoul) | Allow users to login through OpenID Connect Provider -[auth_saml](auth_saml/) | 17.0.1.0.0 | [![vincent-hatakeyama](https://github.com/vincent-hatakeyama.png?size=30px)](https://github.com/vincent-hatakeyama) | SAML2 Authentication +[auth_saml](auth_saml/) | 17.0.1.0.1 | [![vincent-hatakeyama](https://github.com/vincent-hatakeyama.png?size=30px)](https://github.com/vincent-hatakeyama) | SAML2 Authentication [auth_session_timeout](auth_session_timeout/) | 17.0.1.0.0 | | This module disable all inactive sessions since a given delay [auth_signup_verify_email](auth_signup_verify_email/) | 17.0.1.0.0 | | Force uninvited users to use a good email for signup [auth_user_case_insensitive](auth_user_case_insensitive/) | 17.0.1.0.0 | | Makes the user login field case insensitive diff --git a/auth_saml/README.rst b/auth_saml/README.rst index a53a8f65af..8ae4813cd4 100644 --- a/auth_saml/README.rst +++ b/auth_saml/README.rst @@ -7,7 +7,7 @@ SAML2 Authentication !! This file is generated by oca-gen-addon-readme !! !! changes will be overwritten. !! !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! - !! source digest: sha256:29773025a7d79e9696be8e0a1b65361642ef6bc8b6fb8f9cb13a4b4719017c71 + !! source digest: sha256:ffa8efafb4e4dcf93290b09d3910d691b713c29ca2ba54b6b263a9a4336a49b4 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! .. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png @@ -36,14 +36,14 @@ On) between Odoo and other applications of your ecosystem. **Benefits**: -- Reducing the time spent typing different passwords for different - accounts. -- Reducing the time spent in IT support for password oversights. -- Centralizing authentication systems. -- Securing all input levels / exit / access to multiple systems without - prompting users. -- The centralization of access control information for compliance - testing to different standards. +- Reducing the time spent typing different passwords for different + accounts. +- Reducing the time spent in IT support for password oversights. +- Centralizing authentication systems. +- Securing all input levels / exit / access to multiple systems without + prompting users. +- The centralization of access control information for compliance + testing to different standards. **Table of contents** @@ -91,15 +91,22 @@ login screen. Known issues / Roadmap ====================== -- clean up ``auth_saml.request`` +- clean up ``auth_saml.request`` Changelog ========= -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. Bug Tracker =========== @@ -122,28 +129,28 @@ Authors Contributors ------------ -- `XCG Consulting `__: +- `XCG Consulting `__: - - Florent Aide - - Vincent Hatakeyama - - Alexandre Brun - - Houzéfa Abbasbhay - - Szeka Wong + - Florent Aide + - Vincent Hatakeyama + - Alexandre Brun + - Houzéfa Abbasbhay + - Szeka Wong -- Jeremy Co Kim Len -- Jeffery Chen Fan -- Bhavesh Odedra -- `Tecnativa `__: +- Jeremy Co Kim Len +- Jeffery Chen Fan +- Bhavesh Odedra +- `Tecnativa `__: - - Jairo Llopis + - Jairo Llopis -- `GlodoUK `__: +- `GlodoUK `__: - - Karl Southern + - Karl Southern -- `TAKOBI `__: +- `TAKOBI `__: - - Lorenzo Battistini + - Lorenzo Battistini Maintainers ----------- diff --git a/auth_saml/__manifest__.py b/auth_saml/__manifest__.py index f3596d6d52..1146e54fbf 100644 --- a/auth_saml/__manifest__.py +++ b/auth_saml/__manifest__.py @@ -4,7 +4,7 @@ { "name": "SAML2 Authentication", - "version": "17.0.1.0.0", + "version": "17.0.1.0.1", "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 412b5c6994..e7514d1092 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 @@ -158,27 +171,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 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. diff --git a/auth_saml/tests/test_pysaml.py b/auth_saml/tests/test_pysaml.py index 9eedaa5405..78d87b0b20 100644 --- a/auth_saml/tests/test_pysaml.py +++ b/auth_saml/tests/test_pysaml.py @@ -133,17 +133,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, @@ -151,6 +145,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" @@ -261,6 +262,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."""