You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "WHBANG (via GitHub)" <gi...@apache.org> on 2023/04/12 02:54:18 UTC

[GitHub] [incubator-pegasus] WHBANG opened a new pull request, #1433: feat(Ranger): Use Apache Ranger for meta access controller

WHBANG opened a new pull request, #1433:
URL: https://github.com/apache/incubator-pegasus/pull/1433

   ### What problem does this PR solve? <!--add issue link with summary if exists-->
   https://github.com/apache/incubator-pegasus/issues/1054
   
   This patch implements replica access controller using Ranger for ACL.
   
   1. The Ranger policy info of the table is written to the app_envs of the table.
   2. Supported using the policy in the app_envs for ACL when the replica server reads and writes.
   3. Modified some unit tests, and compatible with old and new ACL, some todo done. 
   


-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1433: feat(Ranger): Use Apache Ranger for replica access controller

Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1433:
URL: https://github.com/apache/incubator-pegasus/pull/1433#discussion_r1166797639


##########
src/runtime/security/replica_access_controller.h:
##########
@@ -30,14 +32,39 @@ namespace security {
 class replica_access_controller : public access_controller
 {
 public:
-    explicit replica_access_controller(const std::string &name);
-    bool allowed(message_ex *msg) override;
+    explicit replica_access_controller(const std::string &replica_name);
+
+    // Check whether replica can be accessed, this method is compatible with ACL using
+    // '_allowed_users' and ACL using Ranger policy.
+    bool allowed(message_ex *msg, ranger::access_type req_type) override;
+
+    // Update '_allowed_users' when the app_env(REPLICA_ACCESS_CONTROLLER_ALLOWED_USERS) of the
+    // table changes
     void update_allowed_users(const std::string &users) override;
 
+    // Update '_ranger_policies' when the app_env(REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES) of the
+    // table changes
+    void update_ranger_policies(const std::string &policies) override;
+
+private:
+    // Security check to avoid allowed_users is not empty in special scenarios.
+    void check_allowed_users_valid();
+
 private:
     utils::rw_lock_nr _lock; // [

Review Comment:
   ```suggestion
       mutable utils::rw_lock_nr _lock;
   ```



##########
src/replica/replica.cpp:
##########
@@ -599,5 +599,11 @@ error_code replica::store_app_info(app_info &info, const std::string &path)
     return err;
 }
 
+bool replica::access_controller_allowed(message_ex *msg, ranger::access_type req_type)

Review Comment:
   Declared as `const` ?



##########
src/runtime/security/replica_access_controller.cpp:
##########
@@ -62,10 +88,37 @@ void replica_access_controller::update_allowed_users(const std::string &users)
     utils::split_args(users.c_str(), users_set, ',');
     {
         utils::auto_write_lock l(_lock);
-        // This swap operation is in constant time
-        _users.swap(users_set);
+        _allowed_users.swap(users_set);
         _env_users = users;
+        check_allowed_users_valid();
+    }
+}
+
+void replica_access_controller::update_ranger_policies(const std::string &policies)
+{
+    {
+        utils::auto_read_lock l(_lock);
+        if (_env_policies == policies) {
+            return;
+        }
+    }
+    ranger::acl_policies tmp_policies;
+    std::string tmp_policies_str = policies;

Review Comment:
   ```suggestion
       auto tmp_policies_str = policies;
   ```



##########
src/runtime/security/access_controller.cpp:
##########
@@ -46,14 +46,6 @@ access_controller::access_controller()
 
 access_controller::~access_controller() {}
 
-bool access_controller::pre_check(const std::string &user_name)
-{
-    if (!FLAGS_enable_acl || _super_users.find(user_name) != _super_users.end()) {
-        return true;
-    }
-    return false;
-}
-
 bool access_controller::is_enable_ranger_acl() { return FLAGS_enable_ranger_acl; }

Review Comment:
   ```suggestion
   bool access_controller::is_enable_ranger_acl() const { return FLAGS_enable_ranger_acl; }
   ```



##########
src/runtime/security/replica_access_controller.cpp:
##########
@@ -15,36 +15,61 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include "replica_access_controller.h"
+#include <utility>
+
+// Disable class-memaccess warning to facilitate compilation with gcc>7
+// https://github.com/Tencent/rapidjson/issues/1700
+#pragma GCC diagnostic push
+#if defined(__GNUC__) && __GNUC__ >= 8
+#pragma GCC diagnostic ignored "-Wclass-memaccess"
+#endif
+#include "common/json_helper.h"
+
+#pragma GCC diagnostic pop
 
+#include "replica_access_controller.h"
 #include "runtime/rpc/network.h"
 #include "runtime/rpc/rpc_message.h"
 #include "utils/autoref_ptr.h"
+#include "utils/blob.h"
+#include "utils/flags.h"
 #include "utils/fmt_logging.h"
 #include "utils/strings.h"
 
 namespace dsn {
 namespace security {
-replica_access_controller::replica_access_controller(const std::string &name) { _name = name; }
+DSN_DECLARE_bool(enable_acl);
+DSN_DECLARE_bool(enable_ranger_acl);
+replica_access_controller::replica_access_controller(const std::string &replica_name)
+{
+    _name = replica_name;
+}
 
-bool replica_access_controller::allowed(message_ex *msg)
+bool replica_access_controller::allowed(message_ex *msg, ranger::access_type req_type)

Review Comment:
   Could this function be declared as `const` ? Could use `unordered_set::count` instead of `unordered_set::find`.



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1433: feat(Ranger): Use Apache Ranger for replica access controller

Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1433:
URL: https://github.com/apache/incubator-pegasus/pull/1433#discussion_r1165679426


##########
src/runtime/security/meta_access_controller.cpp:
##########
@@ -65,14 +65,10 @@ meta_access_controller::meta_access_controller(
                                         "RPC_CM_NOTIFY_STOP_SPLIT",
                                         "RPC_CM_QUERY_CHILD_STATE",
                                         "RPC_NEGOTIATION",
-                                        "RPC_CALL_RAW_MESSAGE",
                                         "RPC_CALL_RAW_SESSION_DISCONNECT",
-                                        "RPC_NFS_GET_FILE_SIZE",

Review Comment:
   How about this one? It's not duplicated.



##########
src/runtime/security/replica_access_controller.cpp:
##########
@@ -15,36 +15,61 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include "replica_access_controller.h"
+#include <utility>
+
+// Disable class-memaccess warning to facilitate compilation with gcc>7
+// https://github.com/Tencent/rapidjson/issues/1700
+#pragma GCC diagnostic push
+#if defined(__GNUC__) && __GNUC__ >= 8
+#pragma GCC diagnostic ignored "-Wclass-memaccess"
+#endif
+#include "common/json_helper.h"
+
+#pragma GCC diagnostic pop
 
+#include "replica_access_controller.h"
 #include "runtime/rpc/network.h"
 #include "runtime/rpc/rpc_message.h"
 #include "utils/autoref_ptr.h"
+#include "utils/blob.h"
+#include "utils/flags.h"
 #include "utils/fmt_logging.h"
 #include "utils/strings.h"
 
 namespace dsn {
 namespace security {
-replica_access_controller::replica_access_controller(const std::string &name) { _name = name; }
+DSN_DECLARE_bool(enable_acl);
+DSN_DECLARE_bool(enable_ranger_acl);
+replica_access_controller::replica_access_controller(const std::string &replica_name)
+{
+    _name = replica_name;
+}
 
-bool replica_access_controller::allowed(message_ex *msg)
+bool replica_access_controller::allowed(message_ex *msg, ranger::access_type req_type)
 {
     const std::string &user_name = msg->io_session->get_client_username();
-    if (pre_check(user_name)) {
-        return true;
+    if (!FLAGS_enable_ranger_acl) {
+        if (!FLAGS_enable_acl || is_super_user(user_name)) {
+            return true;
+        }
+        {
+            utils::auto_read_lock l(_lock);
+            // If the user didn't specify any ACL, it means this table is publicly accessible to
+            // everyone. This is a backdoor to allow old-version clients to gracefully upgrade.After
+            // they are finally ensured to be fully upgraded, they can specify some usernames to ACL
+            // and the table will be truly protected.
+            if (!_allowed_users.empty() && _allowed_users.find(user_name) == _allowed_users.end()) {

Review Comment:
   I see.
   How about add some periodic warning logs if: old ACL is enabled && Ranger ACL is disabled && _allowed_users is empty?
   
   (For example replica::update_ac_allowed_users)



##########
src/runtime/security/meta_access_controller.cpp:
##########
@@ -65,14 +65,10 @@ meta_access_controller::meta_access_controller(
                                         "RPC_CM_NOTIFY_STOP_SPLIT",
                                         "RPC_CM_QUERY_CHILD_STATE",
                                         "RPC_NEGOTIATION",
-                                        "RPC_CALL_RAW_MESSAGE",

Review Comment:
   OK, let's adjust the rpc codes in lexicographical order to avoid these mistakes?



##########
src/runtime/security/replica_access_controller.cpp:
##########
@@ -62,10 +87,29 @@ void replica_access_controller::update_allowed_users(const std::string &users)
     utils::split_args(users.c_str(), users_set, ',');
     {
         utils::auto_write_lock l(_lock);
-        // This swap operation is in constant time
-        _users.swap(users_set);
+        _allowed_users.swap(users_set);
         _env_users = users;
     }
 }
+
+void replica_access_controller::start_to_dump_and_sync_policies(const std::string &policies)

Review Comment:
   According to the function logic, how about just rename to `update_policies` ?



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1433: feat(Ranger): Use Apache Ranger for replica access controller

Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1433:
URL: https://github.com/apache/incubator-pegasus/pull/1433#discussion_r1164939541


##########
src/replica/replica_stub.cpp:
##########
@@ -564,6 +566,7 @@ void replica_stub::initialize(bool clear /* = false*/)
     replication_options opts;
     opts.initialize();
     initialize(opts, clear);
+    _access_controller = dsn::make_unique<dsn::security::access_controller>();

Review Comment:
   Use std::make_unique



##########
src/runtime/security/replica_access_controller.cpp:
##########
@@ -15,36 +15,61 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include "replica_access_controller.h"
+#include <utility>
+
+// Disable class-memaccess warning to facilitate compilation with gcc>7
+// https://github.com/Tencent/rapidjson/issues/1700
+#pragma GCC diagnostic push
+#if defined(__GNUC__) && __GNUC__ >= 8
+#pragma GCC diagnostic ignored "-Wclass-memaccess"
+#endif
+#include "common/json_helper.h"
+
+#pragma GCC diagnostic pop
 
+#include "replica_access_controller.h"
 #include "runtime/rpc/network.h"
 #include "runtime/rpc/rpc_message.h"
 #include "utils/autoref_ptr.h"
+#include "utils/blob.h"
+#include "utils/flags.h"
 #include "utils/fmt_logging.h"
 #include "utils/strings.h"
 
 namespace dsn {
 namespace security {
-replica_access_controller::replica_access_controller(const std::string &name) { _name = name; }
+DSN_DECLARE_bool(enable_acl);
+DSN_DECLARE_bool(enable_ranger_acl);
+replica_access_controller::replica_access_controller(const std::string &replica_name)
+{
+    _name = replica_name;
+}
 
-bool replica_access_controller::allowed(message_ex *msg)
+bool replica_access_controller::allowed(message_ex *msg, ranger::access_type req_type)
 {
     const std::string &user_name = msg->io_session->get_client_username();
-    if (pre_check(user_name)) {
-        return true;
+    if (!FLAGS_enable_ranger_acl) {
+        if (!FLAGS_enable_acl || is_super_user(user_name)) {
+            return true;
+        }
+        {
+            utils::auto_read_lock l(_lock);
+            // If the user didn't specify any ACL, it means this table is publicly accessible to
+            // everyone. This is a backdoor to allow old-version clients to gracefully upgrade.After
+            // they are finally ensured to be fully upgraded, they can specify some usernames to ACL
+            // and the table will be truly protected.
+            if (!_allowed_users.empty() && _allowed_users.find(user_name) == _allowed_users.end()) {

Review Comment:
   ```suggestion
               if (_allowed_users.find(user_name) == _allowed_users.end()) {
   ```



##########
src/runtime/security/replica_access_controller.h:
##########
@@ -30,16 +32,19 @@ namespace security {
 class replica_access_controller : public access_controller
 {
 public:
-    explicit replica_access_controller(const std::string &name);
-    bool allowed(message_ex *msg) override;
+    explicit replica_access_controller(const std::string &replica_name);
+    bool allowed(message_ex *msg, ranger::access_type req_type) override;
     void update_allowed_users(const std::string &users) override;
+    void start_to_dump_and_sync_policies(const std::string &policies) override;
 
 private:
     utils::rw_lock_nr _lock; // [
-    std::unordered_set<std::string> _users;
+    std::unordered_set<std::string> _allowed_users;
     std::string _env_users;
     // ]
     std::string _name;
+    std::string _env_policies;

Review Comment:
   I guess it is used for fast judge to check if policies changed, could you add some necessary comments?



##########
src/runtime/security/replica_access_controller.cpp:
##########
@@ -15,36 +15,61 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include "replica_access_controller.h"
+#include <utility>
+
+// Disable class-memaccess warning to facilitate compilation with gcc>7
+// https://github.com/Tencent/rapidjson/issues/1700
+#pragma GCC diagnostic push
+#if defined(__GNUC__) && __GNUC__ >= 8
+#pragma GCC diagnostic ignored "-Wclass-memaccess"
+#endif
+#include "common/json_helper.h"
+
+#pragma GCC diagnostic pop
 
+#include "replica_access_controller.h"
 #include "runtime/rpc/network.h"
 #include "runtime/rpc/rpc_message.h"
 #include "utils/autoref_ptr.h"
+#include "utils/blob.h"
+#include "utils/flags.h"
 #include "utils/fmt_logging.h"
 #include "utils/strings.h"
 
 namespace dsn {
 namespace security {
-replica_access_controller::replica_access_controller(const std::string &name) { _name = name; }
+DSN_DECLARE_bool(enable_acl);
+DSN_DECLARE_bool(enable_ranger_acl);
+replica_access_controller::replica_access_controller(const std::string &replica_name)
+{
+    _name = replica_name;
+}
 
-bool replica_access_controller::allowed(message_ex *msg)
+bool replica_access_controller::allowed(message_ex *msg, ranger::access_type req_type)
 {
     const std::string &user_name = msg->io_session->get_client_username();
-    if (pre_check(user_name)) {
-        return true;
+    if (!FLAGS_enable_ranger_acl) {
+        if (!FLAGS_enable_acl || is_super_user(user_name)) {
+            return true;
+        }
+        {
+            utils::auto_read_lock l(_lock);
+            // If the user didn't specify any ACL, it means this table is publicly accessible to
+            // everyone. This is a backdoor to allow old-version clients to gracefully upgrade.After

Review Comment:
   ```suggestion
               // everyone. This is a backdoor to allow old-version clients to gracefully upgrade. After
   ```



##########
src/runtime/security/meta_access_controller.cpp:
##########
@@ -65,14 +65,10 @@ meta_access_controller::meta_access_controller(
                                         "RPC_CM_NOTIFY_STOP_SPLIT",
                                         "RPC_CM_QUERY_CHILD_STATE",
                                         "RPC_NEGOTIATION",
-                                        "RPC_CALL_RAW_MESSAGE",

Review Comment:
   Why remove these rpc codes?



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1433: feat(Ranger): Use Apache Ranger for replica access controller

Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1433:
URL: https://github.com/apache/incubator-pegasus/pull/1433#discussion_r1166815266


##########
src/runtime/security/replica_access_controller.cpp:
##########
@@ -62,10 +88,37 @@ void replica_access_controller::update_allowed_users(const std::string &users)
     utils::split_args(users.c_str(), users_set, ',');
     {
         utils::auto_write_lock l(_lock);
-        // This swap operation is in constant time
-        _users.swap(users_set);
+        _allowed_users.swap(users_set);
         _env_users = users;
+        check_allowed_users_valid();
+    }
+}
+
+void replica_access_controller::update_ranger_policies(const std::string &policies)
+{
+    {
+        utils::auto_read_lock l(_lock);
+        if (_env_policies == policies) {
+            return;
+        }
+    }
+    ranger::acl_policies tmp_policies;
+    std::string tmp_policies_str = policies;
+    dsn::json::json_forwarder<ranger::acl_policies>::decode(
+        dsn::blob::create_from_bytes(std::move(tmp_policies_str)), tmp_policies);
+    {
+        utils::auto_write_lock l(_lock);
+        _env_policies = policies;
+        _ranger_policies = std::move(tmp_policies);
     }
 }
+
+void replica_access_controller::check_allowed_users_valid()

Review Comment:
   ```suggestion
   void replica_access_controller::check_allowed_users_valid() const
   ```



##########
src/runtime/security/replica_access_controller.h:
##########
@@ -30,14 +32,39 @@ namespace security {
 class replica_access_controller : public access_controller
 {
 public:
-    explicit replica_access_controller(const std::string &name);
-    bool allowed(message_ex *msg) override;
+    explicit replica_access_controller(const std::string &replica_name);
+
+    // Check whether replica can be accessed, this method is compatible with ACL using
+    // '_allowed_users' and ACL using Ranger policy.
+    bool allowed(message_ex *msg, ranger::access_type req_type) override;
+
+    // Update '_allowed_users' when the app_env(REPLICA_ACCESS_CONTROLLER_ALLOWED_USERS) of the
+    // table changes
     void update_allowed_users(const std::string &users) override;
 
+    // Update '_ranger_policies' when the app_env(REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES) of the
+    // table changes
+    void update_ranger_policies(const std::string &policies) override;
+
+private:
+    // Security check to avoid allowed_users is not empty in special scenarios.
+    void check_allowed_users_valid();

Review Comment:
   ```suggestion
       void check_allowed_users_valid() const;
   ```



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] WHBANG commented on a diff in pull request #1433: feat(Ranger): Use Apache Ranger for replica access controller

Posted by "WHBANG (via GitHub)" <gi...@apache.org>.
WHBANG commented on code in PR #1433:
URL: https://github.com/apache/incubator-pegasus/pull/1433#discussion_r1166175671


##########
src/runtime/security/meta_access_controller.cpp:
##########
@@ -65,14 +65,10 @@ meta_access_controller::meta_access_controller(
                                         "RPC_CM_NOTIFY_STOP_SPLIT",
                                         "RPC_CM_QUERY_CHILD_STATE",
                                         "RPC_NEGOTIATION",
-                                        "RPC_CALL_RAW_MESSAGE",
                                         "RPC_CALL_RAW_SESSION_DISCONNECT",
-                                        "RPC_NFS_GET_FILE_SIZE",

Review Comment:
   the RPC of nfs module is managed by the replica_access_controller, so remove it.



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] WHBANG commented on a diff in pull request #1433: feat(Ranger): Use Apache Ranger for replica access controller

Posted by "WHBANG (via GitHub)" <gi...@apache.org>.
WHBANG commented on code in PR #1433:
URL: https://github.com/apache/incubator-pegasus/pull/1433#discussion_r1165019032


##########
src/runtime/security/replica_access_controller.cpp:
##########
@@ -15,36 +15,61 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include "replica_access_controller.h"
+#include <utility>
+
+// Disable class-memaccess warning to facilitate compilation with gcc>7
+// https://github.com/Tencent/rapidjson/issues/1700
+#pragma GCC diagnostic push
+#if defined(__GNUC__) && __GNUC__ >= 8
+#pragma GCC diagnostic ignored "-Wclass-memaccess"
+#endif
+#include "common/json_helper.h"
+
+#pragma GCC diagnostic pop
 
+#include "replica_access_controller.h"
 #include "runtime/rpc/network.h"
 #include "runtime/rpc/rpc_message.h"
 #include "utils/autoref_ptr.h"
+#include "utils/blob.h"
+#include "utils/flags.h"
 #include "utils/fmt_logging.h"
 #include "utils/strings.h"
 
 namespace dsn {
 namespace security {
-replica_access_controller::replica_access_controller(const std::string &name) { _name = name; }
+DSN_DECLARE_bool(enable_acl);
+DSN_DECLARE_bool(enable_ranger_acl);
+replica_access_controller::replica_access_controller(const std::string &replica_name)
+{
+    _name = replica_name;
+}
 
-bool replica_access_controller::allowed(message_ex *msg)
+bool replica_access_controller::allowed(message_ex *msg, ranger::access_type req_type)
 {
     const std::string &user_name = msg->io_session->get_client_username();
-    if (pre_check(user_name)) {
-        return true;
+    if (!FLAGS_enable_ranger_acl) {
+        if (!FLAGS_enable_acl || is_super_user(user_name)) {
+            return true;
+        }
+        {
+            utils::auto_read_lock l(_lock);
+            // If the user didn't specify any ACL, it means this table is publicly accessible to
+            // everyone. This is a backdoor to allow old-version clients to gracefully upgrade.After
+            // they are finally ensured to be fully upgraded, they can specify some usernames to ACL
+            // and the table will be truly protected.
+            if (!_allowed_users.empty() && _allowed_users.find(user_name) == _allowed_users.end()) {

Review Comment:
   The old ACL has left a backdoor for upgrade, and the `_allowed_users` will be cleared during the upgrade, so have to judge whether the list is empty, otherwise the back door will be gone. in other words, when the `_allowed_users` is empty, we want to return `true`



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] WHBANG commented on a diff in pull request #1433: feat(Ranger): Use Apache Ranger for replica access controller

Posted by "WHBANG (via GitHub)" <gi...@apache.org>.
WHBANG commented on code in PR #1433:
URL: https://github.com/apache/incubator-pegasus/pull/1433#discussion_r1164956305


##########
src/runtime/security/meta_access_controller.cpp:
##########
@@ -65,14 +65,10 @@ meta_access_controller::meta_access_controller(
                                         "RPC_CM_NOTIFY_STOP_SPLIT",
                                         "RPC_CM_QUERY_CHILD_STATE",
                                         "RPC_NEGOTIATION",
-                                        "RPC_CALL_RAW_MESSAGE",

Review Comment:
   coding error,duplicated in  input-parameter



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] empiredan merged pull request #1433: feat(Ranger): Use Apache Ranger for replica access controller

Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan merged PR #1433:
URL: https://github.com/apache/incubator-pegasus/pull/1433


-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org