Fix SAML artifact binding failing to validate signed nested samlp:Response by jimmychakkalakal · Pull Request #50862 · keycloak/keycloak

jimmychakkalakal · 2026-07-14T10:32:02Z

Fix SAML artifact binding failing to validate signed nested samlp:Response

Closes #50684

Copilot

Pull request overview

Adds support for validating signed SAML responses nested inside artifact responses.

Changes:

Registers ID attributes on signable nested protocol elements.
Recognizes valid nested protocol signatures.
Adds regression tests for nested and root signatures.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
`JBossSAMLConstants.java`	Defines signable protocol elements.
`SAML2Signature.java`	Registers nested protocol IDs.
`XMLSignatureUtil.java`	Accepts nested protocol signatures.
`SAML2SignatureTest.java`	Tests artifact-response signatures.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

+    public static final Set<JBossSAMLConstants> SIGNED_PROTOCOL_ELEMENTS = Set.of(
+            RESPONSE__PROTOCOL,
+            LOGOUT_REQUEST,
+            LOGOUT_RESPONSE
+    );


keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-07-15T10:33:39Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.cluster.RealmInvalidationClusterTest#crudWithFailover

Keycloak CI - Store IT (oracle)

org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertFalse.failNotFalse(AssertFalse.java:63)
	at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:36)
...

Report flaky test

lhanusov

LGTM

rmartinc

Thanks @jimmychakkalakal! LGTM in general, just a few suggestion to make it more readable. Just for your consideration, they are more nitpicks than real issues.

validate signed nested samlp:Response Closes keycloak#50684 Signed-off-by: Jimmy Chakkalakal <jimmy.chakkalakal@ibm.com>

…re readable Closes keycloak#50342 Signed-off-by: Jimmy Chakkalakal <jimmy.chakkalakal@ibm.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

jimmychakkalakal · 2026-07-17T09:03:34Z

@rmartinc Thanks for the nitpick suggestions. I have implemented those. The code is now more readable and clean. Please review when you have time. Thanks in advance!

rmartinc

Thanks @jimmychakkalakal! LGTM!

…ponse (keycloak#50862) * Fix SAML artifact binding failing to validate signed nested samlp:Response Closes keycloak#50684 Signed-off-by: Jimmy Chakkalakal <jimmy.chakkalakal@ibm.com> * Worked on reviewers nitpicks suggestions, that made code clean and more readable Closes keycloak#50342 Signed-off-by: Jimmy Chakkalakal <jimmy.chakkalakal@ibm.com> --------- Signed-off-by: Jimmy Chakkalakal <jimmy.chakkalakal@ibm.com>

keycloak-github-bot Bot added the team/core-protocols label Jul 14, 2026

jimmychakkalakal requested a review from Copilot July 14, 2026 10:32

Copilot started reviewing on behalf of jimmychakkalakal July 14, 2026 10:32 View session

jimmychakkalakal force-pushed the bug/Fix-50684-SAML_validate_signed_nested_samlp_Response branch from cc6ac67 to 54bc5e4 Compare July 14, 2026 10:33

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread saml-core/src/main/java/org/keycloak/saml/processing/core/util/XMLSignatureUtil.java Outdated

Comment thread saml-core/src/test/java/org/keycloak/saml/processing/api/saml/v2/sig/SAML2SignatureTest.java Outdated

jimmychakkalakal force-pushed the bug/Fix-50684-SAML_validate_signed_nested_samlp_Response branch from 54bc5e4 to 53d0031 Compare July 14, 2026 10:43

jimmychakkalakal requested a review from Copilot July 14, 2026 10:44

Copilot started reviewing on behalf of jimmychakkalakal July 14, 2026 10:44 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread saml-core/src/main/java/org/keycloak/saml/processing/core/util/XMLSignatureUtil.java Outdated

Comment thread saml-core-api/src/main/java/org/keycloak/saml/common/constants/JBossSAMLConstants.java Outdated

jimmychakkalakal force-pushed the bug/Fix-50684-SAML_validate_signed_nested_samlp_Response branch from 53d0031 to b1f1bdf Compare July 14, 2026 10:58

jimmychakkalakal requested a review from Copilot July 14, 2026 11:00

Copilot started reviewing on behalf of jimmychakkalakal July 14, 2026 11:00 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

jimmychakkalakal marked this pull request as ready for review July 14, 2026 11:04

jimmychakkalakal requested a review from a team as a code owner July 14, 2026 11:04

Copilot AI review requested due to automatic review settings July 14, 2026 13:41

jimmychakkalakal force-pushed the bug/Fix-50684-SAML_validate_signed_nested_samlp_Response branch from b1f1bdf to 87eb7ab Compare July 14, 2026 13:41

Copilot started reviewing on behalf of jimmychakkalakal July 14, 2026 13:48 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread saml-core-api/src/main/java/org/keycloak/saml/common/constants/JBossSAMLConstants.java Outdated

Copilot AI review requested due to automatic review settings July 15, 2026 09:09

jimmychakkalakal force-pushed the bug/Fix-50684-SAML_validate_signed_nested_samlp_Response branch from 87eb7ab to 21e7b98 Compare July 15, 2026 09:09

Copilot started reviewing on behalf of jimmychakkalakal July 15, 2026 09:12 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Comment thread saml-core-api/src/main/java/org/keycloak/saml/common/constants/JBossSAMLConstants.java Outdated

Comment on lines +335 to +339

public static final Set<JBossSAMLConstants> SIGNED_PROTOCOL_ELEMENTS = Set.of(

RESPONSE__PROTOCOL,

LOGOUT_REQUEST,

LOGOUT_RESPONSE

);

keycloak-github-bot Bot added the flaky-test label Jul 15, 2026

keycloak-github-bot Bot reviewed Jul 15, 2026

View reviewed changes

lhanusov approved these changes Jul 16, 2026

View reviewed changes

rmartinc requested changes Jul 16, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 17, 2026 08:58

Fix SAML artifact binding failing to

aa7ce7b

validate signed nested samlp:Response Closes keycloak#50684 Signed-off-by: Jimmy Chakkalakal <jimmy.chakkalakal@ibm.com>

Worked on reviewers nitpicks suggestions, that made code clean and mo…

582cb62

…re readable Closes keycloak#50342 Signed-off-by: Jimmy Chakkalakal <jimmy.chakkalakal@ibm.com>

Copilot started reviewing on behalf of jimmychakkalakal July 17, 2026 08:59 View session

jimmychakkalakal force-pushed the bug/Fix-50684-SAML_validate_signed_nested_samlp_Response branch from 66e00d7 to 582cb62 Compare July 17, 2026 08:59

Copilot AI reviewed Jul 17, 2026

View reviewed changes

rmartinc approved these changes Jul 17, 2026

View reviewed changes

pskopek approved these changes Jul 17, 2026

View reviewed changes

pskopek merged commit 86df0eb into keycloak:main Jul 17, 2026
91 checks passed

Uh oh!

Conversation

jimmychakkalakal commented Jul 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot Bot commented Jul 15, 2026

Unreported flaky test detected

org.keycloak.testsuite.cluster.RealmInvalidationClusterTest#crudWithFailover

Uh oh!

lhanusov left a comment

Choose a reason for hiding this comment

Uh oh!

rmartinc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

jimmychakkalakal commented Jul 17, 2026

Uh oh!

rmartinc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants