[MIG] auth_saml: Migration to 19.0

vincent-hatakeyama · vincent-hatakeyama · commit 57bc71ba2e90 · 2026-04-02T16:36:39.000+02:00
diff --git a/auth_saml/README.rst b/auth_saml/README.rst
@@ -21,13 +21,13 @@ SAML2 Authentication
     :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
-    :target: https://github.com/OCA/server-auth/tree/18.0/auth_saml
+    :target: https://github.com/OCA/server-auth/tree/19.0/auth_saml
     :alt: OCA/server-auth
 .. |badge4| image:: https://img.shields.io/badge/weblate-Translate%20me-F47D42.png
-    :target: https://translation.odoo-community.org/projects/server-auth-18-0/server-auth-18-0-auth_saml
+    :target: https://translation.odoo-community.org/projects/server-auth-19-0/server-auth-19-0-auth_saml
     :alt: Translate me on Weblate
 .. |badge5| image:: https://img.shields.io/badge/runboat-Try%20me-875A7B.png
-    :target: https://runboat.odoo-community.org/builds?repo=OCA/server-auth&target_branch=18.0
+    :target: https://runboat.odoo-community.org/builds?repo=OCA/server-auth&target_branch=19.0
     :alt: Try me on Runboat
 
 |badge1| |badge2| |badge3| |badge4| |badge5|
@@ -133,7 +133,7 @@ Bug Tracker
 Bugs are tracked on `GitHub Issues <https://github.com/OCA/server-auth/issues>`_.
 In case of trouble, please check there if your issue has already been reported.
 If you spotted it first, help us to smash it by providing a detailed and welcomed
-`feedback <https://github.com/OCA/server-auth/issues/new?body=module:%20auth_saml%0Aversion:%2018.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.
+`feedback <https://github.com/OCA/server-auth/issues/new?body=module:%20auth_saml%0Aversion:%2019.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.
 
 Do not contact contributors directly about support or help with technical issues.
 
@@ -192,6 +192,6 @@ Current `maintainer <https://odoo-community.org/page/maintainer-role>`__:
 
 |maintainer-vincent-hatakeyama| 
 
-This module is part of the `OCA/server-auth <https://github.com/OCA/server-auth/tree/18.0/auth_saml>`_ project on GitHub.
+This module is part of the `OCA/server-auth <https://github.com/OCA/server-auth/tree/19.0/auth_saml>`_ project on GitHub.
 
 You are welcome to contribute. To learn how please visit https://odoo-community.org/page/Contribute.
diff --git a/auth_saml/__init__.py b/auth_saml/__init__.py
@@ -1 +1 @@
-from . import controllers, models
+from . import controllers, models, wizards
diff --git a/auth_saml/__manifest__.py b/auth_saml/__manifest__.py
@@ -1,18 +1,18 @@
 # Copyright (C) 2020 GlodoUK <https://www.glodo.uk/>
-# Copyright (C) 2010-2016, 2022 XCG Consulting <http://odoo.consulting>
+# Copyright (C) 2010-2016, 2022 XCG Consulting <https://orbeet.io/>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
 
 {
     "name": "SAML2 Authentication",
-    "version": "18.0.1.1.0",
+    "version": "19.0.1.0.0",
     "category": "Tools",
     "author": "XCG Consulting, Odoo Community Association (OCA)",
     "maintainers": ["vincent-hatakeyama"],
     "website": "https://github.com/OCA/server-auth",
     "license": "AGPL-3",
     "depends": ["base_setup", "web"],
     "external_dependencies": {
-        "python": ["pysaml2"],
+        "python": ["pysaml2", "responses"],
         "bin": ["xmlsec1"],
         # special definition used by OCA to install packages
         "deb": ["xmlsec1"],
diff --git a/auth_saml/controllers/main.py b/auth_saml/controllers/main.py
@@ -12,14 +12,13 @@
 
 from odoo import (
     SUPERUSER_ID,
-    _,
     api,
     exceptions,
     http,
     models,
     modules,
 )
-from odoo.http import request
+from odoo.http import Response, request
 from odoo.tools.misc import clean_context
 
 from odoo.addons.web.controllers.home import Home
@@ -35,19 +34,23 @@
 
 def fragment_to_query_string(func):
     @functools.wraps(func)
-    def wrapper(self, **kw):
+    def wrapper(self, *a, **kw):
+        kw.pop("debug", False)
         if not kw:
-            return """<html><head><script>
+            return Response("""<html><head><script>
                 var l = window.location;
                 var q = l.hash.substring(1);
-                var r = '/' + l.search;
+                var r = l.pathname + l.search;
                 if(q.length !== 0) {
                     var s = l.search ? (l.search === '?' ? '' : '&') : '?';
                     r = l.pathname + l.search + s + q;
                 }
+                if (r == l.pathname) {
+                    r = '/';
+                }
                 window.location = r;
-            </script></head><body></body></html>"""
-        return func(self, **kw)
+            </script></head><body></body></html>""")
+        return func(self, *a, **kw)
 
     return wrapper
 
@@ -59,7 +62,7 @@ def wrapper(self, **kw):
 
 class SAMLLogin(Home):
     # Disable pylint self use as the method is meant to be reused in other modules
-    def _list_saml_providers_domain(self):  # pylint: disable=no-self-use
+    def _list_saml_providers_domain(self):
         return []
 
     def list_saml_providers(self, with_autoredirect: bool = False) -> models.Model:
@@ -131,11 +134,11 @@ def web_login(self, *args, **kw):
         if response.is_qweb:
             error = request.params.get("saml_error")
             if error == "no-signup":
-                error = _("Sign up is not allowed on this database.")
+                error = request.env._("Sign up is not allowed on this database.")
             elif error == "access-denied":
-                error = _("Access Denied")
+                error = request.env._("Access Denied")
             elif error == "expired":
-                error = _(
+                error = request.env._(
                     "You do not have access to this database. Please contact support."
                 )
             else:
@@ -179,8 +182,10 @@ def get_auth_request(self, pid):
         )
         if not redirect_url:
             raise Exception(
-                "Failed to get auth request from provider. "
-                "Either misconfigured SAML provider or unknown provider."
+                request.env._(
+                    "Failed to get auth request from provider. "
+                    "Either misconfigured SAML provider or unknown provider."
+                )
             )
 
         redirect = werkzeug.utils.redirect(redirect_url, 303)
@@ -227,25 +232,28 @@ def signin(self, **kw):
                     request.httprequest.url_root.rstrip("/"),
                 )
             )
+            # The response needs to be saved otherwise login does not work.
+            # auth_oauth has a similar code
+            request.env.cr.commit()
             action = state.get("a")
             menu = state.get("m")
             redirect = (
                 werkzeug.urls.url_unquote_plus(state["r"]) if state.get("r") else False
             )
-            url = "/web"
+            url = "/odoo"
             if redirect:
                 url = redirect
             elif action:
-                url = f"/#action={action}"
+                url = f"/odoo/action-{action}"
             elif menu:
-                url = f"/#menu_id={menu}"
+                url = f"/odoo?menu_id={menu}"
 
             credentials_dict = {
                 "login": credentials[1],
                 "token": credentials[2],
                 "type": "saml_token",
             }
-            auth_info = request.session.authenticate(dbname, credentials_dict)
+            auth_info = request.session.authenticate(request.env, credentials_dict)
             resp = request.redirect(_get_login_redirect_url(auth_info["uid"], url), 303)
             resp.autocorrect_location_header = False
             return resp
@@ -277,15 +285,15 @@ def saml_metadata(self, **kw):
 
         if not dbname or not provider:
             _logger.debug("Metadata page asked without database name or provider id")
-            raise request.not_found(_("Missing parameters"))
+            raise request.not_found(request.env._("Missing parameters"))
 
         provider = int(provider)
 
         with modules.registry.Registry(dbname).cursor() as cr:
             env = api.Environment(cr, SUPERUSER_ID, {})
             client = env["auth.saml.provider"].sudo().browse(provider)
             if not client.exists():
-                raise request.not_found(_("Unknown provider"))
+                raise request.not_found(request.env._("Unknown provider"))
 
             return request.make_response(
                 client._metadata_string(
diff --git a/auth_saml/models/__init__.py b/auth_saml/models/__init__.py
@@ -3,7 +3,6 @@
 from . import (
     auth_saml_attribute_mapping,
     auth_saml_provider,
-    auth_saml_request,
     ir_config_parameter,
     res_config_settings,
     res_users,
diff --git a/auth_saml/models/auth_saml_provider.py b/auth_saml/models/auth_saml_provider.py
@@ -10,8 +10,6 @@
 import urllib
 
 import requests
-
-# dependency name is pysaml2 # pylint: disable=W7936
 import saml2
 import saml2.xmldsig as ds
 from saml2.client import Saml2Client
@@ -165,7 +163,7 @@ def _compute_sp_metadata_url(self):
         )
 
         for record in self:
-            if isinstance(record.id, models.NewId):
+            if not record.id:
                 record.sp_metadata_url = False
                 continue
 
@@ -335,8 +333,12 @@ def _validate_auth_response(self, token: str, base_url: str = None):
 
             if not matching_value:
                 raise Exception(
-                    f"Matching attribute {self.matching_attribute} not found "
-                    f"in user attrs: {attrs}"
+                    self.env._(
+                        "Matching attribute %(matching_attribute)s"
+                        " not found in user attrs: %(attrs)s",
+                        matching_attribute=self.matching_attribute,
+                        attrs=attrs,
+                    )
                 )
 
         if matching_value and isinstance(matching_value, list):
@@ -417,8 +419,12 @@ def action_refresh_metadata_from_url(self):
             document = requests.get(provider.idp_metadata_url, timeout=5)
             if document.status_code != 200:
                 raise UserError(
-                    "Unable to download the metadata for "
-                    f"{provider.name}: {document.reason}"
+                    self.env._(
+                        "Unable to download the metadata for"
+                        " %(provider_name)s: %(reason)s",
+                        provider_name=provider.name,
+                        reason=document.reason,
+                    )
                 )
             if document.text != provider.idp_metadata:
                 providers_to_update[provider.id] = document.text
diff --git a/auth_saml/models/res_users.py b/auth_saml/models/res_users.py
@@ -7,7 +7,7 @@
 
 import passlib
 
-from odoo import SUPERUSER_ID, _, api, fields, models, modules, tools
+from odoo import SUPERUSER_ID, api, fields, models, modules, tools
 from odoo.exceptions import AccessDenied, ValidationError
 
 from .ir_config_parameter import ALLOW_SAML_UID_AND_PASSWORD
@@ -153,21 +153,22 @@ def _set_password(self):
         # And also to be able to set password to a blank value
         if not self.allow_saml_and_password():
             saml_users = self.filtered(
-                lambda user: user.sudo().saml_ids
-                and user.id not in self._saml_allowed_user_ids()
-                and user.password
+                lambda user: (
+                    user.sudo().saml_ids
+                    and user.id not in self._saml_allowed_user_ids()
+                    and user.password
+                )
             )
             if saml_users:
                 # same error as an api.constrains because it is a constraint.
                 # a standard constrains would require the use of SQL as the password
                 # field is obfuscated by the base module.
                 raise ValidationError(
-                    _(
-                        "This database disallows users to "
-                        "have both passwords and SAML IDs. "
-                        "Error for logins %s"
+                    self.env._(
+                        "This database disallows users to have both "
+                        "passwords and SAML IDs. Error for logins %s",
+                        saml_users.mapped("login"),
                     )
-                    % saml_users.mapped("login")
                 )
         # handle setting password to NULL
         blank_password_users = self.filtered(lambda user: user.password is False)
diff --git a/auth_saml/tests/test_pysaml.py b/auth_saml/tests/test_pysaml.py
@@ -5,6 +5,7 @@
 import os.path as osp
 import urllib
 from copy import deepcopy
+from unittest import SkipTest
 from unittest.mock import patch
 
 import responses
@@ -49,7 +50,7 @@ def setUp(self):
             }
         )
         self.url_saml_request = (
-            "/auth_saml/get_auth_request?pid=%d" % self.saml_provider.id
+            f"/auth_saml/get_auth_request?pid={self.saml_provider.id}"
         )
 
         self.idp = FakeIDP([self.saml_provider._metadata_string()])
@@ -102,8 +103,7 @@ def test_ensure_provider_appears_on_login(self):
     def test_ensure_provider_appears_on_login_with_redirect_param(self):
         """Test that SAML provider is listed in the login page keeping the redirect"""
         response = self.url_open(
-            "/web/login?redirect=%2Fweb%23action%3D37%26model%3Dir.module.module%26view"
-            "_type%3Dkanban%26menu_id%3D5"
+            "/web/login?redirect=%2Fweb%23action%3D37%26model%3Dir.module.module%26view_type%3Dkanban%26menu_id%3D5"
         )
         self.assertIn("Login with Authentic", response.text)
         self.assertIn(
@@ -184,16 +184,15 @@ def test_get_config_for_provider(self):
 
     def test_ensure_metadata_present(self):
         response = self.url_open(
-            "/auth_saml/metadata?p=%d&d=%s"
-            % (self.saml_provider.id, self.env.cr.dbname)
+            f"/auth_saml/metadata?p={self.saml_provider.id}&d={self.env.cr.dbname}"
         )
 
         self.assertTrue(response.ok)
         self.assertTrue("xml" in response.headers.get("Content-Type"))
 
     def test_ensure_get_auth_request_redirects(self):
         response = self.url_open(
-            "/auth_saml/get_auth_request?pid=%d" % self.saml_provider.id,
+            f"/auth_saml/get_auth_request?pid={self.saml_provider.id}",
             allow_redirects=False,
         )
         self.assertTrue(response.ok)
@@ -460,17 +459,20 @@ def test_disallow_user_password_when_changing_settings(self):
     def test_fragment_to_query_string_no_kw(self):
         """Test the case where no keyword arguments are passed."""
         response = self.decorated_function(self)
-        expected_html = """<html><head><script>
+        expected_html = b"""<html><head><script>
                 var l = window.location;
                 var q = l.hash.substring(1);
-                var r = '/' + l.search;
+                var r = l.pathname + l.search;
                 if(q.length !== 0) {
                     var s = l.search ? (l.search === '?' ? '' : '&') : '?';
                     r = l.pathname + l.search + s + q;
                 }
+                if (r == l.pathname) {
+                    r = '/';
+                }
                 window.location = r;
             </script></head><body></body></html>"""
-        self.assertEqual(response.strip(), expected_html.strip())
+        self.assertEqual(response.data, expected_html)
 
     def test_fragment_to_query_string_with_kw(self):
         """Test the case where keyword arguments are passed."""
@@ -648,9 +650,13 @@ def test_signin_no_relaystate_redirect(self):
 
     def test_signin_redirect_mfa(self):
         """Test redirect to mfa url"""
+        if not self.env["ir.module.module"].search(
+            [("name", "=", "auth_totp"), ("state", "=", "installed")]
+        ):
+            raise SkipTest("auth_totp not installed")
         self.add_provider_to_user()
 
-        redirect_url = self.saml_provider._get_auth_request({"a": "action"})
+        redirect_url = self.saml_provider._get_auth_request({"a": "42"})
         response = self.idp.fake_login(redirect_url)
         unpacked_response = response._unpack()
 
@@ -668,7 +674,7 @@ def test_signin_redirect_mfa(self):
         self.assertTrue(response.ok)
         self.assertEqual(
             response.url,
-            self.base_url() + "/web/login/totp?redirect=%2F%23action%3Daction",
+            self.base_url() + "/web/login/totp?redirect=%2Fodoo%2Faction-42",
         )
 
     def test_action_redirect(self):
@@ -690,7 +696,7 @@ def test_action_redirect(self):
         self.assertTrue(response.ok)
         self.assertEqual(
             response.url,
-            self.base_url() + "/odoo#action=action",
+            self.base_url() + "/odoo/action-action",
         )
 
     def test_menu_redirect(self):
@@ -712,7 +718,7 @@ def test_menu_redirect(self):
         self.assertTrue(response.ok)
         self.assertEqual(
             response.url,
-            self.base_url() + "/odoo#menu_id=12",
+            self.base_url() + "/odoo?menu_id=12",
         )
 
     @responses.activate
diff --git a/auth_saml/views/auth_saml.xml b/auth_saml/views/auth_saml.xml
diff --git a/auth_saml/wizards/__init__.py b/auth_saml/wizards/__init__.py
diff --git a/auth_saml/wizards/auth_saml_request.py b/auth_saml/wizards/auth_saml_request.py
diff --git a/requirements.txt b/requirements.txt

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-from . import controllers, models`
	`1`	`+from . import controllers, models, wizards`