From f00a1711f154c323d1a428ffcb12be2beefbaf60 Mon Sep 17 00:00:00 2001 From: Ricardoalso Date: Thu, 17 Oct 2024 14:32:06 +0200 Subject: [PATCH] auth_saml: Add FOR UPDATE NOWAIT clause to SAML provider query - Enhance concurrency handling when fetching SAML provider data - Prevent potential deadlocks by using NOWAIT - Ensure data consistency during high-traffic scenarios Previous attempt using the locking mechanisms self.env.cr.execute( "SELECT id FROM auth_saml_provider WHERE id in %s FOR UPDATE", (tuple(provider_ids),), ) led to increased latency and deadlocks under high load. We experienced race conditions where two simultaneous login attempts triggered parallel updates of the idp_metadata, leading to unreleased locks. The FOR UPDATE NOWAIT clause addresses this issue by: 1. Acquiring an exclusive lock on the row being updated 2. Failing fast if the lock cannot be acquired (NOWAIT) 3. Ensuring that only one transaction can update the idp_metadata at a time --- auth_saml/models/auth_saml_provider.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/auth_saml/models/auth_saml_provider.py b/auth_saml/models/auth_saml_provider.py index 94c45de06e..8cc0732d07 100644 --- a/auth_saml/models/auth_saml_provider.py +++ b/auth_saml/models/auth_saml_provider.py @@ -308,6 +308,14 @@ def _validate_auth_response(self, token: str, base_url: str = None): except SignatureError: # we have a metadata url: try to refresh the metadata document if self.idp_metadata_url: + self.env.cr.execute( + """ + SELECT id, idp_metadata + FROM auth_saml_provider + WHERE id=%s FOR UPDATE NOWAIT + """, + (self.id,), + ) self.action_refresh_metadata_from_url() # retry: if it fails again, we let the exception flow client = self._get_client_for_provider(base_url) @@ -424,11 +432,6 @@ def action_refresh_metadata_from_url(self): # 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: