[18.0][MIG] auth_saml: Migration to 18.0

auth_saml/__manifest__.py

@@ -4,7 +4,7 @@
 
 {
     "name": "SAML2 Authentication",
-    "version": "17.0.1.0.0",
+    "version": "18.0.1.0.0",
     "category": "Tools",
     "author": "XCG Consulting, Odoo Community Association (OCA)",
     "maintainers": ["vincent-hatakeyama"],

auth_saml/controllers/main.py

@@ -25,6 +25,7 @@
 from odoo.tools.misc import clean_context
 
 from odoo.addons.web.controllers.home import Home
+from odoo.addons.web.controllers.session import Session
 from odoo.addons.web.controllers.utils import _get_login_redirect_url, ensure_db
 
 _logger = logging.getLogger(__name__)
@@ -241,7 +242,13 @@ def signin(self, **kw):
                 url = f"/#action={action}"
             elif menu:
                 url = f"/#menu_id={menu}"
-            pre_uid = request.session.authenticate(*credentials)
+
+            credentials_dict = {
+                "login": credentials[1],
+                "token": credentials[2],
+                "type": "saml_token",
+            }
+            pre_uid = request.session.authenticate(dbname, credentials_dict)
             resp = request.redirect(_get_login_redirect_url(pre_uid, url), 303)
             resp.autocorrect_location_header = False
             return resp
@@ -291,3 +298,22 @@ def saml_metadata(self, **kw):
                 ),
                 [("Content-Type", "text/xml")],
             )
+
+
+class SessionSAML(Session):
+    @http.route("/web/session/logout", type="http", auth="none", readonly=True)
+    def logout(self, redirect="/odoo"):
+        saml_user = (
+            request.env["res.users.saml"]
+            .sudo()
+            .search(
+                [
+                    ("user_id", "=", request.env.user.id),
+                    ("saml_access_token", "!=", False),
+                ]
+            )
+        )
+        if saml_user:
+            _logger.info("Delete saml token")
+            saml_user.saml_access_token = False
+        return super().logout(redirect=redirect)

auth_saml/models/auth_saml_provider.py

@@ -164,7 +164,7 @@ def _compute_sp_metadata_url(self):
             qs = urllib.parse.urlencode({"p": record.id, "d": self.env.cr.dbname})
 
             record.sp_metadata_url = urllib.parse.urljoin(
-                base_url, f"/auth_saml/metadata?{qs}"
+                base_url, (f"/auth_saml/metadata?{qs}")
             )
 
     def _get_cert_key_path(self, field="sp_pem_public"):
@@ -277,6 +277,38 @@ def _get_auth_request(self, extra_state=None, url_root=None):
 
         return redirect_url
 
+    def _logout(self, saml_user, redirect, url_root=None):
+        """
+        build a logout request and give it back to our client
+        """
+        state = {
+            "d": self.env.cr.dbname,
+            "p": self.id,
+        }
+
+        sig_alg = ds.SIG_RSA_SHA1
+        if self.sig_alg:
+            sig_alg = getattr(ds, self.sig_alg)
+
+        saml_client = self._get_client_for_provider(url_root)
+        reqid, info = saml_client.prepare_for_logout(
+            sign=self.sign_authenticate_requests,
+            relay_state=json.dumps(state),
+            sigalg=sig_alg,
+        )
+
+        redirect_url = None
+        # Select the IdP URL to send the AuthN request to
+        for key, value in info["headers"]:
+            if key == "Location":
+                redirect_url = value
+
+        self._store_outstanding_request(reqid)
+
+        saml_user.saml_access_token = None
+
+        return redirect_url
+
     def _validate_auth_response(self, token: str, base_url: str = None):
         """return the validation data corresponding to the access token"""
         self.ensure_one()

auth_saml/models/res_users.py

@@ -76,18 +76,24 @@ def auth_saml(self, provider: int, saml_response: str, base_url: str = None):
         # return user credentials
         return self.env.cr.dbname, login, saml_response
 
-    def _check_credentials(self, password, env):
+    def _check_credentials(self, credential, env):
         """Override to handle SAML auths.
 
         The token can be a password if the user has used the normal form...
         but we are more interested in the case when they are tokens
         and the interesting code is inside the "except" clause.
         """
         try:
-            # Attempt a regular login (via other auth addons) first.
-            return super()._check_credentials(password, env)
+            if self.allow_saml_and_password():
+                # If both SAML and password are allowed we can try first the normal auth
+                return super()._check_credentials(credential, env)
+            else:
+                # If only SAML we go to the except clause
+                raise AccessDenied() from None
 
         except (AccessDenied, passlib.exc.PasswordSizeError):
+            if not (credential["type"] == "saml_token" and credential["token"]):
+                raise
             passwd_allowed = (
                 env["interactive"] or not self.env.user._rpc_api_keys_only()
             )
@@ -100,12 +106,16 @@ def _check_credentials(self, password, env):
                     .search(
                         [
                             ("user_id", "=", self.env.user.id),
-                            ("saml_access_token", "=", password),
+                            ("saml_access_token", "=", credential["token"]),
                         ]
                     )
                 )
                 if token:
-                    return
+                    return {
+                        "uid": self.env.user.id,
+                        "auth_method": "saml",
+                        "mfa": "default",
+                    }
             raise AccessDenied() from None
 
     @api.model

auth_saml/tests/test_pysaml.py

@@ -125,7 +125,7 @@ def test__compute_sp_metadata_url__provider_has_sp_baseurl(self):
             {"p": self.saml_provider.id, "d": self.env.cr.dbname}
         )
         expected_url = urllib.parse.urljoin(
-            "http://example.com", f"/auth_saml/metadata?{expected_qs}"
+            "http://example.com", (f"/auth_saml/metadata?{expected_qs}")
         )
         # Assert that sp_metadata_url is set correctly
         self.assertEqual(self.saml_provider.sp_metadata_url, expected_url)
@@ -332,16 +332,14 @@ def test_disallow_user_password_no_password_set(self):
         with self.assertRaises(ValidationError):
             user.password = "new password"
 
-    def test_disallow_user_password(self):
+    def test_disallow_user_password_on_option_disable(self):
         """Test that existing user password is deleted when adding an SAML provider when
         the disallow option is set."""
+        self.authenticate(user="[email protected]", password="Lu,ums-7vRU>0i]=YDLa")
         # change the option
         self.browse_ref(
             "auth_saml.allow_saml_uid_and_internal_password"
         ).value = "False"
-        # Test that existing user password is deleted when adding an SAML provider
-        self.authenticate(user="[email protected]", password="Lu,ums-7vRU>0i]=YDLa")
-        self.add_provider_to_user()
         with self.assertRaises(AccessDenied):
             self.authenticate(user="[email protected]", password="Lu,ums-7vRU>0i]=YDLa")
 

auth_saml/views/auth_saml.xml

@@ -48,12 +48,12 @@
         <field name="name">auth.saml.provider.list</field>
         <field name="model">auth.saml.provider</field>
         <field name="arch" type="xml">
-            <tree decoration-muted="not active">
+            <list decoration-muted="not active">
                 <field name="sequence" widget="handle" />
                 <field name="name" />
                 <field name="active" widget="boolean_toggle" />
                 <field name="autoredirect" />
-            </tree>
+            </list>
         </field>
     </record>
     <record model="ir.ui.view" id="view_saml_provider_form">
@@ -168,10 +168,10 @@
                                     name="attribute_mapping_ids"
                                     context="{'default_provider_id': id}"
                                 >
-                                    <tree editable="bottom">
+                                    <list editable="bottom">
                                         <field name="attribute_name" />
                                         <field name="field_name" widget="selection" />
-                                    </tree>
+                                    </list>
                                 </field>
                                 <p class="help small">
                                     Mapped attributes are copied from the SAML response at every logon, if available. If multiple values are returned (i.e. a list) then the first value is used.
@@ -191,7 +191,7 @@
     <record model="ir.actions.act_window" id="action_saml_provider">
         <field name="name">Providers</field>
         <field name="res_model">auth.saml.provider</field>
-        <field name="view_mode">tree,form</field>
+        <field name="view_mode">list,form</field>
     </record>
     <menuitem
         id="menu_saml_providers"

auth_saml/views/res_users.xml

@@ -11,10 +11,10 @@
                 <page string="SAML">
                     <group>
                         <field name="saml_ids" nolabel="1" colspan="2">
-                            <tree editable="bottom">
+                            <list editable="bottom">
                                 <field name="saml_provider_id" />
                                 <field name="saml_uid" />
-                            </tree>
+                            </list>
                         </field>
                     </group>
                 </page>