fix: correctly handle duplicate values in setAttribute() by dasniko · Pull Request #48384 · keycloak/keycloak

dasniko · 2026-04-22T19:02:05Z

When an user attribute holds duplicate values (e.g. ["bar", "bar"]) and the caller reduces the count (e.g. to ["bar"]), the change was silently dropped. Both the infinispan cache adapter and the JPA adapter compared old and new values as Sets, so ["bar", "bar"] and ["bar"] both collapsed to {"bar"} and the equality check returned true, causing an early return without persisting any change.

The infinispan adapter's check is the primary failure point in real deployments: it guards the call to getDelegateForUpdate(), so the JPA layer was never reached at all.

Fix: replace Set-based comparison with List-based comparison in both adapters. CollectionUtil.collectionEquals already counts duplicates correctly when given Lists, so ["bar", "bar"] vs. ["bar"] correctly evaluates to not-equal and the update proceeds.

Includes a regression test that verifies duplicate-value reduction is persisted correctly across transaction boundaries.

closes #38526

🤖 AI disclosure: The analysis of the issue and the proposed changes were done with the help of Claude Code. The implementations, especially the new test method, was done myself. Claude also wrote the commit message.

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-04-22T20:12:33Z

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.forms.MultiFactorAuthenticationTest#testAlternativeMechanismsInDifferentSubflows

Keycloak CI - Forms IT (chrome)

org.opentest4j.AssertionFailedError: Expected PasswordPage but was localhost (https://localhost:8543/auth/realms/test/login-actions/authenticate?session_code=ujD4IoVg9WVsYugYJ6VQKgnGQeRQfPPba22uuPfUVt4&execution=d226bd1b-317b-43da-83e2-bf00c3bcf124&client_id=test-app&tab_id=jY96cU7Ttcs&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIn0) ==> expected: <true> but was: <false>
	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.AssertTrue.failNotTrue(AssertTrue.java:63)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
...

Report flaky test

ryanemerson · 2026-04-23T08:29:00Z

I see there's #41432, which you have contributed to, but it appears to be abandoned. Let's proceed here and we can close that PR once this has been merged.

dasniko · 2026-04-23T08:51:33Z

Yes, #41432 is much more complex and does also not work. So I started a completely new approach. And it turns out that it is quite easy to fix.

) When an attribute holds duplicate values (e.g. ["bar", "bar"]) and the caller reduces the count (e.g. to ["bar"]), the change was silently dropped. Both the infinispan cache adapter and the JPA adapter compared old and new values as Sets, so ["bar", "bar"] and ["bar"] both collapsed to {"bar"} and the equality check returned true, causing an early return without persisting any change. The infinispan adapter's check is the primary failure point in real deployments: it guards the call to getDelegateForUpdate(), so the JPA layer was never reached at all. Fix: replace Set-based comparison with List-based comparison in both adapters. CollectionUtil.collectionEquals already counts duplicates correctly when given Lists, so ["bar", "bar"] vs. ["bar"] correctly evaluates to not-equal and the update proceeds. Includes a regression test that verifies duplicate-value reduction is persisted correctly across transaction boundaries. closes keycloak#38526 Signed-off-by: Niko Köbler <[email protected]>

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-04-23T09:53:07Z

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.adapter.servlet.SAMLServletAdapterTest#employeeSigPostNoIdpKeyTestCertSubjectAsKeyNameInKeyInfo

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
	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.AssertTrue.failNotTrue(AssertTrue.java:63)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
...

Report flaky test

ryanemerson

LGTM, thanks @dasniko

Signed-off-by: Alexander Schwartz <[email protected]>

ahus1 · 2026-04-23T10:57:57Z

Thank you for this PR @dasniko and thank you for the first review @ryanemerson.

I've reviewed it and found a similar log in the setSingleAttribute() method. I've pushed one more change and a test case that accompanies it.

The database stores the value as a "bag", which is unordered. Unfortunately there is no native Java type for that, so I chose to sort the list and then compare. Please have a look.

Once one of you has reviewed this change, I'll merge it and prepare the backports.

ryanemerson

Thanks for the second pair of eyes @ahus1, good catch!

dasniko · 2026-04-23T11:10:17Z

Thanks @ahus1! LGTM

keycloak-github-bot · 2026-04-23T11:53:00Z

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.authz.PolicyEvaluationTest#

Keycloak CI - Base IT (3)

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	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.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	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.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	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.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

Report flaky test

keycloak-github-bot

Unreported flaky test detected, please review

ryanemerson · 2026-04-23T12:36:07Z

Re-running the Store model tests as UserSessionInitializerTest passes for me locally.

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-04-23T13:01:47Z

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.authz.PolicyEvaluationTest#

Keycloak CI - Base IT (3)

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	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.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	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.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	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.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

Report flaky test

dasniko requested review from a team as code owners April 22, 2026 19:02

dasniko force-pushed the pr/38526-consistent-multi-value-attributes branch from 20bfcb0 to 505b353 Compare April 22, 2026 19:14

keycloak-github-bot Bot added the flaky-test label Apr 22, 2026

keycloak-github-bot Bot reviewed Apr 22, 2026

View reviewed changes

ryanemerson reviewed Apr 23, 2026

View reviewed changes

Comment thread model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java Outdated

dasniko force-pushed the pr/38526-consistent-multi-value-attributes branch from 505b353 to c8545a8 Compare April 23, 2026 08:53

keycloak-github-bot Bot mentioned this pull request Apr 23, 2026

Flaky test: org.keycloak.testsuite.model.session.UserSessionInitializerTest#testUserSessionPropagationBetweenSites #47978

Open

keycloak-github-bot Bot reviewed Apr 23, 2026

View reviewed changes

ryanemerson previously approved these changes Apr 23, 2026

View reviewed changes

ahus1 dismissed ryanemerson’s stale review via 2721154 April 23, 2026 10:46

ahus1 changed the title ~~fix: correctly handle duplicate values in setAttribute() (#38526)~~ fix: correctly handle duplicate values in setAttribute() Apr 23, 2026

Review

f937a0c

Signed-off-by: Alexander Schwartz <[email protected]>

ahus1 force-pushed the pr/38526-consistent-multi-value-attributes branch from 2721154 to f937a0c Compare April 23, 2026 10:53

ahus1 self-assigned this Apr 23, 2026

ryanemerson approved these changes Apr 23, 2026

View reviewed changes

ahus1 enabled auto-merge (squash) April 23, 2026 11:20

ahus1 approved these changes Apr 23, 2026

View reviewed changes

stianst added the team/core-iam label Apr 23, 2026

keycloak-github-bot Bot reviewed Apr 23, 2026

View reviewed changes

ahus1 merged commit e19df70 into keycloak:main Apr 23, 2026
138 of 140 checks passed

keycloak-github-bot Bot mentioned this pull request Apr 23, 2026

Flaky test: org.keycloak.testsuite.model.session.UserSessionProviderOfflineModelTest#testOfflineSessionLazyLoading #47979

Open

keycloak-github-bot Bot reviewed Apr 23, 2026

View reviewed changes

dasniko deleted the pr/38526-consistent-multi-value-attributes branch April 23, 2026 13:03

ryanemerson mentioned this pull request Apr 24, 2026

Fixed how multi-value attributes are modified in jpa model adapters #41432

Closed

Conversation

dasniko commented Apr 22, 2026

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot Bot commented Apr 22, 2026

Unreported flaky test detected

org.keycloak.testsuite.forms.MultiFactorAuthenticationTest#testAlternativeMechanismsInDifferentSubflows

Uh oh!

Uh oh!

ryanemerson commented Apr 23, 2026

Uh oh!

dasniko commented Apr 23, 2026

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot Bot commented Apr 23, 2026

Unreported flaky test detected

org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#employeeSigPostNoIdpKeyTestCertSubjectAsKeyNameInKeyInfo

Uh oh!

ryanemerson left a comment

Choose a reason for hiding this comment

Uh oh!

ahus1 commented Apr 23, 2026

Uh oh!

ryanemerson left a comment

Choose a reason for hiding this comment

Uh oh!

dasniko commented Apr 23, 2026

Uh oh!

keycloak-github-bot Bot commented Apr 23, 2026

Unreported flaky test detected

org.keycloak.testsuite.authz.PolicyEvaluationTest#

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ryanemerson commented Apr 23, 2026

Uh oh!

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot Bot commented Apr 23, 2026

Unreported flaky test detected

org.keycloak.testsuite.authz.PolicyEvaluationTest#

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants