This is an automated email from the ASF dual-hosted git repository. vihangk1 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push: new 3e25b52 HIVE-25479: Browser SSO auth may fail intermittently (Vihang Karajgaonkar, reviewed by Naveen Gangam)" 3e25b52 is described below commit 3e25b524600c62dfde8d069288a162ee6a59d25d Author: Vihang Karajgaonkar <27933586+vihan...@users.noreply.github.com> AuthorDate: Tue Oct 19 15:57:58 2021 -0700 HIVE-25479: Browser SSO auth may fail intermittently (Vihang Karajgaonkar, reviewed by Naveen Gangam)" Closes (#2728) --- .../jdbc/saml/SimpleSAMLPhpTestBrowserClient.java | 15 ++ .../auth/saml/TestHttpSamlAuthentication.java | 105 ++++++++++++- .../java/org/apache/hive/jdbc/HiveConnection.java | 74 ++++++--- jdbc/src/java/org/apache/hive/jdbc/Utils.java | 2 +- .../hive/jdbc/saml/HiveJdbcBrowserClient.java | 169 ++++++++++----------- .../hive/jdbc/saml/HttpBrowserClientServlet.java | 62 ++++++++ .../jdbc/saml/HttpSamlAuthRequestInterceptor.java | 5 + .../apache/hive/jdbc/saml/IJdbcBrowserClient.java | 42 +++-- .../service/auth/saml/HiveSamlHttpServlet.java | 14 +- 9 files changed, 345 insertions(+), 143 deletions(-) diff --git a/itests/hive-unit/src/test/java/org/apache/hive/jdbc/saml/SimpleSAMLPhpTestBrowserClient.java b/itests/hive-unit/src/test/java/org/apache/hive/jdbc/saml/SimpleSAMLPhpTestBrowserClient.java index 0e2a8b2..97f5de5 100644 --- a/itests/hive-unit/src/test/java/org/apache/hive/jdbc/saml/SimpleSAMLPhpTestBrowserClient.java +++ b/itests/hive-unit/src/test/java/org/apache/hive/jdbc/saml/SimpleSAMLPhpTestBrowserClient.java @@ -36,6 +36,7 @@ public class SimpleSAMLPhpTestBrowserClient extends HiveJdbcBrowserClient { private final String username; private final String password; private final long tokenDelayMs; + private int injectFailureCount = 0; private static final Logger LOG = LoggerFactory .getLogger(SimpleSAMLPhpTestBrowserClient.class); @@ -48,6 +49,10 @@ public class SimpleSAMLPhpTestBrowserClient extends HiveJdbcBrowserClient { this.tokenDelayMs = tokenDelayMs; } + public void setInjectFailureCount(int injectFailureCount) { + this.injectFailureCount = injectFailureCount; + } + @Override protected void openBrowserWindow() throws HiveJdbcBrowserException { // if user and password are null, we fallback to real browser for interactive mode @@ -73,6 +78,16 @@ public class SimpleSAMLPhpTestBrowserClient extends HiveJdbcBrowserClient { } @Override + public void doBrowserSSO() throws HiveJdbcBrowserException { + if (injectFailureCount > 0) { + injectFailureCount--; + throw new HiveJdbcBrowserException( + "This is a injected failure for testing purpose"); + } + super.doBrowserSSO(); + } + + @Override public HiveJdbcBrowserServerResponse getServerResponse() { if (tokenDelayMs > 0) { LOG.debug("Adding a delay of {} msec", tokenDelayMs); diff --git a/itests/hive-unit/src/test/java/org/apache/hive/service/auth/saml/TestHttpSamlAuthentication.java b/itests/hive-unit/src/test/java/org/apache/hive/service/auth/saml/TestHttpSamlAuthentication.java index 2a69ed7..d71fded 100644 --- a/itests/hive-unit/src/test/java/org/apache/hive/service/auth/saml/TestHttpSamlAuthentication.java +++ b/itests/hive-unit/src/test/java/org/apache/hive/service/auth/saml/TestHttpSamlAuthentication.java @@ -21,6 +21,7 @@ package org.apache.hive.service.auth.saml; import static org.apache.hive.jdbc.Utils.JdbcConnectionParams.AUTH_BROWSER_RESPONSE_PORT; import static org.apache.hive.jdbc.Utils.JdbcConnectionParams.AUTH_BROWSER_RESPONSE_TIMEOUT_SECS; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -31,6 +32,10 @@ import com.google.common.io.Files; import com.google.common.io.Resources; import java.io.File; import java.io.IOException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.ServerSocket; +import java.net.Socket; import java.nio.charset.StandardCharsets; import java.sql.ResultSet; import java.sql.SQLException; @@ -48,6 +53,7 @@ import java.util.concurrent.Future; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.conf.HiveConf.ConfVars; import org.apache.hadoop.hive.metastore.MetaStoreTestUtils; +import org.apache.hadoop.hive.ql.metadata.TestHive; import org.apache.hive.jdbc.HiveConnection; import org.apache.hive.jdbc.Utils.JdbcConnectionParams; import org.apache.hive.jdbc.miniHS2.MiniHS2; @@ -56,6 +62,7 @@ import org.apache.hive.jdbc.saml.IJdbcBrowserClient; import org.apache.hive.jdbc.saml.IJdbcBrowserClient.HiveJdbcBrowserException; import org.apache.hive.jdbc.saml.IJdbcBrowserClientFactory; import org.apache.hive.jdbc.saml.SimpleSAMLPhpTestBrowserClient; +import org.apache.thrift.TException; import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -221,6 +228,12 @@ public class TestHttpSamlAuthentication { + "=" + responsePort; } + private static String getSamlJdbcConnectionUrlWithRetry(int numRetries) + throws Exception { + return getSamlJdbcConnectionUrl() + ";" + JdbcConnectionParams.RETRIES + "=" + + numRetries; + } + private static class TestHiveJdbcBrowserClientFactory implements IJdbcBrowserClientFactory { @@ -239,8 +252,8 @@ public class TestHttpSamlAuthentication { @Override public IJdbcBrowserClient create(JdbcConnectionParams connectionParams) throws HiveJdbcBrowserException { - return new SimpleSAMLPhpTestBrowserClient(connectionParams, user - , password, tokenDelayMs); + return new SimpleSAMLPhpTestBrowserClient( + connectionParams, user, password, tokenDelayMs); } } @@ -248,7 +261,6 @@ public class TestHttpSamlAuthentication { * Test HiveConnection which injects a HTMLUnit based browser client. */ private static class TestHiveConnection extends HiveConnection { - public TestHiveConnection(String uri, Properties info, String testUser, String testPass, long tokenDelayMs) throws SQLException { super(uri, info, new TestHiveJdbcBrowserClientFactory(testUser, testPass, tokenDelayMs)); @@ -265,6 +277,23 @@ public class TestHttpSamlAuthentication { } } + private static class TestHiveConnectionWithInjectedFailure extends TestHiveConnection { + private static boolean failureToggle = true; + public TestHiveConnectionWithInjectedFailure(String uri, Properties info, + String testUser, + String testPass) throws SQLException { + super(uri, info, testUser, testPass); + } + + @Override + protected void injectBrowserSSOError() throws Exception { + if (failureToggle) { + failureToggle = false; + throw new Exception("Injected failure"); + } + } + } + /** * Util class to issue multiple connection requests concurrently. */ @@ -376,6 +405,62 @@ public class TestHttpSamlAuthentication { } /** + * Makes sure that the port is released once the SSO flow completes. The test retries + * twice to make sure that there is no flakiness caused by the race between finding + * a free port and immediately some other process acquiring that port later before + * Browser client could be started on it. + */ + @Test + public void testPortReleaseAfterSSO() throws Exception { + setupIDP(true, USER_PASS_MODE); + for (int i=0; i<2; i++) { + int samlResponsePort = MetaStoreTestUtils.findFreePort(); + try (TestHiveConnection connection = new TestHiveConnection( + getSamlJdbcConnectionUrl(30, samlResponsePort), new Properties(), USER1, + USER1_PASSWORD)) { + assertLoggedInUser(connection, USER1); + assertEquals(samlResponsePort, + connection.getBrowserClient().getPort().intValue()); + assertTrue("Port must be released after SSO flow", + isPortAvailable(samlResponsePort)); + break; + } + } + } + + /** + * Here we start HS2 without SAML mode and attempt to use browser SSO against it. + * Test makes sure that connection fails and port is released. + */ + @Test + public void testPortReleaseOnInvalidConfig() throws Exception { + // no IDP setup here; start HS2 in non-SAML mode + miniHS2.start(new HashMap<>()); + int samlResponsePort = MetaStoreTestUtils.findFreePort(); + try (TestHiveConnection connection = new TestHiveConnection( + getSamlJdbcConnectionUrl(30, samlResponsePort), new Properties(), USER1, + USER1_PASSWORD)) { + fail("Connection should not have succeeded since HS2 is not configured correctly"); + } catch (Exception e) { + assertTrue("Port must be released after SSO flow", + isPortAvailable(samlResponsePort)); + } + } + + /** + * Make sure that the given port is free by binding to it and then releasing it. + * @param port + */ + private boolean isPortAvailable(int port) { + try (ServerSocket socket = new ServerSocket(port, 0, + InetAddress.getByName(HiveSamlUtils.LOOP_BACK_INTERFACE))) { + return true; + } catch (IOException e) { + return false; + } + } + + /** * Test exercises group name filtering for users. Only users who belong for a given * groups should be able to log in. user1 belongs to group1, user2 to group2 and user3 * to group1 and group2. @@ -423,6 +508,20 @@ public class TestHttpSamlAuthentication { } } + + /** + * Test injects failure in the first connection attempt and then makes sure that the + * 2nd retry works as expected. + */ + @Test + public void testConnectionRetries() throws Exception { + setupIDP(true, USER_PASS_MODE); + try (HiveConnection connection = new TestHiveConnectionWithInjectedFailure( + getSamlJdbcConnectionUrlWithRetry(2), new Properties(), USER1, USER1_PASSWORD)) { + assertLoggedInUser(connection, USER1); + } + } + /** * Test makes sure that the token received in the server response is not valid * after it is expired. diff --git a/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java b/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java index 4eefded..5fcad06 100644 --- a/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java +++ b/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java @@ -179,7 +179,7 @@ public class HiveConnection implements java.sql.Connection { private Properties clientInfo; private Subject loggedInSubject; private int maxRetries = 1; - private final IJdbcBrowserClient browserClient; + private IJdbcBrowserClient browserClient; /** * Get all direct HiveServer2 URLs from a ZooKeeper based HiveServer2 URL @@ -276,7 +276,6 @@ public class HiveConnection implements java.sql.Connection { sessConfMap = null; isEmbeddedMode = true; initFetchSize = 0; - browserClient = null; } public HiveConnection(String uri, Properties info) throws SQLException { @@ -284,6 +283,11 @@ public class HiveConnection implements java.sql.Connection { } @VisibleForTesting + protected int getNumRetries() { + return maxRetries; + } + + @VisibleForTesting protected HiveConnection(String uri, Properties info, IJdbcBrowserClientFactory browserClientFactory) throws SQLException { try { @@ -340,8 +344,6 @@ public class HiveConnection implements java.sql.Connection { } catch (HiveJdbcBrowserException e) { throw new SQLException(""); } - } else { - browserClient = null; } if (isEmbeddedMode) { client = EmbeddedCLIServicePortal.get(connParams.getHiveConfs()); @@ -1067,33 +1069,58 @@ public class HiveConnection implements java.sql.Connection { //TODO This is a bit hacky. We piggy back on a dummy OpenSession call // to get the redirect response from the server. Instead its probably cleaner to // explicitly do a HTTP post request and get the response. - int numRetry = isBrowserAuthMode() ? 2 : 1; - for (int i=0; i<numRetry; i++) { - try { - openSession(openReq); - } catch (TException e) { - if (isSamlRedirect(e)) { - boolean success = doBrowserSSO(); - if (!success) { - String msg = browserClient.getServerResponse() == null - || browserClient.getServerResponse().getMsg() == null ? "" - : browserClient.getServerResponse().getMsg(); + try { + int numRetry = 1; + if (isBrowserAuthMode()) { + numRetry = 2; + browserClient.startListening(); + } + for (int i=0; i<numRetry; i++) { + try { + openSession(openReq); + } catch (TException e) { + if (isSamlRedirect(e)) { + boolean success = doBrowserSSO(); + if (!success) { + String msg = browserClient.getServerResponse() == null + || browserClient.getServerResponse().getMsg() == null ? "" + : browserClient.getServerResponse().getMsg(); + throw new SQLException( + "Could not establish connection to " + jdbcUriString + ": " + + msg, " 08S01", e); + } + } else { throw new SQLException( - "Could not establish connection to " + jdbcUriString + ": " - + msg, " 08S01", e); + "Could not establish connection to " + jdbcUriString + ": " + e + .getMessage(), " 08S01", e); } - } else { - throw new SQLException( - "Could not establish connection to " + jdbcUriString + ": " + e - .getMessage(), " 08S01", e); + } + } + } catch (HiveJdbcBrowserException e) { + throw new SQLException( + "Could not establish connection to " + jdbcUriString + ": " + e + .getMessage(), " 08S01", e); + } finally { + if (browserClient != null) { + try { + browserClient.close(); + } catch (IOException e) { + LOG.error("Unable to close the browser SSO client : " + e.getMessage(), e); } } } isClosed = false; } - private boolean doBrowserSSO() throws SQLException { + @VisibleForTesting + protected void injectBrowserSSOError() throws Exception { + //no-op + } + + @VisibleForTesting + protected boolean doBrowserSSO() throws SQLException { try { + injectBrowserSSOError(); Preconditions.checkNotNull(browserClient); try (IJdbcBrowserClient bc = browserClient) { browserClient.doBrowserSSO(); @@ -1105,8 +1132,7 @@ public class HiveConnection implements java.sql.Connection { } } catch (Exception ex) { throw new SQLException("Browser based SSO failed: " + ex.getMessage(), - " 08S01", - ex); + " 08S01", ex); } } diff --git a/jdbc/src/java/org/apache/hive/jdbc/Utils.java b/jdbc/src/java/org/apache/hive/jdbc/Utils.java index ffd628c..0fd4820 100644 --- a/jdbc/src/java/org/apache/hive/jdbc/Utils.java +++ b/jdbc/src/java/org/apache/hive/jdbc/Utils.java @@ -85,7 +85,7 @@ public class Utils { // Client param names: // Retry setting - static final String RETRIES = "retries"; + public static final String RETRIES = "retries"; public static final String RETRY_INTERVAL = "retryInterval"; public static final String AUTH_TYPE = "auth"; diff --git a/jdbc/src/java/org/apache/hive/jdbc/saml/HiveJdbcBrowserClient.java b/jdbc/src/java/org/apache/hive/jdbc/saml/HiveJdbcBrowserClient.java index 28732fd..c03c961 100644 --- a/jdbc/src/java/org/apache/hive/jdbc/saml/HiveJdbcBrowserClient.java +++ b/jdbc/src/java/org/apache/hive/jdbc/saml/HiveJdbcBrowserClient.java @@ -20,19 +20,12 @@ package org.apache.hive.jdbc.saml; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import com.google.common.base.Strings; -import com.google.common.util.concurrent.ThreadFactoryBuilder; import java.awt.Desktop; import java.awt.Desktop.Action; -import java.io.BufferedReader; import java.io.IOException; -import java.io.InputStreamReader; import java.io.PrintWriter; import java.io.UnsupportedEncodingException; -import java.net.InetAddress; -import java.net.ServerSocket; import java.net.Socket; -import java.net.SocketTimeoutException; import java.net.URI; import java.net.URISyntaxException; import java.net.URLDecoder; @@ -41,12 +34,15 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.LinkedBlockingDeque; +import java.util.concurrent.TimeUnit; import org.apache.hive.jdbc.Utils.JdbcConnectionParams; import org.apache.hive.service.auth.saml.HiveSamlUtils; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.servlet.ServletHandler; +import org.eclipse.jetty.servlet.ServletHolder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,48 +56,80 @@ public class HiveJdbcBrowserClient implements IJdbcBrowserClient { // error message when the socket times out. @VisibleForTesting public static final String TIMEOUT_ERROR_MSG = "Timed out while waiting for server response"; - private final ServerSocket serverSocket; - private HiveJdbcBrowserServerResponse serverResponse; + // port as parsed from the connection URL; default is 0 which means any available port. + private final int portFromUrl; + // the actual port on the local machine where the web server is running + private Integer serverPort; + // timeout in mill-sec until which browserClient will wait for auth response from the + // HS2 server. + private final long timeoutInMs; + private final BlockingQueue<HiveJdbcBrowserServerResponse> + serverResponseQueue = new LinkedBlockingDeque<>(); protected JdbcBrowserClientContext clientContext; // By default we wait for 2 min unless overridden by a JDBC connection param // browserResponseTimeout private static final int DEFAULT_SOCKET_TIMEOUT_SECS = 120; - private final ExecutorService serverResponseThread = Executors.newSingleThreadExecutor( - new ThreadFactoryBuilder().setNameFormat("Hive-Jdbc-Browser-Client-%d") - .setDaemon(true).build()); + private Server webServer; + private HiveJdbcBrowserServerResponse serverResponse; HiveJdbcBrowserClient(JdbcConnectionParams connectionParams) throws HiveJdbcBrowserException { - serverSocket = getServerSocket(connectionParams.getSessionVars()); + portFromUrl = Integer.parseInt(connectionParams.getSessionVars() + .getOrDefault(JdbcConnectionParams.AUTH_BROWSER_RESPONSE_PORT, "0")); + timeoutInMs = Integer.parseInt( + connectionParams.getSessionVars() + .getOrDefault(JdbcConnectionParams.AUTH_BROWSER_RESPONSE_TIMEOUT_SECS, + String.valueOf(DEFAULT_SOCKET_TIMEOUT_SECS))) * 1000L; } - private ServerSocket getServerSocket(Map<String, String> sessionConf) - throws HiveJdbcBrowserException { - final ServerSocket serverSocket; - int port = Integer.parseInt(sessionConf - .getOrDefault(JdbcConnectionParams.AUTH_BROWSER_RESPONSE_PORT, "0")); - int timeout = Integer.parseInt( - sessionConf.getOrDefault(JdbcConnectionParams.AUTH_BROWSER_RESPONSE_TIMEOUT_SECS, - String.valueOf(DEFAULT_SOCKET_TIMEOUT_SECS))); + @Override + public void startListening() throws HiveJdbcBrowserException { + webServer = new Server(); + ServerConnector serverConnector = new ServerConnector(webServer); + serverConnector.setHost(HiveSamlUtils.LOOP_BACK_INTERFACE); + serverConnector.setPort(portFromUrl); + LOG.info("Browser response timeout is set to {} ms", timeoutInMs); + serverConnector.setIdleTimeout(timeoutInMs); + webServer.addConnector(serverConnector); + ServletHandler servletHandler = new ServletHandler(); + servletHandler.addServletWithMapping( + new ServletHolder(new HttpBrowserClientServlet(this)), "/"); + webServer.setHandler(servletHandler); + webServer.setStopTimeout(30*1000L); try { - serverSocket = new ServerSocket(port, 0, - InetAddress.getByName(HiveSamlUtils.LOOP_BACK_INTERFACE)); - LOG.debug("Browser response timeout is set to {} seconds", timeout); - serverSocket.setSoTimeout(timeout * 1000); - } catch (IOException e) { - throw new HiveJdbcBrowserException("Unable to bind to the localhost"); + webServer.start(); + // we fetch the port after the server is started so that we can get the port + // where the server has bound in case there is no port specified from the URL. + serverPort = ((ServerConnector) webServer.getConnectors()[0]).getLocalPort(); + LOG.debug("Listening on the port {} ", serverPort); + } catch (Exception e) { + throw new HiveJdbcBrowserException("Could not start http server", e); } - return serverSocket; } public Integer getPort() { - return serverSocket.getLocalPort(); + return serverPort; + } + + @Override + public String toString() { + return "HiveJdbcBrowserClient@" + serverPort; + } + + @Override + public HiveJdbcBrowserServerResponse getServerResponse() { + return serverResponse; } @Override public void close() throws IOException { - if (serverSocket != null) { - serverSocket.close(); + if (webServer != null && webServer.isRunning()) { + try { + webServer.stop(); + } catch (Exception e) { + throw new IOException(e); + } + webServer = null; } } @@ -111,7 +139,7 @@ public class HiveJdbcBrowserClient implements IJdbcBrowserClient { // expired reset(); this.clientContext = clientContext; - LOG.trace("Initialized the JDBCBrowser client with URL {}", + LOG.debug("Initialized the JDBCBrowser client with URL {}", clientContext.getSsoUri()); } @@ -121,14 +149,20 @@ public class HiveJdbcBrowserClient implements IJdbcBrowserClient { } public void doBrowserSSO() throws HiveJdbcBrowserException { - Future<Void> serverResponseHandle = waitAsyncForServerResponse(); logDebugInfoUri(clientContext.getSsoUri()); openBrowserWindow(); try { - serverResponseHandle.get(); - } catch (InterruptedException | ExecutionException e) { + waitForServerResponse(timeoutInMs); + } catch (InterruptedException e) { throw new HiveJdbcBrowserException(e); } + if (serverResponse == null) { + throw new HiveJdbcBrowserException(TIMEOUT_ERROR_MSG); + } + if (!serverResponse.isValid()) { + throw new HiveJdbcBrowserException( + "Received invalid response from server. See driver logs for more details"); + } } private void logDebugInfoUri(URI ssoURI) { @@ -136,7 +170,7 @@ public class HiveJdbcBrowserClient implements IJdbcBrowserClient { try { uriParams = getQueryParams(ssoURI); } catch (HiveJdbcBrowserException e) { - LOG.debug("Could get query params of the SSO URI", e); + LOG.info("Could get query params of the SSO URI", e); } LOG.debug("Initializing browser SSO request to URI. Relay state is {}", uriParams.get("RelayState")); @@ -175,7 +209,7 @@ public class HiveJdbcBrowserClient implements IJdbcBrowserClient { .isSupported(Action.BROWSE)) { Desktop.getDesktop().browse(ssoUri); } else { - LOG.debug( + LOG.info( "Desktop mode is not supported. Attempting to use OS " + "commands to open the default browser"); //Desktop is not supported, lets try to open the browser process @@ -201,52 +235,17 @@ public class HiveJdbcBrowserClient implements IJdbcBrowserClient { } } - private Future<Void> waitAsyncForServerResponse() { - return serverResponseThread.submit(() -> { - // listen to the response on the server socket - Socket socket; - try { - LOG.debug("Waiting for a server response on port {} with a timeout of {} ms", - serverSocket.getLocalPort(), serverSocket.getSoTimeout()); - socket = serverSocket.accept(); - } catch (SocketTimeoutException timeoutException) { - throw new HiveJdbcBrowserException(TIMEOUT_ERROR_MSG, - timeoutException); - } catch (IOException e) { - throw new HiveJdbcBrowserException( - "Unexpected error while listening on port " + serverSocket.getLocalPort() - + " for server response", e); - } - try (BufferedReader reader = new BufferedReader(new InputStreamReader( - socket.getInputStream(), StandardCharsets.UTF_8))) { - char[] buffer = new char[16 * 1024]; - // block until you read into the buffer - int len = reader.read(buffer); - String response = String.valueOf(buffer, 0, len); - String[] lines = response.split("\r\n"); - for (String line : lines) { - if (!Strings.isNullOrEmpty(line)) { - if (line.contains("token=")) { - serverResponse = new HiveJdbcBrowserServerResponse(line); - sendBrowserMsg(socket, serverResponse.isSuccessful()); - } else { - LOG.trace("Skipping line {} from server response", line); - } - } - } - if (serverResponse == null) { - throw new HiveJdbcBrowserException("Could not parse the response from server."); - } - } catch (IOException e) { - throw new HiveJdbcBrowserException( - "Unexpected exception while processing server response ", e); - } - return null; - }); + public void addServerResponse(HiveJdbcBrowserServerResponse response) { + serverResponseQueue.add(response); } - public HiveJdbcBrowserServerResponse getServerResponse() { - return serverResponse; + /** + * Waits for a server response until the given timeout value in milliseconds. + * Returns null if timeout. + */ + private void waitForServerResponse(long timeoutInMs) + throws InterruptedException { + serverResponse = serverResponseQueue.poll(timeoutInMs, TimeUnit.MILLISECONDS); } public String getClientIdentifier() { diff --git a/jdbc/src/java/org/apache/hive/jdbc/saml/HttpBrowserClientServlet.java b/jdbc/src/java/org/apache/hive/jdbc/saml/HttpBrowserClientServlet.java new file mode 100644 index 0000000..f83fd95 --- /dev/null +++ b/jdbc/src/java/org/apache/hive/jdbc/saml/HttpBrowserClientServlet.java @@ -0,0 +1,62 @@ +/* + * 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.hive.jdbc.saml; + +import java.io.IOException; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.apache.hive.jdbc.saml.IJdbcBrowserClient.HiveJdbcBrowserServerResponse; +import org.apache.hive.service.auth.saml.HiveSamlUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class HttpBrowserClientServlet extends HttpServlet { + private final HiveJdbcBrowserClient browserClient; + private static final Logger LOG = LoggerFactory.getLogger( + HttpBrowserClientServlet.class); + HttpBrowserClientServlet(HiveJdbcBrowserClient browserClient) { + this.browserClient = browserClient; + } + + @Override + protected void doPost( + HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + LOG.info("Received request from {}:{} for browserClient at port {}", + req.getRemoteAddr(), req.getRemotePort(), browserClient.toString()); + resp.setContentType("text/html"); + resp.setStatus(HttpServletResponse.SC_OK); + String token = req.getParameter(HiveSamlUtils.TOKEN_KEY); + String msg = req.getParameter(HiveSamlUtils.MESSAGE_KEY); + boolean status = Boolean.parseBoolean(req.getParameter(HiveSamlUtils.STATUS_KEY)); + HiveJdbcBrowserServerResponse response = new HiveJdbcBrowserServerResponse(status, + msg, token); + if (response.isSuccessful()) { + resp.getWriter().write("Successfully authenticated. You may close this window."); + } else { + resp.getWriter().write( + "Authentication failed. Please check server logs for details. " + + "You may close this window."); + } + resp.getWriter().flush(); + browserClient.addServerResponse(response); + } +} diff --git a/jdbc/src/java/org/apache/hive/jdbc/saml/HttpSamlAuthRequestInterceptor.java b/jdbc/src/java/org/apache/hive/jdbc/saml/HttpSamlAuthRequestInterceptor.java index 9ada4e5..ad3d89d 100644 --- a/jdbc/src/java/org/apache/hive/jdbc/saml/HttpSamlAuthRequestInterceptor.java +++ b/jdbc/src/java/org/apache/hive/jdbc/saml/HttpSamlAuthRequestInterceptor.java @@ -26,6 +26,8 @@ import org.apache.http.HttpHeaders; import org.apache.http.HttpRequest; import org.apache.http.client.CookieStore; import org.apache.http.protocol.HttpContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * This implements the logic to intercept the HTTP requests from the Hive Jdbc connection @@ -36,6 +38,8 @@ public class HttpSamlAuthRequestInterceptor extends HttpRequestInterceptorBase { private final IJdbcBrowserClient browserClient; private static final String BEARER = "Bearer "; + private static final Logger LOG = LoggerFactory + .getLogger(HttpSamlAuthRequestInterceptor.class); public HttpSamlAuthRequestInterceptor(IJdbcBrowserClient browserClient, CookieStore cookieStore, String cn, boolean isSSL, Map<String, String> additionalHeaders, @@ -56,6 +60,7 @@ public class HttpSamlAuthRequestInterceptor extends HttpRequestInterceptorBase { httpRequest.addHeader(HiveSamlUtils.SSO_CLIENT_IDENTIFIER, clientIdentifier); httpRequest.removeHeaders(HiveSamlUtils.SSO_TOKEN_RESPONSE_PORT); } else { + LOG.debug("Adding response port {}", port); httpRequest.addHeader(HiveSamlUtils.SSO_TOKEN_RESPONSE_PORT, port); } } diff --git a/jdbc/src/java/org/apache/hive/jdbc/saml/IJdbcBrowserClient.java b/jdbc/src/java/org/apache/hive/jdbc/saml/IJdbcBrowserClient.java index c35aeca..6bed420 100644 --- a/jdbc/src/java/org/apache/hive/jdbc/saml/IJdbcBrowserClient.java +++ b/jdbc/src/java/org/apache/hive/jdbc/saml/IJdbcBrowserClient.java @@ -36,6 +36,12 @@ import org.apache.hive.service.auth.saml.HiveSamlUtils; public interface IJdbcBrowserClient extends Closeable { /** + * Start a webserver and bind to a port number as configued. + * @throws HiveJdbcBrowserException + */ + void startListening() throws HiveJdbcBrowserException; + + /** * Execute the browser actions to complete the SSO workflow. This method assumes * that the {@link #init(JdbcBrowserClientContext)} method has been called already * to initialize the state needed for doing the browser based flow. @@ -111,30 +117,10 @@ public interface IJdbcBrowserClient extends Closeable { private final boolean status; private final String token; - public HiveJdbcBrowserServerResponse(String postResponse) { - Map<String, String> params = parseUrlEncodedFormData(postResponse); - status = Boolean.parseBoolean(params.get(HiveSamlUtils.STATUS_KEY)); - msg = params.getOrDefault(HiveSamlUtils.MESSAGE_KEY, ""); - token = params.get(HiveSamlUtils.TOKEN_KEY); - } - - - private Map<String, String> parseUrlEncodedFormData(String line) { - String decoded; - try { - decoded = URLDecoder.decode(line, StandardCharsets.UTF_8.toString()); - } catch (UnsupportedEncodingException e) { - throw new RuntimeException(e); - } - Map<String, String> ret = new HashMap<>(); - for (String params : decoded.split("&")) { - if (params.contains("=")) { - String key = params.substring(0, params.indexOf("=")); - String val = params.substring(params.indexOf("=") + 1); - ret.put(key, val); - } - } - return ret; + public HiveJdbcBrowserServerResponse(boolean status, String msg, String token) { + this.status = status; + this.msg = msg; + this.token = token; } public String getMsg() { @@ -148,5 +134,13 @@ public interface IJdbcBrowserClient extends Closeable { public String getToken() { return token; } + + /** + * A response is valid if the status is true with a non-empty token or + * if status is false + */ + public boolean isValid() { + return !status || (token != null && !token.isEmpty()); + } } } diff --git a/service/src/java/org/apache/hive/service/auth/saml/HiveSamlHttpServlet.java b/service/src/java/org/apache/hive/service/auth/saml/HiveSamlHttpServlet.java index a206e91..4527ed5 100644 --- a/service/src/java/org/apache/hive/service/auth/saml/HiveSamlHttpServlet.java +++ b/service/src/java/org/apache/hive/service/auth/saml/HiveSamlHttpServlet.java @@ -35,7 +35,6 @@ public class HiveSamlHttpServlet extends HttpServlet { .getLogger(HiveSamlHttpServlet.class); private final HiveConf conf; private final ISAMLAuthTokenGenerator tokenGenerator; - private static final String LOOP_BACK_INTERFACE = "127.0.0.1"; public HiveSamlHttpServlet(HiveConf conf) { this.conf = Preconditions.checkNotNull(conf); @@ -56,7 +55,8 @@ public class HiveSamlHttpServlet extends HttpServlet { return; } try { - LOG.info("RelayState = {}. Driver side port on localhost = {}", relayState, port); + LOG.info("RelayState = {}. Driver side port on loopback address is {}", relayState, + port); nameId = HiveSaml2Client.get(conf).validate(request, response); } catch (HttpSamlAuthenticationException e) { if (e instanceof HttpSamlNoGroupsMatchedException) { @@ -77,7 +77,7 @@ public class HiveSamlHttpServlet extends HttpServlet { } private void generateFormData(HttpServletResponse response, String url, String token, - boolean sucess, String msg) { + boolean success, String msg) { StringBuilder sb = new StringBuilder(); sb.append("<html>"); sb.append("<body onload='document.forms[\"form\"].submit()'>"); @@ -86,15 +86,17 @@ public class HiveSamlHttpServlet extends HttpServlet { .format("<input type='hidden' name='%s' value='%s'>", HiveSamlUtils.TOKEN_KEY, token)); sb.append(String.format("<input type='hidden' name='%s' value='%s'>", - HiveSamlUtils.STATUS_KEY, sucess)); + HiveSamlUtils.STATUS_KEY, success)); sb.append(String .format("<input type='hidden' name='%s' value='%s'>", HiveSamlUtils.MESSAGE_KEY, msg)); sb.append("</form>"); sb.append("</body>"); sb.append("</html>"); - try (PrintWriter write = response.getWriter()) { - write.write(sb.toString()); + response.setContentType("text/html"); + try { + response.getWriter().write(sb.toString()); + response.getWriter().flush(); } catch (IOException e) { LOG.error("Could not generate the form data for sending a response to url " + url, e);