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: [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]
