Repository: calcite-avatica Updated Branches: refs/heads/master c3f4611d4 -> e5ad50385
[CALCITE-1487] Set the request as handled with authentication failures Project: http://git-wip-us.apache.org/repos/asf/calcite-avatica/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite-avatica/commit/e5ad5038 Tree: http://git-wip-us.apache.org/repos/asf/calcite-avatica/tree/e5ad5038 Diff: http://git-wip-us.apache.org/repos/asf/calcite-avatica/diff/e5ad5038 Branch: refs/heads/master Commit: e5ad503858c47b4336adbb1ade8399818a9323d1 Parents: c3f4611 Author: Josh Elser <[email protected]> Authored: Thu Jul 27 17:32:29 2017 -0400 Committer: Josh Elser <[email protected]> Committed: Thu Jul 27 17:32:29 2017 -0400 ---------------------------------------------------------------------- .../avatica/server/AbstractAvaticaHandler.java | 4 +++- .../calcite/avatica/server/AvaticaJsonHandler.java | 2 +- .../avatica/server/AvaticaProtobufHandler.java | 2 +- .../avatica/server/AbstractAvaticaHandlerTest.java | 15 ++++++++++----- 4 files changed, 15 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite-avatica/blob/e5ad5038/server/src/main/java/org/apache/calcite/avatica/server/AbstractAvaticaHandler.java ---------------------------------------------------------------------- diff --git a/server/src/main/java/org/apache/calcite/avatica/server/AbstractAvaticaHandler.java b/server/src/main/java/org/apache/calcite/avatica/server/AbstractAvaticaHandler.java index 3a7ccf3..edd9f0d 100644 --- a/server/src/main/java/org/apache/calcite/avatica/server/AbstractAvaticaHandler.java +++ b/server/src/main/java/org/apache/calcite/avatica/server/AbstractAvaticaHandler.java @@ -20,6 +20,7 @@ import org.apache.calcite.avatica.AvaticaSeverity; import org.apache.calcite.avatica.remote.AuthenticationType; import org.apache.calcite.avatica.remote.Service.ErrorResponse; +import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; import java.io.IOException; @@ -52,7 +53,7 @@ public abstract class AbstractAvaticaHandler extends AbstractHandler * @param response The response to the user's request * @return True if request can proceed, false otherwise. */ - public boolean isUserPermitted(AvaticaServerConfiguration serverConfig, + public boolean isUserPermitted(AvaticaServerConfiguration serverConfig, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException { // Make sure that we drop any unauthenticated users out first. if (null != serverConfig) { @@ -61,6 +62,7 @@ public abstract class AbstractAvaticaHandler extends AbstractHandler if (null == remoteUser) { response.setStatus(HttpURLConnection.HTTP_UNAUTHORIZED); response.getOutputStream().write(UNAUTHORIZED_ERROR.serialize().toByteArray()); + baseRequest.setHandled(true); return false; } } http://git-wip-us.apache.org/repos/asf/calcite-avatica/blob/e5ad5038/server/src/main/java/org/apache/calcite/avatica/server/AvaticaJsonHandler.java ---------------------------------------------------------------------- diff --git a/server/src/main/java/org/apache/calcite/avatica/server/AvaticaJsonHandler.java b/server/src/main/java/org/apache/calcite/avatica/server/AvaticaJsonHandler.java index 15d2d3c..ff45400 100644 --- a/server/src/main/java/org/apache/calcite/avatica/server/AvaticaJsonHandler.java +++ b/server/src/main/java/org/apache/calcite/avatica/server/AvaticaJsonHandler.java @@ -90,7 +90,7 @@ public class AvaticaJsonHandler extends AbstractAvaticaHandler { HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { try (final Context ctx = requestTimer.start()) { - if (!isUserPermitted(serverConfig, request, response)) { + if (!isUserPermitted(serverConfig, baseRequest, request, response)) { LOG.debug("HTTP request from {} is unauthenticated and authentication is required", request.getRemoteAddr()); return; http://git-wip-us.apache.org/repos/asf/calcite-avatica/blob/e5ad5038/server/src/main/java/org/apache/calcite/avatica/server/AvaticaProtobufHandler.java ---------------------------------------------------------------------- diff --git a/server/src/main/java/org/apache/calcite/avatica/server/AvaticaProtobufHandler.java b/server/src/main/java/org/apache/calcite/avatica/server/AvaticaProtobufHandler.java index cfa19c1..acb957b 100644 --- a/server/src/main/java/org/apache/calcite/avatica/server/AvaticaProtobufHandler.java +++ b/server/src/main/java/org/apache/calcite/avatica/server/AvaticaProtobufHandler.java @@ -92,7 +92,7 @@ public class AvaticaProtobufHandler extends AbstractAvaticaHandler { throws IOException, ServletException { try (final Context ctx = this.requestTimer.start()) { // Check if the user is OK to proceed. - if (!isUserPermitted(serverConfig, request, response)) { + if (!isUserPermitted(serverConfig, baseRequest, request, response)) { LOG.debug("HTTP request from {} is unauthenticated and authentication is required", request.getRemoteAddr()); return; http://git-wip-us.apache.org/repos/asf/calcite-avatica/blob/e5ad5038/server/src/test/java/org/apache/calcite/avatica/server/AbstractAvaticaHandlerTest.java ---------------------------------------------------------------------- diff --git a/server/src/test/java/org/apache/calcite/avatica/server/AbstractAvaticaHandlerTest.java b/server/src/test/java/org/apache/calcite/avatica/server/AbstractAvaticaHandlerTest.java index 66eb361..0583436 100644 --- a/server/src/test/java/org/apache/calcite/avatica/server/AbstractAvaticaHandlerTest.java +++ b/server/src/test/java/org/apache/calcite/avatica/server/AbstractAvaticaHandlerTest.java @@ -18,6 +18,7 @@ package org.apache.calcite.avatica.server; import org.apache.calcite.avatica.remote.AuthenticationType; +import org.eclipse.jetty.server.Request; import org.hamcrest.BaseMatcher; import org.hamcrest.Description; import org.junit.Before; @@ -44,15 +45,17 @@ public class AbstractAvaticaHandlerTest { private AbstractAvaticaHandler handler; private AvaticaServerConfiguration config; + private Request baseRequest; private HttpServletRequest request; private HttpServletResponse response; @Before public void setup() throws Exception { handler = mock(AbstractAvaticaHandler.class); config = mock(AvaticaServerConfiguration.class); + baseRequest = new Request(null, null); request = mock(HttpServletRequest.class); response = mock(HttpServletResponse.class); - when(handler.isUserPermitted(config, request, response)).thenCallRealMethod(); + when(handler.isUserPermitted(config, baseRequest, request, response)).thenCallRealMethod(); } @Test public void disallowUnauthenticatedUsers() throws Exception { @@ -62,8 +65,10 @@ public class AbstractAvaticaHandlerTest { when(request.getRemoteUser()).thenReturn(null); when(response.getOutputStream()).thenReturn(os); - assertFalse(handler.isUserPermitted(config, request, response)); + assertFalse(handler.isUserPermitted(config, baseRequest, request, response)); + // The request should be marked as "handled" + assertTrue(baseRequest.isHandled()); verify(response).setStatus(HttpURLConnection.HTTP_UNAUTHORIZED); // Make sure that the serialized ErrorMessage looks reasonable verify(os).write(argThat(new BaseMatcher<byte[]>() { @@ -86,16 +91,16 @@ public class AbstractAvaticaHandlerTest { @Test public void allowAuthenticatedUsers() throws Exception { when(config.getAuthenticationType()).thenReturn(AuthenticationType.SPNEGO); when(request.getRemoteUser()).thenReturn("user1"); - assertTrue(handler.isUserPermitted(config, request, response)); + assertTrue(handler.isUserPermitted(config, baseRequest, request, response)); } @Test public void allowAllUsersWhenNoAuthenticationIsNeeded() throws Exception { when(config.getAuthenticationType()).thenReturn(AuthenticationType.NONE); when(request.getRemoteUser()).thenReturn(null); - assertTrue(handler.isUserPermitted(config, request, response)); + assertTrue(handler.isUserPermitted(config, baseRequest, request, response)); when(request.getRemoteUser()).thenReturn("user1"); - assertTrue(handler.isUserPermitted(config, request, response)); + assertTrue(handler.isUserPermitted(config, baseRequest, request, response)); } }
