[IMP] auth_saml: download the provider metadata #602
Conversation
|
Hi @sbidoul, @vincent-hatakeyama, |
gurneyalex
force-pushed
the
15.0-auth_saml-download_metadata
branch
from
89ad9e2 to
0bed723
Compare
|
I don’t think the changes to auth_oidc should be included. Keycloak also provides a link to the metadata (in the form https:///realms//protocol/saml/descriptor). I’m not sure giving specific configuration information like this is pertinent. Coverage can be improved by using a mock and the fake idp already in the tests. |
gurneyalex
force-pushed
the
15.0-auth_saml-download_metadata
branch
from
0bed723 to
984a1fe
Compare
|
Thanks @vincent-hatakeyama I'll update the PR. I guess you are refering to the "help" message? The issue we are facing with Office365 is real: the provided metadata suddenly stops being able to validate the saml messages, and noone can authenticate. Updating the metadata with the content served from the original URL does solve the problem. |
| settings = deepcopy(CONFIG) | ||
| settings["key_file"] = osp.join( | ||
| osp.dirname(__file__), "data", "key_idp_expired.pem" | ||
| ) | ||
| settings["cert"] = osp.join( | ||
| osp.dirname(__file__), "data", "key_idp_expired.pem" | ||
| ) | ||
| expired_idp = FakeIDP(settings=settings) | ||
| self.saml_provider.idp_metadata = expired_idp.get_metadata() |
There was a problem hiding this comment.
@vincent-hatakeyama I think this is not the correct way to trigger an error when checking the signature to test the refresh (I'm not familiar enough with the SAML idp vs. sp internals). Could you offer a suggestion?
The goal is to get this https://github.com/OCA/server-auth/pull/602/files#diff-19fdbbb274613d822573aba74ad5575708e31bddf1688f6d909e589370f6ec3fR311 covered by a test.
There was a problem hiding this comment.
And indeed, the block is not tested
|
Looks good from functional point of view 👍 |
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.
gurneyalex
force-pushed
the
15.0-auth_saml-download_metadata
branch
from
92fed8b to
31441ef
Compare
1. download the metadata with a timeout to avoid getting stuck 2. lock the record after completing the download
|
/ocabot merge patch |
|
On my way to merge this fine PR! |
|
Congratulations, your PR was merged at 300557b. Thanks a lot for contributing to OCA. ❤️ |
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.