diff --git a/auth_saml/README.rst b/auth_saml/README.rst index ea6b223ea1..f7d1ad8fca 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:3fcac74e9beda7cf4b033bd925615869ba6499576aabc948428f2cce34b6b790 + !! source digest: sha256:56a6042e204ca8c553db8eb36de4b1ad7ae8e1e9d5abe598a8398f5e17da7e7f !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! .. |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 78e6c96f33..cbba99a293 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,14 @@ 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 @@ -232,10 +244,19 @@ def _get_config_for_provider(self, base_url: str = None) -> Saml2Config: "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) -> Saml2Client: sp_config = self._get_config_for_provider(base_url) @@ -280,13 +301,26 @@ def _get_auth_request(self, extra_state=None, url_root=None): 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": @@ -370,3 +404,42 @@ 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 + + providers_to_update = {} + for provider in providers: + document = requests.get(provider.idp_metadata_url, timeout=5) + if document.status_code != 200: + raise UserError( + f"Unable to download the metadata for {provider.name}: {document.reason}" + ) + if document.text != provider.idp_metadata: + providers_to_update[provider.id] = document.text + + if not providers_to_update: + return False + + # lock the records we might update, so that multiple simultaneous login + # attempts will not cause concurrent updates + provider_ids = tuple(providers_to_update.keys()) + self.env.cr.execute( + "SELECT id FROM auth_saml_provider WHERE id in %s FOR UPDATE", + (tuple(provider_ids),), + ) + updated = False + for provider in providers: + if provider.id in providers_to_update: + provider.idp_metadata = providers_to_update[provider.id] + _logger.info( + "Updated metadata for provider %s from %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..3eb7d6c67f 100644 --- a/auth_saml/tests/fake_idp.py +++ b/auth_saml/tests/fake_idp.py @@ -105,8 +105,9 @@ 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}}) diff --git a/auth_saml/tests/test_pysaml.py b/auth_saml/tests/test_pysaml.py index 7549e2546f..1de8ea275a 100644 --- a/auth_saml/tests/test_pysaml.py +++ b/auth_saml/tests/test_pysaml.py @@ -2,12 +2,16 @@ import base64 import html import os +import os.path as osp +from copy import deepcopy from unittest.mock import patch +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") @@ -359,3 +363,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 a0f4e1b855..9bda039d93 100644 --- a/auth_saml/views/auth_saml.xml +++ b/auth_saml/views/auth_saml.xml @@ -76,6 +76,21 @@ + +