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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
