fix: acl forward compatible (OpenAtomFoundation#2459)

* fix: ACL user authentication errors * blacklist instead of acl user * add rename command (OpenAtomFoundation#2462) * support config get userblacklist ---------
cheniujh · Mar 8, 2024 · 17d3d6e · 17d3d6e
1 parent e6a9ae8
commit 17d3d6e
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 11 deletions.
diff --git a/conf/pika.conf b/conf/pika.conf
@@ -79,7 +79,7 @@ requirepass :
 # [NOTICE] The value of this parameter must match the "requirepass" setting on the master.
 masterauth :
 
-# The [password of user], which is empty by default.(Deprecated)
+# The [password of user], which is empty by default.
 # [NOTICE] If this user password is the same as admin password (including both being empty),
 # the value of this parameter will be ignored and all users are considered as administrators,
 # in this scenario, users are not subject to the restrictions imposed by the userblacklist.
@@ -91,7 +91,7 @@ masterauth :
 # [Advice] It's recommended to add high-risk commands to this list.
 # [Format] Commands should be separated by ",". For example: FLUSHALL, SHUTDOWN, KEYS, CONFIG
 # By default, this list is empty.
-userblacklist :
+# userblacklist :
 
 # Running Mode of Pika, The current version only supports running in "classic mode".
 # If set to 'classic', Pika will create multiple DBs whose number is the value of configure item "databases".

diff --git a/include/acl.h b/include/acl.h
@@ -365,6 +365,8 @@ class Acl {
 
   void UpdateDefaultUserPassword(const std::string& pass);
 
+  void InitLimitUser(const std::string& bl, bool limit_exist);
+
   // After the user channel is modified, determine whether the current channel needs to be disconnected
   void KillPubsubClientsIfNeeded(const std::shared_ptr<User>& origin, const std::shared_ptr<User>& newUser);
 
@@ -380,6 +382,7 @@ class Acl {
   static std::vector<std::string> GetAllCategoryName();
 
   static const std::string DefaultUser;
+  static const std::string DefaultLimitUser;
   static const int64_t LogGroupingMaxTimeDelta;
 
   // Adds a new entry in the ACL log, making sure to delete the old entry

diff --git a/include/pika_conf.h b/include/pika_conf.h
@@ -178,6 +178,10 @@ class PikaConf : public pstd::BaseConf {
     std::shared_lock l(rwlock_);
     return masterauth_;
   }
+  std::string userpass() {
+    std::shared_lock l(rwlock_);
+    return userpass_;
+  }
   std::string bgsave_path() {
     std::shared_lock l(rwlock_);
     return bgsave_path_;
@@ -377,6 +381,11 @@ class PikaConf : public pstd::BaseConf {
     return pstd::Set2String(slow_cmd_set_, ',');
   }
 
+  const std::string GetUserBlackList() {
+    std::shared_lock l(rwlock_);
+    return userblacklist_;
+  }
+
   bool is_slow_cmd(const std::string& cmd) {
     std::shared_lock l(rwlock_);
     return slow_cmd_set_.find(cmd) != slow_cmd_set_.end();
@@ -709,6 +718,7 @@ class PikaConf : public pstd::BaseConf {
   std::string replication_id_;
   std::string requirepass_;
   std::string masterauth_;
+  std::string userpass_;
   std::atomic<bool> classic_mode_;
   int databases_ = 0;
   int default_slot_num_ = 1;
@@ -762,6 +772,7 @@ class PikaConf : public pstd::BaseConf {
 
   std::string network_interface_;
 
+  std::string userblacklist_;
   std::vector<std::string> users_;  // acl user rules
 
   std::string aclFile_;

diff --git a/src/acl.cc b/src/acl.cc
@@ -294,7 +294,16 @@ std::vector<std::string> User::AllChannelKey() {
 pstd::Status Acl::Initialization() {
   AddUser(CreateDefaultUser());
   UpdateDefaultUserPassword(g_pika_conf->requirepass());
+
   auto status = LoadUsersAtStartup();
+  auto u = GetUser(DefaultLimitUser);
+  bool limit_exist = true;
+  if (nullptr == u) {
+    AddUser(CreatedUser(DefaultLimitUser));
+    limit_exist = false;
+  }
+  InitLimitUser(g_pika_conf->GetUserBlackList(), limit_exist);
+
   if (!status.ok()) {
     return status;
   }
@@ -472,6 +481,41 @@ void Acl::UpdateDefaultUserPassword(const std::string& pass) {
   }
 }
 
+void Acl::InitLimitUser(const std::string& bl, bool limit_exist) {
+  auto pass = g_pika_conf->userpass();
+  std::vector<std::string> blacklist;
+  pstd::StringSplit(bl, ',', blacklist);
+  std::unique_lock wl(mutex_);
+  auto u = GetUser(DefaultLimitUser);
+  if (limit_exist) {
+    if (!bl.empty()) {
+      u->SetUser("+@all");
+      for(auto& cmd : blacklist) {
+        cmd = pstd::StringTrim(cmd, " ");
+        u->SetUser("-" + cmd);
+      }
+      u->SetUser("on");
+      if (!pass.empty()) {
+        u->SetUser(">"+pass);
+      }
+    }
+  } else {
+    if (pass.empty()) {
+      u->SetUser("nopass");
+    } else {
+      u->SetUser(">"+pass);
+    }
+    u->SetUser("on");
+    u->SetUser("+@all");
+    u->SetUser("~*");
+    u->SetUser("&*");
+
+    for(auto& cmd : blacklist) {
+      cmd = pstd::StringTrim(cmd, " ");
+      u->SetUser("-" + cmd);
+    }
+  }
+}
 // bool Acl::CheckUserCanExec(const std::shared_ptr<Cmd>& cmd, const PikaCmdArgsType& argv) { cmd->name(); }
 
 std::shared_ptr<User> Acl::CreateDefaultUser() {
@@ -725,6 +769,7 @@ std::array<std::pair<std::string, uint32_t>, 3> Acl::SelectorFlags = {{
 }};
 
 const std::string Acl::DefaultUser = "default";
+const std::string Acl::DefaultLimitUser = "limit";
 const int64_t Acl::LogGroupingMaxTimeDelta = 60000;
 
 void Acl::AddLogEntry(int32_t reason, int32_t context, const std::string& username, const std::string& object,

diff --git a/src/pika_acl.cc b/src/pika_acl.cc
@@ -106,6 +106,10 @@ void PikaAclCmd::DelUser() {
       res().SetRes(CmdRes::kErrOther, "The 'default' user cannot be removed");
       return;
     }
+    if (it->data() == Acl::DefaultLimitUser) {
+      res().SetRes(CmdRes::kErrOther, "The 'limit' user cannot be removed");
+      return;
+    }
   }
 
   std::vector<std::string> userNames(argv_.begin() + 2, argv_.end());

diff --git a/src/pika_admin.cc b/src/pika_admin.cc
@@ -270,15 +270,24 @@ void AuthCmd::Do() {
   std::string pwd = "";
   bool defaultAuth = false;
   if (argv_.size() == 2) {
-    userName = Acl::DefaultUser;
     pwd = argv_[1];
-    defaultAuth = true;
+//    defaultAuth = true;
   } else {
     userName = argv_[1];
     pwd = argv_[2];
   }
 
-  auto authResult = AuthenticateUser(name(), userName, pwd, conn, defaultAuth);
+  AuthResult authResult;
+  if (userName == "") {
+    //  default
+    authResult = AuthenticateUser(name(), Acl::DefaultUser, pwd, conn, true);
+    if (authResult != AuthResult::OK && authResult != AuthResult::NO_REQUIRE_PASS) {
+      //  Limit
+      authResult = AuthenticateUser(name(), Acl::DefaultLimitUser, pwd, conn, defaultAuth);
+    }
+  } else {
+    authResult = AuthenticateUser(name(), userName, pwd, conn, defaultAuth);
+  }
 
   switch (authResult) {
     case AuthResult::INVALID_CONN:
@@ -1569,7 +1578,11 @@ void ConfigCmd::ConfigGet(std::string& ret) {
     EncodeString(&config_body, "slow-cmd-thread-pool-size");
     EncodeNumber(&config_body, g_pika_conf->slow_cmd_thread_pool_size());
   }
-
+  if (pstd::stringmatch(pattern.data(), "userblacklist", 1) != 0) {
+    elements += 2;
+    EncodeString(&config_body, "userblacklist");
+    EncodeString(&config_body, g_pika_conf -> GetUserBlackList());
+  }
   if (pstd::stringmatch(pattern.data(), "slow-cmd-list", 1) != 0) {
     elements += 2;
     EncodeString(&config_body, "slow-cmd-list");

diff --git a/src/pika_conf.cc b/src/pika_conf.cc
@@ -47,7 +47,7 @@ int PikaConf::Load() {
   GetConfStr("replication-id", &replication_id_);
   GetConfStr("requirepass", &requirepass_);
   GetConfStr("masterauth", &masterauth_);
-  //  GetConfStr("userpass", &userpass_);
+  GetConfStr("userpass", &userpass_);
   GetConfInt("maxclients", &maxclients_);
   if (maxclients_ <= 0) {
     maxclients_ = 20000;
@@ -463,6 +463,8 @@ int PikaConf::Load() {
   network_interface_ = "";
   GetConfStr("network-interface", &network_interface_);
 
+  // userblacklist
+  GetConfStr("userblacklist", &userblacklist_);
   // acl users
   GetConfStrMulti("user", &users_);
 
@@ -643,8 +645,8 @@ int PikaConf::ConfigRewrite() {
   SetConfInt("timeout", timeout_);
   SetConfStr("requirepass", requirepass_);
   SetConfStr("masterauth", masterauth_);
-  //  SetConfStr("userpass", userpass_);
-  //  SetConfStr("userblacklist", userblacklist);
+  SetConfStr("userpass", userpass_);
+  SetConfStr("userblacklist", userblacklist_);
   SetConfStr("dump-prefix", bgsave_prefix_);
   SetConfInt("maxclients", maxclients_);
   SetConfInt("dump-expire", expire_dump_days_);

diff --git a/tests/integration/server_test.go b/tests/integration/server_test.go
@@ -49,7 +49,7 @@ var _ = Describe("Server", func() {
 			r = client.Do(ctx, "config", "set", "requirepass", "foobar")
 			Expect(r.Val()).To(Equal("OK"))
 
-			r = client.Do(ctx, "AUTH", "wrong!")
+			r = client.Do(ctx, "AUTH", "default", "wrong!")
 			Expect(r.Err()).To(MatchError("WRONGPASS invalid username-password pair or user is disabled."))
 
 			// r = client.Do(ctx, "AUTH", "foo", "bar")

diff --git a/tests/unit/acl.tcl b/tests/unit/acl.tcl
@@ -9,7 +9,7 @@ start_server {tags {"acl external:skip"}} {
 
     test {Coverage: ACL USERS} {
         r ACL USERS
-    } {default newuser}
+    } {default limit newuser}
 
     test {Usernames can not contain spaces or null characters} {
         catch {r ACL setuser "a a"} err