acelyc111 commented on code in PR #1379:
URL:
https://github.com/apache/incubator-pegasus/pull/1379#discussion_r1122696493
##########
src/runtime/security/access_controller.cpp:
##########
@@ -26,11 +26,22 @@
namespace dsn {
namespace security {
DSN_DEFINE_bool(security, enable_acl, false, "whether enable access controller
or not");
-DSN_TAG_VARIABLE(enable_acl, FT_MUTABLE);
Review Comment:
nit: Remove the blank line.
##########
src/runtime/security/access_controller.cpp:
##########
@@ -26,11 +26,22 @@
namespace dsn {
namespace security {
DSN_DEFINE_bool(security, enable_acl, false, "whether enable access controller
or not");
-DSN_TAG_VARIABLE(enable_acl, FT_MUTABLE);
-DSN_DEFINE_string(security, super_users, "", "super user for access
controller");
+DSN_DEFINE_bool(security, enable_ranger_acl, false, "whether enable access
controller or not");
-access_controller::access_controller() { utils::split_args(FLAGS_super_users,
_super_users, ','); }
+DSN_DEFINE_string(security,
+ super_users,
+ "",
Review Comment:
What happen if enable acl but leave super_users as empty?
##########
src/runtime/security/access_controller.cpp:
##########
@@ -26,11 +26,22 @@
namespace dsn {
namespace security {
DSN_DEFINE_bool(security, enable_acl, false, "whether enable access controller
or not");
-DSN_TAG_VARIABLE(enable_acl, FT_MUTABLE);
-DSN_DEFINE_string(security, super_users, "", "super user for access
controller");
+DSN_DEFINE_bool(security, enable_ranger_acl, false, "whether enable access
controller or not");
Review Comment:
```suggestion
DSN_DEFINE_bool(security, enable_ranger_acl, false, "whether enable access
controller integrate to Apache Ranger or not");
```
##########
src/runtime/security/access_controller.cpp:
##########
@@ -40,6 +51,14 @@ bool access_controller::pre_check(const std::string
&user_name)
return true;
}
return false;
+ return _super_users.find(user_name) != _super_users.end();
+}
+
+bool access_controller::is_enable_ranger_acl() { return
FLAGS_enable_ranger_acl; }
Review Comment:
Why not use `FLAGS_enable_ranger_acl` directly?
##########
src/runtime/security/access_controller.h:
##########
@@ -31,27 +33,41 @@ class access_controller
access_controller();
virtual ~access_controller() = 0;
- /**
- * update the access controller
- * acls - the new acls to update
- **/
- virtual void update(const std::string &acls){};
+ // Update the access controller.
+ // users - the new allowed users to update
+ virtual void update_allowed_users(const std::string &users) {}
+
+ // Check whether the Ranger ACL is enabled or not.
+ bool is_enable_ranger_acl();
+
+ // Check if the message received is allowd to access the system.
+ // msg - the message received
+ virtual bool allowed(message_ex *msg, dsn::ranger::access_type req_type) {
return false; }
- /**
- * check if the message received is allowd to do something.
- * msg - the message received
- **/
+ // Check if the message received is allowd to access the table.
+ // msg - the message received
+ // app_name - tables involved in ACL
+ virtual bool allowed(message_ex *msg, const std::string &app_name) {
return false; }
+
+ // (TODO:wanghao) : this method will be deleted in the next patch.
Review Comment:
```suggestion
// TODO(wanghao): this method will be deleted in the next patch.
```
--
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]
