Skip to content

map default proper given name attribute #35943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
dasniko wants to merge 2 commits into
base: main
Choose a base branch
from
Open

map default proper given name attribute #35943

dasniko wants to merge 2 commits into from

Conversation

@dasniko
Copy link
Contributor

@dasniko dasniko commented Dec 16, 2024

closes #35941

@dasniko dasniko marked this pull request as ready for review February 19, 2025 20:30
@dasniko dasniko requested a review from a team as a code owner February 19, 2025 20:30
@thomasdarimont
Copy link
Contributor

thomasdarimont commented Feb 19, 2025 •
edited
Loading

Your suggested change changes the LDAP mapper "username-cn", instead of the "first name" mapper, is this a mistake?

The first name mapper is currently using CN as you mentioned here:
https://github.com/keycloak/keycloak/blob/main/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java#L386

@dasniko dasniko marked this pull request as draft February 19, 2025 20:33
@dasniko
Copy link
Contributor Author

dasniko commented Feb 19, 2025

Yes, thanks for the hint.
Don't know how I mixed that up... will fix it.

@dasniko dasniko marked this pull request as ready for review February 19, 2025 20:42
@dasniko dasniko marked this pull request as draft February 19, 2025 21:24
@dasniko dasniko force-pushed the 35941 branch 2 times, most recently from 1005245 to f434112 Compare February 20, 2025 06:41
keycloak-github-bot[bot]
keycloak-github-bot bot reviewed Feb 20, 2025
View reviewed changes
Copy link

@keycloak-github-bot keycloak-github-bot bot left a comment

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@keycloak-github-bot
Copy link

keycloak-github-bot bot commented Feb 20, 2025

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.actions.RequiredActionPriorityTest#executeRequiredActionWithCustomPriorityAppliesSamePriorityToSessionAndUserActions

Keycloak CI - Base IT (1)

java.lang.AssertionError: Event expected
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertNotNull(Assert.java:713)
	at org.keycloak.testsuite.AssertEvents.poll(AssertEvents.java:91)
...

Report flaky test

org.keycloak.testsuite.actions.RequiredActionPriorityTest#executeRequiredActionsWithDefaultPriority

Keycloak CI - Base IT (1)

java.lang.AssertionError: Expected TermsAndConditionsPage but was Sign in to test (https://localhost:8543/auth/realms/test/login-actions/required-action?execution=TERMS_AND_CONDITIONS&client_id=test-app&tab_id=vl6y2gONr0M&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIiwic3QiOiIxNzMxMzI4Zi1kMjY2LTRjZTItOTMzNy0zMzQ0Yzg4MjM5OGMifQ)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.actions.RequiredActionPriorityTest#setupTotpAfterUpdatePassword

Keycloak CI - Base IT (1)

java.lang.AssertionError: Expected AppPage but was Sign in to test (https://localhost:8543/auth/realms/test/login-actions/required-action?execution=UPDATE_PROFILE&client_id=test-app&tab_id=u4kAw5nFONE&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIiwic3QiOiIxMWMxMWViZS05MGI0LTRjMjUtOGQ2Yy1kODdkNWM3ZmM0OTIifQ)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

keycloak-github-bot[bot]
keycloak-github-bot bot reviewed Feb 20, 2025
View reviewed changes
Copy link

@keycloak-github-bot keycloak-github-bot bot left a comment

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@keycloak-github-bot
Copy link

keycloak-github-bot bot commented Feb 20, 2025

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.actions.RequiredActionPriorityTest#executeRequiredActionWithCustomPriorityAppliesSamePriorityToSessionAndUserActions

Keycloak CI - Base IT (1)

java.lang.AssertionError: Event expected
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertNotNull(Assert.java:713)
	at org.keycloak.testsuite.AssertEvents.poll(AssertEvents.java:91)
...

Report flaky test

org.keycloak.testsuite.actions.RequiredActionPriorityTest#executeRequiredActionsWithDefaultPriority

Keycloak CI - Base IT (1)

java.lang.AssertionError: Expected TermsAndConditionsPage but was Sign in to test (https://localhost:8543/auth/realms/test/login-actions/required-action?execution=TERMS_AND_CONDITIONS&client_id=test-app&tab_id=xUmmi3q0znE&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIiwic3QiOiIyNzM4ODgwZC1lZTFlLTQ4ZGItYmIxZi0yYjYxOTIwOThkYmUifQ)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.actions.RequiredActionPriorityTest#setupTotpAfterUpdatePassword

Keycloak CI - Base IT (1)

java.lang.AssertionError: Expected AppPage but was Sign in to test (https://localhost:8543/auth/realms/test/login-actions/required-action?execution=UPDATE_PROFILE&client_id=test-app&tab_id=k0hTTV5ISmw&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIiwic3QiOiI1MDg4YTBmNC04YTdjLTQxNTQtYWI0Mi1hNmEzMzA2OTQ2MjEifQ)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.organization.admin.OrganizationInvitationLinkTest#testInviteNewUserRegistrationCustomRegistrationFlow

Keycloak CI - Base IT (3)

java.lang.AssertionError: 

Expected: is <false>
     but: was <true>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testFullNameMapperWriteOnly

Keycloak CI - Base IT (5)

java.lang.AssertionError: expected:<null> but was:<James2>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:120)
	at org.junit.Assert.assertEquals(Assert.java:146)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testImpossibleToChangeNonLDAPMappedAttributes

Keycloak CI - Base IT (5)

jakarta.ws.rs.BadRequestException: HTTP 400 Bad Request
	at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.handleErrorStatus(ClientInvocation.java:236)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.extractors.DefaultEntityExtractorFactory$3.extractEntity(DefaultEntityExtractorFactory.java:41)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.ClientInvoker.invokeSync(ClientInvoker.java:136)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.ClientInvoker.invoke(ClientInvoker.java:103)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testSearch

Keycloak CI - Base IT (5)

jakarta.ws.rs.InternalServerErrorException: HTTP 500 Internal Server Error
	at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.handleErrorStatus(ClientInvocation.java:250)
	at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.extractResult(ClientInvocation.java:216)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.extractors.BodyEntityExtractor.extractEntity(BodyEntityExtractor.java:59)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.ClientInvoker.invokeSync(ClientInvoker.java:136)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testSearchWithCustomLDAPFilter

Keycloak CI - Base IT (5)

jakarta.ws.rs.InternalServerErrorException: HTTP 500 Internal Server Error
	at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.handleErrorStatus(ClientInvocation.java:250)
	at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.extractResult(ClientInvocation.java:216)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.extractors.BodyEntityExtractor.extractEntity(BodyEntityExtractor.java:59)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.ClientInvoker.invokeSync(ClientInvoker.java:136)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#caseInSensitiveImport

Keycloak CI - Base IT (5)

java.lang.IllegalArgumentException: No enum constant org.keycloak.testsuite.pages.AppPage.RequestType.Sign in to test
	at java.base/java.lang.Enum.valueOf(Enum.java:293)
	at org.keycloak.testsuite.pages.AppPage$RequestType.valueOf(AppPage.java:58)
	at org.keycloak.testsuite.pages.AppPage.getRequestType(AppPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#caseInsensitiveSearch

Keycloak CI - Base IT (5)

jakarta.ws.rs.BadRequestException: HTTP 400 Bad Request
	at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.handleErrorStatus(ClientInvocation.java:236)
	at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.extractResult(ClientInvocation.java:216)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.extractors.BodyEntityExtractor.extractEntity(BodyEntityExtractor.java:59)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.ClientInvoker.invokeSync(ClientInvoker.java:136)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#deleteFederationLink

Keycloak CI - Base IT (5)

java.lang.AssertionError: Expected OAuthGrantPage but was Sign in to test (https://localhost:8543/auth/realms/test/login-actions/authenticate?execution=cfc97a11-2be6-4d51-97aa-8eac953c4283&client_id=third-party&tab_id=m9Gfk8BR5-c&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIiwic3QiOiJlOWZlN2JhZS0yOTFmLTRjOWUtYTdlNS01N2E5MjYxYzhhYzMifQ)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#ldapPasswordChangeWithAccountConsole

Keycloak CI - Base IT (5)

java.lang.RuntimeException: User with username johnkeycloak not found
	at org.keycloak.testsuite.util.AccountHelper.getUserRepresentation(AccountHelper.java:36)
	at org.keycloak.testsuite.util.AccountHelper.getUserResource(AccountHelper.java:43)
	at org.keycloak.testsuite.util.AccountHelper.updatePassword(AccountHelper.java:55)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.ldapPasswordChangeWithAccountConsole(LDAPProvidersIntegrationTest.java:507)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#ldapPasswordChangeWithAdminEndpointAndRequiredAction

Keycloak CI - Base IT (5)

java.lang.NullPointerException: Cannot invoke "org.keycloak.representations.idm.UserRepresentation.getId()" because the return value of "org.keycloak.testsuite.admin.ApiUtil.findUserByUsername(org.keycloak.admin.client.resource.RealmResource, String)" is null
	at org.keycloak.testsuite.admin.ApiUtil.findUserByUsernameId(ApiUtil.java:162)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.ldapPasswordChangeWithAdminEndpointAndRequiredAction(LDAPProvidersIntegrationTest.java:538)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#loginLdap

Keycloak CI - Base IT (5)

java.lang.IllegalArgumentException: No enum constant org.keycloak.testsuite.pages.AppPage.RequestType.Sign in to test
	at java.base/java.lang.Enum.valueOf(Enum.java:293)
	at org.keycloak.testsuite.pages.AppPage$RequestType.valueOf(AppPage.java:58)
	at org.keycloak.testsuite.pages.AppPage.getRequestType(AppPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#loginLdapWithDirectGrant

Keycloak CI - Base IT (5)

java.lang.AssertionError: expected:<200> but was:<401>
	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

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#loginLdapWithEmail

Keycloak CI - Base IT (5)

java.lang.IllegalArgumentException: No enum constant org.keycloak.testsuite.pages.AppPage.RequestType.Sign in to test
	at java.base/java.lang.Enum.valueOf(Enum.java:293)
	at org.keycloak.testsuite.pages.AppPage$RequestType.valueOf(AppPage.java:58)
	at org.keycloak.testsuite.pages.AppPage.getRequestType(AppPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#registerExistingLdapUser

Keycloak CI - Base IT (5)

java.lang.AssertionError: Expected RegisterPage but was Sign in to test (https://localhost:8543/auth/realms/test/login-actions/registration?execution=5e5b270d-c734-45c0-94e3-05b72dc0cd1b&client_id=test-app&tab_id=2k9CJbeKIqc&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIiwic3QiOiIxOGE2MTdlNS05N2FkLTQwNzUtYWQzNC01MzI3ZDQ0ZDI3NTEifQ)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#registerUserLdapSuccess

Keycloak CI - Base IT (5)

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertNotNull(Assert.java:713)
	at org.junit.Assert.assertNotNull(Assert.java:723)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testAlwaysReadValueFromLdapCached

Keycloak CI - Base IT (5)

org.keycloak.testsuite.runonserver.RunOnServerException: java.lang.NullPointerException
	at org.keycloak.testsuite.client.KeycloakTestingClient$Server.run(KeycloakTestingClient.java:175)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.testAlwaysReadValueFromLdapCached(LDAPProvidersIntegrationTest.java:1429)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testAlwaysReadValueFromLdapNoCache

Keycloak CI - Base IT (5)

org.keycloak.testsuite.runonserver.RunOnServerException: java.lang.NullPointerException
	at org.keycloak.testsuite.client.KeycloakTestingClient$Server.run(KeycloakTestingClient.java:175)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.testAlwaysReadValueFromLdapNoCache(LDAPProvidersIntegrationTest.java:1475)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testCacheUser

Keycloak CI - Base IT (5)

java.lang.RuntimeException: org.keycloak.testsuite.runonserver.RunOnServerException: java.lang.NullPointerException
	at org.keycloak.testsuite.client.KeycloakTestingClient$Server.fetch(KeycloakTestingClient.java:146)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.testCacheUser(LDAPProvidersIntegrationTest.java:1333)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testCaseSensitiveAttributeName

Keycloak CI - Base IT (5)

org.keycloak.testsuite.runonserver.RunOnServerException: java.lang.NullPointerException
	at org.keycloak.testsuite.client.KeycloakTestingClient$Server.run(KeycloakTestingClient.java:175)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.testCaseSensitiveAttributeName(LDAPProvidersIntegrationTest.java:684)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testCommaInUsername

Keycloak CI - Base IT (5)

java.lang.IllegalArgumentException: No enum constant org.keycloak.testsuite.pages.AppPage.RequestType.Sign in to test
	at java.base/java.lang.Enum.valueOf(Enum.java:293)
	at org.keycloak.testsuite.pages.AppPage$RequestType.valueOf(AppPage.java:58)
	at org.keycloak.testsuite.pages.AppPage.getRequestType(AppPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testEmailVerifiedFromImport

Keycloak CI - Base IT (5)

jakarta.ws.rs.InternalServerErrorException: HTTP 500 Internal Server Error
	at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.handleErrorStatus(ClientInvocation.java:250)
	at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.extractResult(ClientInvocation.java:216)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.extractors.BodyEntityExtractor.extractEntity(BodyEntityExtractor.java:59)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.ClientInvoker.invokeSync(ClientInvoker.java:136)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testHardcodedAttributeMapperTest

Keycloak CI - Base IT (5)

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertNotNull(Assert.java:713)
	at org.junit.Assert.assertNotNull(Assert.java:723)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testHardcodedGroupMapper

Keycloak CI - Base IT (5)

org.keycloak.testsuite.runonserver.RunOnServerException: java.lang.NullPointerException
	at org.keycloak.testsuite.client.KeycloakTestingClient$Server.run(KeycloakTestingClient.java:175)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.testHardcodedGroupMapper(LDAPProvidersIntegrationTest.java:862)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testHardcodedRoleMapper

Keycloak CI - Base IT (5)

org.keycloak.testsuite.runonserver.RunOnServerException: java.lang.NullPointerException
	at org.keycloak.testsuite.client.KeycloakTestingClient$Server.run(KeycloakTestingClient.java:175)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.testHardcodedRoleMapper(LDAPProvidersIntegrationTest.java:821)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testImportExistingUserFromLDAP

Keycloak CI - Base IT (5)

java.lang.AssertionError: expected:<Email already exists.> but was:<null>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:120)
	at org.junit.Assert.assertEquals(Assert.java:146)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testLDAPUserDeletionImport

Keycloak CI - Base IT (5)

jakarta.ws.rs.InternalServerErrorException: HTTP 500 Internal Server Error
	at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.handleErrorStatus(ClientInvocation.java:250)
	at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.extractResult(ClientInvocation.java:216)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.extractors.BodyEntityExtractor.extractEntity(BodyEntityExtractor.java:59)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.ClientInvoker.invokeSync(ClientInvoker.java:136)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testLDAPUserRefreshCache

Keycloak CI - Base IT (5)

org.keycloak.testsuite.runonserver.RunOnServerException: java.lang.NullPointerException
	at org.keycloak.testsuite.client.KeycloakTestingClient$Server.run(KeycloakTestingClient.java:175)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.testLDAPUserRefreshCache(LDAPProvidersIntegrationTest.java:1294)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testReadonly

Keycloak CI - Base IT (5)

java.lang.NullPointerException: Cannot invoke "org.keycloak.representations.idm.UserRepresentation.getId()" because "user" is null
	at org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest.assertFederatedUserLink(LDAPProvidersIntegrationNoImportTest.java:85)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.testReadonly(LDAPProvidersIntegrationTest.java:937)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testRemoveFederatedUser

Keycloak CI - Base IT (5)

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertFalse(Assert.java:65)
	at org.junit.Assert.assertFalse(Assert.java:75)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testSearchWithPartiallyCachedUser

Keycloak CI - Base IT (5)

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertNotNull(Assert.java:713)
	at org.junit.Assert.assertNotNull(Assert.java:723)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testSyncRegistrationEmailRDNDefaultValue

Keycloak CI - Base IT (5)

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertFalse(Assert.java:65)
	at org.junit.Assert.assertFalse(Assert.java:75)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testSyncRegistrationEmailRDNNoDefault

Keycloak CI - Base IT (5)

java.lang.AssertionError: expected:<400> but was:<409>
	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

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testSyncRegistrationForceDefault

Keycloak CI - Base IT (5)

jakarta.ws.rs.WebApplicationException: Create method returned status Conflict (Code: 409); expected status: Created (201). Response body: {"errorMessage":"User exists with same username"}
	at org.keycloak.testsuite.admin.ApiUtil.getCreatedId(ApiUtil.java:63)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.testSyncRegistrationForceDefault(LDAPProvidersIntegrationTest.java:163)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testSyncRegistrationOff

Keycloak CI - Base IT (5)

jakarta.ws.rs.WebApplicationException: Create method returned status Conflict (Code: 409); expected status: Created (201). Response body: {"errorMessage":"User exists with same username"}
	at org.keycloak.testsuite.admin.ApiUtil.getCreatedId(ApiUtil.java:63)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.testSyncRegistrationOff(LDAPProvidersIntegrationTest.java:142)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

org.keycloak.testsuite.federation.ldap.noimport.LDAPProvidersIntegrationNoImportTest#testUserAttributeLDAPStorageMapperHandlingUsernameLowercasing

Keycloak CI - Base IT (5)

org.keycloak.testsuite.runonserver.RunOnServerException: java.lang.NullPointerException
	at org.keycloak.testsuite.client.KeycloakTestingClient$Server.run(KeycloakTestingClient.java:175)
	at org.keycloak.testsuite.federation.ldap.LDAPProvidersIntegrationTest.testUserAttributeLDAPStorageMapperHandlingUsernameLowercasing(LDAPProvidersIntegrationTest.java:1528)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
...

Report flaky test

keycloak-github-bot[bot]
keycloak-github-bot bot reviewed Feb 20, 2025
View reviewed changes
Copy link

@keycloak-github-bot keycloak-github-bot bot left a comment

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@keycloak-github-bot
Copy link

keycloak-github-bot bot commented Feb 20, 2025

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.actions.RequiredActionPriorityTest#executeRequiredActionWithCustomPriorityAppliesSamePriorityToSessionAndUserActions

Keycloak CI - Base IT (1)

java.lang.AssertionError: Event expected
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertNotNull(Assert.java:713)
	at org.keycloak.testsuite.AssertEvents.poll(AssertEvents.java:91)
...

Report flaky test

org.keycloak.testsuite.actions.RequiredActionPriorityTest#executeRequiredActionsWithDefaultPriority

Keycloak CI - Base IT (1)

java.lang.AssertionError: Expected TermsAndConditionsPage but was Sign in to test (https://localhost:8543/auth/realms/test/login-actions/required-action?execution=TERMS_AND_CONDITIONS&client_id=test-app&tab_id=NQclBE9_6AI&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIiwic3QiOiI0M2ZiNDZkYi1lZWVlLTRhZjQtYjlmMi1kZjEzOWJiMDg3MmIifQ)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.actions.RequiredActionPriorityTest#setupTotpAfterUpdatePassword

Keycloak CI - Base IT (1)

java.lang.AssertionError: Expected AppPage but was Sign in to test (https://localhost:8543/auth/realms/test/login-actions/required-action?execution=UPDATE_PROFILE&client_id=test-app&tab_id=XsfH8gGKMZ0&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIiwic3QiOiI4MGI3MTQzMC1iMTRkLTQ0MDItOTcwYy1mZjBjNjRiMGI0YzAifQ)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.organization.admin.OrganizationInvitationLinkTest#testInviteNewUserRegistrationCustomRegistrationFlow

Keycloak CI - Base IT (3)

java.lang.AssertionError: 

Expected: is <false>
     but: was <true>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
...

Report flaky test

@dasniko dasniko marked this pull request as ready for review February 20, 2025 19:49
@dasniko dasniko requested review from a team as code owners February 20, 2025 19:49
@dasniko
Copy link
Contributor Author

dasniko commented Feb 20, 2025

I don't get these flaky tests... perhaps someone from the maintainers can explain what happens here? "Port already in use"???

pedroigor
pedroigor reviewed Feb 20, 2025
View reviewed changes
Copy link
Contributor

@pedroigor pedroigor left a comment

Choose a reason for hiding this comment

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

Thanks, @dasniko. I'm not sure why we have been using CN to map the first name. We probably need a bit of history from @sguilhen, @mposolda or @rmartinc.

For me, your proposal makes more sense and I'm wondering what is the idea behind this entire block https://github.com/keycloak/keycloak/blob/main/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java#L342-L382.

I think we need to change that one too.

federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java
UserAttributeLDAPStorageMapper.IS_MANDATORY_IN_LDAP, "true");
realm.addComponentModel(mapperModel);

if (editMode == UserStorageProvider.EditMode.WRITABLE) {
Copy link
Contributor

@pedroigor pedroigor Feb 20, 2025

Choose a reason for hiding this comment

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

If Edit Mode is READ_ONLY we still want to map this attribute from LDAP, don't we? Or is there a reason to have it only if writable?

Copy link
Contributor Author

@dasniko dasniko Feb 20, 2025

Choose a reason for hiding this comment

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

I'm not 100% sure.
When READ_ONLY we have the first and last name as name information from single mappers coming from LDAP. fullname is not a real user attribute in Keycloak, this is always composed of first and lastname. So, IMHO this is only required in WRITEABLE mode, but I might be wrong here.

@dasniko
Copy link
Contributor Author

dasniko commented Feb 20, 2025

For me, your proposal makes more sense and I'm wondering what is the idea behind this entire block https://github.com/keycloak/keycloak/blob/main/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java#L342-L382.

I think we need to change that one too.

Yes, this makes probably sense. Before we start to change this, we should clarify what we want to achieve. TBH, I don't have that extensive knowledge of LDAP and even less on ActiveDirectory...

keycloak-github-bot[bot]
keycloak-github-bot bot reviewed Feb 20, 2025
View reviewed changes
Copy link

@keycloak-github-bot keycloak-github-bot bot left a comment

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@keycloak-github-bot
Copy link

keycloak-github-bot bot commented Feb 20, 2025

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.actions.RequiredActionPriorityTest#executeRequiredActionWithCustomPriorityAppliesSamePriorityToSessionAndUserActions

Keycloak CI - Base IT (1)

java.lang.AssertionError: Event expected
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertNotNull(Assert.java:713)
	at org.keycloak.testsuite.AssertEvents.poll(AssertEvents.java:91)
...

Report flaky test

org.keycloak.testsuite.actions.RequiredActionPriorityTest#executeRequiredActionsWithDefaultPriority

Keycloak CI - Base IT (1)

java.lang.AssertionError: Expected TermsAndConditionsPage but was Sign in to test (https://localhost:8543/auth/realms/test/login-actions/required-action?execution=TERMS_AND_CONDITIONS&client_id=test-app&tab_id=NQclBE9_6AI&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIiwic3QiOiI0M2ZiNDZkYi1lZWVlLTRhZjQtYjlmMi1kZjEzOWJiMDg3MmIifQ)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.actions.RequiredActionPriorityTest#setupTotpAfterUpdatePassword

Keycloak CI - Base IT (1)

java.lang.AssertionError: Expected AppPage but was Sign in to test (https://localhost:8543/auth/realms/test/login-actions/required-action?execution=UPDATE_PROFILE&client_id=test-app&tab_id=XsfH8gGKMZ0&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIiwic3QiOiI4MGI3MTQzMC1iMTRkLTQ0MDItOTcwYy1mZjBjNjRiMGI0YzAifQ)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.organization.admin.OrganizationInvitationLinkTest#testInviteNewUserRegistrationCustomRegistrationFlow

Keycloak CI - Base IT (3)

java.lang.AssertionError: 

Expected: is <false>
     but: was <true>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
...

Report flaky test

@pedroigor
Copy link
Contributor

pedroigor commented Feb 20, 2025

Btw, errors does not seem related and I found that the test OrganizationInvitationLinkTest is failing. I'm investigating.

@pedroigor
Copy link
Contributor

pedroigor commented Feb 20, 2025

Yes, this makes probably sense. Before we start to change this, we should clarify what we want to achieve. TBH, I don't have that extensive knowledge of LDAP and even less on ActiveDirectory...

I'm also learning more about this area/component but the givenName is a standard [1] LDAP attribute and it should always be mapped to the user attribute firstName. By doing that, that block I mentioned could be reviewed because we won't have conflicts anymore when the CN is used as the username attribute.

That block assumes the vendor is AD without even checking if the vendor is actually AD.

In general, I think we should:

[1] https://datatracker.ietf.org/doc/html/rfc4519#section-2.12

@keycloak-github-bot
Copy link

keycloak-github-bot bot commented Feb 21, 2025

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.actions.RequiredActionPriorityTest#executeRequiredActionWithCustomPriorityAppliesSamePriorityToSessionAndUserActions

Keycloak CI - Base IT (1)

java.lang.AssertionError: Event expected
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertNotNull(Assert.java:713)
	at org.keycloak.testsuite.AssertEvents.poll(AssertEvents.java:91)
...

Report flaky test

org.keycloak.testsuite.actions.RequiredActionPriorityTest#executeRequiredActionsWithDefaultPriority

Keycloak CI - Base IT (1)

java.lang.AssertionError: Expected TermsAndConditionsPage but was Sign in to test (https://localhost:8543/auth/realms/test/login-actions/required-action?execution=TERMS_AND_CONDITIONS&client_id=test-app&tab_id=nXr60-HZx0E&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIiwic3QiOiI4ZjRlOTYwNS01MWVmLTRjYjEtYWQzMi0zNGY4NGVhOTZiZmEifQ)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

org.keycloak.testsuite.actions.RequiredActionPriorityTest#setupTotpAfterUpdatePassword

Keycloak CI - Base IT (1)

java.lang.AssertionError: Expected AppPage but was Sign in to test (https://localhost:8543/auth/realms/test/login-actions/required-action?execution=UPDATE_PROFILE&client_id=test-app&tab_id=IbI3By-oeKU&client_data=eyJydSI6Imh0dHBzOi8vbG9jYWxob3N0Ojg1NDMvYXV0aC9yZWFsbXMvbWFzdGVyL2FwcC9hdXRoIiwicnQiOiJjb2RlIiwic3QiOiJmMTg0YmFmYS0yYmJmLTQ1ODMtOTMyNC1mMjg0ZDlhOGY0M2QifQ)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
...

Report flaky test

keycloak-github-bot[bot]
keycloak-github-bot bot reviewed Feb 21, 2025
View reviewed changes
Copy link

@keycloak-github-bot keycloak-github-bot bot left a comment

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@dasniko
Copy link
Contributor Author

dasniko commented Feb 21, 2025

By doing that, that block I mentioned could be reviewed because we won't have conflicts anymore when the CN is used as the username attribute.

I don't understand why cn should be the username attribute. Per RFC cn is the "common name" which refers to the persons full name[1]. The uid is IMHO the proper field for the username[2].
Is this a special case, that some vendor uses the cn as the username?

[1] https://datatracker.ietf.org/doc/html/rfc4519#section-2.3
[2] https://datatracker.ietf.org/doc/html/rfc4519#section-2.39

@dasniko
Copy link
Contributor Author

dasniko commented Feb 21, 2025

@pedroigor I pushed the refactoring of the if block in a separate commit (3163096), please have a look if it is the way you meant.

This was referenced Feb 21, 2025
Closed
Closed
Closed
@pedroigor
Copy link
Contributor

pedroigor commented Feb 21, 2025

I don't understand why cn should be the username attribute. Per RFC cn is the "common name" which refers to the persons full name[1]. The uid is IMHO the proper field for the username[2]. Is this a special case, that some vendor uses the cn as the username?

Same here. But I think for historical reasons, not sure if still valid, it is the case for some MSAD deployments. We need some input here from @mposolda or @sguilhen ...

The uid is IMHO the proper field for the username[2].
Is this a special case, that some vendor uses the cn as the username?

For some vendors yes but MSAD works differently, I think. IIRC, uid is multivalued and there is no guarantee for uniqueness when using this vendor. Not sure how others handle this attribute but most of time I see uid being used for vendors like (based on) OpenLDAP. Perhaps that is the reason why people don't use it in MSAD.

pedroigor
pedroigor reviewed Feb 21, 2025
View reviewed changes
Copy link
Contributor

@pedroigor pedroigor left a comment

Choose a reason for hiding this comment

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

@dasniko The changes to that "block" makes sense to me.

federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java
UserAttributeLDAPStorageMapper.IS_MANDATORY_IN_LDAP, "true");
realm.addComponentModel(mapperModel);

// For AD deployments with "sAMAccountName" as username and writable, we need to map "cn" as username as well (this is needed so we can register new users from KC into LDAP).
Copy link
Contributor

@pedroigor pedroigor Feb 21, 2025

Choose a reason for hiding this comment

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

Just wondering if this mapper is about making sure there is a value set for CN when creating users given that firstName and lastName might be optional.

If so, I think the mapper for CN might involve a specific logic to check whether the first and last names are optional and if so, fallback to the username value. Instead of mapping to a single attribute. This logic should not be specific to AD, I think, and other vendors will have the same mapper.

This is kinda what the FullNameLDAPStorageMapper does but it does make much sense to put the fullname into the username on KC side.

@dasniko
Copy link
Contributor Author

dasniko commented Feb 24, 2025

@pedroigor Is there anything I can/shall do currently? Or are we waiting just for some input from @mposolda and @sguilhen?

I initially "only" wanted to change this wrong default mapping between first- and common name. IMHO this is, what my PR does. Any further change should be done in a separate PR/issue!?

@sguilhen
Copy link
Contributor

sguilhen commented Mar 19, 2025

Sorry for the late reply @pedroigor and @dasniko . I think the changes here make sense. I'm not aware of any historical reason for mapping the cn attribute as the username, but I might be missing something.

@rmartinc Do you have anything to add here?

@dasniko
Copy link
Contributor Author

dasniko commented May 2, 2025

Hi @pedroigor , how do we want to proceed here with this issue/PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pedroigor pedroigor pedroigor left review comments

+1 more reviewer

@keycloak-github-bot keycloak-github-bot[bot] keycloak-github-bot[bot] left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

flaky-test team/cloud-native team/core-clients team/core-iam

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve wrong(?) default mapping of first name to LDAP 'cn' attribute to 'givenName'

4 participants

@dasniko @thomasdarimont @pedroigor @sguilhen