search

OCA / server-auth

https://odoo-community.org/psc-teams/tools-30
GNU Affero General Public License v3.0
150 stars 403 forks source link

[IMP] auth_saml: download the provider metadata #602

Closed gurneyalex closed 3 months ago

gurneyalex commented 8 months ago

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.

OCA-git-bot commented 8 months ago

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

vincent-hatakeyama commented 8 months ago

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 commented 8 months ago

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.

apf-c2c commented 5 months ago

Looks good from functional point of view :+1:

gurneyalex commented 3 months ago

/ocabot merge patch

OCA-git-bot commented 3 months ago

On my way to merge this fine PR! Prepared branch 15.0-ocabot-merge-pr-602-by-gurneyalex-bump-patch, awaiting test results.

OCA-git-bot commented 3 months ago

Congratulations, your PR was merged at 300557bcddc70accad9ba2ce775fccdcda2c047b. Thanks a lot for contributing to OCA. ❤️