Skip to content

Commit

Permalink
auth_saml: Add FOR UPDATE NOWAIT clause to SAML provider query
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
Ricardoalso committed Oct 17, 2024
1 parent 1219cbb commit f00a171
Showing 1 changed file with 8 additions and 5 deletions.
13 changes: 8 additions & 5 deletions auth_saml/models/auth_saml_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(

Check warning on line 311 in auth_saml/models/auth_saml_provider.py

View check run for this annotation

Codecov / codecov/patch

auth_saml/models/auth_saml_provider.py#L311

Added line #L311 was not covered by tests
"""
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)
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit f00a171

Please sign in to comment.