Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMP][16.0] auth_saml: download the provider metadata #739

Open
wants to merge 2 commits into
base: 16.0
Choose a base branch
from

Conversation

dutrieuc
Copy link

@dutrieuc dutrieuc commented Dec 18, 2024

Reopening of #647
Migration to 16 of #602 feature

@OCA-git-bot
Copy link
Contributor

Hi @vincent-hatakeyama,
some modules you are maintaining are being modified, check this out!

@dutrieuc dutrieuc changed the title 16.0 auth saml metadata [IMP][16.0] auth_saml: download the provider metadata Dec 18, 2024
@dutrieuc dutrieuc marked this pull request as ready for review December 18, 2024 19:07
@dutrieuc dutrieuc force-pushed the 16.0-auth_saml_metadata branch 2 times, most recently from c0156e7 to aa1f1e1 Compare December 19, 2024 10:30
@dutrieuc
Copy link
Author

Hello @vincent-hatakeyama,
Should I ping someone for review ?

Comment on lines 430 to 433
provider_ids = tuple(providers_to_update.keys())
self.env.cr.execute(
"SELECT id FROM auth_saml_provider WHERE id in %s FOR UPDATE",
(tuple(providers.ids),),
(tuple(provider_ids),),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

provider_idsis already a tuple, why use tuple again?

@vincent-hatakeyama
Copy link
Contributor

The last 3 commits need to be squashed and their title fixed.

The second commit title also does not match OCA standards.

gurneyalex and others added 2 commits January 22, 2025 17:07
On Office365, what you get when configuring an application for SAML
authentication is the URL of the federation metadata document. This URL
is stable, but the content of the document is not. I suspect some of the
encryption keys can be updated / renewed over time. The result is that
the configured provider in Odoo suddenly stops working, because the
messages sent by the Office365 provider can no longer be validated by
Odoo (because the federation document is out of date). Downloading the
new version and updating the auth.saml.provider record fixes the issue.

This PR adds a new field to store the URL of the metadata document. When
this field is set on a provider, you get a button next to it in the form
view to download the document from the URL. The button will not update
the document if it has not changed.

Additionally, when a SignatureError happens, we check if downloading the
document again fixes the issue.
Fix logic of SELECT FOR UDPDATE to only lock records whose metadata will
be updated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants