From 85ccd64e01f3b5d0fc73a25361b2844192b838c3 Mon Sep 17 00:00:00 2001 From: mposolda Date: Fri, 11 Mar 2016 18:06:35 +0100 Subject: [PATCH] KEYCLOAK-2643 Added write-only property to LDAP full-name attribute mapper --- .../ldap/LDAPFederationProviderFactory.java | 3 +- .../mappers/FullNameLDAPFederationMapper.java | 14 ++++++++ .../FullNameLDAPFederationMapperFactory.java | 31 ++++++++++++++--- .../HardcodedLDAPRoleMapperFactory.java | 2 +- ...rAttributeLDAPFederationMapperFactory.java | 2 +- .../GroupLDAPFederationMapperFactory.java | 2 +- .../role/RoleLDAPFederationMapperFactory.java | 2 +- .../MSADUserAccountControlMapperFactory.java | 2 +- .../mappers/UserFederationMapperFactory.java | 3 +- .../admin/UserFederationProviderResource.java | 2 +- .../FederationProvidersIntegrationTest.java | 33 ++++++++++++++++++- .../admin/messages/messages_en.properties | 2 ++ 12 files changed, 84 insertions(+), 14 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java index 0250b1103f..51a3c8cb70 100755 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java @@ -165,7 +165,8 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi // For read-only LDAP, we map "cn" as full name mapperModel = KeycloakModelUtils.createUserFederationMapperModel("full name", newProviderModel.getId(), FullNameLDAPFederationMapperFactory.PROVIDER_ID, FullNameLDAPFederationMapper.LDAP_FULL_NAME_ATTRIBUTE, LDAPConstants.CN, - UserAttributeLDAPFederationMapper.READ_ONLY, readOnly); + FullNameLDAPFederationMapper.READ_ONLY, readOnly, + FullNameLDAPFederationMapper.WRITE_ONLY, "false"); realm.addUserFederationMapper(mapperModel); } } diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java index 113a57d325..b94b5b497c 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java @@ -40,6 +40,8 @@ public class FullNameLDAPFederationMapper extends AbstractLDAPFederationMapper { public static final String LDAP_FULL_NAME_ATTRIBUTE = "ldap.full.name.attribute"; public static final String READ_ONLY = "read.only"; + public static final String WRITE_ONLY = "write.only"; + public FullNameLDAPFederationMapper(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, RealmModel realm) { super(mapperModel, ldapProvider, realm); @@ -47,6 +49,10 @@ public class FullNameLDAPFederationMapper extends AbstractLDAPFederationMapper { @Override public void onImportUserFromLDAP(LDAPObject ldapUser, UserModel user, boolean isCreate) { + if (isWriteOnly()) { + return; + } + String ldapFullNameAttrName = getLdapFullNameAttrName(); String fullName = ldapUser.getAttributeAsString(ldapFullNameAttrName); if (fullName == null) { @@ -117,6 +123,10 @@ public class FullNameLDAPFederationMapper extends AbstractLDAPFederationMapper { @Override public void beforeLDAPQuery(LDAPQuery query) { + if (isWriteOnly()) { + return; + } + String ldapFullNameAttrName = getLdapFullNameAttrName(); query.addReturningLdapAttribute(ldapFullNameAttrName); @@ -178,4 +188,8 @@ public class FullNameLDAPFederationMapper extends AbstractLDAPFederationMapper { private boolean isReadOnly() { return parseBooleanParameter(mapperModel, READ_ONLY); } + + private boolean isWriteOnly() { + return parseBooleanParameter(mapperModel, WRITE_ONLY); + } } diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapperFactory.java index bc4b34a42e..32826b2dda 100755 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapperFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapperFactory.java @@ -43,12 +43,17 @@ public class FullNameLDAPFederationMapperFactory extends AbstractLDAPFederationM static { ProviderConfigProperty userModelAttribute = createConfigProperty(FullNameLDAPFederationMapper.LDAP_FULL_NAME_ATTRIBUTE, "LDAP Full Name Attribute", - "Name of LDAP attribute, which contains fullName of user. In most cases it will be 'cn' ", ProviderConfigProperty.STRING_TYPE, null); + "Name of LDAP attribute, which contains fullName of user. Usually it will be 'cn' ", ProviderConfigProperty.STRING_TYPE, null); configProperties.add(userModelAttribute); - ProviderConfigProperty readOnly = createConfigProperty(UserAttributeLDAPFederationMapper.READ_ONLY, "Read Only", + ProviderConfigProperty readOnly = createConfigProperty(FullNameLDAPFederationMapper.READ_ONLY, "Read Only", "For Read-only is data imported from LDAP to Keycloak DB, but it's not saved back to LDAP when user is updated in Keycloak.", ProviderConfigProperty.BOOLEAN_TYPE, null); configProperties.add(readOnly); + + ProviderConfigProperty writeOnly = createConfigProperty(FullNameLDAPFederationMapper.WRITE_ONLY, "Write Only", + "For Write-only is data propagated to LDAP when user is created or updated in Keycloak. But this mapper is not used to propagate data from LDAP back into Keycloak. " + + "This setting is useful if you configured separate firstName and lastName attribute mappers and you want to use those to read attribute from LDAP into Keycloak", ProviderConfigProperty.BOOLEAN_TYPE, null); + configProperties.add(writeOnly); } @Override @@ -78,8 +83,11 @@ public class FullNameLDAPFederationMapperFactory extends AbstractLDAPFederationM defaultValues.put(FullNameLDAPFederationMapper.LDAP_FULL_NAME_ATTRIBUTE, LDAPConstants.CN); - String readOnly = config.getEditMode() == UserFederationProvider.EditMode.WRITABLE ? "false" : "true"; - defaultValues.put(UserAttributeLDAPFederationMapper.READ_ONLY, readOnly); + boolean readOnly = config.getEditMode() != UserFederationProvider.EditMode.WRITABLE; + defaultValues.put(FullNameLDAPFederationMapper.READ_ONLY, String.valueOf(readOnly)); + + String writeOnly = String.valueOf(!readOnly); + defaultValues.put(FullNameLDAPFederationMapper.WRITE_ONLY, writeOnly); return defaultValues; } @@ -90,8 +98,21 @@ public class FullNameLDAPFederationMapperFactory extends AbstractLDAPFederationM } @Override - public void validateConfig(RealmModel realm, UserFederationMapperModel mapperModel) throws FederationConfigValidationException { + public void validateConfig(RealmModel realm, UserFederationProviderModel fedProviderModel, UserFederationMapperModel mapperModel) throws FederationConfigValidationException { checkMandatoryConfigAttribute(FullNameLDAPFederationMapper.LDAP_FULL_NAME_ATTRIBUTE, "LDAP Full Name Attribute", mapperModel); + + boolean readOnly = AbstractLDAPFederationMapper.parseBooleanParameter(mapperModel, FullNameLDAPFederationMapper.READ_ONLY); + boolean writeOnly = AbstractLDAPFederationMapper.parseBooleanParameter(mapperModel, FullNameLDAPFederationMapper.WRITE_ONLY); + + LDAPConfig cfg = new LDAPConfig(fedProviderModel.getConfig()); + UserFederationProvider.EditMode editMode = cfg.getEditMode(); + + if (writeOnly && cfg.getEditMode() != UserFederationProvider.EditMode.WRITABLE) { + throw new FederationConfigValidationException("ldapErrorCantWriteOnlyForReadOnlyLdap"); + } + if (writeOnly && readOnly) { + throw new FederationConfigValidationException("ldapErrorCantWriteOnlyAndReadOnly"); + } } @Override diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/HardcodedLDAPRoleMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/HardcodedLDAPRoleMapperFactory.java index 73a4cef364..1ca93f5b2a 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/HardcodedLDAPRoleMapperFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/HardcodedLDAPRoleMapperFactory.java @@ -77,7 +77,7 @@ public class HardcodedLDAPRoleMapperFactory extends AbstractLDAPFederationMapper } @Override - public void validateConfig(RealmModel realm, UserFederationMapperModel mapperModel) throws FederationConfigValidationException { + public void validateConfig(RealmModel realm, UserFederationProviderModel fedProviderModel, UserFederationMapperModel mapperModel) throws FederationConfigValidationException { String roleName = mapperModel.getConfig().get(HardcodedLDAPRoleMapper.ROLE); if (roleName == null) { throw new FederationConfigValidationException("Role can't be null"); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapperFactory.java index f90eec3308..b0a7faae4a 100755 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapperFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapperFactory.java @@ -101,7 +101,7 @@ public class UserAttributeLDAPFederationMapperFactory extends AbstractLDAPFedera } @Override - public void validateConfig(RealmModel realm, UserFederationMapperModel mapperModel) throws FederationConfigValidationException { + public void validateConfig(RealmModel realm, UserFederationProviderModel fedProviderModel, UserFederationMapperModel mapperModel) throws FederationConfigValidationException { checkMandatoryConfigAttribute(UserAttributeLDAPFederationMapper.USER_MODEL_ATTRIBUTE, "User Model Attribute", mapperModel); checkMandatoryConfigAttribute(UserAttributeLDAPFederationMapper.LDAP_ATTRIBUTE, "LDAP Attribute", mapperModel); } diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapperFactory.java index e43a896002..fb6056b743 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapperFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapperFactory.java @@ -186,7 +186,7 @@ public class GroupLDAPFederationMapperFactory extends AbstractLDAPFederationMapp } @Override - public void validateConfig(RealmModel realm, UserFederationMapperModel mapperModel) throws FederationConfigValidationException { + public void validateConfig(RealmModel realm, UserFederationProviderModel fedProviderModel, UserFederationMapperModel mapperModel) throws FederationConfigValidationException { checkMandatoryConfigAttribute(GroupMapperConfig.GROUPS_DN, "LDAP Groups DN", mapperModel); checkMandatoryConfigAttribute(GroupMapperConfig.MODE, "Mode", mapperModel); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapperFactory.java index fa5b13d1f0..25a7ad1bc8 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapperFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapperFactory.java @@ -180,7 +180,7 @@ public class RoleLDAPFederationMapperFactory extends AbstractLDAPFederationMappe } @Override - public void validateConfig(RealmModel realm, UserFederationMapperModel mapperModel) throws FederationConfigValidationException { + public void validateConfig(RealmModel realm, UserFederationProviderModel fedProviderModel, UserFederationMapperModel mapperModel) throws FederationConfigValidationException { checkMandatoryConfigAttribute(RoleMapperConfig.ROLES_DN, "LDAP Roles DN", mapperModel); checkMandatoryConfigAttribute(RoleMapperConfig.MODE, "Mode", mapperModel); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/msad/MSADUserAccountControlMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/msad/MSADUserAccountControlMapperFactory.java index 13c605bd17..36c494dcdd 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/msad/MSADUserAccountControlMapperFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/msad/MSADUserAccountControlMapperFactory.java @@ -75,7 +75,7 @@ public class MSADUserAccountControlMapperFactory extends AbstractLDAPFederationM } @Override - public void validateConfig(RealmModel realm, UserFederationMapperModel mapperModel) throws FederationConfigValidationException { + public void validateConfig(RealmModel realm, UserFederationProviderModel fedProviderModel, UserFederationMapperModel mapperModel) throws FederationConfigValidationException { } @Override diff --git a/server-spi/src/main/java/org/keycloak/mappers/UserFederationMapperFactory.java b/server-spi/src/main/java/org/keycloak/mappers/UserFederationMapperFactory.java index dcf0b773df..661462cc1f 100644 --- a/server-spi/src/main/java/org/keycloak/mappers/UserFederationMapperFactory.java +++ b/server-spi/src/main/java/org/keycloak/mappers/UserFederationMapperFactory.java @@ -53,10 +53,11 @@ public interface UserFederationMapperFactory extends ProviderFactory