Skip to content

Commit

Permalink
Fix logic of SELECT FOR UDPDATE to avoid deadlocks
Browse files Browse the repository at this point in the history
1. download the metadata with a timeout to avoid getting stuck
2. lock the record after completing the download
  • Loading branch information
Ricardoalso authored and gurneyalex committed May 14, 2024
1 parent 31441ef commit ad253fb
Showing 1 changed file with 18 additions and 8 deletions.
26 changes: 18 additions & 8 deletions auth_saml/models/auth_saml_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -411,21 +411,31 @@ def action_refresh_metadata_from_url(self):
)
if not providers:
return False

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

View check run for this annotation

Codecov / codecov/patch

auth_saml/models/auth_saml_provider.py#L413

Added line #L413 was not covered by tests

providers_to_update = {}
for provider in providers:
document = requests.get(provider.idp_metadata_url, timeout=5)
if document.status_code != 200:
raise UserError(

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

View check run for this annotation

Codecov / codecov/patch

auth_saml/models/auth_saml_provider.py#L419

Added line #L419 was not covered by tests
f"Unable to download the metadata for {provider.name}: {document.reason}"
)
if document.text != provider.idp_metadata:
providers_to_update[provider.id] = document.text

# 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),),
(tuple(providers_to_update.keys()),),
)
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 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,
)
if document.text != provider.idp_metadata:
provider.idp_metadata = document.text
_logger.info("Updated provider metadata for %s", provider.name)
updated = True

return updated

0 comments on commit ad253fb

Please sign in to comment.