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

Certificate validation for RSA is optional? #225

Closed
gotjosh opened this issue Oct 30, 2019 · 3 comments
Closed

Certificate validation for RSA is optional? #225

gotjosh opened this issue Oct 30, 2019 · 3 comments

Comments

@gotjosh
Copy link
Contributor

gotjosh commented Oct 30, 2019

Hi @crewjam πŸ‘‹,

First of all, thank you very much for your hard work on this library πŸ™‡β€β™‚οΈ. It has helped us provide a SAML integration that our customers love. Sadly, I'm not here only to deliver good news.

We've faced many problems while attempting to use the library with Keycloak as an IdP. One of them is that when using encrypted assertions (using RSA encryption) they don't provide a certificate within the ACS for us to validate against.

We've opted to remove this validation in a fork we've recently created. However, we would love to continue using upstream and contribute back if possible.

Please let me know if this is something that makes sense as I would love to put up a PR for you to review.

You can find more details of the change here: grafana#1

@crewjam
Copy link
Owner

crewjam commented Oct 30, 2019

Thanks for the kind words, @gotjosh. Your change looks good to me. My only concern is that the name of the function (validateRSAKey) seems misleading at this point. If you wanted to do a PR with the change you referenced plus naming the function something like validateRSAKeyIfPresent we could merge that.

Thanks for the contribution!

@gotjosh
Copy link
Contributor Author

gotjosh commented Oct 30, 2019

Thank you for the fast response @crewjam. PR incoming.

@crewjam
Copy link
Owner

crewjam commented Oct 30, 2019

ref PR #226

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

No branches or pull requests

2 participants