diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/NettyHBaseSaslRpcClientHandler.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/NettyHBaseSaslRpcClientHandler.java index 48e631c76299..cc71355d4297 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/NettyHBaseSaslRpcClientHandler.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/NettyHBaseSaslRpcClientHandler.java @@ -145,6 +145,12 @@ public byte[] run() throws Exception { // Mechanisms which have multiple steps will not return true on `SaslClient#isComplete()` // until the handshake has fully completed. Mechanisms which only send a single buffer may // return true on `isComplete()` after that initial response is calculated. + + // HBASE-28337 We still want to check if the SaslClient completed the handshake, because + // there are certain mechs like PLAIN which doesn't have a server response after the + // initial authentication request. We cannot remove this tryComplete(), otherwise mechs + // like PLAIN will fail with call timeout. + tryComplete(ctx); } catch (Exception e) { // the exception thrown by handlerAdded will not be passed to the exceptionCaught below // because netty will remove a handler if handlerAdded throws an exception. diff --git a/hbase-examples/src/test/java/org/apache/hadoop/hbase/security/provider/example/TestShadeSaslAuthenticationProvider.java b/hbase-examples/src/test/java/org/apache/hadoop/hbase/security/provider/example/TestShadeSaslAuthenticationProvider.java index 4ee753b1d26e..1d0bb40b4406 100644 --- a/hbase-examples/src/test/java/org/apache/hadoop/hbase/security/provider/example/TestShadeSaslAuthenticationProvider.java +++ b/hbase-examples/src/test/java/org/apache/hadoop/hbase/security/provider/example/TestShadeSaslAuthenticationProvider.java @@ -27,8 +27,6 @@ import java.io.File; import java.io.IOException; import java.io.OutputStreamWriter; -import java.io.PrintWriter; -import java.io.StringWriter; import java.nio.charset.StandardCharsets; import java.security.PrivilegedExceptionAction; import java.util.ArrayList; @@ -67,10 +65,8 @@ import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.CommonFSUtils; import org.apache.hadoop.hbase.util.Pair; -import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.minikdc.MiniKdc; import org.apache.hadoop.security.UserGroupInformation; -import org.apache.hadoop.security.token.SecretManager.InvalidToken; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -82,8 +78,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.hbase.thirdparty.com.google.common.base.Throwables; - @Category({ MediumTests.class, SecurityTests.class }) public class TestShadeSaslAuthenticationProvider { private static final Logger LOG = @@ -210,21 +204,23 @@ public String run() throws Exception { @Test public void testPositiveAuthentication() throws Exception { final Configuration clientConf = new Configuration(CONF); - try (Connection conn = ConnectionFactory.createConnection(clientConf)) { + try (Connection conn1 = ConnectionFactory.createConnection(clientConf)) { UserGroupInformation user1 = UserGroupInformation.createUserForTesting("user1", new String[0]); - user1.addToken(ShadeClientTokenUtil.obtainToken(conn, "user1", USER1_PASSWORD)); + user1.addToken(ShadeClientTokenUtil.obtainToken(conn1, "user1", USER1_PASSWORD)); user1.doAs(new PrivilegedExceptionAction() { @Override public Void run() throws Exception { - try (Table t = conn.getTable(tableName)) { - Result r = t.get(new Get(Bytes.toBytes("r1"))); - assertNotNull(r); - assertFalse("Should have read a non-empty Result", r.isEmpty()); - final Cell cell = r.getColumnLatestCell(Bytes.toBytes("f1"), Bytes.toBytes("q1")); - assertTrue("Unexpected value", CellUtil.matchingValue(cell, Bytes.toBytes("1"))); + try (Connection conn = ConnectionFactory.createConnection(clientConf)) { + try (Table t = conn.getTable(tableName)) { + Result r = t.get(new Get(Bytes.toBytes("r1"))); + assertNotNull(r); + assertFalse("Should have read a non-empty Result", r.isEmpty()); + final Cell cell = r.getColumnLatestCell(Bytes.toBytes("f1"), Bytes.toBytes("q1")); + assertTrue("Unexpected value", CellUtil.matchingValue(cell, Bytes.toBytes("1"))); - return null; + return null; + } } } }); @@ -262,7 +258,6 @@ public Void run() throws Exception { } catch (Exception e) { LOG.info("Caught exception in negative Master connectivity test", e); assertEquals("Found unexpected exception", pair.getSecond(), e.getClass()); - validateRootCause(Throwables.getRootCause(e)); } return null; } @@ -279,7 +274,6 @@ public Void run() throws Exception { } catch (Exception e) { LOG.info("Caught exception in negative RegionServer connectivity test", e); assertEquals("Found unexpected exception", pair.getSecond(), e.getClass()); - validateRootCause(Throwables.getRootCause(e)); } return null; } @@ -293,19 +287,4 @@ public Void run() throws Exception { } }); } - - void validateRootCause(Throwable rootCause) { - LOG.info("Root cause was", rootCause); - if (rootCause instanceof RemoteException) { - RemoteException re = (RemoteException) rootCause; - IOException actualException = re.unwrapRemoteException(); - assertEquals(InvalidToken.class, actualException.getClass()); - } else { - StringWriter writer = new StringWriter(); - rootCause.printStackTrace(new PrintWriter(writer)); - String text = writer.toString(); - assertTrue("Message did not contain expected text", - text.contains(InvalidToken.class.getName())); - } - } }