Merge PR #483 into 15.0

Signed-off-by vincent-hatakeyama

auth_saml/controllers/main.py

@@ -1,12 +1,13 @@
 # Copyright (C) 2020 GlodoUK <https://www.glodo.uk/>
-# Copyright (C) 2010-2016, 2022 XCG Consulting <https://xcg-consulting.fr/>
+# Copyright (C) 2010-2016, 2022-2023 XCG Consulting <https://xcg-consulting.fr/>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
 
 import functools
 import json
 import logging
 
 import werkzeug.utils
+from werkzeug.exceptions import BadRequest
 from werkzeug.urls import url_quote_plus
 
 import odoo
@@ -54,7 +55,7 @@ def wrapper(self, req, **kw):
 
 
 class SAMLLogin(Home):
-    def _list_saml_providers_domain(self):
+    def _list_saml_providers_domain(self):  # pylint: disable=no-self-use
         return []
 
     def list_saml_providers(self, with_autoredirect: bool = False) -> models.Model:
@@ -67,7 +68,8 @@ def list_saml_providers(self, with_autoredirect: bool = False) -> models.Model:
         if with_autoredirect:
             domain.append(("autoredirect", "=", True))
         providers = request.env["auth.saml.provider"].sudo().search_read(domain)
-
+        for provider in providers:
+            provider["auth_link"] = self._auth_saml_request_link(provider)
         return providers
 
     def _saml_autoredirect(self):
@@ -79,11 +81,21 @@ def _saml_autoredirect(self):
         )
         if autoredirect_providers and not disable_autoredirect:
             return werkzeug.utils.redirect(
-                "/auth_saml/get_auth_request?pid=%d" % autoredirect_providers[0]["id"],
+                self._auth_saml_request_link(autoredirect_providers[0]),
                 303,
             )
         return None
 
+    def _auth_saml_request_link(self, provider: models.Model):
+        """Return the auth request link for the provided provider"""
+        params = {
+            "pid": provider["id"],
+        }
+        redirect = request.params.get("redirect")
+        if redirect:
+            params["redirect"] = redirect
+        return "/auth_saml/get_auth_request?%s" % werkzeug.urls.url_encode(params)
+
     @http.route()
     def web_client(self, s_action=None, **kw):
         ensure_db()
@@ -144,7 +156,6 @@ def _get_saml_extra_relaystate(self):
         The provider will automatically set things like the dbname, provider
         id, etc.
         """
-
         redirect = request.params.get("redirect") or "web"
         if not redirect.startswith(("//", "http://", "https://")):
             redirect = "{}{}".format(
@@ -199,6 +210,8 @@ def signin(self, req, **kw):
         state = json.loads(kw["RelayState"])
         provider = state["p"]
         dbname = state["d"]
+        if not http.db_filter([dbname]):
+            return BadRequest()
         context = state.get("c", {})
         registry = registry_get(dbname)
 
@@ -216,8 +229,15 @@ def signin(self, req, **kw):
                 )
                 action = state.get("a")
                 menu = state.get("m")
+                redirect = (
+                    werkzeug.urls.url_unquote_plus(state["r"])
+                    if state.get("r")
+                    else False
+                )
                 url = "/"
-                if action:
+                if redirect:
+                    url = redirect
+                elif action:
                     url = "/#action=%s" % action
                 elif menu:
                     url = "/#menu_id=%s" % menu

auth_saml/models/auth_saml_provider.py

@@ -191,7 +191,7 @@ def _get_cert_key_path(self, field="sp_pem_public"):
 
         return keys_path
 
-    def _get_config_for_provider(self, base_url: str = None):
+    def _get_config_for_provider(self, base_url: str = None) -> Saml2Config:
         """
         Internal helper to get a configured Saml2Client
         """
@@ -237,7 +237,7 @@ def _get_config_for_provider(self, base_url: str = None):
         sp_config.allow_unknown_attributes = True
         return sp_config
 
-    def _get_client_for_provider(self, base_url: str = None):
+    def _get_client_for_provider(self, base_url: str = None) -> Saml2Client:
         sp_config = self._get_config_for_provider(base_url)
         saml_client = Saml2Client(config=sp_config)
         return saml_client
@@ -336,7 +336,7 @@ def _store_outstanding_request(self, reqid):
             {"saml_provider_id": self.id, "saml_request_id": reqid}
         )
 
-    def _metadata_string(self, valid=None, base_url=None):
+    def _metadata_string(self, valid=None, base_url: str = None):
         self.ensure_one()
 
         sp_config = self._get_config_for_provider(base_url)

auth_saml/readme/HISTORY.rst

@@ -1,3 +1,13 @@
+15.0.1.4.0
+~~~~~~~~~~
+
+Handle redirect after authentification.
+
+15.0.1.3.0
+~~~~~~~~~~
+
+Improve login page.
+
 15.0.1.1.0
 ~~~~~~~~~~
 

auth_saml/tests/test_pysaml.py

@@ -1,5 +1,8 @@
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
 import base64
+import html
 import os
+from unittest.mock import patch
 
 from odoo.exceptions import AccessDenied, UserError, ValidationError
 from odoo.tests import HttpCase, tagged
@@ -85,6 +88,21 @@ def test_ensure_provider_appears_on_login(self):
         self.assertIn("Login with Authentic", response.text)
         self.assertIn(self.url_saml_request, response.text)
 
+    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"
+        )
+        self.assertIn("Login with Authentic", response.text)
+        self.assertIn(
+            "/auth_saml/get_auth_request?pid={}&redirect=%2Fweb%23action%3D37%26mod"
+            "el%3Dir.module.module%26view_type%3Dkanban%26menu_id%3D5".format(
+                self.saml_provider.id
+            ),
+            response.text,
+        )
+
     def test_ensure_metadata_present(self):
         response = self.url_open(
             "/auth_saml/metadata?p=%d&d=%s"
@@ -96,7 +114,7 @@ def test_ensure_metadata_present(self):
 
     def test_ensure_get_auth_request_redirects(self):
         response = self.url_open(
-            "/auth_saml/get_auth_request?pid=%d" % (self.saml_provider.id),
+            "/auth_saml/get_auth_request?pid=%d" % self.saml_provider.id,
             allow_redirects=False,
         )
         self.assertTrue(response.ok)
@@ -160,14 +178,15 @@ def test_login_with_saml(self):
         self.assertEqual(200, response.status_code)
         unpacked_response = response._unpack()
 
-        (_database, login, token) = (
+        (database, login, token) = (
             self.env["res.users"]
             .sudo()
             .auth_saml(
                 self.saml_provider.id, unpacked_response.get("SAMLResponse"), None
             )
         )
 
+        self.assertEqual(database, self.env.cr.dbname)
         self.assertEqual(login, self.user.login)
 
         # We should not be able to log in with the wrong token
@@ -273,3 +292,47 @@ def test_disallow_user_admin_can_have_password(self):
         ).value = "False"
         # Test base.user_admin exception
         self.env.ref("base.user_admin").password = "nNRST4j*->sEatNGg._!"
+
+    def test_db_filtering(self):
+        # change filter to only allow our db.
+        with patch("odoo.http.db_filter", new=lambda *args, **kwargs: []):
+            self.add_provider_to_user()
+
+            redirect_url = self.saml_provider._get_auth_request()
+            response = self.idp.fake_login(redirect_url)
+            unpacked_response = response._unpack()
+
+            for key in unpacked_response:
+                unpacked_response[key] = html.unescape(unpacked_response[key])
+            response = self.url_open("/auth_saml/signin", data=unpacked_response)
+            self.assertFalse(response.ok)
+            self.assertIn(response.status_code, [400, 404])
+
+    def test_redirect_after_login(self):
+        """Test that providing a redirect will be kept after SAML login."""
+        self.add_provider_to_user()
+
+        redirect_url = self.saml_provider._get_auth_request(
+            {
+                "r": "%2Fweb%23action%3D37%26model%3Dir.module.module%26view_type%3Dkan"
+                "ban%26menu_id%3D5"
+            }
+        )
+        response = self.idp.fake_login(redirect_url)
+        unpacked_response = response._unpack()
+
+        for key in unpacked_response:
+            unpacked_response[key] = html.unescape(unpacked_response[key])
+        response = self.url_open(
+            "/auth_saml/signin",
+            data=unpacked_response,
+            allow_redirects=True,
+            timeout=300,
+        )
+        self.assertTrue(response.ok)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response.url,
+            self.base_url()
+            + "/web#action=37&model=ir.module.module&view_type=kanban&menu_id=5",
+        )

auth_saml/views/auth_saml.xml

@@ -11,7 +11,7 @@
                     t-foreach="saml_providers"
                     t-as="p"
                     class="list-group-item list-group-item-action py-2"
-                    t-att-href="'/auth_saml/get_auth_request?pid=%s'%p['id']"
+                    t-att-href="p['auth_link']"
                 >
                     <i t-att-class="p['css_class']" />
                     <t t-esc="p['body']" />