[IMP] auth_saml: pre-commit auto fixes

OCA · Nov 10, 2024 · f23be80 · f23be80
1 parent c81cf43
commit f23be80
Show file tree

Hide file tree

Showing 10 changed files with 72 additions and 59 deletions.
diff --git a/auth_saml/README.rst b/auth_saml/README.rst
@@ -17,13 +17,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/17.0/auth_saml
+    :target: https://github.com/OCA/server-auth/tree/18.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-17-0/server-auth-17-0-auth_saml
+    :target: https://translation.odoo-community.org/projects/server-auth-18-0/server-auth-18-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=17.0
+    :target: https://runboat.odoo-community.org/builds?repo=OCA/server-auth&target_branch=18.0
     :alt: Try me on Runboat
 
 |badge1| |badge2| |badge3| |badge4| |badge5|
@@ -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,7 +91,7 @@ login screen.
 Known issues / Roadmap
 ======================
 
--  clean up ``auth_saml.request``
+- clean up ``auth_saml.request``
 
 Changelog
 =========
@@ -107,7 +107,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:%2017.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:%2018.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.
 
@@ -122,28 +122,28 @@ Authors
 Contributors
 ------------
 
--  `XCG Consulting <https://xcg-consulting.fr/>`__:
+- `XCG Consulting <https://xcg-consulting.fr/>`__:
 
-   -  Florent Aide <florent.aide@xcg-consulting.fr>
-   -  Vincent Hatakeyama <vincent.hatakeyama@xcg-consulting.fr>
-   -  Alexandre Brun
-   -  Houzéfa Abbasbhay <houzefa.abba@xcg-consulting.fr>
-   -  Szeka Wong <szeka.wong@xcg-consulting.fr>
+  - Florent Aide <florent.aide@xcg-consulting.fr>
+  - Vincent Hatakeyama <vincent.hatakeyama@xcg-consulting.fr>
+  - Alexandre Brun
+  - Houzéfa Abbasbhay <houzefa.abba@xcg-consulting.fr>
+  - Szeka Wong <szeka.wong@xcg-consulting.fr>
 
--  Jeremy Co Kim Len <jeremy.cokimlen@vinci-concessions.com>
--  Jeffery Chen Fan <jeffery9@gmail.com>
--  Bhavesh Odedra <bodedra@opensourceintegrators.com>
--  `Tecnativa <https://www.tecnativa.com/>`__:
+- Jeremy Co Kim Len <jeremy.cokimlen@vinci-concessions.com>
+- Jeffery Chen Fan <jeffery9@gmail.com>
+- Bhavesh Odedra <bodedra@opensourceintegrators.com>
+- `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
 -----------
@@ -166,6 +166,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/17.0/auth_saml>`_ project on GitHub.
+This module is part of the `OCA/server-auth <https://github.com/OCA/server-auth/tree/18.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/__manifest__.py b/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"],

diff --git a/auth_saml/controllers/main.py b/auth_saml/controllers/main.py
@@ -100,7 +100,7 @@ def _auth_saml_request_link(self, provider: models.Model):
         redirect = request.params.get("redirect")
         if redirect:
             params["redirect"] = redirect
-        return "/auth_saml/get_auth_request?%s" % werkzeug.urls.url_encode(params)
+        return f"/auth_saml/get_auth_request?{werkzeug.urls.url_encode(params)}"
 
     @http.route()
     def web_client(self, s_action=None, **kw):
@@ -238,10 +238,16 @@ def signin(self, **kw):
             if redirect:
                 url = redirect
             elif action:
-                url = "/#action=%s" % action
+                url = f"/#action={action}"
             elif menu:
-                url = "/#menu_id=%s" % menu
-            pre_uid = request.session.authenticate(*credentials)
+                url = f"/#menu_id={menu}"
+
+            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

diff --git a/auth_saml/models/auth_saml_provider.py b/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, ("/auth_saml/metadata?%s" % qs)
+                base_url, (f"/auth_saml/metadata?{qs}")
             )
 
     def _get_cert_key_path(self, field="sp_pem_public"):

diff --git a/auth_saml/models/res_users.py b/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

diff --git a/auth_saml/tests/fake_idp.py b/auth_saml/tests/fake_idp.py
@@ -25,20 +25,18 @@
         "aa": {
             "endpoints": {
                 "attribute_service": [
-                    ("%s/aap" % BASE, BINDING_HTTP_POST),
+                    (f"{BASE}/aap", BINDING_HTTP_POST),
                 ]
             },
         },
         "aq": {
-            "endpoints": {
-                "authn_query_service": [("%s/aqs" % BASE, BINDING_HTTP_POST)]
-            },
+            "endpoints": {"authn_query_service": [(f"{BASE}/aqs", BINDING_HTTP_POST)]},
         },
         "idp": {
             "endpoints": {
                 "single_sign_on_service": [
-                    ("%s/sso/redirect" % BASE, BINDING_HTTP_REDIRECT),
-                    ("%s/sso/post" % BASE, BINDING_HTTP_POST),
+                    (f"{BASE}/sso/redirect", BINDING_HTTP_REDIRECT),
+                    (f"{BASE}/sso/post", BINDING_HTTP_POST),
                 ],
             },
             "policy": {
@@ -87,7 +85,7 @@ def _unpack(self, ver="SAMLResponse"):
         """
         _str = self.text
 
-        sr_str = 'name="%s" value="' % ver
+        sr_str = f'name="{ver}" value="'
         rs_str = 'name="RelayState" value="'
 
         i = _str.find(sr_str)

diff --git a/auth_saml/tests/test_pysaml.py b/auth_saml/tests/test_pysaml.py
@@ -92,10 +92,8 @@ def test_ensure_provider_appears_on_login_with_redirect_param(self):
         )
         self.assertIn("Login with Authentic", response.text)
         self.assertIn(
-            "/auth_saml/get_auth_request?pid={}&amp;redirect=%2Fweb%23action%3D37%26mod"
-            "el%3Dir.module.module%26view_type%3Dkanban%26menu_id%3D5".format(
-                self.saml_provider.id
-            ),
+            f"/auth_saml/get_auth_request?pid={self.saml_provider.id}&amp;redirect=%2Fweb%23action%3D37%26mod"
+            "el%3Dir.module.module%26view_type%3Dkanban%26menu_id%3D5",
             response.text,
         )
 
@@ -127,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", ("/auth_saml/metadata?%s" % 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)

diff --git a/auth_saml/views/auth_saml.xml b/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"

diff --git a/auth_saml/views/res_users.xml b/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>

diff --git a/requirements.txt b/requirements.txt
@@ -1,2 +1,3 @@
 # generated from manifests external_dependencies
+pysaml2
 python-jose