Github user sohami commented on a diff in the pull request:
https://github.com/apache/drill/pull/1040#discussion_r158184891
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestSpnegoAuthentication.java
---
@@ -0,0 +1,597 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.server;
+
+
+import com.google.common.collect.Lists;
+import com.typesafe.config.ConfigValueFactory;
+import org.apache.commons.codec.binary.Base64;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.common.scanner.persistence.ScanResult;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.exception.DrillbitStartupException;
+import org.apache.drill.exec.rpc.security.AuthenticatorProviderImpl;
+import org.apache.drill.exec.rpc.security.KerberosHelper;
+import org.apache.drill.exec.rpc.security.plain.PlainFactory;
+import
org.apache.drill.exec.rpc.user.security.testing.UserAuthenticatorTestImpl;
+import org.apache.drill.exec.server.options.SystemOptionManager;
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import
org.apache.drill.exec.server.rest.auth.DrillHttpSecurityHandlerProvider;
+import org.apache.drill.exec.server.rest.auth.DrillSpnegoAuthenticator;
+import org.apache.drill.exec.server.rest.auth.DrillSpnegoLoginService;
+import org.apache.drill.exec.server.rest.auth.SpnegoUtil;
+import org.apache.drill.test.BaseDirTestWatcher;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.security.authentication.util.KerberosName;
+import org.apache.hadoop.security.authentication.util.KerberosUtil;
+import org.apache.kerby.kerberos.kerb.client.JaasKrbUtil;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.security.Authenticator;
+import org.eclipse.jetty.security.DefaultIdentityService;
+import org.eclipse.jetty.security.UserAuthentication;
+import org.eclipse.jetty.server.Authentication;
+import org.eclipse.jetty.server.UserIdentity;
+import org.ietf.jgss.GSSContext;
+import org.ietf.jgss.GSSManager;
+import org.ietf.jgss.GSSName;
+import org.ietf.jgss.Oid;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.mockito.Mockito;
+import sun.security.jgss.GSSUtil;
+
+import javax.security.auth.Subject;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.lang.reflect.Field;
+import java.security.PrivilegedExceptionAction;
+
+import static junit.framework.TestCase.fail;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Test {@link SpnegoUtil}, {@link DrillSpnegoAuthenticator} and {@link
DrillSpnegoLoginService}
+ */
+public class TestSpnegoAuthentication {
+
+ private static KerberosHelper spnegoHelper;
+
+ private static final String primaryName = "HTTP";
+
+ private static final BaseDirTestWatcher dirTestWatcher = new
BaseDirTestWatcher();
+
+
+ @BeforeClass
+ public static void setupTest() throws Exception {
+ spnegoHelper = new
KerberosHelper(TestSpnegoAuthentication.class.getSimpleName(), primaryName);
+ spnegoHelper.setupKdc(dirTestWatcher.getTmpDir());
+
+
+ sun.security.krb5.Config.refresh();
+
+ // (2) Reset the default realm.
+ final Field defaultRealm =
KerberosName.class.getDeclaredField("defaultRealm");
+ defaultRealm.setAccessible(true);
+ defaultRealm.set(null, KerberosUtil.getDefaultRealm());
+ }
+
+ /**
+ * Both SPNEGO and FORM mechanism is enabled for WebServer in
configuration. Test to see if the respective security
+ * handlers are created successfully or not.
+ * @throws Exception
+ */
+ @Test
+ public void testSPNEGOAndFORMEnabled() throws Exception {
+
+ final DrillConfig newConfig = new DrillConfig(DrillConfig.create()
+ .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED,
+ ConfigValueFactory.fromAnyRef(true))
+ .withValue(ExecConstants.HTTP_AUTHENTICATION_MECHANISMS,
+ ConfigValueFactory.fromIterable(Lists.newArrayList("form",
"spnego")))
+ .withValue(ExecConstants.HTTP_SPNEGO_PRINCIPAL,
+ ConfigValueFactory.fromAnyRef(spnegoHelper.SERVER_PRINCIPAL))
+ .withValue(ExecConstants.HTTP_SPNEGO_KEYTAB,
+
ConfigValueFactory.fromAnyRef(spnegoHelper.serverKeytab.toString())),
+ false);
+
+ final ScanResult scanResult = ClassPathScanner.fromPrescan(newConfig);
+ final AuthenticatorProviderImpl authenticatorProvider =
Mockito.mock(AuthenticatorProviderImpl.class);
+
Mockito.when(authenticatorProvider.containsFactory(PlainFactory.SIMPLE_NAME)).thenReturn(true);
+
+ final DrillbitContext context = Mockito.mock(DrillbitContext.class);
+ Mockito.when(context.getClasspathScan()).thenReturn(scanResult);
+ Mockito.when(context.getConfig()).thenReturn(newConfig);
+
Mockito.when(context.getAuthProvider()).thenReturn(authenticatorProvider);
+
+ final DrillHttpSecurityHandlerProvider securityProvider = new
DrillHttpSecurityHandlerProvider(newConfig, context);
+ assertTrue(securityProvider.isFormEnabled());
+ assertTrue(securityProvider.isSpnegoEnabled());
+ }
+
+ /**
+ * Validate if FORM security handler is created successfully when only
form is configured as auth mechanism
+ * @throws Exception
+ */
+ @Test
+ public void testOnlyFORMEnabled() throws Exception {
+
+ final DrillConfig newConfig = new DrillConfig(DrillConfig.create()
+ .withValue(ExecConstants.HTTP_AUTHENTICATION_MECHANISMS,
+ ConfigValueFactory.fromIterable(Lists.newArrayList("form")))
+ .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED,
+ ConfigValueFactory.fromAnyRef(true))
+ .withValue(ExecConstants.HTTP_SPNEGO_PRINCIPAL,
+ ConfigValueFactory.fromAnyRef(spnegoHelper.SERVER_PRINCIPAL))
+ .withValue(ExecConstants.HTTP_SPNEGO_KEYTAB,
+
ConfigValueFactory.fromAnyRef(spnegoHelper.serverKeytab.toString())),
+ false);
+
+ final ScanResult scanResult = ClassPathScanner.fromPrescan(newConfig);
+ final AuthenticatorProviderImpl authenticatorProvider =
Mockito.mock(AuthenticatorProviderImpl.class);
+
Mockito.when(authenticatorProvider.containsFactory(PlainFactory.SIMPLE_NAME)).thenReturn(true);
+
+ final DrillbitContext context = Mockito.mock(DrillbitContext.class);
+ Mockito.when(context.getClasspathScan()).thenReturn(scanResult);
+ Mockito.when(context.getConfig()).thenReturn(newConfig);
+
Mockito.when(context.getAuthProvider()).thenReturn(authenticatorProvider);
+
+ final DrillHttpSecurityHandlerProvider securityProvider = new
DrillHttpSecurityHandlerProvider(newConfig, context);
+ assertTrue(securityProvider.isFormEnabled());
+ assertTrue(!securityProvider.isSpnegoEnabled());
+ }
+
+ /**
+ * Validate failure in creating FORM security handler when PAM
authenticator is absent. PAM authenticator is provided
+ * via {@link PlainFactory#getAuthenticator()}
+ * @throws Exception
+ */
+ @Test
+ public void testFORMEnabledWithPlainDisabled() throws Exception {
+ try {
+ final DrillConfig newConfig = new DrillConfig(DrillConfig.create()
+ .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED,
+ ConfigValueFactory.fromAnyRef(true))
+ .withValue(ExecConstants.HTTP_AUTHENTICATION_MECHANISMS,
+ ConfigValueFactory.fromIterable(Lists.newArrayList("form")))
+ .withValue(ExecConstants.HTTP_SPNEGO_PRINCIPAL,
+ ConfigValueFactory.fromAnyRef(spnegoHelper.SERVER_PRINCIPAL))
+ .withValue(ExecConstants.HTTP_SPNEGO_KEYTAB,
+
ConfigValueFactory.fromAnyRef(spnegoHelper.serverKeytab.toString())),
+ false);
+
+ final ScanResult scanResult =
ClassPathScanner.fromPrescan(newConfig);
+ final AuthenticatorProviderImpl authenticatorProvider =
Mockito.mock(AuthenticatorProviderImpl.class);
+
Mockito.when(authenticatorProvider.containsFactory(PlainFactory.SIMPLE_NAME)).thenReturn(false);
+
+ final DrillbitContext context = Mockito.mock(DrillbitContext.class);
+ Mockito.when(context.getClasspathScan()).thenReturn(scanResult);
+ Mockito.when(context.getConfig()).thenReturn(newConfig);
+
Mockito.when(context.getAuthProvider()).thenReturn(authenticatorProvider);
+
+ final DrillHttpSecurityHandlerProvider securityProvider =
+ new DrillHttpSecurityHandlerProvider(newConfig, context);
+ fail();
+ } catch(Exception ex) {
+ assertTrue(ex instanceof DrillbitStartupException);
+ }
+ }
+
+ /**
+ * Validate only SPNEGO security handler is configured properly when
enabled via configuration
+ * @throws Exception
+ */
+ @Test
+ public void testOnlySPNEGOEnabled() throws Exception {
+
+ final DrillConfig newConfig = new DrillConfig(DrillConfig.create()
+ .withValue(ExecConstants.HTTP_AUTHENTICATION_MECHANISMS,
+ ConfigValueFactory.fromIterable(Lists.newArrayList("spnego")))
+ .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED,
+ ConfigValueFactory.fromAnyRef(true))
+ .withValue(ExecConstants.HTTP_SPNEGO_PRINCIPAL,
+ ConfigValueFactory.fromAnyRef(spnegoHelper.SERVER_PRINCIPAL))
+ .withValue(ExecConstants.HTTP_SPNEGO_KEYTAB,
+
ConfigValueFactory.fromAnyRef(spnegoHelper.serverKeytab.toString())),
+ false);
+
+ final ScanResult scanResult = ClassPathScanner.fromPrescan(newConfig);
+ final AuthenticatorProviderImpl authenticatorProvider =
Mockito.mock(AuthenticatorProviderImpl.class);
+
Mockito.when(authenticatorProvider.containsFactory(PlainFactory.SIMPLE_NAME)).thenReturn(false);
+
+ final DrillbitContext context = Mockito.mock(DrillbitContext.class);
+ Mockito.when(context.getClasspathScan()).thenReturn(scanResult);
+ Mockito.when(context.getConfig()).thenReturn(newConfig);
+
Mockito.when(context.getAuthProvider()).thenReturn(authenticatorProvider);
+
+ final DrillHttpSecurityHandlerProvider securityProvider = new
DrillHttpSecurityHandlerProvider(newConfig, context);
+
+ assertTrue(!securityProvider.isFormEnabled());
+ assertTrue(securityProvider.isSpnegoEnabled());
+ }
+
+ /**
+ * Validate when none of the security mechanism is specified in the
+ * {@link ExecConstants#HTTP_AUTHENTICATION_MECHANISMS}, FORM security
handler is still configured correctly when
+ * authentication is enabled along with PAM authenticator module.
+ * @throws Exception
+ */
+ @Test
+ public void testConfigBackwardCompatibility() throws Exception {
+
+ final DrillConfig newConfig = new DrillConfig(DrillConfig.create()
+ .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED,
+ ConfigValueFactory.fromAnyRef(true)),
+ false);
+
+ final ScanResult scanResult = ClassPathScanner.fromPrescan(newConfig);
+ final AuthenticatorProviderImpl authenticatorProvider =
Mockito.mock(AuthenticatorProviderImpl.class);
+
Mockito.when(authenticatorProvider.containsFactory(PlainFactory.SIMPLE_NAME)).thenReturn(true);
+
+ final DrillbitContext context = Mockito.mock(DrillbitContext.class);
+ Mockito.when(context.getClasspathScan()).thenReturn(scanResult);
+ Mockito.when(context.getConfig()).thenReturn(newConfig);
+
Mockito.when(context.getAuthProvider()).thenReturn(authenticatorProvider);
+
+ final DrillHttpSecurityHandlerProvider securityProvider = new
DrillHttpSecurityHandlerProvider(newConfig, context);
+
+ assertTrue(securityProvider.isFormEnabled());
+ assertTrue(!securityProvider.isSpnegoEnabled());
+ }
+
+ /**
+ * Validate behavior of {@link SpnegoUtil} class when provided with
different configuration's for SPNEGO via
+ * DrillConfig
+ * @throws Exception
+ */
+ @Test
+ public void testSpnegoUtil() throws Exception {
--- End diff --
I kept all the tests in same file to avoid the need for setup again. But
have now moved into separate class files as per your feedback.
---
