[IMP][16.0] auth_saml: download the provider metadata#739
[IMP][16.0] auth_saml: download the provider metadata#739OCA-git-bot merged 2 commits intoOCA:16.0from
Conversation
|
Hi @vincent-hatakeyama, |
dutrieuc
changed the title
dutrieuc
force-pushed
the
16.0-auth_saml_metadata
branch
2 times, most recently
from
c0156e7 to
aa1f1e1
Compare
|
Hello @vincent-hatakeyama, |
| 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),), |
There was a problem hiding this comment.
provider_idsis already a tuple, why use tuple again?
|
The last 3 commits need to be squashed and their title fixed. The second commit title also does not match OCA standards. |
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
dutrieuc
force-pushed
the
16.0-auth_saml_metadata
branch
from
589c76d to
11343aa
Compare
|
@vincent-hatakeyama Is it OK for you now? |
StefanRijnhart
left a comment
StefanRijnhart
left a comment
There was a problem hiding this comment.
Thanks for the port! Would you like to port this further, at least to 18.0 so that it will be included in future versions from now on?
|
/ocabot merge minor |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at 6df1cae. Thanks a lot for contributing to OCA. ❤️ |
|
@StefanRijnhart I'll find some time to add it in 18 |
6 participants
Reopening of #647
Migration to 16 of #602 feature