From afaa42e2c3701f356db5f1cd17f95174185975f9 Mon Sep 17 00:00:00 2001 From: vrenaville Date: Thu, 8 Feb 2024 11:53:21 +0100 Subject: [PATCH] [IMP][14.0] auth_saml: download the provider metadata --- auth_saml/README.rst | 6 +- auth_saml/models/auth_saml_provider.py | 79 ++++++++++++++++++++--- auth_saml/readme/CONFIGURE.rst | 4 ++ auth_saml/tests/data/cert_idp_expired.pem | 24 +++++++ auth_saml/tests/data/key_idp_expired.pem | 28 ++++++++ auth_saml/tests/fake_idp.py | 6 +- auth_saml/tests/test_pysaml.py | 59 ++++++++++++++++- auth_saml/views/auth_saml.xml | 19 +++++- 8 files changed, 208 insertions(+), 17 deletions(-) create mode 100644 auth_saml/tests/data/cert_idp_expired.pem create mode 100644 auth_saml/tests/data/key_idp_expired.pem diff --git a/auth_saml/README.rst b/auth_saml/README.rst index 1ce3228e82..99fd7f7c02 100644 --- a/auth_saml/README.rst +++ b/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:99008eaf94a8c59ea3984b9310ddbf58dddccd1aa4164c58a973a48e9a2f8cc1 + !! source digest: sha256:881c0c4fdf9c79d45da334ccfec481c23aeaf07986bd03321c82760c3fd188d7 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! .. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png @@ -81,6 +81,10 @@ by using the query parameter ``disable_autoredirect``, as in ``https://example.com/web/login?disable_autoredirect=`` The login is also displayed if there is an error with SAML login, in order to display any error message. +If you are using Office365 as identity provider, set up the federation metadata document +rather than the document itself. This will allow the module to refresh the document when +needed. + Usage ===== diff --git a/auth_saml/models/auth_saml_provider.py b/auth_saml/models/auth_saml_provider.py index 96e1960f88..a82e6b7c59 100644 --- a/auth_saml/models/auth_saml_provider.py +++ b/auth_saml/models/auth_saml_provider.py @@ -9,13 +9,17 @@ import tempfile import urllib.parse +import requests + # dependency name is pysaml2 # pylint: disable=W7936 import saml2 import saml2.xmldsig as ds from saml2.client import Saml2Client from saml2.config import Config as Saml2Config +from saml2.sigver import SignatureError from odoo import api, fields, models +from odoo.exceptions import UserError _logger = logging.getLogger(__name__) @@ -42,6 +46,13 @@ class AuthSamlProvider(models.Model): ), required=True, ) + idp_metadata_url = fields.Char( + string="Identity Provider Metadata URL", + help="Some SAML providers, notably Office365 can have a metadata " + "document which changes over time, and they provide a URL to the " + "document instead. When this field is set, the metadata can be " + "fetched from the provided URL.", + ) sp_baseurl = fields.Text( string="Override Base URL", help="""Base URL sent to Odoo with this, rather than automatically @@ -229,10 +240,19 @@ def _get_config_for_provider(self, base_url: str = None): "cert_file": self._get_cert_key_path("sp_pem_public"), "key_file": self._get_cert_key_path("sp_pem_private"), } - sp_config = Saml2Config() - sp_config.load(settings) - sp_config.allow_unknown_attributes = True - return sp_config + try: + sp_config = Saml2Config() + sp_config.load(settings) + sp_config.allow_unknown_attributes = True + return sp_config + except saml2.SAMLError: + if self.env.context.get("saml2_retry_after_refresh_metadata", False): + raise + # Retry after refresh metadata + self.action_refresh_metadata_from_url() + return self.with_context( + saml2_retry_after_refresh_metatata=1 + )._get_config_for_provider(base_url) def _get_client_for_provider(self, base_url: str = None): sp_config = self._get_config_for_provider(base_url) @@ -278,12 +298,26 @@ def _validate_auth_response(self, token: str, base_url: str = None): """return the validation data corresponding to the access token""" self.ensure_one() - client = self._get_client_for_provider(base_url) - response = client.parse_authn_request_response( - token, - saml2.entity.BINDING_HTTP_POST, - self._get_outstanding_requests_dict(), - ) + try: + client = self._get_client_for_provider(base_url) + response = client.parse_authn_request_response( + token, + saml2.entity.BINDING_HTTP_POST, + self._get_outstanding_requests_dict(), + ) + except SignatureError: + # we have a metadata url: try to refresh the metadata document + if self.idp_metadata_url: + self.action_refresh_metadata_from_url() + # retry: if it fails again, we let the exception flow + client = self._get_client_for_provider(base_url) + response = client.parse_authn_request_response( + token, + saml2.entity.BINDING_HTTP_POST, + self._get_outstanding_requests_dict(), + ) + else: + raise matching_value = None if self.matching_attribute == "subject.nameId": @@ -367,3 +401,28 @@ def _hook_validate_auth_response(self, response, matching_value): vals[attribute.field_name] = attribute_value return {"mapped_attrs": vals} + + def action_refresh_metadata_from_url(self): + providers = self.search( + [("idp_metadata_url", "ilike", "http%"), ("id", "in", self.ids)] + ) + if not providers: + return False + # lock the records we might update, so that multiple simultaneous login + # attempts will not cause concurrent updates + self.env.cr.execute( + "SELECT id FROM auth_saml_provider WHERE id in %s FOR UPDATE", + (tuple(providers.ids),), + ) + updated = False + for provider in providers: + document = requests.get(provider.idp_metadata_url) + if document.status_code != 200: + raise UserError( + f"Unable to download the metadata for {provider.name}: {document.reason}" + ) + if document.text != provider.idp_metadata: + provider.idp_metadata = document.text + _logger.info("Updated provider metadata for %s", provider.name) + updated = True + return updated diff --git a/auth_saml/readme/CONFIGURE.rst b/auth_saml/readme/CONFIGURE.rst index 270886b630..b08476110c 100644 --- a/auth_saml/readme/CONFIGURE.rst +++ b/auth_saml/readme/CONFIGURE.rst @@ -15,3 +15,7 @@ with the highest priority. It is still possible to access the login without redi by using the query parameter ``disable_autoredirect``, as in ``https://example.com/web/login?disable_autoredirect=`` The login is also displayed if there is an error with SAML login, in order to display any error message. + +If you are using Office365 as identity provider, set up the federation metadata document +rather than the document itself. This will allow the module to refresh the document when +needed. diff --git a/auth_saml/tests/data/cert_idp_expired.pem b/auth_saml/tests/data/cert_idp_expired.pem new file mode 100644 index 0000000000..d2c320a4b9 --- /dev/null +++ b/auth_saml/tests/data/cert_idp_expired.pem @@ -0,0 +1,24 @@ +-----BEGIN CERTIFICATE----- +MIID7TCCAtWgAwIBAgIUDBX/LJ1BPZOhb2vrDnwIasyEi+AwDQYJKoZIhvcNAQEL +BQAwgYUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMQ4wDAYDVQQH +DAVQYXJpczEMMAoGA1UECgwDT0NBMQwwCgYDVQQLDANPQ0ExFDASBgNVBAMMC2V4 +YW1wbGUuY29tMR8wHQYJKoZIhvcNAQkBFhB0ZXN0QGV4YW1wbGUuY29tMB4XDTIz +MDEwMTExMDAyN1oXDTIzMDEzMTExMDAyN1owgYUxCzAJBgNVBAYTAkFVMRMwEQYD +VQQIDApTb21lLVN0YXRlMQ4wDAYDVQQHDAVQYXJpczEMMAoGA1UECgwDT0NBMQww +CgYDVQQLDANPQ0ExFDASBgNVBAMMC2V4YW1wbGUuY29tMR8wHQYJKoZIhvcNAQkB +FhB0ZXN0QGV4YW1wbGUuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC +AQEAvgeLRr1Q9aS/t8ZuC7/pZRHTB6sqamVwXyR7zh0v51yH7xBy9xs4zJWKneRn +OJw46IogYhY+dyNWElbY+Ckcc6z1eJONiHNtOKAy07VtfhisGviRv1kLE56SHGgW +fIXrOuFqj6F1yTfKyLtq2RZBzmbMTNG7z89rO2hqdTWqhyof9OGWtecrM7Ei9PnL +tqULhQyh6n47KnIXfBMLIeQG7d/WyGU5CnO7yhHkS/51E9gI6g5G0VoueBVFErCl +rjo0clMJrFVpanOG2USGgLfPkomSIv9ZL4SreFN27sbhTbkVWxbk7AOCFCQcaBIv +RThpRrA9YRv2dB/X4yIi7UrrPwIDAQABo1MwUTAdBgNVHQ4EFgQU4WFoM/SL6qvT +jV4YUwH3rggBqyIwHwYDVR0jBBgwFoAU4WFoM/SL6qvTjV4YUwH3rggBqyIwDwYD +VR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAYG+CTENdcmiKmyYg/n6H +59RRgaLHSszjsKYdM5Uy/PmwfvPTRuxP38UhF0gWJTsLxWRjI0ejzdy5vzvxpoYl +3pEyYxPzl+jfGA6AUSOfzTT/ksi7SZWak3cqJDFCdnfaCSjYyi+nKz5y6Bm1/fMy +/3zJX92ELIWc8cTUItUIWjlITmxgLhIGr1wYaCinxkaEopGqkX85RYFaWKyYa5ok +8MnoYbPrh/i9EekHUMBMPKWN3tWMMEROTtX9hmxSSTtgdQCahBaOCCU+m8PSNKEc +UA8nSStaolv8t6aOyEb/Kzs7WSbd7v1ovZsy2FYmIRn0eHz8fpMAw2qk7mol6iao +GQ== +-----END CERTIFICATE----- diff --git a/auth_saml/tests/data/key_idp_expired.pem b/auth_saml/tests/data/key_idp_expired.pem new file mode 100644 index 0000000000..496f309a12 --- /dev/null +++ b/auth_saml/tests/data/key_idp_expired.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQC+B4tGvVD1pL+3 +xm4Lv+llEdMHqypqZXBfJHvOHS/nXIfvEHL3GzjMlYqd5Gc4nDjoiiBiFj53I1YS +Vtj4KRxzrPV4k42Ic204oDLTtW1+GKwa+JG/WQsTnpIcaBZ8hes64WqPoXXJN8rI +u2rZFkHOZsxM0bvPz2s7aGp1NaqHKh/04Za15yszsSL0+cu2pQuFDKHqfjsqchd8 +Ewsh5Abt39bIZTkKc7vKEeRL/nUT2AjqDkbRWi54FUUSsKWuOjRyUwmsVWlqc4bZ +RIaAt8+SiZIi/1kvhKt4U3buxuFNuRVbFuTsA4IUJBxoEi9FOGlGsD1hG/Z0H9fj +IiLtSus/AgMBAAECggEBAKuXUFJeHL7TNzMRAMmnT28uOypPiwtr8Z5X6Vtiy6DU +0wIyDj3H3PAPkI2mcvaRSmngYAFyKJGX3N7OgTkElmZ1pWptgn3WDKf3MC4vQ2F7 +kd0A20q3cuMSaskvzC5BFvmiFoD/wMYjlP7RDVhdWqqv9IbhVAAAQcnxLUANZ6CH +/xrieGuYavs62pSu5fnke7zRozdD1Mb7/oolAnycaLuoi1eZBh8wW8EJyFSxcZ5A +pYF5kNqbwAdOZ22Tygxwu7lnh8PUOKxf9pTmO6uUYAJcn/Z3ZHtnBYsjU/LkfNPV +hYLu1bKftm6UEZYwCXE3/ygop1q648NvCvtJB+Gbj9ECgYEA8nB+hS+7MLgi/dv8 +FCMJ9HBN76/nlwjOCTZIyIhCs5Jc6zJQGiDNLUFM/1mpBKUWWAss3g0dmJq32ish +apsCUxabzWuKi44fDMEterJrGDWquyJK+jNPqfqOORLdMf0edNfZbjUxev7D52Ak +4Ej3Ggy/fENd8QWLK6PZHV5X1MUCgYEAyKiWlawh7l8eBrba8UFQ4n1HiK/2uEud +yQOLceSRmW/xC6ZCiR0ILinrtZWRxqQg+ZSS24hjnHhcdnRw8TRXx22TkTwGfAXW +wKesPrtGJrn0ADuZwPkGewyeHPsisXNSiuGLPcLiOCoNNYgbIWJ2RknM1Xw+2p8C +qYU8Si6l6DMCgYEA20v4ld7sExCszjZ72XcsXQhs5v+Vm9/iByEsSwA+XZJqLHFx +VYEQNvxXeq8OnN37zR4msqDogY6J+XWEH5shSiksO28ofj3LRk1DJzZWeyqoSeem +LJXXXKkAlw3COaJ9NzG8Qt0o6dmjORqVoK8/nTekyfFh+0+JaKsoDFG3XwUCgYBN +tq2Ljj0d+wzAAPXO1kMjVO3tjGj7e53CinLpS2LwkCBFKMFAJVRTvLyjeSgaTNrQ +jrBKAgrCQQNehT5wzJrqjA/JAfxo8EH6H3ZgXVuQCBjuNicYS9ossfhStRj8rPNd +AnlRFDdVFUREZVBMn7u7AT4puJMHTOpVCVsOR/7NbQKBgApyR1WfsxZYi8vzVosQ +jnMIW18lnZN3s6auyEvmpVowx0U0yd9QU3HHX1j8Kfq7D9uERCwBtIfn9fzZrZnu +Xgbi9LMUT1z1jFXxJNgzaNmm0JHW9cD24BWNeQ60uxaRiGGmCyfmgqrGOXSn2R8w +KoWEnnunZ9nehcD9dkWcH5zG +-----END PRIVATE KEY----- diff --git a/auth_saml/tests/fake_idp.py b/auth_saml/tests/fake_idp.py index 1d958233bd..e5a4dc1649 100644 --- a/auth_saml/tests/fake_idp.py +++ b/auth_saml/tests/fake_idp.py @@ -105,11 +105,11 @@ def _unpack(self, ver="SAMLResponse"): class FakeIDP(Server): - def __init__(self, metadatas=None): - settings = CONFIG + def __init__(self, metadatas=None, settings=None): + if settings is None: + settings = CONFIG if metadatas: settings.update({"metadata": {"inline": metadatas}}) - config = Saml2Config() config.load(settings) config.allow_unknown_attributes = True diff --git a/auth_saml/tests/test_pysaml.py b/auth_saml/tests/test_pysaml.py index e7263f71f9..8e7ea43d00 100644 --- a/auth_saml/tests/test_pysaml.py +++ b/auth_saml/tests/test_pysaml.py @@ -1,10 +1,14 @@ import base64 import os +import os.path as osp +from copy import deepcopy + +import responses from odoo.exceptions import AccessDenied, UserError, ValidationError from odoo.tests import HttpCase, tagged -from .fake_idp import FakeIDP +from .fake_idp import CONFIG, FakeIDP @tagged("saml", "post_install", "-at_install") @@ -296,3 +300,56 @@ def test_disallow_user_password_when_changing_settings(self): self.authenticate( user="user@example.com", password="NesTNSte9340D720te>/-A" ) + + @responses.activate + def test_download_metadata(self): + expected_metadata = self.idp.get_metadata() + responses.add( + responses.GET, + "http://localhost:8000/metadata", + status=200, + content_type="text/xml", + body=expected_metadata, + ) + self.saml_provider.idp_metadata_url = "http://localhost:8000/metadata" + self.saml_provider.idp_metadata = "" + self.saml_provider.action_refresh_metadata_from_url() + self.assertEqual(self.saml_provider.idp_metadata, expected_metadata) + + @responses.activate + def test_login_with_saml_metadata_empty(self): + self.saml_provider.idp_metadata_url = "http://localhost:8000/metadata" + self.saml_provider.idp_metadata = "" + expected_metadata = self.idp.get_metadata() + responses.add( + responses.GET, + "http://localhost:8000/metadata", + status=200, + content_type="text/xml", + body=expected_metadata, + ) + self.test_login_with_saml() + self.assertEqual(self.saml_provider.idp_metadata, expected_metadata) + + @responses.activate + def test_login_with_saml_metadata_key_changed(self): + settings = deepcopy(CONFIG) + settings["key_file"] = osp.join( + osp.dirname(__file__), "data", "key_idp_expired.pem" + ) + settings["cert"] = osp.join( + osp.dirname(__file__), "data", "key_idp_expired.pem" + ) + expired_idp = FakeIDP(settings=settings) + self.saml_provider.idp_metadata = expired_idp.get_metadata() + self.saml_provider.idp_metadata_url = "http://localhost:8000/metadata" + up_to_date_metadata = self.idp.get_metadata() + self.assertNotEqual(self.saml_provider.idp_metadata, up_to_date_metadata) + responses.add( + responses.GET, + "http://localhost:8000/metadata", + status=200, + content_type="text/xml", + body=up_to_date_metadata, + ) + self.test_login_with_saml() diff --git a/auth_saml/views/auth_saml.xml b/auth_saml/views/auth_saml.xml index e4e2f99149..cabe0f095f 100644 --- a/auth_saml/views/auth_saml.xml +++ b/auth_saml/views/auth_saml.xml @@ -72,8 +72,23 @@ - -