From 80ff7b92db521027a1bcfad4dc99dfd933c9a9c8 Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 8 Jun 2015 17:39:13 +0200 Subject: [PATCH] KEYCLOAK-886 Reduce some LDAP info logging to trace and debug --- .../ldap/idm/query/internal/OrCondition.java | 2 -- .../idm/store/ldap/LDAPIdentityStore.java | 18 ++++++++--------- .../idm/store/ldap/LDAPOperationManager.java | 20 +++++++++---------- .../mappers/RoleLDAPFederationMapper.java | 7 ++----- .../models/UserFederationManager.java | 2 +- .../models/UserFederationProvider.java | 4 +++- 6 files changed, 24 insertions(+), 29 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/OrCondition.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/OrCondition.java index 436355b27f..d898ffd23a 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/OrCondition.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/OrCondition.java @@ -1,7 +1,5 @@ package org.keycloak.federation.ldap.idm.query.internal; -import java.util.List; - import org.keycloak.federation.ldap.idm.query.Condition; import org.keycloak.federation.ldap.idm.query.QueryParameter; diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java index 03b23a31bb..3dbfd0a399 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java @@ -181,8 +181,8 @@ public class LDAPIdentityStore implements IdentityStore { public boolean validatePassword(LDAPObject user, String password) { String userDN = user.getDn().toString(); - if (logger.isDebugEnabled()) { - logger.debugf("Using DN [%s] for authentication of user", userDN); + if (logger.isTraceEnabled()) { + logger.tracef("Using DN [%s] for authentication of user", userDN); } if (operationManager.authenticate(userDN, password)) { @@ -259,7 +259,9 @@ public class LDAPIdentityStore implements IdentityStore { filter.append(getObjectClassesFilter(identityQuery.getObjectClasses())); filter.append(")"); - logger.infof("Using filter for LDAP search: %s", filter); + if (logger.isTraceEnabled()) { + logger.tracef("Using filter for LDAP search: %s . Searching in DN: %s", filter, identityQuery.getSearchDn()); + } return filter; } @@ -378,10 +380,6 @@ public class LDAPIdentityStore implements IdentityStore { ldapObject.setDn(dn); ldapObject.setRdnAttributeName(dn.getFirstRdnAttrName()); - if (logger.isTraceEnabled()) { - logger.tracef("Populating LDAP Object from DN [%s]", entryDN); - } - NamingEnumeration ldapAttributes = attributes.getAll(); // Exact name of attributes might be different @@ -415,9 +413,6 @@ public class LDAPIdentityStore implements IdentityStore { if (ldapAttributeName.equalsIgnoreCase(LDAPConstants.OBJECT_CLASS)) { ldapObject.setObjectClasses(attrValues); } else { - if (logger.isTraceEnabled()) { - logger.tracef("Populating ldap attribute [%s] with value [%s] for DN [%s].", ldapAttributeName, attrValues.toString(), entryDN); - } if (attrValues.size() == 1) { ldapObject.setAttribute(ldapAttributeName, attrValues.iterator().next()); } else { @@ -431,6 +426,9 @@ public class LDAPIdentityStore implements IdentityStore { } } + if (logger.isTraceEnabled()) { + logger.tracef("Found ldap object [%s] and populated with the attributes [%s]. Read-only attributes are [%s]", ldapObject.getDn().toString(), ldapObject.getAttributes(), ldapObject.getReadOnlyAttributeNames()); + } return ldapObject; } catch (Exception e) { diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java index fd88f392c0..8d934f3a11 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java @@ -128,8 +128,8 @@ public class LDAPOperationManager { execute(new LdapOperation() { @Override public SearchResult execute(LdapContext context) throws NamingException { - if (logger.isDebugEnabled()) { - logger.debugf("Removing entry with DN [%s]", entryDn); + if (logger.isTraceEnabled()) { + logger.tracef("Removing entry with DN [%s]", entryDn); } destroySubcontext(context, entryDn); return null; @@ -357,8 +357,8 @@ public class LDAPOperationManager { public void modifyAttributes(final String dn, final ModificationItem[] mods) { try { - if (logger.isDebugEnabled()) { - logger.debugf("Modifying attributes for entry [%s]: [", dn); + if (logger.isTraceEnabled()) { + logger.tracef("Modifying attributes for entry [%s]: [", dn); for (ModificationItem item : mods) { Object values; @@ -369,10 +369,10 @@ public class LDAPOperationManager { values = "No values"; } - logger.debugf(" Op [%s]: %s = %s", item.getModificationOp(), item.getAttribute().getID(), values); + logger.tracef(" Op [%s]: %s = %s", item.getModificationOp(), item.getAttribute().getID(), values); } - logger.debugf("]"); + logger.tracef("]"); } execute(new LdapOperation() { @@ -389,18 +389,18 @@ public class LDAPOperationManager { public void createSubContext(final String name, final Attributes attributes) { try { - if (logger.isDebugEnabled()) { - logger.debugf("Creating entry [%s] with attributes: [", name); + if (logger.isTraceEnabled()) { + logger.tracef("Creating entry [%s] with attributes: [", name); NamingEnumeration all = attributes.getAll(); while (all.hasMore()) { Attribute attribute = all.next(); - logger.debugf(" %s = %s", attribute.getID(), attribute.get()); + logger.tracef(" %s = %s", attribute.getID(), attribute.get()); } - logger.debugf("]"); + logger.tracef("]"); } execute(new LdapOperation() { diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java index 2a78169c04..165309c1e7 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java @@ -79,8 +79,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { RoleContainerModel roleContainer = getTargetRoleContainer(mapperModel, realm); RoleModel role = roleContainer.getRole(roleName); - // TODO: debug - logger.infof("Granting role [%s] to user [%s] during import from LDAP", roleName, user.getUsername()); + logger.debugf("Granting role [%s] to user [%s] during import from LDAP", roleName, user.getUsername()); user.grantRole(role); } } @@ -94,8 +93,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { // Sync roles from LDAP tree and create them in local Keycloak DB (if they don't exist here yet) protected void syncRolesFromLDAP(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, RealmModel realm) { if (!rolesSyncedModels.contains(mapperModel.getId())) { - // TODO: debug - logger.infof("Syncing roles from LDAP into Keycloak DB. Mapper is [%s], LDAP provider is [%s]", mapperModel.getName(), ldapProvider.getModel().getDisplayName()); + logger.debugf("Syncing roles from LDAP into Keycloak DB. Mapper is [%s], LDAP provider is [%s]", mapperModel.getName(), ldapProvider.getModel().getDisplayName()); LDAPIdentityQuery ldapQuery = createRoleQuery(mapperModel, ldapProvider); @@ -108,7 +106,6 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { String roleName = ldapRole.getAttributeAsString(rolesRdnAttr); if (roleContainer.getRole(roleName) == null) { - // TODO: debug logger.infof("Syncing role [%s] from LDAP to keycloak DB", roleName); roleContainer.addRole(roleName); } diff --git a/model/api/src/main/java/org/keycloak/models/UserFederationManager.java b/model/api/src/main/java/org/keycloak/models/UserFederationManager.java index 93045c1f19..4682dddbfa 100755 --- a/model/api/src/main/java/org/keycloak/models/UserFederationManager.java +++ b/model/api/src/main/java/org/keycloak/models/UserFederationManager.java @@ -20,7 +20,7 @@ public class UserFederationManager implements UserProvider { protected KeycloakSession session; - // Set of already validated/proxied users during this session. Key is user ID + // Set of already validated/proxied federation users during this session. Key is user ID private Map managedUsers = new HashMap<>(); public UserFederationManager(KeycloakSession session) { diff --git a/model/api/src/main/java/org/keycloak/models/UserFederationProvider.java b/model/api/src/main/java/org/keycloak/models/UserFederationProvider.java index 7c97d675ee..8fdd45ac16 100755 --- a/model/api/src/main/java/org/keycloak/models/UserFederationProvider.java +++ b/model/api/src/main/java/org/keycloak/models/UserFederationProvider.java @@ -44,11 +44,12 @@ public interface UserFederationProvider extends Provider { /** * Gives the provider an option to validate if user still exists in federation backend and then proxy UserModel loaded from local storage. - * This method is called whenever a UserModel is pulled from local storage. + * This method is called whenever a UserModel is pulled from Keycloak local storage. * For example, the LDAP provider proxies the UserModel and does on-demand synchronization with * LDAP whenever UserModel update methods are invoked. It also overrides UserModel.updateCredential for the * credential types it supports * + * @param realm * @param local * @return null if user is no longer valid or proxy object otherwise */ @@ -122,6 +123,7 @@ public interface UserFederationProvider extends Provider { * Is the Keycloak UserModel still valid and/or existing in federated storage? Keycloak may call this method * in various user operations. The local storage may be deleted if this method returns false. * + * @param realm * @param local * @return */