From 37a13a9158347e3b0b866092a0331c223661202b Mon Sep 17 00:00:00 2001
From: zhao liwei <zlw5307@163.com>
Date: Fri, 25 Dec 2020 15:51:02 +0800
Subject: [PATCH] refactor(security): remove mandatory on client side (#708)

---
 src/runtime/security/client_negotiation.cpp   |  6 +++---
 src/runtime/security/negotiation_manager.cpp  |  3 +--
 src/runtime/test/client_negotiation_test.cpp  | 20 +++++--------------
 src/runtime/test/negotiation_manager_test.cpp | 19 ++++++++----------
 4 files changed, 17 insertions(+), 31 deletions(-)

diff --git a/src/runtime/security/client_negotiation.cpp b/src/runtime/security/client_negotiation.cpp
index 7aef0359c7..8cfc21434f 100644
--- a/src/runtime/security/client_negotiation.cpp
+++ b/src/runtime/security/client_negotiation.cpp
@@ -51,7 +51,7 @@ void client_negotiation::handle_response(error_code err, const negotiation_respo
 {
     if (err != ERR_OK) {
         // ERR_HANDLER_NOT_FOUND means server is old version, which doesn't support authentication
-        if (ERR_HANDLER_NOT_FOUND == err && !FLAGS_mandatory_auth) {
+        if (ERR_HANDLER_NOT_FOUND == err) {
             ddebug_f("{}: treat negotiation succeed because server is old version, which doesn't "
                      "support authentication",
                      _name);
@@ -62,8 +62,8 @@ void client_negotiation::handle_response(error_code err, const negotiation_respo
         return;
     }
 
-    // make the negotiation succeed if server doesn't enable auth and the auth is not mandantory
-    if (negotiation_status::type::SASL_AUTH_DISABLE == response.status && !FLAGS_mandatory_auth) {
+    // make the negotiation succeed if server doesn't enable auth
+    if (negotiation_status::type::SASL_AUTH_DISABLE == response.status) {
         ddebug_f("{}: treat negotiation succeed as server doesn't enable it", _name);
         succ_negotiation();
         return;
diff --git a/src/runtime/security/negotiation_manager.cpp b/src/runtime/security/negotiation_manager.cpp
index 7a086dcf2f..67582061d8 100644
--- a/src/runtime/security/negotiation_manager.cpp
+++ b/src/runtime/security/negotiation_manager.cpp
@@ -110,8 +110,7 @@ bool negotiation_manager::on_rpc_recv_msg(message_ex *msg)
 bool negotiation_manager::on_rpc_send_msg(message_ex *msg)
 {
     // if try_pend_message return true, it means the msg is pended to the resend message queue
-    return !FLAGS_mandatory_auth || in_white_list(msg->rpc_code()) ||
-           !msg->io_session->try_pend_message(msg);
+    return in_white_list(msg->rpc_code()) || !msg->io_session->try_pend_message(msg);
 }
 
 std::shared_ptr<negotiation> negotiation_manager::get_negotiation(negotiation_rpc rpc)
diff --git a/src/runtime/test/client_negotiation_test.cpp b/src/runtime/test/client_negotiation_test.cpp
index 5be170d56d..e60c2d66f2 100644
--- a/src/runtime/test/client_negotiation_test.cpp
+++ b/src/runtime/test/client_negotiation_test.cpp
@@ -95,26 +95,16 @@ TEST_F(client_negotiation_test, handle_response)
     {
         error_code resp_err;
         negotiation_status::type resp_status;
-        bool mandatory_auth;
         negotiation_status::type neg_status;
-    } tests[] = {{ERR_TIMEOUT,
-                  negotiation_status::type::SASL_SELECT_MECHANISMS,
-                  false,
-                  negotiation_status::type::SASL_AUTH_FAIL},
-                 {ERR_OK,
-                  negotiation_status::type::SASL_AUTH_DISABLE,
-                  true,
-                  negotiation_status::type::SASL_AUTH_FAIL},
-                 {ERR_OK,
-                  negotiation_status::type::SASL_AUTH_DISABLE,
-                  false,
-                  negotiation_status::type::SASL_SUCC}};
+    } tests[] = {
+        {ERR_TIMEOUT,
+         negotiation_status::type::SASL_SELECT_MECHANISMS,
+         negotiation_status::type::SASL_AUTH_FAIL},
+        {ERR_OK, negotiation_status::type::SASL_AUTH_DISABLE, negotiation_status::type::SASL_SUCC}};
 
-    DSN_DECLARE_bool(mandatory_auth);
     for (const auto &test : tests) {
         negotiation_response resp;
         resp.status = test.resp_status;
-        FLAGS_mandatory_auth = test.mandatory_auth;
         handle_response(test.resp_err, resp);
 
         ASSERT_EQ(get_negotiation_status(), test.neg_status);
diff --git a/src/runtime/test/negotiation_manager_test.cpp b/src/runtime/test/negotiation_manager_test.cpp
index 49646a2c0a..1d888d63af 100644
--- a/src/runtime/test/negotiation_manager_test.cpp
+++ b/src/runtime/test/negotiation_manager_test.cpp
@@ -116,20 +116,17 @@ TEST_F(negotiation_manager_test, on_rpc_send_msg)
     {
         task_code rpc_code;
         bool negotiation_succeed;
-        bool mandatory_auth;
         bool return_value;
-    } tests[] = {{RPC_NEGOTIATION, false, true, true},
-                 {RPC_NEGOTIATION_ACK, false, true, true},
-                 {fd::RPC_FD_FAILURE_DETECTOR_PING, false, true, true},
-                 {fd::RPC_FD_FAILURE_DETECTOR_PING_ACK, false, true, true},
-                 {RPC_HTTP_SERVICE, false, true, true},
-                 {RPC_HTTP_SERVICE_ACK, false, true, true},
-                 {service::RPC_NFS_COPY, true, true, true},
-                 {service::RPC_NFS_COPY, false, false, true},
-                 {service::RPC_NFS_COPY, false, true, false}};
+    } tests[] = {{RPC_NEGOTIATION, false, true},
+                 {RPC_NEGOTIATION_ACK, false, true},
+                 {fd::RPC_FD_FAILURE_DETECTOR_PING, false, true},
+                 {fd::RPC_FD_FAILURE_DETECTOR_PING_ACK, false, true},
+                 {RPC_HTTP_SERVICE, false, true},
+                 {RPC_HTTP_SERVICE_ACK, false, true},
+                 {service::RPC_NFS_COPY, true, true},
+                 {service::RPC_NFS_COPY, false, false}};
 
     for (const auto &test : tests) {
-        FLAGS_mandatory_auth = test.mandatory_auth;
         message_ptr msg = dsn::message_ex::create_request(test.rpc_code, 0, 0);
         auto sim_session = create_fake_session();
         msg->io_session = sim_session;