Merge pull request #548 from OCA/17.0

Syncing from upstream OCA/server-auth (17.0)

Author: bt-admin

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

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 <https://xcg-consulting.fr/>`__:
+- `XCG Consulting <https://xcg-consulting.fr/>`__:
 
-   -  Florent Aide <[email protected]>
-   -  Vincent Hatakeyama <[email protected]>
-   -  Alexandre Brun
-   -  Houzéfa Abbasbhay <[email protected]>
-   -  Szeka Wong <[email protected]>
+  - Florent Aide <[email protected]>
+  - Vincent Hatakeyama <[email protected]>
+  - Alexandre Brun
+  - Houzéfa Abbasbhay <[email protected]>
+  - Szeka Wong <[email protected]>
 
--  Jeremy Co Kim Len <[email protected]>
--  Jeffery Chen Fan <[email protected]>
--  Bhavesh Odedra <[email protected]>
--  `Tecnativa <https://www.tecnativa.com/>`__:
+- Jeremy Co Kim Len <[email protected]>
+- Jeffery Chen Fan <[email protected]>
+- Bhavesh Odedra <[email protected]>
+- `Tecnativa <https://www.tecnativa.com/>`__:
 
-   -  Jairo Llopis
+  - Jairo Llopis
 
--  `GlodoUK <https://www.glodo.uk/>`__:
+- `GlodoUK <https://www.glodo.uk/>`__:
 
-   -  Karl Southern
+  - Karl Southern
 
--  `TAKOBI <https://takobi.online/>`__:
+- `TAKOBI <https://takobi.online/>`__:
 
-   -  Lorenzo Battistini
+  - Lorenzo Battistini
 
 Maintainers
 -----------

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"],

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()

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.

auth_saml/tests/test_pysaml.py

@@ -133,24 +133,25 @@ 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": "[email protected]", "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,
                 {"attribute_name": "nick_name", "field_name": "name"},
             ),  # 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": "[email protected]", "givenName": "New", "last_name": "User"}
+        )
+        self._add_mapping_to_provider()
         # Call the method
         result = self.saml_provider._hook_validate_auth_response(
             fake_response, "[email protected]"
@@ -261,6 +262,17 @@ def test_login_with_saml(self):
         # User should now be able to log in with the token
         self.authenticate(user="[email protected]", 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, "[email protected]")
+        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, "[email protected]")
+
     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."""