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

feat(saml): make getting providers from metadata non-panic #1464

Merged
merged 2 commits into from
Jun 6, 2024

Conversation

shentschel
Copy link
Contributor

Description

Changes the error behaviour when initialising saml providers. Instead of stoping the application now there will be a warning that the provider could not be loaded. The provider in question will be ignored on startup.

Closes: #1445

* show a warning if a provider cannot be fetched by its metadata url
* skip the provider in provisioning state

Closes: #1445
@shentschel shentschel requested a review from FreddyDevelop May 21, 2024 09:43
Copy link
Contributor

@FreddyDevelop FreddyDevelop left a comment

Choose a reason for hiding this comment

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

Can we add information (e.g. domain or name) to the error message to identify the saml provider which is not working?
I got this error message while testing the PR, but when you have multiple identity provider it is hard/impossible to identify the provider which is not working: failed to initialize provider: unable to unmarshal idp metadata response body to xml: expected element type <EntityDescriptor> but have <html>

adds the idp config provider name to error message when
the provider host cannot be parsed from metadata url or
fetching metadata document fails.

Closes: #1445
@shentschel
Copy link
Contributor Author

I extended the information with the idpconfig provider name.

@shentschel shentschel requested a review from FreddyDevelop May 29, 2024 09:01
@FreddyDevelop FreddyDevelop merged commit d551f32 into main Jun 6, 2024
12 checks passed
@FreddyDevelop FreddyDevelop deleted the feat/1445-no-saml-metadata-panic branch June 6, 2024 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[FEAT]: Do not panic if getting SAML metadatdata fails
2 participants