security: cleanup the logging in the native stores

A lot of messages were being logged at the info level in the native user and roles
stores. This changes the logging to be more selective in the cases where the index
does not exist or the error is really an error and the user should be notified.

Closes elastic/elasticsearch#1339

Original commit: elastic/x-pack-elasticsearch@0bc0d9bf7a
This commit is contained in:
jaymode 2016-03-15 10:45:23 -04:00
parent e3551a7570
commit 60500ec6af
2 changed files with 93 additions and 56 deletions

View File

@ -60,6 +60,7 @@ import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.RemoteTransportException; import org.elasticsearch.transport.RemoteTransportException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
@ -148,7 +149,7 @@ public class ESNativeUsersStore extends AbstractComponent implements ClusterStat
if (t instanceof IndexNotFoundException) { if (t instanceof IndexNotFoundException) {
logger.trace("failed to retrieve user [{}] since security index does not exist", username); logger.trace("failed to retrieve user [{}] since security index does not exist", username);
} else { } else {
logger.info("failed to retrieve user [{}]", t, username); logger.debug("failed to retrieve user [{}]", t, username);
} }
// We don't invoke the onFailure listener here, instead // We don't invoke the onFailure listener here, instead
@ -186,8 +187,12 @@ public class ESNativeUsersStore extends AbstractComponent implements ClusterStat
// This function is MADNESS! But it works, don't think about it too hard... // This function is MADNESS! But it works, don't think about it too hard...
client.search(request, new ActionListener<SearchResponse>() { client.search(request, new ActionListener<SearchResponse>() {
private SearchResponse lastResponse = null;
@Override @Override
public void onResponse(final SearchResponse resp) { public void onResponse(final SearchResponse resp) {
lastResponse = resp;
boolean hasHits = resp.getHits().getHits().length > 0; boolean hasHits = resp.getHits().getHits().length > 0;
if (hasHits) { if (hasHits) {
for (SearchHit hit : resp.getHits().getHits()) { for (SearchHit hit : resp.getHits().getHits()) {
@ -200,19 +205,9 @@ public class ESNativeUsersStore extends AbstractComponent implements ClusterStat
.setScroll(scrollKeepAlive).request(); .setScroll(scrollKeepAlive).request();
client.searchScroll(scrollRequest, this); client.searchScroll(scrollRequest, this);
} else { } else {
ClearScrollRequest clearScrollRequest = client.prepareClearScroll().addScrollId(resp.getScrollId()).request(); if (resp.getScrollId() != null) {
client.clearScroll(clearScrollRequest, new ActionListener<ClearScrollResponse>() { clearScrollResponse(resp.getScrollId());
@Override
public void onResponse(ClearScrollResponse response) {
// cool, it cleared, we don't really care though...
} }
@Override
public void onFailure(Throwable t) {
// Not really much to do here except for warn about it...
logger.warn("failed to clear scroll [{}] after retrieving all users", t, resp.getScrollId());
}
});
// Finally, return the list of users // Finally, return the list of users
listener.onResponse(Collections.unmodifiableList(users)); listener.onResponse(Collections.unmodifiableList(users));
} }
@ -220,23 +215,28 @@ public class ESNativeUsersStore extends AbstractComponent implements ClusterStat
@Override @Override
public void onFailure(Throwable t) { public void onFailure(Throwable t) {
// attempt to clear scroll response
if (lastResponse != null && lastResponse.getScrollId() != null) {
clearScrollResponse(lastResponse.getScrollId());
}
if (t instanceof IndexNotFoundException) { if (t instanceof IndexNotFoundException) {
logger.trace("could not retrieve users because security index does not exist"); logger.trace("could not retrieve users because security index does not exist");
} else { // We don't invoke the onFailure listener here, instead just pass an empty list
logger.info("failed to retrieve users", t);
}
// We don't invoke the onFailure listener here, instead
// we call the response with an empty list
listener.onResponse(Collections.emptyList()); listener.onResponse(Collections.emptyList());
} else {
listener.onFailure(t);
}
} }
}); });
} catch (Exception e) { } catch (Exception e) {
logger.error("unable to retrieve users", e); logger.error("unable to retrieve users {}", e, Arrays.toString(usernames));
listener.onFailure(e); listener.onFailure(e);
} }
} }
private UserAndPassword getUserAndPassword(String username) { private UserAndPassword getUserAndPassword(final String username) {
final AtomicReference<UserAndPassword> userRef = new AtomicReference<>(null); final AtomicReference<UserAndPassword> userRef = new AtomicReference<>(null);
final CountDownLatch latch = new CountDownLatch(1); final CountDownLatch latch = new CountDownLatch(1);
getUserAndPassword(username, new LatchedActionListener<>(new ActionListener<UserAndPassword>() { getUserAndPassword(username, new LatchedActionListener<>(new ActionListener<UserAndPassword>() {
@ -247,19 +247,23 @@ public class ESNativeUsersStore extends AbstractComponent implements ClusterStat
@Override @Override
public void onFailure(Throwable t) { public void onFailure(Throwable t) {
logger.info("failed to retrieve user", t); if (t instanceof IndexNotFoundException) {
logger.trace("failed to retrieve user [{}] since security index does not exist", t, username);
} else {
logger.error("failed to retrieve user [{}]", t, username);
}
} }
}, latch)); }, latch));
try { try {
latch.await(30, TimeUnit.SECONDS); latch.await(30, TimeUnit.SECONDS);
} catch (InterruptedException e) { } catch (InterruptedException e) {
logger.info("timed out retrieving user"); logger.error("timed out retrieving user [{}]", username);
return null; return null;
} }
return userRef.get(); return userRef.get();
} }
private void getUserAndPassword(String user, final ActionListener<UserAndPassword> listener) { private void getUserAndPassword(final String user, final ActionListener<UserAndPassword> listener) {
try { try {
GetRequest request = client.prepareGet(ShieldTemplateService.SECURITY_INDEX_NAME, USER_DOC_TYPE, user).request(); GetRequest request = client.prepareGet(ShieldTemplateService.SECURITY_INDEX_NAME, USER_DOC_TYPE, user).request();
client.get(request, new ActionListener<GetResponse>() { client.get(request, new ActionListener<GetResponse>() {
@ -271,9 +275,9 @@ public class ESNativeUsersStore extends AbstractComponent implements ClusterStat
@Override @Override
public void onFailure(Throwable t) { public void onFailure(Throwable t) {
if (t instanceof IndexNotFoundException) { if (t instanceof IndexNotFoundException) {
logger.trace("could not retrieve user because security index does not exist", t); logger.trace("could not retrieve user [{}] because security index does not exist", t, user);
} else { } else {
logger.info("failed to retrieve user", t); logger.error("failed to retrieve user [{}]", t, user);
} }
// We don't invoke the onFailure listener here, instead // We don't invoke the onFailure listener here, instead
// we call the response with a null user // we call the response with a null user
@ -281,10 +285,10 @@ public class ESNativeUsersStore extends AbstractComponent implements ClusterStat
} }
}); });
} catch (IndexNotFoundException infe) { } catch (IndexNotFoundException infe) {
logger.trace("could not retrieve user because security index does not exist"); logger.trace("could not retrieve user [{}] because security index does not exist", user);
listener.onResponse(null); listener.onResponse(null);
} catch (Exception e) { } catch (Exception e) {
logger.error("unable to retrieve user", e); logger.error("unable to retrieve user [{}]", e, user);
listener.onFailure(e); listener.onFailure(e);
} }
} }
@ -501,6 +505,22 @@ public class ESNativeUsersStore extends AbstractComponent implements ClusterStat
listeners.add(listener); listeners.add(listener);
} }
private void clearScrollResponse(String scrollId) {
ClearScrollRequest clearScrollRequest = client.prepareClearScroll().addScrollId(scrollId).request();
client.clearScroll(clearScrollRequest, new ActionListener<ClearScrollResponse>() {
@Override
public void onResponse(ClearScrollResponse response) {
// cool, it cleared, we don't really care though...
}
@Override
public void onFailure(Throwable t) {
// Not really much to do here except for warn about it...
logger.warn("failed to clear scroll [{}]", t, scrollId);
}
});
}
private <Response> void clearRealmCache(String username, ActionListener<Response> listener, Response response) { private <Response> void clearRealmCache(String username, ActionListener<Response> listener, Response response) {
SecurityClient securityClient = new SecurityClient(client); SecurityClient securityClient = new SecurityClient(client);
ClearRealmCacheRequest request = securityClient.prepareClearRealmCache() ClearRealmCacheRequest request = securityClient.prepareClearRealmCache()
@ -565,7 +585,7 @@ public class ESNativeUsersStore extends AbstractComponent implements ClusterStat
Map<String, Object> metadata = (Map<String, Object>) sourceMap.get(User.Fields.METADATA.getPreferredName()); Map<String, Object> metadata = (Map<String, Object>) sourceMap.get(User.Fields.METADATA.getPreferredName());
return new UserAndPassword(new User(username, roles, fullName, email, metadata), password.toCharArray()); return new UserAndPassword(new User(username, roles, fullName, email, metadata), password.toCharArray());
} catch (Exception e) { } catch (Exception e) {
logger.error("error in the format of get response for user", e); logger.error("error in the format of data for user [{}]", e, username);
return null; return null;
} }
} }

View File

@ -44,7 +44,6 @@ import org.elasticsearch.shield.action.role.ClearRolesCacheRequest;
import org.elasticsearch.shield.action.role.ClearRolesCacheResponse; import org.elasticsearch.shield.action.role.ClearRolesCacheResponse;
import org.elasticsearch.shield.action.role.DeleteRoleRequest; import org.elasticsearch.shield.action.role.DeleteRoleRequest;
import org.elasticsearch.shield.action.role.PutRoleRequest; import org.elasticsearch.shield.action.role.PutRoleRequest;
import org.elasticsearch.shield.authc.AuthenticationService;
import org.elasticsearch.shield.authz.RoleDescriptor; import org.elasticsearch.shield.authz.RoleDescriptor;
import org.elasticsearch.shield.authz.permission.Role; import org.elasticsearch.shield.authz.permission.Role;
import org.elasticsearch.shield.authz.store.RolesStore; import org.elasticsearch.shield.authz.store.RolesStore;
@ -52,6 +51,7 @@ import org.elasticsearch.shield.client.SecurityClient;
import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.threadpool.ThreadPool;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
@ -101,8 +101,7 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore,
private volatile boolean shieldIndexExists = false; private volatile boolean shieldIndexExists = false;
@Inject @Inject
public ESNativeRolesStore(Settings settings, Provider<InternalClient> clientProvider, public ESNativeRolesStore(Settings settings, Provider<InternalClient> clientProvider, ThreadPool threadPool) {
Provider<AuthenticationService> authProvider, ThreadPool threadPool) {
super(settings); super(settings);
this.clientProvider = clientProvider; this.clientProvider = clientProvider;
this.threadPool = threadPool; this.threadPool = threadPool;
@ -192,8 +191,12 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore,
// This function is MADNESS! But it works, don't think about it too hard... // This function is MADNESS! But it works, don't think about it too hard...
client.search(request, new ActionListener<SearchResponse>() { client.search(request, new ActionListener<SearchResponse>() {
private SearchResponse lastResponse = null;
@Override @Override
public void onResponse(SearchResponse resp) { public void onResponse(SearchResponse resp) {
lastResponse = resp;
boolean hasHits = resp.getHits().getHits().length > 0; boolean hasHits = resp.getHits().getHits().length > 0;
if (hasHits) { if (hasHits) {
for (SearchHit hit : resp.getHits().getHits()) { for (SearchHit hit : resp.getHits().getHits()) {
@ -206,19 +209,9 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore,
.setScroll(scrollKeepAlive).request(); .setScroll(scrollKeepAlive).request();
client.searchScroll(scrollRequest, this); client.searchScroll(scrollRequest, this);
} else { } else {
ClearScrollRequest clearScrollRequest = client.prepareClearScroll().addScrollId(resp.getScrollId()).request(); if (resp.getScrollId() != null) {
client.clearScroll(clearScrollRequest, new ActionListener<ClearScrollResponse>() { clearScollRequest(resp.getScrollId());
@Override
public void onResponse(ClearScrollResponse response) {
// cool, it cleared, we don't really care though...
} }
@Override
public void onFailure(Throwable t) {
// Not really much to do here except for warn about it...
logger.warn("failed to clear scroll after retrieving all roles", t);
}
});
// Finally, return the list of users // Finally, return the list of users
listener.onResponse(Collections.unmodifiableList(roles)); listener.onResponse(Collections.unmodifiableList(roles));
} }
@ -226,18 +219,22 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore,
@Override @Override
public void onFailure(Throwable t) { public void onFailure(Throwable t) {
// attempt to clear the scroll request
if (lastResponse != null && lastResponse.getScrollId() != null) {
clearScollRequest(lastResponse.getScrollId());
}
if (t instanceof IndexNotFoundException) { if (t instanceof IndexNotFoundException) {
logger.trace("could not retrieve roles because security index does not exist"); logger.trace("could not retrieve roles because security index does not exist");
// since this is expected to happen at times, we just call the listener with an empty list
listener.onResponse(Collections.<RoleDescriptor>emptyList());
} else { } else {
logger.info("failed to retrieve roles", t); listener.onFailure(t);
} }
// We don't invoke the onFailure listener here, instead
// we call the response with an empty list
listener.onResponse(Collections.emptyList());
} }
}); });
} catch (Exception e) { } catch (Exception e) {
logger.error("unable to retrieve roles", e); logger.error("unable to retrieve roles {}", e, Arrays.toString(names));
listener.onFailure(e); listener.onFailure(e);
} }
} }
@ -283,7 +280,7 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore,
public void putRole(final PutRoleRequest request, final RoleDescriptor role, final ActionListener<Boolean> listener) { public void putRole(final PutRoleRequest request, final RoleDescriptor role, final ActionListener<Boolean> listener) {
if (state() != State.STARTED) { if (state() != State.STARTED) {
logger.trace("attempted to put role before service was started"); logger.trace("attempted to put role [{}] before service was started", request.name());
listener.onResponse(false); listener.onResponse(false);
} }
try { try {
@ -302,12 +299,12 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore,
@Override @Override
public void onFailure(Throwable e) { public void onFailure(Throwable e) {
logger.error("failed to put role to the index", e); logger.error("failed to put role [{}]", e, request.name());
listener.onFailure(e); listener.onFailure(e);
} }
}); });
} catch (Exception e) { } catch (Exception e) {
logger.error("unable to put role", e); logger.error("unable to put role [{}]", e, request.name());
listener.onFailure(e); listener.onFailure(e);
} }
@ -336,14 +333,18 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore,
@Override @Override
public void onFailure(Throwable t) { public void onFailure(Throwable t) {
logger.info("failed to retrieve role", t); if (t instanceof IndexNotFoundException) {
logger.trace("failed to retrieve role [{}] since security index does not exist", t, roleId);
} else {
logger.error("failed to retrieve role [{}]", t, roleId);
}
} }
}, latch)); }, latch));
try { try {
latch.await(30, TimeUnit.SECONDS); latch.await(30, TimeUnit.SECONDS);
} catch (InterruptedException e) { } catch (InterruptedException e) {
logger.info("timed out retrieving role"); logger.error("timed out retrieving role [{}]", roleId);
} }
GetResponse response = getRef.get(); GetResponse response = getRef.get();
@ -371,7 +372,7 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore,
GetRequest request = client.prepareGet(ShieldTemplateService.SECURITY_INDEX_NAME, ROLE_DOC_TYPE, role).request(); GetRequest request = client.prepareGet(ShieldTemplateService.SECURITY_INDEX_NAME, ROLE_DOC_TYPE, role).request();
client.get(request, listener); client.get(request, listener);
} catch (IndexNotFoundException e) { } catch (IndexNotFoundException e) {
logger.trace("security index does not exist", e); logger.trace("unable to retrieve role [{}] since security index does not exist", e, role);
listener.onResponse(new GetResponse( listener.onResponse(new GetResponse(
new GetResult(ShieldTemplateService.SECURITY_INDEX_NAME, ROLE_DOC_TYPE, role, -1, false, null, null))); new GetResult(ShieldTemplateService.SECURITY_INDEX_NAME, ROLE_DOC_TYPE, role, -1, false, null, null)));
} catch (Exception e) { } catch (Exception e) {
@ -380,6 +381,22 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore,
} }
} }
private void clearScollRequest(final String scrollId) {
ClearScrollRequest clearScrollRequest = client.prepareClearScroll().addScrollId(scrollId).request();
client.clearScroll(clearScrollRequest, new ActionListener<ClearScrollResponse>() {
@Override
public void onResponse(ClearScrollResponse response) {
// cool, it cleared, we don't really care though...
}
@Override
public void onFailure(Throwable t) {
// Not really much to do here except for warn about it...
logger.warn("failed to clear scroll [{}] after retrieving roles", t, scrollId);
}
});
}
// FIXME hack for testing // FIXME hack for testing
public void reset() { public void reset() {
final State state = state(); final State state = state();
@ -452,7 +469,7 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore,
try { try {
return RoleDescriptor.parse(name, sourceBytes); return RoleDescriptor.parse(name, sourceBytes);
} catch (Exception e) { } catch (Exception e) {
logger.warn("unable to deserialize role from response", e); logger.error("error in the format of data for role [{}]", e, name);
return null; return null;
} }
} }