8000 Add per-mapper UUID attribute decoding toggle for Group LDAP mapper · keycloak/keycloak@512331e · GitHub
[go: up one dir, main page]
Skip to content

Commit 512331e

Browse files
sguilhenpedroigor
sguilhen
authored and
pedroigor
committed
Add per-mapper UUID attribute decoding toggle for Group LDAP mapper
- when enabled, the UUID LDAP attribute (e.g. objectGUID) in mapped group attributes is decoded to UUID string format Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>
1 parent b8c593b commit 512331e

10 files changed

Lines changed: 87 additions & 27 deletions

File tree

‎docs/documentation/upgrading/topics/changes/changes-26_7_0.adoc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ The User Attribute LDAP mapper now includes a *Decode Binary Attribute As* confi
139139

140140
This change reverts a regression introduced in 26.6.1 where the `objectGUID` binary attribute format was silently changed from base64 to UUID string for user attribute mappers. Existing binary attribute mappers are automatically migrated to use *base64* decoding to preserve their current behavior. If you were relying on the UUID string format introduced in 26.6.x, explicitly set the decoder to *uuid* after upgrading.
141141

142+
The Group LDAP mapper also includes a new *Decode UUID Attribute to UUID Format* toggle. When enabled, the UUID LDAP attribute (e.g. `objectGUID` in Active Directory) listed in "Mapped Group Attributes" is decoded to UUID string format instead of base64. New group mappers default to enabled (UUID format). Existing group mappers are automatically migrated to disabled (base64) to preserve their current behavior.
143+
142144
=== DPoP not supported for implicit and hybrid flows
143145

144146
DPoP (Demonstrating Proof of Possession) is no longer supported for implicit and hybrid flows. If a client has the *Require DPoP bound tokens* option enabled, authorization requests using implicit or hybrid response types will be rejected with an error.

‎federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,8 @@ private void updateAttributesOfKCGroup(GroupModel kcGroup, LDAPObject ldapGroup)
361361
if (attrValues == null) {
362362
kcGroup.removeAttribute(attrName);
363363
} else {
364-
if (attrName.equalsIgnoreCase(ldapConfig.getUuidLDAPAttributeName())) {
364+
if (config.isDecodeGroupUuidAttribute()
365+
&& attrName.equalsIgnoreCase(ldapConfig.getUuidLDAPAttributeName())) {
365366
attrValues = attrValues.stream()
366367
.map(v -> LDAPUtil.decodeBase64ToUuid(v, ldapConfig))
367368
.collect(Collectors.toCollection(java.util.LinkedHashSet::new));

‎federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapperFactory.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,13 @@ private static List<ProviderConfigProperty> getProps(ComponentModel parent) {
210210
"Leave this empty if no additional group attributes are required to be mapped in Keycloak. ")
211211
.type(ProviderConfigProperty.STRING_TYPE)
212212
.add()
213+
.property().name(GroupMapperConfig.DECODE_GROUP_UUID_ATTRIBUTE)
214+
.label("Decode UUID Attribute to UUID Format")
215+
.helpText("If on, the UUID LDAP attribute (e.g. objectGUID in Active Directory) listed in 'Mapped Group Attributes' " +
216+
"is decoded to UUID string format. If off, the attribute is kept as a base64-encoded string.")
217+
.type(ProviderConfigProperty.BOOLEAN_TYPE)
218+
.defaultValue("true")
219+
.add()
213220
.property().name(GroupMapperConfig.DROP_NON_EXISTING_GROUPS_DURING_SYNC)
214221
.label("Drop non-existing groups during sync")
215222
.helpText("If this flag is true, then during sync of groups from LDAP to Keycloak, we will keep just those Keycloak groups, which still exists in LDAP. Rest will be deleted")

‎federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupMapperConfig.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ public class GroupMapperConfig extends CommonLDAPGroupMapperConfig {
5555
// Name of attributes of the LDAP group object, which will be mapped as attributes of Group in Keycloak
5656
public static final String MAPPED_GROUP_ATTRIBUTES = "mapped.group.attributes";
5757

58+
// Controls whether the UUID LDAP attribute (e.g. objectGUID) is decoded to UUID format or kept as base64
59+
public static final String DECODE_GROUP_UUID_ATTRIBUTE = "decode.group.uuid.attribute";
60+
5861
// During sync of groups from LDAP to Keycloak, we will keep just those Keycloak groups, which still exists in LDAP. Rest will be deleted
5962
public static final String DROP_NON_EXISTING_GROUPS_DURING_SYNC = "drop.non.existing.groups.during.sync";
6063

@@ -131,6 +134,10 @@ public String getCustomLdapFilter() {
131134
return mapperModel.getConfig().getFirst(GROUPS_LDAP_FILTER);
132135
}
133136

137+
public boolean isDecodeGroupUuidAttribute() {
138+
return AbstractLDAPStorageMapper.parseBooleanParameter(mapperModel, DECODE_GROUP_UUID_ATTRIBUTE);
139+
}
140+
134141
public boolean isDropNonExistingGroupsDuringSync() {
135142
return AbstractLDAPStorageMapper.parseBooleanParameter(mapperModel, DROP_NON_EXISTING_GROUPS_DURING_SYNC);
136143
}

‎model/storage-private/src/main/java/org/keycloak/migration/migrators/MigrateTo26_7_0.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public void migrateRealm(KeycloakSession session, RealmModel realm) {
5454
renameDynamicScopeAttributes(realm);
5555
migrateParameterizedScopeTypes(realm);
5656
migrateLdapBinaryAttributeMappers(realm);
57+
migrateLdapGroupAttributeMappers(realm);
5758
}
5859

5960
private void migrateParameterizedScopeTypes(RealmModel realm) {
@@ -184,4 +185,16 @@ private void migrateLdapBinaryAttributeMappers(RealmModel realm) {
184185
realm.updateComponent(c);
185186
});
186187
}
188+
189+
private void migrateLdapGroupAttributeMappers(RealmModel realm) {
190+
realm.getComponentsStream(realm.getId(), UserStorageProvider.class.getName())
191+
.filter(c -> LDAPConstants.LDAP_PROVIDER.equals(c.getProviderId()))
192+
.flatMap(ldap -> realm.getComponentsStream(ldap.getId(), "org.keycloak.storage.ldap.mappers.LDAPStorageMapper"))
193+
.filter(c -> "group-ldap-mapper".equals(c.getProviderId()))
194+
.filter(c -> c.getConfig().getFirst("decode.group.uuid.attribute") == null)
195+
.forEach(c -> {
196+
c.getConfig().putSingle("decode.group.uuid.attribute", "false");
197+
realm.updateComponent(c);
198+
});
199+
}
187200
}

‎testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -468,11 +468,10 @@ protected void testMigrationTo26_4_0() {
468468
testSamlEncryptionAttributes(migrationRealm);
469469
}
470470

471-
protected void testMigrationTo26_7_0(boolean testLdapBinaryAttributeDecoderMigration) {
471+
protected void testMigrationTo26_7_0() {
472472
testParameterizedScopeTypesMigration(migrationRealm);
473-
if (testLdapBinaryAttributeDecoderMigration) {
474-
testLdapBinaryAttributeDecoderMigration(migrationRealm2);
475-
}
473+
testLdapBinaryAttributeDecoderMigration(migrationRealm2);
474+
testLdapGroupAttributeDecoderMigration(migrationRealm2);
476475
}
477476

478477
private void testParameterizedScopeTypesMigration(RealmResource realm) {
@@ -1379,6 +1378,22 @@ private void testLdapBinaryAttributeDecoderMigration(final RealmResource realm)
13791378
assertThat(binaryMapper.getConfig().getFirst("binary.attribute.decoder"), equalTo("base64"));
13801379
}
13811380

1381+
private void testLdapGroupAttributeDecoderMigration(final RealmResource realm) {
1382+
RealmRepresentation rep = realm.toRepresentation();
1383+
List<ComponentRepresentation> ldapProviders = realm.components().query(rep.getId(), UserStorageProvider.class.getName());
1384+
assertThat(ldapProviders.size(), equalTo(1));
1385+
String ldapProviderId = ldapProviders.get(0).getId();
1386+
1387+
List<ComponentRepresentation> mappers = realm.components().query(ldapProviderId,
1388+
"org.keycloak.storage.ldap.mappers.LDAPStorageMapper");
1389+
ComponentRepresentation groupMapper = mappers.stream()
1390+
.filter(c -> "group-ldap-mapper".equals(c.getProviderId()))
1391+
.findFirst()
1392+
.orElse(null);
1393+
assertNotNull(groupMapper, "Group mapper not found");
1394+
assertThat(groupMapper.getConfig().getFirst("decode.group.uuid.attribute"), equalTo("false"));
1395+
}
1396+
13821397
private void testHS512KeyCreated(RealmResource realm) {
13831398
List<ComponentRepresentation> keyProviders = realm.components().query(realm.toRepresentation().getId(), KeyProvider.class.getName());
13841399
Assertions.assertTrue(keyProviders.stream().anyMatch(c -> "hmac-generated".equals(c.getProviderId())

‎testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/JsonFileImport1903MigrationTest.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ public void migration19_0_3Test() throws Exception {
7373
testMigrationTo25_0_0();
7474
testMigrationTo26_0_0(true);
7575
testMigrationTo26_3_0();
76-
testMigrationTo26_7_0(true);
7776
}
7877

7978
@Test

‎testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/MigrationTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ public void migration19_xTest() throws Exception{
7777
testMigrationTo26_1_0(true);
7878
testMigrationTo26_3_0();
7979
testMigrationTo26_4_0();
80-
testMigrationTo26_7_0(true);
8180
}
8281

8382
@Test
@@ -94,6 +93,6 @@ public void migration24_xTest() throws Exception{
9493
testMigrationTo26_1_0(true);
9594
testMigrationTo26_3_0();
9695
testMigrationTo26_4_0();
97-
testMigrationTo26_7_0(false);
96+
testMigrationTo26_7_0();
9897
}
9998
}

‎testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-19.0.3.json‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2981,24 +2981,7 @@
29812981
"id": "307bba27-f8da-4c4d-a1b9-3c1bf49f824e",
29822982
"name": "ldap",
29832983
"providerId": "ldap",
2984-
"subComponents": {
2985-
"org.keycloak.storage.ldap.mappers.LDAPStorageMapper": [
2986-
{
2987-
"id": "a1b2c3d4-e5f6-7890-abcd-ef1234567890",
2988-
"name": "objectGUID",
2989-
"providerId": "user-attribute-ldap-mapper",
2990-
"subComponents": {},
2991-
"config": {
2992-
"ldap.attribute": [ "objectGUID" ],
2993-
"is.binary.attribute": [ "true" ],
2994-
"always.read.value.from.ldap": [ "true" ],
2995-
"user.model.attribute": [ "objectGUID" ],
2996-
"read.only": [ "true" ],
2997-
"is.mandatory.in.ldap": [ "false" ]
2998-
}
2999-
}
3000-
]
3001-
},
2984+
"subComponents": {},
30022985
"config": {
30032986
"pagination": [ "false" ],
30042987
"fullSyncPeriod": [ "-1" ],

‎testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-24.0.4.json‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5355,7 +5355,41 @@
53555355
"id" : "307bba27-f8da-4c4d-a1b9-3c1bf49f824e",
53565356
"name" : "ldap",
53575357
"providerId" : "ldap",
5358-
"subComponents" : { },
5358+
"subComponents" : {
5359+
"org.keycloak.storage.ldap.mappers.LDAPStorageMapper" : [ {
5360+
"id" : "a1b2c3d4-e5f6-7890-abcd-ef1234567890",
5361+
"name" : "objectGUID",
5362+
"providerId" : "user-attribute-ldap-mapper",
5363+
"subComponents" : { },
5364+
"config" : {
5365+
"ldap.attribute" : [ "objectGUID" ],
5366+
"is.binary.attribute" : [ "true" ],
5367+
"always.read.value.from.ldap" : [ "true" ],
5368+
"user.model.attribute" : [ "objectGUID" ],
5369+
"read.only" : [ "true" ],
5370+
"is.mandatory.in.ldap" : [ "false" ]
5371+
}
5372+
}, {
5373+
"id" : "b2c3d4e5-f6a7-8901-bcde-f12345678901",
5374+
"name" : "groups",
5375+
"providerId" : "group-ldap-mapper",
5376+
"subComponents" : { },
5377+
"config" : {
5378+
"groups.dn" : [ "ou=Groups,dc=keycloak,dc=org" ],
5379+
"group.name.ldap.attribute" : [ "cn" ],
5380+
"group.object.classes" : [ "groupOfNames" ],
5381+
"preserve.group.inheritance" : [ "true" ],
5382+
"ignore.missing.groups" : [ "false" ],
5383+
"mapped.group.attributes" : [ "objectGUID" ],
5384+
"membership.ldap.attribute" : [ "member" ],
5385+
"membership.attribute.type" : [ "DN" ],
5386+
"membership.user.ldap.attribute" : [ "uid" ],
5387+
"mode" : [ "LDAP_ONLY" ],
5388+
"user.roles.retrieve.strategy" : [ "LOAD_GROUPS_BY_MEMBER_ATTRIBUTE" ],
5389+
"drop.non.existing.groups.during.sync" : [ "false" ]
5390+
}
5391+
} ]
5392+
},
53595393
"config" : {
53605394
"fullSyncPeriod" : [ "-1" ],
53615395
"pagination" : [ "false" ],

0 commit comments

Comments
 (0)
0