Do not lower-case the username from the IdP when creating the federated identity

[pedroigor]

Closes #28495

[pedroigor]

Do we want a switch to enable/disable this behavior? This change can break existing deployments expecting the original username to be in lower-case.

[sguilhen]

It is probably a good idea to do it, IMO.

[mposolda]

Can we just update this and document in the migration guide? Hopefully we can avoid (yet another) switch. But not 100% sure... If you guys rather think, we should keep backwards compatibility, I don't want to block this.

[pedroigor]

I'd go then with the path of not adding any switch but updating both release and upgrade guides. I think most of the time, you really want to keep the original username. Also, I think one could always use a mapper to lowercase the value if they really need this data into tokens.

@thomasdarimont @sschu @tnorimat Based on your experience, do you have any strong opinions about this? Does it make sense to introduce a setting to keep storing the original username in lowercase (case-insensitive) when federating users from a broker?

[tnorimat]

@pedroigor Hello,
From my side, there is no strong opinion about it. IMO, it makes sense to me to add a switch make sense to keep backward compatibility.

[sschu]

I also would try to not break compatibility, otherwise people will end up with an upper/lower case mixture. Maybe we could keep the old behaviour for existing realms and introduce the new behaviour for new realms only? One option to not use a switch here would be to edit the template for new realms to always use a username template mapper in its first broker login flow. We could document that in order to change the behaviour for existing realms, you can add a username template mapper.

Besides that, I think the search logic in the JPAUserProvider needs to be adapted since currently it always searches for lower username only, see

keycloak/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java

Line 938 in 45e5e6c

orPredicates.add(builder.equal(from.get(USERNAME), value));

and

keycloak/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java

Line 994 in 45e5e6c

predicates.add(builder.equal(root.get(key), value.toLowerCase()));

[pedroigor]

@sschu Those references are related to the querying users and not their federated identities. AFAIK, we don't have any query that uses https://github.com/keycloak/keycloak/blob/main/model/jpa/src/main/java/org/keycloak/models/jpa/entities/FederatedIdentityEntity.java#L64 in a predicate.

I like though your suggestion to keep the existing behavior for existing realms while introducing the new behavior only for new ones. I'm wondering if we should consider binding realms with a version so that we can change behavior like this one only for realms greater than or equal to the version where a realm was created?

[sschu]

Ah, I assumed this was targeted at the username that Keycloak set's for a user based on the username from the external IDP. That's why I was also looking at the UsernameTemplateMapper. Binding switches to realm versions sounds like a nice idea to me. But. I would probably try to get more people's opinion on this since this could probably be used in several places.

[pedroigor]

I've updated the PR to introduce a Case-sensitive username switch to the broker configuration so that you can enable it and have case-sensitive usernames when storing the federated identity.

[sguilhen]

Accidentally approved the PR - I meant to comment that there are test failures that seem related @pedroigor

[pedroigor]

Fixed the failure by making sure we respect the value from the JSON configuration instead of forcing a lower-case when importing federating identities.

[keycloak-github-bot]

Unreported flaky test detected, please review

[keycloak-github-bot]

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.BruteForceTest#testNonExistingAccounts

Keycloak CI - Forms IT (chrome)

org.openqa.selenium.TimeoutException: 
timeout: Timed out receiving message from renderer: 10.000
  (Session info: chrome-headless-shell=125.0.6422.60)
Build info: version: '3.14.0', revision: 'aacccce0', time: '2018-08-02T20:19:58.91Z'
System info: host: 'fv-az661-983', ip: '10.1.0.34', os.name: 'Linux', os.arch: 'amd64', os.version: '6.5.0-1021-azure', java.version: '17.0.11'
...

Report flaky test

org.keycloak.testsuite.model.session.UserSessionPersisterProviderTest#testMigrateSession

Keycloak CI - Store Model Tests

java.lang.AssertionError: expected:<3> but was:<0>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
...

Report flaky test