Skip to content

fix: improve SAML signature validation for redirect binding#621

Open
gsaandy wants to merge 10 commits into
crewjam:mainfrom
gsaandy:fix/single-logout
Open

fix: improve SAML signature validation for redirect binding#621
gsaandy wants to merge 10 commits into
crewjam:mainfrom
gsaandy:fix/single-logout

Conversation

@gsaandy

@gsaandy gsaandy commented May 17, 2025

Copy link
Copy Markdown

This resolves the merge conflicts in #449

In addition to the changes mentioned in the above PR, it also fixes the following

  • Removes the signature validation using SAMLResponse payload for HTTP-Redirect binding
  • Fix the Signature validation failures for ADFS because of decode/encode while reconstructing the sign data
  • fix: add missing Signature and SigAlg query params with single logout HTTTP-Redirect binding request

Tested single logout with Okta and Microsoft Entra ID(Azure AD)

Jguer and others added 3 commits January 2, 2023 13:34
add support for sha1 & sha512

add tests

use query sign in redirect

implement review feedback

- Return error if signature is unsupported
- wrap errors

Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>
Co-authored-by: Orgad Shaneh <orgads@gmail.com>
- Removes the signature validation using SAMLResponse payload for HTTP-Redirect binding
- Fix the Signature validation failures for ADFS because of decode/encode while reconstructing the sign data
@gsaandy gsaandy requested a review from crewjam as a code owner May 17, 2025 22:27
@gsaandy

gsaandy commented May 17, 2025

Copy link
Copy Markdown
Author

Review please @crewjam @andreas-kupries @Jguer @omerkarj

@gsaandy

gsaandy commented May 20, 2025

Copy link
Copy Markdown
Author

@crewjam - just wondering if you’ve had a chance to take a look at this PR. I'm happy to help with any changes needed to get it fixed and merged

@gsaandy

gsaandy commented May 24, 2025

Copy link
Copy Markdown
Author

@crewjam - just wondering if you’ve had a chance to take a look at this PR. I'm happy to help with any changes needed to get it fixed and merged

Hello @crewjam - Any chance this can be reviewed.

@gsaandy

gsaandy commented Jun 7, 2025

Copy link
Copy Markdown
Author

@crewjam - could you help to review this PR.

@Lumengrid

Copy link
Copy Markdown

any ETA on this ?

@gsaandy

gsaandy commented Jun 21, 2025

Copy link
Copy Markdown
Author

@crewjam - could you help to review this?

@gsaandy

gsaandy commented Jul 11, 2025

Copy link
Copy Markdown
Author

Hello @crewjam , it's been about two months with no response on this PR. Just checking in - wondering if the repo is still actively maintained?

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

Successfully merging this pull request may close these issues.

3 participants