HBASE-28337 Positive connection test in TestShadeSaslAuthenticationProvider runs with Kerberos instead of Shade authentication (#5659)

Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

Author: anmolnar

hbase-client/src/main/java/org/apache/hadoop/hbase/security/NettyHBaseSaslRpcClientHandler.java

@@ -130,6 +130,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.

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<Void>() {
         @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()));
-    }
-  }
 }