From 7e2bb9c1aab2e033027267b1e5174e9cf83b969a Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 22 Dec 2018 07:34:14 -0500 Subject: [PATCH] Fix NPE in CachingUsernamePasswordRealm (#36953) This commit fixes an NPE in the CachingUsernamePasswordRealm when the cache is disabled. --- .../support/CachingUsernamePasswordRealm.java | 4 ++- .../CachingUsernamePasswordRealmTests.java | 29 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/CachingUsernamePasswordRealm.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/CachingUsernamePasswordRealm.java index 3924370fb33..faec4b8c665 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/CachingUsernamePasswordRealm.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/CachingUsernamePasswordRealm.java @@ -119,6 +119,7 @@ public abstract class CachingUsernamePasswordRealm extends UsernamePasswordRealm * @param listener to be called at completion */ private void authenticateWithCache(UsernamePasswordToken token, ActionListener listener) { + assert cache != null; try { final AtomicBoolean authenticationInCache = new AtomicBoolean(true); final ListenableFuture listenableCacheEntry = cache.computeIfAbsent(token.principal(), k -> { @@ -200,7 +201,7 @@ public abstract class CachingUsernamePasswordRealm extends UsernamePasswordRealm } protected int getCacheSize() { - return cache.count(); + return cache == null ? -1 : cache.count(); } protected abstract void doAuthenticate(UsernamePasswordToken token, ActionListener listener); @@ -221,6 +222,7 @@ public abstract class CachingUsernamePasswordRealm extends UsernamePasswordRealm } private void lookupWithCache(String username, ActionListener listener) { + assert cache != null; try { final AtomicBoolean lookupInCache = new AtomicBoolean(true); final ListenableFuture listenableCacheEntry = cache.computeIfAbsent(username, key -> { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/CachingUsernamePasswordRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/CachingUsernamePasswordRealmTests.java index ab824dcb34d..5bce054b42f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/CachingUsernamePasswordRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/CachingUsernamePasswordRealmTests.java @@ -92,30 +92,59 @@ public class CachingUsernamePasswordRealmTests extends ESTestCase { assertThat(realm.cacheHasher, sameInstance(Hasher.resolve(cachingHashAlgo))); } + public void testCacheSizeWhenCacheDisabled() { + final RealmConfig.RealmIdentifier identifier = new RealmConfig.RealmIdentifier("caching", "test_realm"); + final Settings settings = Settings.builder() + .put(globalSettings) + .put(RealmSettings.getFullSettingKey(identifier, CachingUsernamePasswordRealmSettings.CACHE_TTL_SETTING), -1) + .build(); + + final RealmConfig config = + new RealmConfig(identifier, settings, TestEnvironment.newEnvironment(globalSettings), new ThreadContext(Settings.EMPTY)); + final CachingUsernamePasswordRealm realm = new CachingUsernamePasswordRealm(config, threadPool) { + @Override + protected void doAuthenticate(UsernamePasswordToken token, ActionListener listener) { + listener.onResponse(AuthenticationResult.success(new User("username", new String[]{"r1", "r2", "r3"}))); + } + + @Override + protected void doLookupUser(String username, ActionListener listener) { + listener.onFailure(new UnsupportedOperationException("this method should not be called")); + } + }; + assertThat(realm.getCacheSize(), equalTo(-1)); + } + public void testAuthCache() { AlwaysAuthenticateCachingRealm realm = new AlwaysAuthenticateCachingRealm(globalSettings, threadPool); SecureString pass = new SecureString("pass"); PlainActionFuture future = new PlainActionFuture<>(); realm.authenticate(new UsernamePasswordToken("a", pass), future); future.actionGet(); + assertThat(realm.getCacheSize(), equalTo(1)); future = new PlainActionFuture<>(); realm.authenticate(new UsernamePasswordToken("b", pass), future); future.actionGet(); + assertThat(realm.getCacheSize(), equalTo(2)); future = new PlainActionFuture<>(); realm.authenticate(new UsernamePasswordToken("c", pass), future); future.actionGet(); + assertThat(realm.getCacheSize(), equalTo(3)); assertThat(realm.authInvocationCounter.intValue(), is(3)); future = new PlainActionFuture<>(); realm.authenticate(new UsernamePasswordToken("a", pass), future); future.actionGet(); + assertThat(realm.getCacheSize(), equalTo(3)); future = new PlainActionFuture<>(); realm.authenticate(new UsernamePasswordToken("b", pass), future); future.actionGet(); + assertThat(realm.getCacheSize(), equalTo(3)); future = new PlainActionFuture<>(); realm.authenticate(new UsernamePasswordToken("c", pass), future); future.actionGet(); + assertThat(realm.getCacheSize(), equalTo(3)); assertThat(realm.authInvocationCounter.intValue(), is(3)); assertThat(realm.lookupInvocationCounter.intValue(), is(0));