[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/drill/pull/1040


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158184949
  
--- 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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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 

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158113430
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillHttpSecurityHandlerProvider.java
 ---
@@ -0,0 +1,187 @@
+/*
+ * 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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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.rest.auth;
+
+import com.google.common.base.Preconditions;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.common.map.CaseInsensitiveMap;
+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.AuthStringUtil;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.eclipse.jetty.security.ConstraintSecurityHandler;
+import org.eclipse.jetty.security.authentication.SessionAuthentication;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.Request;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+
+public class DrillHttpSecurityHandlerProvider extends 
ConstraintSecurityHandler {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillHttpSecurityHandlerProvider.class);
+
+  private final Map 
securityHandlers =
+  CaseInsensitiveMap.newHashMapWithExpectedSize(5);
+
+  public DrillHttpSecurityHandlerProvider(DrillConfig config, 
DrillbitContext drillContext)
+  throws DrillbitStartupException {
+
+
Preconditions.checkState(config.getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED));
+final Set configuredMechanisms = new HashSet<>();
+
+if (config.hasPath(ExecConstants.HTTP_AUTHENTICATION_MECHANISMS)) {
+  
configuredMechanisms.addAll(AuthStringUtil.asSet(config.getStringList(ExecConstants.HTTP_AUTHENTICATION_MECHANISMS)));
+} else { // for backward compatibility
+  configuredMechanisms.add(FORMSecurityHanlder.HANDLER_NAME);
+}
+
+  final ScanResult scan = drillContext.getClasspathScan();
+  final Collection factoryImpls =
+  
scan.getImplementations(DrillHttpConstraintSecurityHandler.class);
+  logger.debug("Found DrillHttpConstraintSecurityHandler 
implementations: {}", factoryImpls);
+  for (final Class clazz 
: factoryImpls) {
+
+// If all the configured mechanisms handler is added then break 
out of this loop
+if (configuredMechanisms.isEmpty()) {
+  break;
+}
+
+Constructor 
validConstructor = null;
+for (final Constructor c : clazz.getConstructors()) {
+  final Class[] params = c.getParameterTypes();
+  if (params.length == 0) {
+validConstructor = (Constructor) c; // unchecked
+break;
+  }
+}
+
+if (validConstructor == null) {
+  logger.warn("Skipping DrillHttpConstraintSecurityHandler class 
{}. It must implement at least one" +
+  " constructor with signature [{}()]", 
clazz.getCanonicalName(), clazz.getName());
+  continue;
+}
+
+try {
+  final DrillHttpConstraintSecurityHandler instance = 
validConstructor.newInstance();
+  if (configuredMechanisms.remove(instance.getImplName())) {
+ 

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158112333
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillHttpSecurityHandlerProvider.java
 ---
@@ -0,0 +1,187 @@
+/*
+ * 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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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.rest.auth;
+
+import com.google.common.base.Preconditions;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.common.map.CaseInsensitiveMap;
+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.AuthStringUtil;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.eclipse.jetty.security.ConstraintSecurityHandler;
+import org.eclipse.jetty.security.authentication.SessionAuthentication;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.Request;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+
+public class DrillHttpSecurityHandlerProvider extends 
ConstraintSecurityHandler {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillHttpSecurityHandlerProvider.class);
+
+  private final Map 
securityHandlers =
+  CaseInsensitiveMap.newHashMapWithExpectedSize(5);
+
+  public DrillHttpSecurityHandlerProvider(DrillConfig config, 
DrillbitContext drillContext)
+  throws DrillbitStartupException {
+
+
Preconditions.checkState(config.getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED));
+final Set configuredMechanisms = new HashSet<>();
+
+if (config.hasPath(ExecConstants.HTTP_AUTHENTICATION_MECHANISMS)) {
+  
configuredMechanisms.addAll(AuthStringUtil.asSet(config.getStringList(ExecConstants.HTTP_AUTHENTICATION_MECHANISMS)));
+} else { // for backward compatibility
+  configuredMechanisms.add(FORMSecurityHanlder.HANDLER_NAME);
+}
+
+  final ScanResult scan = drillContext.getClasspathScan();
+  final Collection factoryImpls =
+  
scan.getImplementations(DrillHttpConstraintSecurityHandler.class);
+  logger.debug("Found DrillHttpConstraintSecurityHandler 
implementations: {}", factoryImpls);
+  for (final Class clazz 
: factoryImpls) {
+
+// If all the configured mechanisms handler is added then break 
out of this loop
+if (configuredMechanisms.isEmpty()) {
+  break;
+}
+
+Constructor 
validConstructor = null;
+for (final Constructor c : clazz.getConstructors()) {
+  final Class[] params = c.getParameterTypes();
+  if (params.length == 0) {
+validConstructor = (Constructor) c; // unchecked
+break;
+  }
+}
+
+if (validConstructor == null) {
+  logger.warn("Skipping DrillHttpConstraintSecurityHandler class 
{}. It must implement at least one" +
+  " constructor with signature [{}()]", 
clazz.getCanonicalName(), clazz.getName());
+  continue;
+}
+
+try {
+  final DrillHttpConstraintSecurityHandler instance = 
validConstructor.newInstance();
+  if (configuredMechanisms.remove(instance.getImplName())) {
+ 

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158099889
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogInLogOutResources.java
 ---
@@ -69,23 +79,94 @@ public Viewable getLoginPage(@Context 
HttpServletRequest request, @Context HttpS
 return ViewableWithPermissions.createLoginPage(null);
   }
 
+  @GET
+  @Path(WebServerConstants.SPENGO_LOGIN_RESOURCE_PATH)
+  @Produces(MediaType.TEXT_HTML)
+  public Viewable getSpnegologin(@Context HttpServletRequest request, 
@Context HttpServletResponse response,
+ @Context SecurityContext sc, @Context 
UriInfo uriInfo,
+ 
@QueryParam(WebServerConstants.REDIRECT_QUERY_PARM) String redirect) throws 
Exception {
+if (AuthDynamicFeature.isUserLoggedIn(sc)) {
+  request.getRequestDispatcher("/").forward(request, response);
+  return null;
+}
+
+final String errorString = "Invalid SPNEGO credentials or SPNEGO is 
not configured";
+MainLoginPageModel model = new MainLoginPageModel(errorString);
+return ViewableWithPermissions.createMainLoginPage(model);
+  }
+
   // Request type is POST because POST request which contains the login 
credentials are invalid and the request is
   // dispatched here directly.
   @POST
-  @Path("/login")
+  @Path(WebServerConstants.FORM_LOGIN_RESOURCE_PATH)
   @Produces(MediaType.TEXT_HTML)
   public Viewable getLoginPageAfterValidationError() {
 return ViewableWithPermissions.createLoginPage("Invalid 
username/password credentials.");
   }
 
   @GET
-  @Path("/logout")
+  @Path(WebServerConstants.LOGOUT_RESOURCE_PATH)
   public void logout(@Context HttpServletRequest req, @Context 
HttpServletResponse resp) throws Exception {
 final HttpSession session = req.getSession();
 if (session != null) {
   session.invalidate();
 }
 
-req.getRequestDispatcher("/").forward(req, resp);
+
req.getRequestDispatcher(WebServerConstants.WEBSERVER_ROOT_PATH).forward(req, 
resp);
+  }
+
+  @GET
+  @Path(WebServerConstants.MAIN_LOGIN_RESOURCE_PATH)
+  @Produces(MediaType.TEXT_HTML)
+  public Viewable getMainLoginPage(@Context HttpServletRequest request, 
@Context HttpServletResponse response,
+   @Context SecurityContext sc, @Context 
UriInfo uriInfo,
+   
@QueryParam(WebServerConstants.REDIRECT_QUERY_PARM) String redirect) throws 
Exception {
+if (!StringUtils.isEmpty(redirect)) {
+  // If the URL has redirect in it, set the redirect URI in session, 
so that after the login is successful, request
+  // is forwarded to the redirect page.
+  final HttpSession session = request.getSession(true);
+  final URI destURI = UriBuilder.fromUri(URLDecoder.decode(redirect, 
"UTF-8")).build();
+  session.setAttribute(FormAuthenticator.__J_URI, destURI.toString());
+}
+
+MainLoginPageModel model = new MainLoginPageModel(null);
+return ViewableWithPermissions.createMainLoginPage(model);
+  }
+
+  public class MainLoginPageModel {
+
+public String error;
--- End diff --

Done


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158112289
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillHttpSecurityHandlerProvider.java
 ---
@@ -0,0 +1,187 @@
+/*
+ * 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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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.rest.auth;
+
+import com.google.common.base.Preconditions;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.common.map.CaseInsensitiveMap;
+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.AuthStringUtil;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.eclipse.jetty.security.ConstraintSecurityHandler;
+import org.eclipse.jetty.security.authentication.SessionAuthentication;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.Request;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+
+public class DrillHttpSecurityHandlerProvider extends 
ConstraintSecurityHandler {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillHttpSecurityHandlerProvider.class);
+
+  private final Map 
securityHandlers =
+  CaseInsensitiveMap.newHashMapWithExpectedSize(5);
+
+  public DrillHttpSecurityHandlerProvider(DrillConfig config, 
DrillbitContext drillContext)
+  throws DrillbitStartupException {
+
+
Preconditions.checkState(config.getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED));
+final Set configuredMechanisms = new HashSet<>();
+
+if (config.hasPath(ExecConstants.HTTP_AUTHENTICATION_MECHANISMS)) {
+  
configuredMechanisms.addAll(AuthStringUtil.asSet(config.getStringList(ExecConstants.HTTP_AUTHENTICATION_MECHANISMS)));
+} else { // for backward compatibility
+  configuredMechanisms.add(FORMSecurityHanlder.HANDLER_NAME);
+}
+
+  final ScanResult scan = drillContext.getClasspathScan();
+  final Collection factoryImpls =
+  
scan.getImplementations(DrillHttpConstraintSecurityHandler.class);
+  logger.debug("Found DrillHttpConstraintSecurityHandler 
implementations: {}", factoryImpls);
+  for (final Class clazz 
: factoryImpls) {
+
+// If all the configured mechanisms handler is added then break 
out of this loop
+if (configuredMechanisms.isEmpty()) {
+  break;
+}
+
+Constructor 
validConstructor = null;
+for (final Constructor c : clazz.getConstructors()) {
+  final Class[] params = c.getParameterTypes();
+  if (params.length == 0) {
+validConstructor = (Constructor) c; // unchecked
+break;
+  }
+}
+
+if (validConstructor == null) {
+  logger.warn("Skipping DrillHttpConstraintSecurityHandler class 
{}. It must implement at least one" +
+  " constructor with signature [{}()]", 
clazz.getCanonicalName(), clazz.getName());
+  continue;
+}
+
+try {
+  final DrillHttpConstraintSecurityHandler instance = 
validConstructor.newInstance();
+  if (configuredMechanisms.remove(instance.getImplName())) {
+ 

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158164861
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoAuthenticator.java
 ---
@@ -0,0 +1,150 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.rest.auth;
+
+
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.apache.parquet.Strings;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpVersion;
+import org.eclipse.jetty.security.ServerAuthException;
+import org.eclipse.jetty.security.UserAuthentication;
+import org.eclipse.jetty.security.authentication.DeferredAuthentication;
+import org.eclipse.jetty.security.authentication.SessionAuthentication;
+import org.eclipse.jetty.security.authentication.SpnegoAuthenticator;
+import org.eclipse.jetty.server.Authentication;
+import org.eclipse.jetty.server.HttpChannel;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Response;
+import org.eclipse.jetty.server.UserIdentity;
+
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.IOException;
+
+/**
+ * Custom SpnegoAuthenticator for Drill which provides following:
+ * 1) Perform SPNEGO authentication only when spnegoLogin resource is 
requested. This helps to avoid authentication
+ *for each and every resource which the JETTY provided authenticator 
does.
+ * 2) Helps to redirect to the target URL after authentication is done 
successfully.
+ * 3) Clear-Up in memory session information once LogOut is triggered. 
Such that any future request also trigger the
+ *SPNEGO authentication.
+ */
+public class DrillSpnegoAuthenticator extends SpnegoAuthenticator {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillSpnegoAuthenticator.class);
+
+  public DrillSpnegoAuthenticator(String authMethod) {
+super(authMethod);
+  }
+
+  @Override
+  public Authentication validateRequest(ServletRequest request, 
ServletResponse response, boolean mandatory)
+  throws ServerAuthException {
+
+HttpServletRequest req = (HttpServletRequest) request;
+HttpServletResponse res = (HttpServletResponse) response;
+HttpSession session = req.getSession(true);
+final Authentication authentication =
+(Authentication) 
session.getAttribute("org.eclipse.jetty.security.UserIdentity");
+String uri = req.getRequestURI();
+
+//If the Request URI is for /spnegoLogin then perform login
+mandatory |= uri.equals(WebServerConstants.SPENGO_LOGIN_RESOURCE_PATH);
--- End diff --

Changed. 


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158135221
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/SpnegoUtil.java
 ---
@@ -0,0 +1,115 @@
+/*
+ * 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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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.rest.auth;
+
+import com.google.common.base.Preconditions;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.CommonConfigurationKeys;
+import org.apache.hadoop.security.UserGroupInformation;
+
+public class SpnegoUtil {
+
+  private UserGroupInformation loggedInUgi;
+
+  //private String realm;
+
+  private String principal;
+
+  private String keytab;
+
+  public SpnegoUtil(DrillConfig config) {
+
+keytab = config.hasPath(ExecConstants.HTTP_SPNEGO_KEYTAB) ?
+config.getString(ExecConstants.HTTP_SPNEGO_KEYTAB) :
+null;
+
+principal = config.hasPath(ExecConstants.HTTP_SPNEGO_PRINCIPAL) ?
+config.getString(ExecConstants.HTTP_SPNEGO_PRINCIPAL) :
+null;
+  }
+
+  //Reads the SPNEGO principal from the config file
+  public String getSpnegoPrincipal() {
+return principal;
+  }
+
+  public void validateSpnegoConfig() throws DrillException {
+
+StringBuilder errorMsg = new StringBuilder();
+
+if (principal != null && keytab != null) {
+  return;
+}
+
+if (principal == null) {
+  errorMsg.append("\nConfiguration ");
+  errorMsg.append(ExecConstants.HTTP_SPNEGO_PRINCIPAL);
+  errorMsg.append(" is not found");
+}
+
+if (keytab == null) {
+  errorMsg.append("\nConfiguration ");
+  errorMsg.append(ExecConstants.HTTP_SPNEGO_KEYTAB);
+  errorMsg.append(" is not found");
+}
+
+throw new DrillException(errorMsg.toString());
+  }
+
+  //Performs the Server login to KDC for SPNEGO
+  private UserGroupInformation loginAndReturnUgi() throws DrillException {
+
+validateSpnegoConfig();
+
+UserGroupInformation ugi;
+try {
+  // Check if security is not enabled and try to set the security 
parameter to login the principal.
+  // After the login is performed reset the static UGI state.
+  if (!UserGroupInformation.isSecurityEnabled()) {
+final Configuration newConfig = new Configuration();
+
newConfig.set(CommonConfigurationKeys.HADOOP_SECURITY_AUTHENTICATION,
+UserGroupInformation.AuthenticationMethod.KERBEROS.toString());
+
+UserGroupInformation.setConfiguration(newConfig);
+ugi = 
UserGroupInformation.loginUserFromKeytabAndReturnUGI(principal, keytab);
+
+// Reset the original configuration for static UGI
+UserGroupInformation.setConfiguration(new Configuration());
--- End diff --

There is no concurrency issue here since this will be called only once 
during WebServer setup time.


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158120552
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/SPNEGOSecurityHandler.java
 ---
@@ -0,0 +1,37 @@
+/*
+ * 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
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.rest.auth;
+
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.server.DrillbitContext;
+
+public class SPNEGOSecurityHandler extends 
DrillHttpConstraintSecurityHandler {
--- End diff --

Renamed.


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158166373
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoAuthenticator.java
 ---
@@ -0,0 +1,150 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.rest.auth;
+
+
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.apache.parquet.Strings;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpVersion;
+import org.eclipse.jetty.security.ServerAuthException;
+import org.eclipse.jetty.security.UserAuthentication;
+import org.eclipse.jetty.security.authentication.DeferredAuthentication;
+import org.eclipse.jetty.security.authentication.SessionAuthentication;
+import org.eclipse.jetty.security.authentication.SpnegoAuthenticator;
+import org.eclipse.jetty.server.Authentication;
+import org.eclipse.jetty.server.HttpChannel;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Response;
+import org.eclipse.jetty.server.UserIdentity;
+
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.IOException;
+
+/**
+ * Custom SpnegoAuthenticator for Drill which provides following:
+ * 1) Perform SPNEGO authentication only when spnegoLogin resource is 
requested. This helps to avoid authentication
+ *for each and every resource which the JETTY provided authenticator 
does.
+ * 2) Helps to redirect to the target URL after authentication is done 
successfully.
+ * 3) Clear-Up in memory session information once LogOut is triggered. 
Such that any future request also trigger the
+ *SPNEGO authentication.
+ */
+public class DrillSpnegoAuthenticator extends SpnegoAuthenticator {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillSpnegoAuthenticator.class);
+
+  public DrillSpnegoAuthenticator(String authMethod) {
+super(authMethod);
+  }
+
+  @Override
+  public Authentication validateRequest(ServletRequest request, 
ServletResponse response, boolean mandatory)
+  throws ServerAuthException {
+
+HttpServletRequest req = (HttpServletRequest) request;
+HttpServletResponse res = (HttpServletResponse) response;
+HttpSession session = req.getSession(true);
+final Authentication authentication =
+(Authentication) 
session.getAttribute("org.eclipse.jetty.security.UserIdentity");
+String uri = req.getRequestURI();
+
+//If the Request URI is for /spnegoLogin then perform login
+mandatory |= uri.equals(WebServerConstants.SPENGO_LOGIN_RESOURCE_PATH);
+
+//For logout remove the attribute from the session that holds 
UserIdentity
+if (authentication != null && 
uri.equals(WebServerConstants.LOGOUT_RESOURCE_PATH)) {
+  logger.debug("Logging out user {}", req.getRemoteAddr());
+  session.removeAttribute("org.eclipse.jetty.security.UserIdentity");
+  return null;
+} else if (authentication != null) { // Since already logged in just 
return the session attribute
+  return authentication;
+} else { // The session is not yet authenticated
+  final String header = 
req.getHeader(HttpHeader.AUTHORIZATION.asString());
+  if (!mandatory) {
+return new DeferredAuthentication(this);
+  } else if (header == null) {
+try {
+  if (DeferredAuthentication.isDeferred(res)) {
+return Authentication.UNAUTHENTICATED;
+  } else {
+res.setHeader(HttpHeader.WWW_AUTHENTICATE.asString(), 
HttpHeader.NEGOTIATE.asString());
+res.sendError(401);
+logger.debug("SPNEGOAuthenticator: Sending 

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158348706
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillErrorHandler.java
 ---
@@ -0,0 +1,45 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.rest.auth;
+
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.eclipse.jetty.server.handler.ErrorHandler;
+
+import javax.servlet.http.HttpServletRequest;
+import java.io.IOException;
+import java.io.Writer;
+
+/**
+ * Custom ErrorHandler class for Drill's WebServer to have better error 
message in case when SPNEGO login failed and
+ * what to do next. In all other cases this would use the generic error 
page.
+ */
+public class DrillErrorHandler extends ErrorHandler {
+
+  @Override
+  protected void writeErrorPageMessage(HttpServletRequest request, Writer 
writer,
+   int code, String message, String 
uri) throws IOException {
+
+super.writeErrorPageMessage(request, writer, code, message, uri);
+
+if (uri.equals(WebServerConstants.SPENGO_LOGIN_RESOURCE_PATH)) {
--- End diff --

There is a reason for doing it like this for handling a special case which 
is when server send 401 error code (standard HTTP error page) along with 
Negotiate header to client and expect client to respond back with SPNEGO token. 
In case when client is not configured for SPNEGO it will not respond and server 
will not get chance to reply back with an error page. In this scenario we 
wanted to provide a message to client on what to do next and hence the special 
handling.


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158112439
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillHttpSecurityHandlerProvider.java
 ---
@@ -0,0 +1,187 @@
+/*
+ * 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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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.rest.auth;
+
+import com.google.common.base.Preconditions;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.common.map.CaseInsensitiveMap;
+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.AuthStringUtil;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.eclipse.jetty.security.ConstraintSecurityHandler;
+import org.eclipse.jetty.security.authentication.SessionAuthentication;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.Request;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+
+public class DrillHttpSecurityHandlerProvider extends 
ConstraintSecurityHandler {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillHttpSecurityHandlerProvider.class);
+
+  private final Map 
securityHandlers =
+  CaseInsensitiveMap.newHashMapWithExpectedSize(5);
+
+  public DrillHttpSecurityHandlerProvider(DrillConfig config, 
DrillbitContext drillContext)
+  throws DrillbitStartupException {
+
+
Preconditions.checkState(config.getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED));
+final Set configuredMechanisms = new HashSet<>();
+
+if (config.hasPath(ExecConstants.HTTP_AUTHENTICATION_MECHANISMS)) {
+  
configuredMechanisms.addAll(AuthStringUtil.asSet(config.getStringList(ExecConstants.HTTP_AUTHENTICATION_MECHANISMS)));
+} else { // for backward compatibility
+  configuredMechanisms.add(FORMSecurityHanlder.HANDLER_NAME);
+}
+
+  final ScanResult scan = drillContext.getClasspathScan();
+  final Collection factoryImpls =
+  
scan.getImplementations(DrillHttpConstraintSecurityHandler.class);
+  logger.debug("Found DrillHttpConstraintSecurityHandler 
implementations: {}", factoryImpls);
+  for (final Class clazz 
: factoryImpls) {
+
+// If all the configured mechanisms handler is added then break 
out of this loop
+if (configuredMechanisms.isEmpty()) {
+  break;
+}
+
+Constructor 
validConstructor = null;
+for (final Constructor c : clazz.getConstructors()) {
+  final Class[] params = c.getParameterTypes();
+  if (params.length == 0) {
+validConstructor = (Constructor) c; // unchecked
+break;
+  }
+}
+
+if (validConstructor == null) {
+  logger.warn("Skipping DrillHttpConstraintSecurityHandler class 
{}. It must implement at least one" +
+  " constructor with signature [{}()]", 
clazz.getCanonicalName(), clazz.getName());
+  continue;
+}
+
+try {
+  final DrillHttpConstraintSecurityHandler instance = 
validConstructor.newInstance();
+  if (configuredMechanisms.remove(instance.getImplName())) {
+ 

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158099695
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogInLogOutResources.java
 ---
@@ -69,23 +79,94 @@ public Viewable getLoginPage(@Context 
HttpServletRequest request, @Context HttpS
 return ViewableWithPermissions.createLoginPage(null);
   }
 
+  @GET
+  @Path(WebServerConstants.SPENGO_LOGIN_RESOURCE_PATH)
+  @Produces(MediaType.TEXT_HTML)
+  public Viewable getSpnegologin(@Context HttpServletRequest request, 
@Context HttpServletResponse response,
+ @Context SecurityContext sc, @Context 
UriInfo uriInfo,
+ 
@QueryParam(WebServerConstants.REDIRECT_QUERY_PARM) String redirect) throws 
Exception {
+if (AuthDynamicFeature.isUserLoggedIn(sc)) {
+  request.getRequestDispatcher("/").forward(request, response);
--- End diff --

Done


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158164495
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoAuthenticator.java
 ---
@@ -0,0 +1,150 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.rest.auth;
+
+
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.apache.parquet.Strings;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpVersion;
+import org.eclipse.jetty.security.ServerAuthException;
+import org.eclipse.jetty.security.UserAuthentication;
+import org.eclipse.jetty.security.authentication.DeferredAuthentication;
+import org.eclipse.jetty.security.authentication.SessionAuthentication;
+import org.eclipse.jetty.security.authentication.SpnegoAuthenticator;
+import org.eclipse.jetty.server.Authentication;
+import org.eclipse.jetty.server.HttpChannel;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Response;
+import org.eclipse.jetty.server.UserIdentity;
+
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.IOException;
+
+/**
+ * Custom SpnegoAuthenticator for Drill which provides following:
+ * 1) Perform SPNEGO authentication only when spnegoLogin resource is 
requested. This helps to avoid authentication
+ *for each and every resource which the JETTY provided authenticator 
does.
+ * 2) Helps to redirect to the target URL after authentication is done 
successfully.
+ * 3) Clear-Up in memory session information once LogOut is triggered. 
Such that any future request also trigger the
+ *SPNEGO authentication.
+ */
+public class DrillSpnegoAuthenticator extends SpnegoAuthenticator {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillSpnegoAuthenticator.class);
+
+  public DrillSpnegoAuthenticator(String authMethod) {
+super(authMethod);
+  }
+
+  @Override
+  public Authentication validateRequest(ServletRequest request, 
ServletResponse response, boolean mandatory)
+  throws ServerAuthException {
+
+HttpServletRequest req = (HttpServletRequest) request;
+HttpServletResponse res = (HttpServletResponse) response;
+HttpSession session = req.getSession(true);
+final Authentication authentication =
+(Authentication) 
session.getAttribute("org.eclipse.jetty.security.UserIdentity");
+String uri = req.getRequestURI();
+
+//If the Request URI is for /spnegoLogin then perform login
+mandatory |= uri.equals(WebServerConstants.SPENGO_LOGIN_RESOURCE_PATH);
+
+//For logout remove the attribute from the session that holds 
UserIdentity
+if (authentication != null && 
uri.equals(WebServerConstants.LOGOUT_RESOURCE_PATH)) {
--- End diff --

Refactored this method to be more readable.


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158111965
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillHttpSecurityHandlerProvider.java
 ---
@@ -0,0 +1,187 @@
+/*
+ * 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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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.rest.auth;
+
+import com.google.common.base.Preconditions;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.common.map.CaseInsensitiveMap;
+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.AuthStringUtil;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.eclipse.jetty.security.ConstraintSecurityHandler;
+import org.eclipse.jetty.security.authentication.SessionAuthentication;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.Request;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+
+public class DrillHttpSecurityHandlerProvider extends 
ConstraintSecurityHandler {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillHttpSecurityHandlerProvider.class);
+
+  private final Map 
securityHandlers =
+  CaseInsensitiveMap.newHashMapWithExpectedSize(5);
--- End diff --

I just chose a little higher number than 2 to avoid resizing of hashtable 
(based on default loadfactor of 0.75), but looked into the implementation and 
seems like initial size of 2 will also work since it will allocate 4 space for 
it.


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158118536
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/FORMSecurityHanlder.java
 ---
@@ -0,0 +1,50 @@
+/*
+ * 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
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.rest.auth;
+
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.rpc.security.plain.PlainFactory;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.eclipse.jetty.security.authentication.FormAuthenticator;
+
+public class FORMSecurityHanlder extends 
DrillHttpConstraintSecurityHandler {
--- End diff --

Changed.


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158117082
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoLoginService.java
 ---
@@ -0,0 +1,151 @@
+/*
+ * 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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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.rest.auth;
+
+
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.options.SystemOptionManager;
+import org.apache.drill.exec.util.ImpersonationUtil;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.eclipse.jetty.security.DefaultIdentityService;
+import org.eclipse.jetty.security.SpnegoLoginService;
+import org.eclipse.jetty.server.UserIdentity;
+import org.eclipse.jetty.util.B64Code;
+import org.ietf.jgss.GSSContext;
+import org.ietf.jgss.GSSCredential;
+import org.ietf.jgss.GSSException;
+import org.ietf.jgss.GSSManager;
+import org.ietf.jgss.GSSName;
+import org.ietf.jgss.Oid;
+
+import javax.security.auth.Subject;
+import java.lang.reflect.Field;
+import java.security.Principal;
+import java.security.PrivilegedExceptionAction;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Custom implementation of DrillSpnegoLoginService to avoid the need of 
passing targetName in a config file,
+ * to include the SPNEGO OID and the way UserIdentity is created.
+ */
+public class DrillSpnegoLoginService extends SpnegoLoginService {
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillSpnegoLoginService.class);
+
+  private static final String TARGET_NAME_FIELD_NAME = "_targetName";
+
+  private final DrillbitContext drillContext;
+
+  private final SpnegoUtil spnegoUtil;
+
+  private final UserGroupInformation loggedInUgi;
+
+  public DrillSpnegoLoginService(DrillbitContext drillBitContext) throws 
DrillException {
+super(DrillSpnegoLoginService.class.getName());
+setIdentityService(new DefaultIdentityService());
+drillContext = drillBitContext;
+
+// Load and verify SPNEGO config. Then Login using creds to get an UGI 
instance
+spnegoUtil = new SpnegoUtil(drillBitContext.getConfig());
+spnegoUtil.validateSpnegoConfig();
+loggedInUgi = spnegoUtil.getLoggedInUgi();
+  }
+
+  @Override
+  protected void doStart() throws Exception {
+// Override the parent implementation, setting _targetName to be the 
serverPrincipal
+// without the need for a one-line file to do the same thing.
+final Field targetNameField = 
SpnegoLoginService.class.getDeclaredField(TARGET_NAME_FIELD_NAME);
+targetNameField.setAccessible(true);
+targetNameField.set(this, spnegoUtil.getSpnegoPrincipal());
+  }
+
+  @Override
+  public UserIdentity login(final String username, final Object 
credentials) {
+
+UserIdentity identity = null;
+try {
+  identity = loggedInUgi.doAs(new 
PrivilegedExceptionAction() {
+@Override
+public UserIdentity run() {
+  return spnegoLogin(username, credentials);
+}
+  });
+} catch (Exception e) {
+  logger.error("Failed to login using SPNEGO");
--- End diff --

Done.


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158118993
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/FORMSecurityHanlder.java
 ---
@@ -0,0 +1,50 @@
+/*
+ * 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
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.rest.auth;
+
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.rpc.security.plain.PlainFactory;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.eclipse.jetty.security.authentication.FormAuthenticator;
+
+public class FORMSecurityHanlder extends 
DrillHttpConstraintSecurityHandler {
+  //private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(FORMSecurityHanlder.class);
+
+  public static final String HANDLER_NAME = "FORM";
+
+  @Override
+  public String getImplName() {
+return HANDLER_NAME;
--- End diff --

Didn't used it earlier since it was not used consistently by Jetty itself. 
See 
`org.eclipse.jetty.security.authentication.FormAuthenticator::getAuthMethod()`


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158117860
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoLoginService.java
 ---
@@ -0,0 +1,151 @@
+/*
+ * 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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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.rest.auth;
+
+
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.server.options.SystemOptionManager;
+import org.apache.drill.exec.util.ImpersonationUtil;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.eclipse.jetty.security.DefaultIdentityService;
+import org.eclipse.jetty.security.SpnegoLoginService;
+import org.eclipse.jetty.server.UserIdentity;
+import org.eclipse.jetty.util.B64Code;
+import org.ietf.jgss.GSSContext;
+import org.ietf.jgss.GSSCredential;
+import org.ietf.jgss.GSSException;
+import org.ietf.jgss.GSSManager;
+import org.ietf.jgss.GSSName;
+import org.ietf.jgss.Oid;
+
+import javax.security.auth.Subject;
+import java.lang.reflect.Field;
+import java.security.Principal;
+import java.security.PrivilegedExceptionAction;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Custom implementation of DrillSpnegoLoginService to avoid the need of 
passing targetName in a config file,
+ * to include the SPNEGO OID and the way UserIdentity is created.
+ */
+public class DrillSpnegoLoginService extends SpnegoLoginService {
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillSpnegoLoginService.class);
+
+  private static final String TARGET_NAME_FIELD_NAME = "_targetName";
+
+  private final DrillbitContext drillContext;
+
+  private final SpnegoUtil spnegoUtil;
+
+  private final UserGroupInformation loggedInUgi;
+
+  public DrillSpnegoLoginService(DrillbitContext drillBitContext) throws 
DrillException {
+super(DrillSpnegoLoginService.class.getName());
+setIdentityService(new DefaultIdentityService());
+drillContext = drillBitContext;
+
+// Load and verify SPNEGO config. Then Login using creds to get an UGI 
instance
+spnegoUtil = new SpnegoUtil(drillBitContext.getConfig());
+spnegoUtil.validateSpnegoConfig();
+loggedInUgi = spnegoUtil.getLoggedInUgi();
+  }
+
+  @Override
+  protected void doStart() throws Exception {
+// Override the parent implementation, setting _targetName to be the 
serverPrincipal
+// without the need for a one-line file to do the same thing.
+final Field targetNameField = 
SpnegoLoginService.class.getDeclaredField(TARGET_NAME_FIELD_NAME);
+targetNameField.setAccessible(true);
+targetNameField.set(this, spnegoUtil.getSpnegoPrincipal());
+  }
+
+  @Override
+  public UserIdentity login(final String username, final Object 
credentials) {
+
+UserIdentity identity = null;
+try {
+  identity = loggedInUgi.doAs(new 
PrivilegedExceptionAction() {
+@Override
+public UserIdentity run() {
+  return spnegoLogin(username, credentials);
+}
+  });
+} catch (Exception e) {
+  logger.error("Failed to login using SPNEGO");
+}
+
+return identity;
+  }
+
+  private UserIdentity spnegoLogin(String username, Object credentials) {
--- End diff --

Removed.


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158166736
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/rpc/security/KerberosHelper.java
 ---
@@ -43,16 +45,20 @@
   public String SERVER_PRINCIPAL;
   private final String testName;
 
-  private File keytabDir;
+  public File keytabDir;
--- End diff --

Have changed this back to private since change was not required.


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158100823
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java 
---
@@ -183,17 +180,19 @@ public void start() throws Exception {
 }
   }
 
-  private ServletContextHandler createServletContextHandler(final boolean 
authEnabled) {
+  private ServletContextHandler createServletContextHandler(final boolean 
authEnabled) throws Exception {
--- End diff --

Done


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158137486
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoAuthenticator.java
 ---
@@ -0,0 +1,150 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.rest.auth;
+
+
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.apache.parquet.Strings;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpVersion;
+import org.eclipse.jetty.security.ServerAuthException;
+import org.eclipse.jetty.security.UserAuthentication;
+import org.eclipse.jetty.security.authentication.DeferredAuthentication;
+import org.eclipse.jetty.security.authentication.SessionAuthentication;
+import org.eclipse.jetty.security.authentication.SpnegoAuthenticator;
+import org.eclipse.jetty.server.Authentication;
+import org.eclipse.jetty.server.HttpChannel;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Response;
+import org.eclipse.jetty.server.UserIdentity;
+
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.IOException;
+
+/**
+ * Custom SpnegoAuthenticator for Drill which provides following:
+ * 1) Perform SPNEGO authentication only when spnegoLogin resource is 
requested. This helps to avoid authentication
+ *for each and every resource which the JETTY provided authenticator 
does.
+ * 2) Helps to redirect to the target URL after authentication is done 
successfully.
+ * 3) Clear-Up in memory session information once LogOut is triggered. 
Such that any future request also trigger the
+ *SPNEGO authentication.
+ */
+public class DrillSpnegoAuthenticator extends SpnegoAuthenticator {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillSpnegoAuthenticator.class);
+
+  public DrillSpnegoAuthenticator(String authMethod) {
+super(authMethod);
+  }
+
+  @Override
+  public Authentication validateRequest(ServletRequest request, 
ServletResponse response, boolean mandatory)
+  throws ServerAuthException {
+
+HttpServletRequest req = (HttpServletRequest) request;
+HttpServletResponse res = (HttpServletResponse) response;
+HttpSession session = req.getSession(true);
+final Authentication authentication =
+(Authentication) 
session.getAttribute("org.eclipse.jetty.security.UserIdentity");
+String uri = req.getRequestURI();
+
+//If the Request URI is for /spnegoLogin then perform login
+mandatory |= uri.equals(WebServerConstants.SPENGO_LOGIN_RESOURCE_PATH);
+
+//For logout remove the attribute from the session that holds 
UserIdentity
+if (authentication != null && 
uri.equals(WebServerConstants.LOGOUT_RESOURCE_PATH)) {
+  logger.debug("Logging out user {}", req.getRemoteAddr());
+  session.removeAttribute("org.eclipse.jetty.security.UserIdentity");
--- End diff --

Changed in all the places.


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158120686
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/SPNEGOSecurityHandler.java
 ---
@@ -0,0 +1,37 @@
+/*
+ * 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
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.rest.auth;
+
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.server.DrillbitContext;
+
+public class SPNEGOSecurityHandler extends 
DrillHttpConstraintSecurityHandler {
+  //private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(SPNEGOSecurityHandler.class);
+
+  public static final String HANDLER_NAME = "SPNEGO";
+
+  @Override
+  public String getImplName() {
+return HANDLER_NAME;
--- End diff --

Done


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158121657
  
--- Diff: exec/rpc/src/main/java/org/apache/drill/exec/rpc/BasicServer.java 
---
@@ -202,6 +202,9 @@ public int bind(final int initialPort, boolean 
allowPortHunting) {
 if (e instanceof BindException && allowPortHunting) {
   continue;
 }
+
+e.printStackTrace();
--- End diff --

Not sure how this got introduced. Thanks for catching.


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158135735
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/SpnegoUtil.java
 ---
@@ -0,0 +1,115 @@
+/*
+ * 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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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.rest.auth;
+
+import com.google.common.base.Preconditions;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.CommonConfigurationKeys;
+import org.apache.hadoop.security.UserGroupInformation;
+
+public class SpnegoUtil {
+
+  private UserGroupInformation loggedInUgi;
+
+  //private String realm;
+
+  private String principal;
+
+  private String keytab;
+
+  public SpnegoUtil(DrillConfig config) {
+
+keytab = config.hasPath(ExecConstants.HTTP_SPNEGO_KEYTAB) ?
+config.getString(ExecConstants.HTTP_SPNEGO_KEYTAB) :
+null;
+
+principal = config.hasPath(ExecConstants.HTTP_SPNEGO_PRINCIPAL) ?
+config.getString(ExecConstants.HTTP_SPNEGO_PRINCIPAL) :
+null;
+  }
+
+  //Reads the SPNEGO principal from the config file
+  public String getSpnegoPrincipal() {
+return principal;
+  }
+
+  public void validateSpnegoConfig() throws DrillException {
+
+StringBuilder errorMsg = new StringBuilder();
+
+if (principal != null && keytab != null) {
+  return;
+}
+
+if (principal == null) {
+  errorMsg.append("\nConfiguration ");
+  errorMsg.append(ExecConstants.HTTP_SPNEGO_PRINCIPAL);
+  errorMsg.append(" is not found");
+}
+
+if (keytab == null) {
+  errorMsg.append("\nConfiguration ");
+  errorMsg.append(ExecConstants.HTTP_SPNEGO_KEYTAB);
+  errorMsg.append(" is not found");
+}
+
+throw new DrillException(errorMsg.toString());
+  }
+
+  //Performs the Server login to KDC for SPNEGO
+  private UserGroupInformation loginAndReturnUgi() throws DrillException {
+
+validateSpnegoConfig();
+
+UserGroupInformation ugi;
+try {
+  // Check if security is not enabled and try to set the security 
parameter to login the principal.
+  // After the login is performed reset the static UGI state.
+  if (!UserGroupInformation.isSecurityEnabled()) {
+final Configuration newConfig = new Configuration();
+
newConfig.set(CommonConfigurationKeys.HADOOP_SECURITY_AUTHENTICATION,
+UserGroupInformation.AuthenticationMethod.KERBEROS.toString());
+
+UserGroupInformation.setConfiguration(newConfig);
+ugi = 
UserGroupInformation.loginUserFromKeytabAndReturnUGI(principal, keytab);
+
+// Reset the original configuration for static UGI
+UserGroupInformation.setConfiguration(new Configuration());
+  } else {
+ugi = 
UserGroupInformation.loginUserFromKeytabAndReturnUGI(principal, keytab);
+  }
+} catch (Exception e) {
+  throw new DrillException(String.format("Login failed for %s with 
given keytab", principal));
--- End diff --

Done


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158137533
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillSpnegoAuthenticator.java
 ---
@@ -0,0 +1,150 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.rest.auth;
+
+
+import org.apache.drill.exec.server.rest.WebServerConstants;
+import org.apache.parquet.Strings;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpVersion;
+import org.eclipse.jetty.security.ServerAuthException;
+import org.eclipse.jetty.security.UserAuthentication;
+import org.eclipse.jetty.security.authentication.DeferredAuthentication;
+import org.eclipse.jetty.security.authentication.SessionAuthentication;
+import org.eclipse.jetty.security.authentication.SpnegoAuthenticator;
+import org.eclipse.jetty.server.Authentication;
+import org.eclipse.jetty.server.HttpChannel;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Response;
+import org.eclipse.jetty.server.UserIdentity;
+
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.IOException;
+
+/**
+ * Custom SpnegoAuthenticator for Drill which provides following:
+ * 1) Perform SPNEGO authentication only when spnegoLogin resource is 
requested. This helps to avoid authentication
+ *for each and every resource which the JETTY provided authenticator 
does.
+ * 2) Helps to redirect to the target URL after authentication is done 
successfully.
+ * 3) Clear-Up in memory session information once LogOut is triggered. 
Such that any future request also trigger the
+ *SPNEGO authentication.
+ */
+public class DrillSpnegoAuthenticator extends SpnegoAuthenticator {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(DrillSpnegoAuthenticator.class);
+
+  public DrillSpnegoAuthenticator(String authMethod) {
+super(authMethod);
+  }
+
+  @Override
+  public Authentication validateRequest(ServletRequest request, 
ServletResponse response, boolean mandatory)
+  throws ServerAuthException {
+
+HttpServletRequest req = (HttpServletRequest) request;
+HttpServletResponse res = (HttpServletResponse) response;
+HttpSession session = req.getSession(true);
+final Authentication authentication =
+(Authentication) 
session.getAttribute("org.eclipse.jetty.security.UserIdentity");
+String uri = req.getRequestURI();
+
+//If the Request URI is for /spnegoLogin then perform login
+mandatory |= uri.equals(WebServerConstants.SPENGO_LOGIN_RESOURCE_PATH);
+
+//For logout remove the attribute from the session that holds 
UserIdentity
+if (authentication != null && 
uri.equals(WebServerConstants.LOGOUT_RESOURCE_PATH)) {
+  logger.debug("Logging out user {}", req.getRemoteAddr());
+  session.removeAttribute("org.eclipse.jetty.security.UserIdentity");
+  return null;
+} else if (authentication != null) { // Since already logged in just 
return the session attribute
+  return authentication;
+} else { // The session is not yet authenticated
+  final String header = 
req.getHeader(HttpHeader.AUTHORIZATION.asString());
+  if (!mandatory) {
+return new DeferredAuthentication(this);
+  } else if (header == null) {
+try {
+  if (DeferredAuthentication.isDeferred(res)) {
+return Authentication.UNAUTHENTICATED;
+  } else {
+res.setHeader(HttpHeader.WWW_AUTHENTICATE.asString(), 
HttpHeader.NEGOTIATE.asString());
+res.sendError(401);
+logger.debug("SPNEGOAuthenticator: Sending 

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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 

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158135822
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/SpnegoUtil.java
 ---
@@ -0,0 +1,115 @@
+/*
+ * 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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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.rest.auth;
+
+import com.google.common.base.Preconditions;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.CommonConfigurationKeys;
+import org.apache.hadoop.security.UserGroupInformation;
+
+public class SpnegoUtil {
--- End diff --

Renamed to SpnegoConfig


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158106496
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/auth/DrillHttpConstraintSecurityHandler.java
 ---
@@ -0,0 +1,61 @@
+/*
+ * 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
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * 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.rest.auth;
+
+import com.google.common.collect.ImmutableSet;
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.eclipse.jetty.security.ConstraintMapping;
+import org.eclipse.jetty.security.ConstraintSecurityHandler;
+import org.eclipse.jetty.security.LoginService;
+import org.eclipse.jetty.security.authentication.LoginAuthenticator;
+
+import java.util.Collections;
+import java.util.Set;
+
+import static 
org.apache.drill.exec.server.rest.auth.DrillUserPrincipal.ADMIN_ROLE;
+import static 
org.apache.drill.exec.server.rest.auth.DrillUserPrincipal.AUTHENTICATED_ROLE;
+
+/**
+ * Accessor class that extends the ConstraintSecurityHandler to expose 
protected method's for start and stop of Handler.
+ * This is needed since now {@link DrillHttpSecurityHandlerProvider} 
composes of 2 security handler -
--- End diff --

Changed


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158099938
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogInLogOutResources.java
 ---
@@ -69,23 +79,94 @@ public Viewable getLoginPage(@Context 
HttpServletRequest request, @Context HttpS
 return ViewableWithPermissions.createLoginPage(null);
   }
 
+  @GET
+  @Path(WebServerConstants.SPENGO_LOGIN_RESOURCE_PATH)
+  @Produces(MediaType.TEXT_HTML)
+  public Viewable getSpnegologin(@Context HttpServletRequest request, 
@Context HttpServletResponse response,
+ @Context SecurityContext sc, @Context 
UriInfo uriInfo,
+ 
@QueryParam(WebServerConstants.REDIRECT_QUERY_PARM) String redirect) throws 
Exception {
+if (AuthDynamicFeature.isUserLoggedIn(sc)) {
+  request.getRequestDispatcher("/").forward(request, response);
+  return null;
+}
+
+final String errorString = "Invalid SPNEGO credentials or SPNEGO is 
not configured";
+MainLoginPageModel model = new MainLoginPageModel(errorString);
+return ViewableWithPermissions.createMainLoginPage(model);
+  }
+
   // Request type is POST because POST request which contains the login 
credentials are invalid and the request is
   // dispatched here directly.
   @POST
-  @Path("/login")
+  @Path(WebServerConstants.FORM_LOGIN_RESOURCE_PATH)
   @Produces(MediaType.TEXT_HTML)
   public Viewable getLoginPageAfterValidationError() {
 return ViewableWithPermissions.createLoginPage("Invalid 
username/password credentials.");
   }
 
   @GET
-  @Path("/logout")
+  @Path(WebServerConstants.LOGOUT_RESOURCE_PATH)
   public void logout(@Context HttpServletRequest req, @Context 
HttpServletResponse resp) throws Exception {
 final HttpSession session = req.getSession();
 if (session != null) {
   session.invalidate();
 }
 
-req.getRequestDispatcher("/").forward(req, resp);
+
req.getRequestDispatcher(WebServerConstants.WEBSERVER_ROOT_PATH).forward(req, 
resp);
+  }
+
+  @GET
+  @Path(WebServerConstants.MAIN_LOGIN_RESOURCE_PATH)
+  @Produces(MediaType.TEXT_HTML)
+  public Viewable getMainLoginPage(@Context HttpServletRequest request, 
@Context HttpServletResponse response,
+   @Context SecurityContext sc, @Context 
UriInfo uriInfo,
+   
@QueryParam(WebServerConstants.REDIRECT_QUERY_PARM) String redirect) throws 
Exception {
+if (!StringUtils.isEmpty(redirect)) {
+  // If the URL has redirect in it, set the redirect URI in session, 
so that after the login is successful, request
+  // is forwarded to the redirect page.
+  final HttpSession session = request.getSession(true);
+  final URI destURI = UriBuilder.fromUri(URLDecoder.decode(redirect, 
"UTF-8")).build();
+  session.setAttribute(FormAuthenticator.__J_URI, destURI.toString());
+}
+
+MainLoginPageModel model = new MainLoginPageModel(null);
+return ViewableWithPermissions.createMainLoginPage(model);
+  }
+
+  public class MainLoginPageModel {
+
+public String error;
+
+MainLoginPageModel(String error) {
--- End diff --

Since the class is used only by `LogInLogOutResources` I have made it 
private and left constructor as package-private


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158102763
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServerConstants.java
 ---
@@ -0,0 +1,44 @@
+/*
+ * 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
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.rest;
+
+/**
+ * Holds various constants used by WebServer components.
+ */
+public class WebServerConstants {
+  //private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(WebServerConstants.class);
--- End diff --

1. Removed logger. 
2. I prefer to use class since interface is mostly used for defining a 
service contract. If it's an interface then any class can implement it and will 
get access to all the constants which might not be required. I have changed the 
class to be final and made constructor private so that it can't be extended and 
instantiated by mistake.


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158099736
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogInLogOutResources.java
 ---
@@ -69,23 +79,94 @@ public Viewable getLoginPage(@Context 
HttpServletRequest request, @Context HttpS
 return ViewableWithPermissions.createLoginPage(null);
   }
 
+  @GET
+  @Path(WebServerConstants.SPENGO_LOGIN_RESOURCE_PATH)
+  @Produces(MediaType.TEXT_HTML)
+  public Viewable getSpnegologin(@Context HttpServletRequest request, 
@Context HttpServletResponse response,
+ @Context SecurityContext sc, @Context 
UriInfo uriInfo,
+ 
@QueryParam(WebServerConstants.REDIRECT_QUERY_PARM) String redirect) throws 
Exception {
+if (AuthDynamicFeature.isUserLoggedIn(sc)) {
+  request.getRequestDispatcher("/").forward(request, response);
+  return null;
+}
+
+final String errorString = "Invalid SPNEGO credentials or SPNEGO is 
not configured";
+MainLoginPageModel model = new MainLoginPageModel(errorString);
+return ViewableWithPermissions.createMainLoginPage(model);
+  }
+
   // Request type is POST because POST request which contains the login 
credentials are invalid and the request is
   // dispatched here directly.
   @POST
-  @Path("/login")
+  @Path(WebServerConstants.FORM_LOGIN_RESOURCE_PATH)
   @Produces(MediaType.TEXT_HTML)
   public Viewable getLoginPageAfterValidationError() {
 return ViewableWithPermissions.createLoginPage("Invalid 
username/password credentials.");
   }
 
   @GET
-  @Path("/logout")
+  @Path(WebServerConstants.LOGOUT_RESOURCE_PATH)
   public void logout(@Context HttpServletRequest req, @Context 
HttpServletResponse resp) throws Exception {
 final HttpSession session = req.getSession();
 if (session != null) {
   session.invalidate();
 }
 
-req.getRequestDispatcher("/").forward(req, resp);
+
req.getRequestDispatcher(WebServerConstants.WEBSERVER_ROOT_PATH).forward(req, 
resp);
+  }
+
+  @GET
+  @Path(WebServerConstants.MAIN_LOGIN_RESOURCE_PATH)
+  @Produces(MediaType.TEXT_HTML)
+  public Viewable getMainLoginPage(@Context HttpServletRequest request, 
@Context HttpServletResponse response,
+   @Context SecurityContext sc, @Context 
UriInfo uriInfo,
+   
@QueryParam(WebServerConstants.REDIRECT_QUERY_PARM) String redirect) throws 
Exception {
+if (!StringUtils.isEmpty(redirect)) {
+  // If the URL has redirect in it, set the redirect URI in session, 
so that after the login is successful, request
+  // is forwarded to the redirect page.
+  final HttpSession session = request.getSession(true);
+  final URI destURI = UriBuilder.fromUri(URLDecoder.decode(redirect, 
"UTF-8")).build();
+  session.setAttribute(FormAuthenticator.__J_URI, destURI.toString());
+}
--- End diff --

Done. Added private method `updateSessionRedirectInfo`


---

[GitHub] drill pull request #1040: DRILL-5425: Support HTTP Kerberos auth using SPNEG...

[sohami]

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1040#discussion_r158100206
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogInLogOutResources.java
 ---
@@ -69,23 +79,94 @@ public Viewable getLoginPage(@Context 
HttpServletRequest request, @Context HttpS
 return ViewableWithPermissions.createLoginPage(null);
   }
 
+  @GET
+  @Path(WebServerConstants.SPENGO_LOGIN_RESOURCE_PATH)
+  @Produces(MediaType.TEXT_HTML)
+  public Viewable getSpnegologin(@Context HttpServletRequest request, 
@Context HttpServletResponse response,
+ @Context SecurityContext sc, @Context 
UriInfo uriInfo,
+ 
@QueryParam(WebServerConstants.REDIRECT_QUERY_PARM) String redirect) throws 
Exception {
+if (AuthDynamicFeature.isUserLoggedIn(sc)) {
+  request.getRequestDispatcher("/").forward(request, response);
+  return null;
+}
+
+final String errorString = "Invalid SPNEGO credentials or SPNEGO is 
not configured";
+MainLoginPageModel model = new MainLoginPageModel(errorString);
+return ViewableWithPermissions.createMainLoginPage(model);
+  }
+
   // Request type is POST because POST request which contains the login 
credentials are invalid and the request is
   // dispatched here directly.
   @POST
-  @Path("/login")
+  @Path(WebServerConstants.FORM_LOGIN_RESOURCE_PATH)
   @Produces(MediaType.TEXT_HTML)
   public Viewable getLoginPageAfterValidationError() {
 return ViewableWithPermissions.createLoginPage("Invalid 
username/password credentials.");
   }
 
   @GET
-  @Path("/logout")
+  @Path(WebServerConstants.LOGOUT_RESOURCE_PATH)
   public void logout(@Context HttpServletRequest req, @Context 
HttpServletResponse resp) throws Exception {
 final HttpSession session = req.getSession();
 if (session != null) {
   session.invalidate();
 }
 
-req.getRequestDispatcher("/").forward(req, resp);
+
req.getRequestDispatcher(WebServerConstants.WEBSERVER_ROOT_PATH).forward(req, 
resp);
+  }
+
+  @GET
+  @Path(WebServerConstants.MAIN_LOGIN_RESOURCE_PATH)
+  @Produces(MediaType.TEXT_HTML)
+  public Viewable getMainLoginPage(@Context HttpServletRequest request, 
@Context HttpServletResponse response,
+   @Context SecurityContext sc, @Context 
UriInfo uriInfo,
+   
@QueryParam(WebServerConstants.REDIRECT_QUERY_PARM) String redirect) throws 
Exception {
+if (!StringUtils.isEmpty(redirect)) {
+  // If the URL has redirect in it, set the redirect URI in session, 
so that after the login is successful, request
+  // is forwarded to the redirect page.
+  final HttpSession session = request.getSession(true);
+  final URI destURI = UriBuilder.fromUri(URLDecoder.decode(redirect, 
"UTF-8")).build();
+  session.setAttribute(FormAuthenticator.__J_URI, destURI.toString());
+}
+
+MainLoginPageModel model = new MainLoginPageModel(null);
+return ViewableWithPermissions.createMainLoginPage(model);
+  }
+
+  public class MainLoginPageModel {
+
+public String error;
+
+MainLoginPageModel(String error) {
+  this.error = error;
+}
+
+public boolean isSpnegoEnabled() {
+  return 
LogInLogOutResources.isSpnegoEnabled(workManager.getContext().getConfig());
+}
+
+public boolean isFormEnabled() {
+  return 
LogInLogOutResources.isFormEnabled(workManager.getContext().getConfig());
+}
+
+public String getError() {
+  return error;
+}
+  }
+
+  private static boolean isSpnegoEnabled(DrillConfig config) {
--- End diff --

Done


---