Skip to content

fix: sign SAML AuthnRequest with rsa-sha256 - #2750

Open
xlgmokha wants to merge 1 commit into
masterfrom
auth-1547
Open

fix: sign SAML AuthnRequest with rsa-sha256#2750
xlgmokha wants to merge 1 commit into
masterfrom
auth-1547

Conversation

@xlgmokha

@xlgmokha xlgmokha commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Bug fix. Update crewjam/saml to support rsa-sha256 signatures and use it.

What is the current behavior?

Generates a SAML Authnrequest with a rsa-sha1 signature.

What is the new behavior?

Generates a SAML Authnrequest with a rsa-sha256 signature.

Additional context

@xlgmokha xlgmokha changed the title fix: update to github.com/crewjam/saml@v0.5.1 chore: update to github.com/crewjam/saml@v0.5.1 Aug 26, 2026
@xlgmokha xlgmokha self-assigned this Aug 26, 2026
@blacksmith-sh

This comment has been minimized.

@xlgmokha
xlgmokha marked this pull request as ready for review August 26, 2026 23:17
@xlgmokha
xlgmokha requested a review from a team as a code owner August 26, 2026 23:17
@xlgmokha
xlgmokha force-pushed the auth-1547 branch 3 times, most recently from c2f4f97 to 416068a Compare September 1, 2026 17:01
@xlgmokha xlgmokha changed the title chore: update to github.com/crewjam/saml@v0.5.1 fix: sign SAML AuthnRequest with rsa-sha256 Sep 1, 2026
Bump to github.com/crewjam/saml@v0.5.1 to support RSA-SHA256

crewjam/saml@v0.4.14...v0.5.1
Comment thread internal/api/saml.go
})

provider.AuthnNameIDFormat = saml.PersistentNameIDFormat
provider.SignatureMethod = dsig.RSASHA256SignatureMethod

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How sure are you this isn't going to break existing customers?

@hf hf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Likely won't be an issue but needs careful rollout.

xlgmokha commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

I will try to see if I can gate this change so that it is scoped to specific instances.

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.

2 participants