Apache9 commented on code in PR #6601:
URL: https://github.com/apache/hbase/pull/6601#discussion_r1938280564
##########
hbase-client/src/test/java/org/apache/hadoop/hbase/security/TestSaslUtil.java:
##########
@@ -61,4 +62,118 @@ public void testInitSaslProperties() {
props = SaslUtil.initSaslProperties("");
assertEquals("auth", props.get(Sasl.QOP));
}
+
+ @Test
+ public void testVerifyQop() {
+ String nullQop = null;
+ String authentication = "auth";
+ String integrity = "auth-int";
+ String confidentality = "auth-conf";
+ String anyQop = "auth-conf,auth-int,auth";
+
+ // Empty requested, got empty
+ try {
+ SaslUtil.verifyNegotiatedQop(nullQop, nullQop);
+ } catch (Exception e) {
+ fail("Should not have thrown exception");
+ }
+
+ // Auth requested, got null
+ try {
+ SaslUtil.verifyNegotiatedQop(authentication, nullQop);
+ } catch (Exception e) {
+ fail("Should not have thrown exception");
+ }
+
+ // Auth requested, got auth
+ try {
+ SaslUtil.verifyNegotiatedQop(authentication, authentication);
+ } catch (Exception e) {
+ fail("Should not have thrown exception");
+ }
+
+ // Auth requested, got confidentality.
+ try {
+ SaslUtil.verifyNegotiatedQop(authentication, confidentality);
+ fail("Should have thrown exception");
Review Comment:
Use assertThrows
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/SimpleServerRpcConnection.java:
##########
@@ -252,13 +253,14 @@ private void saslReadAndProcess(ByteBuff saslToken)
throws IOException, Interrup
doRawSaslReply(SaslStatus.SUCCESS, new BytesWritable(replyToken),
null, null);
}
if (saslServer.isComplete()) {
- String qop = saslServer.getNegotiatedQop();
- useWrap = qop != null && !"auth".equalsIgnoreCase(qop);
+ String negotiatedQop = saslServer.getNegotiatedQop();
Review Comment:
So in the old code, SimpleServerRpcConnection does not call
finishSaslNegotiation? Seems a bit strange...
##########
hbase-client/src/test/java/org/apache/hadoop/hbase/security/TestSaslUtil.java:
##########
@@ -61,4 +62,118 @@ public void testInitSaslProperties() {
props = SaslUtil.initSaslProperties("");
assertEquals("auth", props.get(Sasl.QOP));
}
+
+ @Test
+ public void testVerifyQop() {
+ String nullQop = null;
+ String authentication = "auth";
+ String integrity = "auth-int";
+ String confidentality = "auth-conf";
+ String anyQop = "auth-conf,auth-int,auth";
+
+ // Empty requested, got empty
+ try {
+ SaslUtil.verifyNegotiatedQop(nullQop, nullQop);
+ } catch (Exception e) {
+ fail("Should not have thrown exception");
Review Comment:
Just let the exception be thrown? In this way we could see more detailed
failure message...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
