[Kerberos] Refactoring and remove configs with defaults (#32152)

bizybot · web-flow · commit f0df110263ab · 2018-07-20T08:37:42.000+10:00
This commit does some refactoring to remove support package
and move classes to kerberos package.
That was the only class in that package, so no need for it to be in
separate package.
Changes done to use default values for jaas configuration options
for the ones which we can use defaults.
Fix couple of random failures in tests.
Modified `refreshKrb5Config` to use default value `false` in
KerberosTicketValidator. If the krb5.conf file is modified then we
will need to restart JVM as the config will not be refreshed.
For testing, `refreshKrb5Config` is set to `true` as we keep
changing the kdc port. This is set in SpnegoClient and only for tests.
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealm.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealm.java
@@ -20,7 +20,6 @@
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
 import org.elasticsearch.xpack.core.security.authc.kerberos.KerberosRealmSettings;
 import org.elasticsearch.xpack.core.security.user.User;
-import org.elasticsearch.xpack.security.authc.kerberos.support.KerberosTicketValidator;
 import org.elasticsearch.xpack.security.authc.support.CachingRealm;
 import org.elasticsearch.xpack.security.authc.support.UserRoleMapper;
 import org.elasticsearch.xpack.security.authc.support.mapper.NativeRoleMappingStore;
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosTicketValidator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosTicketValidator.java
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-package org.elasticsearch.xpack.security.authc.kerberos.support;
+package org.elasticsearch.xpack.security.authc.kerberos;
 
 import org.apache.logging.log4j.Logger;
 import org.elasticsearch.ExceptionsHelper;
@@ -262,8 +262,6 @@ public AppConfigurationEntry[] getAppConfigurationEntry(final String name) {
             options.put("useKeyTab", Boolean.TRUE.toString());
             options.put("storeKey", Boolean.TRUE.toString());
             options.put("doNotPrompt", Boolean.TRUE.toString());
-            options.put("renewTGT", Boolean.FALSE.toString());
-            options.put("refreshKrb5Config", Boolean.TRUE.toString());
             options.put("isInitiator", Boolean.FALSE.toString());
             options.put("debug", Boolean.toString(krbDebug));
 
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealmAuthenticateFailedTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealmAuthenticateFailedTests.java
@@ -67,18 +67,17 @@ public void testAuthenticateDifferentFailureScenarios() throws LoginException, G
         final boolean nullKerberosAuthnToken = rarely();
         final KerberosAuthenticationToken kerberosAuthenticationToken =
                 nullKerberosAuthnToken ? null : new KerberosAuthenticationToken(decodedTicket);
-
-        final PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
-        kerberosRealm.authenticate(kerberosAuthenticationToken, future);
-        AuthenticationResult result = future.actionGet();
-        assertThat(result, is(notNullValue()));
         if (nullKerberosAuthnToken) {
-            assertThat(result.getStatus(), is(equalTo(AuthenticationResult.Status.CONTINUE)));
+            expectThrows(AssertionError.class,
+                    () -> kerberosRealm.authenticate(kerberosAuthenticationToken, PlainActionFuture.newFuture()));
         } else {
+            final PlainActionFuture<AuthenticationResult> future = new PlainActionFuture<>();
+            kerberosRealm.authenticate(kerberosAuthenticationToken, future);
+            AuthenticationResult result = future.actionGet();
+            assertThat(result, is(notNullValue()));
             if (validTicket) {
                 final String expectedUsername = maybeRemoveRealmName(username);
-                final User expectedUser = new User(expectedUsername, roles.toArray(new String[roles.size()]), null, null, null,
-                        true);
+                final User expectedUser = new User(expectedUsername, roles.toArray(new String[roles.size()]), null, null, null, true);
                 assertSuccessAuthenticationResult(expectedUser, outToken, result);
             } else {
                 assertThat(result.getStatus(), is(equalTo(AuthenticationResult.Status.TERMINATE)));
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealmBootstrapCheckTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealmBootstrapCheckTests.java
@@ -14,7 +14,6 @@
 import org.elasticsearch.xpack.core.security.authc.RealmSettings;
 import org.elasticsearch.xpack.core.security.authc.kerberos.KerberosRealmSettings;
 import org.elasticsearch.xpack.core.security.authc.pki.PkiRealmSettings;
-import org.elasticsearch.xpack.security.authc.kerberos.support.KerberosTestCase;
 
 import java.io.IOException;
 import java.nio.file.Path;
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealmCacheTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealmCacheTests.java
@@ -12,7 +12,6 @@
 import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
 import org.elasticsearch.xpack.core.security.authc.kerberos.KerberosRealmSettings;
 import org.elasticsearch.xpack.core.security.user.User;
-import org.elasticsearch.xpack.security.authc.kerberos.support.KerberosTestCase;
 import org.elasticsearch.xpack.security.authc.support.UserRoleMapper.UserData;
 import org.ietf.jgss.GSSException;
 
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealmSettingsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealmSettingsTests.java
@@ -10,7 +10,6 @@
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.core.security.authc.kerberos.KerberosRealmSettings;
-import org.elasticsearch.xpack.security.authc.kerberos.support.KerberosTestCase;
 
 import java.io.IOException;
 import java.nio.file.Files;
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealmTestCase.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealmTestCase.java
@@ -6,13 +6,12 @@
 
 package org.elasticsearch.xpack.security.authc.kerberos;
 
-import com.google.common.collect.Sets;
-
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.threadpool.TestThreadPool;
@@ -23,8 +22,6 @@
 import org.elasticsearch.xpack.core.security.authc.kerberos.KerberosRealmSettings;
 import org.elasticsearch.xpack.core.security.support.Exceptions;
 import org.elasticsearch.xpack.core.security.user.User;
-import org.elasticsearch.xpack.security.authc.kerberos.support.KerberosTestCase;
-import org.elasticsearch.xpack.security.authc.kerberos.support.KerberosTicketValidator;
 import org.elasticsearch.xpack.security.authc.support.UserRoleMapper;
 import org.elasticsearch.xpack.security.authc.support.mapper.NativeRoleMappingStore;
 import org.elasticsearch.xpack.security.support.SecurityIndexManager;
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosTestCase.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosTestCase.java
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-package org.elasticsearch.xpack.security.authc.kerberos.support;
+package org.elasticsearch.xpack.security.authc.kerberos;
 
 import org.apache.logging.log4j.Logger;
 import org.elasticsearch.ExceptionsHelper;
@@ -69,6 +69,7 @@ public abstract class KerberosTestCase extends ESTestCase {
         unsupportedLocaleLanguages.add("hi");
         unsupportedLocaleLanguages.add("uz");
         unsupportedLocaleLanguages.add("fa");
+        unsupportedLocaleLanguages.add("ks");
     }
 
     @BeforeClass
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosTicketValidatorTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosTicketValidatorTests.java
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-package org.elasticsearch.xpack.security.authc.kerberos.support;
+package org.elasticsearch.xpack.security.authc.kerberos;
 
 import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.common.collect.Tuple;
@@ -13,6 +13,7 @@
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.xpack.core.security.authc.kerberos.KerberosRealmSettings;
+import org.elasticsearch.xpack.security.authc.kerberos.KerberosTicketValidator;
 import org.ietf.jgss.GSSException;
 
 import java.io.IOException;
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/SimpleKdcLdapServer.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/SimpleKdcLdapServer.java
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-package org.elasticsearch.xpack.security.authc.kerberos.support;
+package org.elasticsearch.xpack.security.authc.kerberos;
 
 import com.unboundid.ldap.listener.InMemoryDirectoryServer;
 import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig;
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/SimpleKdcLdapServerTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/SimpleKdcLdapServerTests.java
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-package org.elasticsearch.xpack.security.authc.kerberos.support;
+package org.elasticsearch.xpack.security.authc.kerberos;
 
 import com.unboundid.ldap.sdk.LDAPConnection;
 import com.unboundid.ldap.sdk.SearchResult;
@@ -17,6 +17,7 @@
 import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.xpack.core.security.authc.kerberos.KerberosRealmSettings;
 import org.elasticsearch.xpack.security.authc.kerberos.KerberosAuthenticationToken;
+import org.elasticsearch.xpack.security.authc.kerberos.KerberosTicketValidator;
 import org.elasticsearch.xpack.security.authc.ldap.support.LdapUtils;
 import org.ietf.jgss.GSSException;
 
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/SpnegoClient.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/SpnegoClient.java
@@ -4,14 +4,15 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-package org.elasticsearch.xpack.security.authc.kerberos.support;
+package org.elasticsearch.xpack.security.authc.kerberos;
 
 import org.apache.logging.log4j.Logger;
 import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.logging.ESLoggerFactory;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.xpack.security.authc.kerberos.KerberosTicketValidator;
 import org.ietf.jgss.GSSContext;
 import org.ietf.jgss.GSSCredential;
 import org.ietf.jgss.GSSException;
@@ -148,7 +149,7 @@ public void close() throws LoginException, GSSException, PrivilegedActionExcepti
     }
 
     /**
-     * @return {@code true} If the context was established
+     * @return {@code true} If the gss security context was established
      */
     boolean isEstablished() {
         return gssContext.isEstablished();
@@ -196,12 +197,10 @@ public AppConfigurationEntry[] getAppConfigurationEntry(final String name) {
             final Map<String, String> options = new HashMap<>();
             options.put("principal", principal);
             options.put("storeKey", Boolean.TRUE.toString());
-            options.put("useTicketCache", Boolean.FALSE.toString());
-            options.put("useKeyTab", Boolean.FALSE.toString());
-            options.put("renewTGT", Boolean.FALSE.toString());
-            options.put("refreshKrb5Config", Boolean.TRUE.toString());
             options.put("isInitiator", Boolean.TRUE.toString());
             options.put("debug", Boolean.TRUE.toString());
+            // Refresh Krb5 config during tests as the port keeps changing for kdc server
+            options.put("refreshKrb5Config", Boolean.TRUE.toString());
 
             return new AppConfigurationEntry[] { new AppConfigurationEntry(SUN_KRB5_LOGIN_MODULE,
                     AppConfigurationEntry.LoginModuleControlFlag.REQUIRED, Collections.unmodifiableMap(options)) };
diff --git a/x-pack/qa/kerberos-tests/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/SpnegoHttpClientConfigCallbackHandler.java b/x-pack/qa/kerberos-tests/src/test/java/org/elasticsearch/xpack/security/authc/kerberos/SpnegoHttpClientConfigCallbackHandler.java
@@ -304,10 +304,8 @@ private abstract static class AbstractJaasConf extends Configuration {
         public AppConfigurationEntry[] getAppConfigurationEntry(final String name) {
             final Map<String, String> options = new HashMap<>();
             options.put("principal", userPrincipalName);
-            options.put("refreshKrb5Config", Boolean.TRUE.toString());
             options.put("isInitiator", Boolean.TRUE.toString());
             options.put("storeKey", Boolean.TRUE.toString());
-            options.put("renewTGT", Boolean.FALSE.toString());
             options.put("debug", Boolean.toString(enableDebugLogs));
             addOptions(options);
             return new AppConfigurationEntry[] { new AppConfigurationEntry(SUN_KRB5_LOGIN_MODULE,
