Skip to content
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

Ldap log#116 #117

Merged
merged 4 commits into from
Mar 18, 2015
Merged

Ldap log#116 #117

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3d068f0
Merge remote-tracking branch 'upstream/master'
luan-cestari Mar 13, 2015
b660979
Merge remote-tracking branch 'upstream/master'
luan-cestari Mar 17, 2015
f9d7c99
Inserted some log statements
luan-cestari Mar 17, 2015
80ee513
Fixed one log level entry
luan-cestari Mar 17, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion auth/src/main/java/com/redhat/lightblue/rest/auth/jboss/CertLdapLoginModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ public class CertLdapLoginModule extends CertRolesLoginModule {

@Override
public void initialize(Subject subject, CallbackHandler callbackHandler, Map<String,?> sharedState, Map<String,?> options) {
addValidOptions(ALL_VALID_OPTIONS);
LOGGER.debug("CertLdapLoginModule#initialize was invoked");
addValidOptions(ALL_VALID_OPTIONS);
super.initialize(subject, callbackHandler, sharedState, options);
}

Expand Down
5 changes: 5 additions & 0 deletions auth/src/main/java/com/redhat/lightblue/rest/auth/ldap/LDAPCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public void onRemoval(@ParametersAreNonnullByDefault RemovalNotification notific
}
}
).build();
LOGGER.debug("LDAPCache: ldapCacheSession was created on a static block");

userRolesCacheSession = CacheBuilder.newBuilder()
.concurrencyLevel(10) // handle 10 concurrent request without a problem
Expand All @@ -54,17 +55,21 @@ public void onRemoval(@ParametersAreNonnullByDefault RemovalNotification notific
}
}
).build();
LOGGER.debug("LDAPCache: userRolesCacheSession was created on a static block");
}

public static Cache<LDAPCacheKey, SearchResult> getLDAPCacheSession() {
LOGGER.debug("LDAPCache#getLDAPCacheSession");
return ldapCacheSession;
}

public static Cache<LDAPCacheKey, List<String>> getUserRolesCacheSession() {
LOGGER.debug("LDAPCache#getUserRolesCacheSession");
return userRolesCacheSession;
}

public static void invalidateKey(LDAPCacheKey key) {
LOGGER.debug("LDAPCache#invalidateKey was invoked");
ldapCacheSession.invalidate(key);
userRolesCacheSession.invalidate(key);
}
Expand Down
4 changes: 4 additions & 0 deletions auth/src/main/java/com/redhat/lightblue/rest/auth/ldap/LDAPSearcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ public class LDAPSearcher {
private static final Logger LOGGER = Logger.getLogger(LDAPSearcher.class);

public static SearchResult searchLDAPServer(LDAPCacheKey ldapCacheKey) throws NamingException, LDAPUserNotFoundException, LDAPMultipleUserFoundException {
LOGGER.debug("LDAPSearcher#searchLDAPServer was invoked and it will call the remote LDAP server");

// Extension a: returns an exception as the LDAP server is down (eg.: this can be meaningful to use the cache )
NamingEnumeration<SearchResult> results = ldapCacheKey.ldapContext.search(ldapCacheKey.ldapSearchBase, ldapCacheKey.searchFilter, ldapCacheKey.searchControls);
SearchResult searchResult = null;
Expand All @@ -29,9 +31,11 @@ public static SearchResult searchLDAPServer(LDAPCacheKey ldapCacheKey) throws Na
}

// Basic flow: returns the unique entry from LDAP server
LOGGER.debug("LDAPSearcher#searchLDAPServer could retrieve the values from the remote LDAP Server");
return searchResult;
} else {
// Extension c: returns an exception to notify that the user was not found (eg.: this can be meaningful to evict the key )
LOGGER.debug("LDAPSearcher#searchLDAPServer could NOT retrieve the user from the remote LDAP Server");
throw new LDAPUserNotFoundException();
}
}
Expand Down
17 changes: 13 additions & 4 deletions auth/src/main/java/com/redhat/lightblue/rest/auth/ldap/LdapFindUserByUidCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class LdapFindUserByUidCommand extends HystrixCommand<SearchResult> {
private static final Logger LOGGER = Logger.getLogger(LightblueLdapRoleProvider.class);

static {
LOGGER.debug("Invoking ServoGraphiteSetup#initialize on a static block");
ServoGraphiteSetup.initialize();
}

Expand All @@ -31,22 +32,26 @@ public class LdapFindUserByUidCommand extends HystrixCommand<SearchResult> {
public LdapFindUserByUidCommand(LdapContext ldapContext, String ldapSearchBase, String uid) {
super(HystrixCommand.Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GROUPKEY)).
andCommandKey(HystrixCommandKey.Factory.asKey(GROUPKEY + ":" + LdapFindUserByUidCommand.class.getSimpleName())));
LOGGER.debug("Creating LdapFindUserByUidCommand");
//check if the informed parameters are valid
if (Strings.isNullOrEmpty(uid)) {
LOGGER.error("uid informed in LdapFindUserByUidCommand constructor is empty or null");
throw new IllegalArgumentException(String.format(INVALID_PARAM, "uid"));
} else if (Strings.isNullOrEmpty(ldapSearchBase)) {
LOGGER.error("ldapSearchBase informed in LdapFindUserByUidCommand constructor is empty or null");
throw new IllegalArgumentException(String.format(INVALID_PARAM, "ldapSearchBase"));
} else if (ldapContext == null) {
LOGGER.error("ldapContext informed in LdapFindUserByUidCommand constructor is null");
throw new IllegalArgumentException(String.format(INVALID_PARAM, "ldapContext"));
}

this.cacheKey = new LDAPCacheKey(uid, ldapContext, ldapSearchBase, "(uid=" + uid + ")", SearchControls.SUBTREE_SCOPE);
}

@Override
protected SearchResult run() throws Exception {
SearchResult searchResult = null;
LOGGER.debug("LdapFindUserByUidCommand#run was invoked");

SearchResult searchResult = null;
try {
searchResult = LDAPSearcher.searchLDAPServer(cacheKey);
} catch (NamingException e) {
Expand All @@ -57,7 +62,7 @@ protected SearchResult run() throws Exception {
// Return null in case the User not found or multiple Users were found (which is inconsistent)

if (e instanceof LDAPUserNotFoundException)
LOGGER.info("No result found roles for user: " + cacheKey.uid, e);
LOGGER.error("No result found roles for user: " + cacheKey.uid, e);
else {
LOGGER.error("Multiples users found and only one was expected for user: " + cacheKey.uid, e);
}
Expand All @@ -68,6 +73,7 @@ protected SearchResult run() throws Exception {
LDAPCache.invalidateKey(cacheKey);
}
}
LOGGER.debug("LdapFindUserByUidCommand#run : user found! Adding it to the cache");
LDAPCache.getLDAPCacheSession().put(cacheKey, searchResult);

return searchResult;
Expand All @@ -78,7 +84,7 @@ This methods is executed for all types of failure such as run() failure, timeout
*/
@Override
protected SearchResult getFallback() {
LOGGER.info("Error during the execution of the command. Falling back to the cache");
LOGGER.error("Error during the execution of the command. Falling back to the cache");
Copy link
Member

Choose a reason for hiding this comment

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

Should we log this at error level or something lower, maybe warn? Because we have a fallback, seems it could be a warning and if the fallback fails we'd have an error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree. I will switch to warn level . At the first sight I thought this would not impact being this way but as it can have a heavy load it could wast a lot of space and cpu .

return new FallbackViaLDAPServerProblemCommand(cacheKey, getFailedExecutionException()).execute();

}
Expand All @@ -96,19 +102,22 @@ private static class FallbackViaLDAPServerProblemCommand extends HystrixCommand<
public FallbackViaLDAPServerProblemCommand(LDAPCacheKey cacheKey, Throwable failedExecutionThrowable) {
super(HystrixCommand.Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GROUPKEY)).
andCommandKey(HystrixCommandKey.Factory.asKey(GROUPKEY + ":" + FallbackViaLDAPServerProblemCommand.class.getSimpleName())));
LOGGER.debug("FallbackViaLDAPServerProblemCommand constructor");
this.cacheKey = cacheKey;
this.failedExecutionThrowable = failedExecutionThrowable;
}

@Override
protected SearchResult run() throws Exception {
LOGGER.debug("FallbackViaLDAPServerProblemCommand#run was invoked and the following Exception caused the fallback", failedExecutionThrowable);
SearchResult searchResult = LDAPCache.getLDAPCacheSession().getIfPresent(cacheKey);
if (searchResult == null) {
CachedLDAPUserNotFoundException e = new CachedLDAPUserNotFoundException();
LOGGER.error("Failed to connect to the server and no result found roles for user: " + cacheKey.uid, e);
throw e;
}
// was able to use the cache or use the LDAP server on the second retry
LOGGER.debug("FallbackViaLDAPServerProblemCommand#run : user found!");
return searchResult;
}

Expand Down
10 changes: 9 additions & 1 deletion auth/src/main/java/com/redhat/lightblue/rest/auth/ldap/LightblueLdapRoleProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public class LightblueLdapRoleProvider implements LightblueRoleProvider {
String ldapSearchBase;

public LightblueLdapRoleProvider(String server, String searchBase, String bindDn, String bindDNPwd) throws NamingException {
LOGGER.debug("Creating LightblueLdapRoleProvider");
Hashtable<String, Object> env = new Hashtable<>();
env.put(Context.SECURITY_AUTHENTICATION, "simple");
if (bindDn != null) {
Expand All @@ -54,13 +55,14 @@ public LightblueLdapRoleProvider(String server, String searchBase, String bindDn
env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
env.put(Context.PROVIDER_URL, server);
ldapSearchBase = searchBase;
LOGGER.debug("Creating InitialLdapContext ");
ldapContext = new InitialLdapContext(env, null);
}

@Override
public List<String> getUserRoles(String userName) {
LOGGER.debug("Invoking LightblueLdapRoleProvider#getUserRoles");
List<String> userRoles = new ArrayList<>();

try {
userRoles.addAll(getUserRolesFromCache(userName));

Expand All @@ -74,32 +76,38 @@ public List<String> getUserRoles(String userName) {
LOGGER.error("Naming problem with LDAP for user: " + userName, ne);
} catch (HystrixRuntimeException ce) {
// Not found in cache, returns an empty list
LOGGER.error("Not found in cache, returns an empty list " + userName, ce);
}

return userRoles;
}

@Override
public Collection<String> getUsersInGroup(String groupName) {
LOGGER.error("Invoking LightblueLdapRoleProvider#getUsersInGroup (not supported))");
throw new UnsupportedOperationException("Not yet implemented");
}

@Override
public void flushRoleCache(String roleName) {
LOGGER.error("Invoking LightblueLdapRoleProvider#flushRoleCache (not supported))");
throw new UnsupportedOperationException("Not yet implemented");
}

@Override
public void flushUserCache(String userName) {
LOGGER.error("Invoking LightblueLdapRoleProvider#flushUserCache (not supported))");
throw new UnsupportedOperationException("Not yet implemented");
}

private List<String> getUserRolesFromCache(String userName) {
LOGGER.debug("Invoking LightblueLdapRoleProvider#getUserRolesFromCache");
LDAPCacheKey cacheKey = new LDAPCacheKey(userName, ldapContext, ldapSearchBase, "(uid=" + userName + ")", SearchControls.SUBTREE_SCOPE);
return LDAPCache.getUserRolesCacheSession().getIfPresent(cacheKey);
}

private List<String> getUserRolesFromLdap(SearchResult ldapUser) throws NamingException {
LOGGER.debug("Invoking LightblueLdapRoleProvider#getUserRolesFromLdap");
List<String> groups = new ArrayList<>();

//if no user found it should return an empty list (I think)
Expand Down