From fed34929aeb7d9f245cee538740b49b42dec4715 Mon Sep 17 00:00:00 2001
From: Sven-Torben Janus <sven-torben.janus@conciso.de>
Date: Thu, 9 Apr 2020 18:15:04 +0200
Subject: [PATCH] KEYCLOAK-13817 Fix X509 auth fails

when attribute value is always read from LDAP and import is enabled

When userattribute value is always read from LDAP, then the value is not
available in the local store. Therfore, KC will not find a user by that
attribute in the local store. When querying the LDAP storage
provider, the user will be found. However, when it is also available in
the local store (though without the attribute) it will not get imported
and therefore not returned with the result set of the LDAP storage
provider. Hence, the user will not be found at all.

This change adds the user to the result set of the LDAP user stoage
provider, iff the attribute user by the search is set to always read
value from LDAP.
---
 .../storage/ldap/LDAPStorageProvider.java     | 25 ++++++++++++++-----
 .../UserAttributeLDAPStorageMapper.java       |  5 +++-
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
index 70499a55e6..15ecb919f4 100755
--- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
+++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
@@ -35,7 +35,6 @@ import org.keycloak.credential.CredentialAuthentication;
 import org.keycloak.credential.CredentialInput;
 import org.keycloak.credential.CredentialInputUpdater;
 import org.keycloak.credential.CredentialInputValidator;
-import org.keycloak.credential.CredentialModel;
 import org.keycloak.federation.kerberos.impl.KerberosUsernamePasswordAuthenticator;
 import org.keycloak.federation.kerberos.impl.SPNEGOAuthenticator;
 import org.keycloak.models.*;
@@ -58,10 +57,7 @@ import org.keycloak.storage.ldap.idm.query.internal.LDAPQuery;
 import org.keycloak.storage.ldap.idm.query.internal.LDAPQueryConditionsBuilder;
 import org.keycloak.storage.ldap.idm.store.ldap.LDAPIdentityStore;
 import org.keycloak.storage.ldap.kerberos.LDAPProviderKerberosConfig;
-import org.keycloak.storage.ldap.mappers.LDAPOperationDecorator;
-import org.keycloak.storage.ldap.mappers.LDAPStorageMapper;
-import org.keycloak.storage.ldap.mappers.LDAPStorageMapperManager;
-import org.keycloak.storage.ldap.mappers.PasswordUpdateCallback;
+import org.keycloak.storage.ldap.mappers.*;
 import org.keycloak.storage.user.ImportedUserValidation;
 import org.keycloak.storage.user.UserLookupProvider;
 import org.keycloak.storage.user.UserQueryProvider;
@@ -237,9 +233,12 @@ public class LDAPStorageProvider implements UserStorageProvider,
 
              for (LDAPObject ldapUser : ldapObjects) {
                  String ldapUsername = LDAPUtils.getUsername(ldapUser, this.ldapIdentityStore.getConfig());
-                 if (session.userLocalStorage().getUserByUsername(ldapUsername, realm) == null) {
+                 UserModel localUser = session.userLocalStorage().getUserByUsername(ldapUsername, realm);
+                 if (localUser == null) {
                      UserModel imported = importUserFromLDAP(session, realm, ldapUser);
                      searchResults.add(imported);
+                 } else if (shouldUserAttributeBeAlwaysReadFromLdap(realm, attrName)) {
+                     searchResults.add(proxy(realm, localUser, ldapUser));
                  }
              }
 
@@ -247,6 +246,20 @@ public class LDAPStorageProvider implements UserStorageProvider,
          }
     }
 
+    private boolean shouldUserAttributeBeAlwaysReadFromLdap(RealmModel realm, String userAttributeName) {
+        List<ComponentModel> mapperModels = realm.getComponents(model.getId(), LDAPStorageMapper.class.getName());
+        return mapperModels.stream().anyMatch(mapperModel -> shouldUserAttributeBeAlwaysReadFromLdap(mapperModel, userAttributeName));
+    }
+
+    private boolean shouldUserAttributeBeAlwaysReadFromLdap(ComponentModel mapperModel, String userAttributeName) {
+        LDAPStorageMapper mapper = mapperManager.getMapper(mapperModel);
+        if (UserAttributeLDAPStorageMapper.class.isAssignableFrom(mapper.getClass())) {
+            UserAttributeLDAPStorageMapper userAttributeMapper = (UserAttributeLDAPStorageMapper) mapper;
+            return userAttributeName.equals(userAttributeMapper.getUserModelAttribute()) && userAttributeMapper.isAlwaysReadValueFromLdap();
+        }
+        return false;
+    }
+
     public boolean synchronizeRegistrations() {
         return "true".equalsIgnoreCase(model.getConfig().getFirst(LDAPConstants.SYNC_REGISTRATIONS)) && editMode == UserStorageProvider.EditMode.WRITABLE;
     }
diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/UserAttributeLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/UserAttributeLDAPStorageMapper.java
index 6f3f454c83..eb83f53cd4 100644
--- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/UserAttributeLDAPStorageMapper.java
+++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/UserAttributeLDAPStorageMapper.java
@@ -415,7 +415,7 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
         }
     }
 
-    private String getUserModelAttribute() {
+    public String getUserModelAttribute() {
         return mapperModel.getConfig().getFirst(USER_MODEL_ATTRIBUTE);
     }
 
@@ -431,6 +431,9 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
         return parseBooleanParameter(mapperModel, READ_ONLY);
     }
 
+    public boolean isAlwaysReadValueFromLdap() {
+        return parseBooleanParameter(mapperModel, ALWAYS_READ_VALUE_FROM_LDAP);
+    }
 
     protected void setPropertyOnUserModel(Property<Object> userModelProperty, UserModel user, String ldapAttrValue) {
         if (ldapAttrValue == null) {