feat(acl): add support of multiple passwords (#3189)

* add support for multiple passwords
* add support for deleting passwords
* add support for resetpass
* add tests
* always prefix passwords with hashtag when printed

src/server/acl/acl_family.cc

@@ -64,8 +64,7 @@ void AclFamily::List(CmdArgList args, ConnectionContext* cntx) {
 
   for (const auto& [username, user] : registry) {
     std::string buffer = "user ";
-    const std::string_view pass = user.Password();
-    const std::string password = pass == "nopass" ? "nopass" : PrettyPrintSha(pass);
+    const std::string password = PasswordsToString(user.Passwords(), user.HasNopass(), false);
 
     const std::string acl_keys = AclKeysToString(user.Keys());
     const std::string maybe_space_com = acl_keys.empty() ? "" : " ";
@@ -75,7 +74,7 @@ void AclFamily::List(CmdArgList args, ConnectionContext* cntx) {
 
     using namespace std::string_view_literals;
 
-    absl::StrAppend(&buffer, username, " ", user.IsActive() ? "on "sv : "off "sv, password, " ",
+    absl::StrAppend(&buffer, username, " ", user.IsActive() ? "on "sv : "off "sv, password,
                     acl_keys, maybe_space_com, acl_cat_and_commands);
 
     cntx->SendSimpleString(buffer);
@@ -196,9 +195,7 @@ std::string AclFamily::RegistryToString() const {
   std::string result;
   for (auto& [username, user] : registry) {
     std::string command = "USER ";
-    const std::string_view pass = user.Password();
-    const std::string password =
-        pass == "nopass" ? "nopass " : absl::StrCat("#", PrettyPrintSha(pass, true), " ");
+    const std::string password = PasswordsToString(user.Passwords(), user.HasNopass(), true);
 
     const std::string acl_keys = AclKeysToString(user.Keys());
     const std::string maybe_space = acl_keys.empty() ? "" : " ";
@@ -495,7 +492,10 @@ void AclFamily::GetUser(CmdArgList args, ConnectionContext* cntx) {
   }
   auto& user = registry.find(username)->second;
   std::string status = user.IsActive() ? "on" : "off";
-  auto pass = user.Password();
+  auto pass = PasswordsToString(user.Passwords(), user.HasNopass(), false);
+  if (!pass.empty()) {
+    pass.pop_back();
+  }
 
   auto* rb = static_cast<facade::RedisReplyBuilder*>(cntx->reply_builder());
   rb->StartArray(8);
@@ -509,7 +509,7 @@ void AclFamily::GetUser(CmdArgList args, ConnectionContext* cntx) {
   }
 
   rb->SendSimpleString("passwords");
-  if (pass != "nopass") {
+  if (pass != "nopass" && !pass.empty()) {
     rb->SendSimpleString(pass);
   } else {
     rb->SendEmptyArray();
@@ -647,7 +647,7 @@ void AclFamily::Init(facade::Listener* main_listener, UserRegistry* registry) {
   registry_ = registry;
   config_registry.RegisterMutable("requirepass", [this](const absl::CommandLineFlag& flag) {
     User::UpdateRequest rqst;
-    rqst.password = flag.CurrentValue();
+    rqst.passwords.push_back({flag.CurrentValue()});
     registry_->MaybeAddAndUpdate("default", std::move(rqst));
     return true;
   });

src/server/acl/acl_family_test.cc

@@ -47,16 +47,67 @@ TEST_F(AclFamilyTest, AclSetUser) {
   EXPECT_THAT(resp, "OK");
   resp = Run({"ACL", "LIST"});
   auto vec = resp.GetVec();
-  EXPECT_THAT(
-      vec, UnorderedElementsAre("user default on nopass ~* +@all", "user vlad off nopass -@all"));
+  EXPECT_THAT(vec, UnorderedElementsAre("user default on nopass ~* +@all", "user vlad off -@all"));
 
   resp = Run({"ACL", "SETUSER", "vlad", "+ACL"});
   EXPECT_THAT(resp, "OK");
 
   resp = Run({"ACL", "LIST"});
   vec = resp.GetVec();
-  EXPECT_THAT(vec, UnorderedElementsAre("user default on nopass ~* +@all",
-                                        "user vlad off nopass -@all +acl"));
+  EXPECT_THAT(vec,
+              UnorderedElementsAre("user default on nopass ~* +@all", "user vlad off -@all +acl"));
+
+  resp = Run({"ACL", "SETUSER", "vlad", "on", ">pass", ">temp"});
+  EXPECT_THAT(resp, "OK");
+
+  resp = Run({"ACL", "LIST"});
+  vec = resp.GetVec();
+  EXPECT_THAT(vec.size(), 2);
+  auto contains_vlad = [](const auto& vec) {
+    const std::string default_user = "user default on nopass ~* +@all";
+    const std::string a_permutation = "user vlad on #a6864eb339b0e1f #d74ff0ee8da3b98 -@all +acl";
+    const std::string b_permutation = "user vlad on #d74ff0ee8da3b98 #a6864eb339b0e1f -@all +acl";
+    std::string_view other;
+    if (vec[0] == default_user) {
+      other = vec[1].GetView();
+    } else if (vec[1] == default_user) {
+      other = vec[0].GetView();
+    } else {
+      return false;
+    }
+
+    return other == a_permutation || other == b_permutation;
+  };
+
+  EXPECT_THAT(contains_vlad(vec), true);
+
+  resp = Run({"AUTH", "vlad", "pass"});
+  EXPECT_THAT(resp, "OK");
+
+  resp = Run({"AUTH", "vlad", "temp"});
+  EXPECT_THAT(resp, "OK");
+
+  resp = Run({"AUTH", "default", R"("")"});
+  EXPECT_THAT(resp, "OK");
+
+  resp = Run({"ACL", "SETUSER", "vlad", ">another"});
+  EXPECT_THAT(resp, "OK");
+
+  resp = Run({"ACL", "SETUSER", "vlad", "<another"});
+  EXPECT_THAT(resp, "OK");
+
+  resp = Run({"ACL", "LIST"});
+  vec = resp.GetVec();
+  EXPECT_THAT(vec.size(), 2);
+  EXPECT_THAT(contains_vlad(vec), true);
+
+  resp = Run({"ACL", "SETUSER", "vlad", "resetpass"});
+  EXPECT_THAT(resp, "OK");
+
+  resp = Run({"ACL", "LIST"});
+  vec = resp.GetVec();
+  EXPECT_THAT(vec,
+              UnorderedElementsAre("user default on nopass ~* +@all", "user vlad on -@all +acl"));
 }
 
 TEST_F(AclFamilyTest, AclDelUser) {
@@ -105,8 +156,8 @@ TEST_F(AclFamilyTest, AclList) {
   resp = Run({"ACL", "LIST"});
   auto vec = resp.GetVec();
   EXPECT_THAT(vec, UnorderedElementsAre("user default on nopass ~* +@all",
-                                        "user kostas off d74ff0ee8da3b98 -@all +@admin",
-                                        "user adi off d74ff0ee8da3b98 -@all +@fast"));
+                                        "user kostas off #d74ff0ee8da3b98 -@all +@admin",
+                                        "user adi off #d74ff0ee8da3b98 -@all +@fast"));
 }
 
 TEST_F(AclFamilyTest, AclAuth) {
@@ -154,19 +205,17 @@ TEST_F(AclFamilyTest, TestAllCategories) {
       EXPECT_THAT(resp, "OK");
 
       resp = Run({"ACL", "LIST"});
-      EXPECT_THAT(resp.GetVec(),
-                  UnorderedElementsAre("user default on nopass ~* +@all",
-                                       absl::StrCat("user kostas off nopass -@all ", "+@",
-                                                    absl::AsciiStrToLower(cat))));
+      EXPECT_THAT(resp.GetVec(), UnorderedElementsAre("user default on nopass ~* +@all",
+                                                      absl::StrCat("user kostas off -@all ", "+@",
+                                                                   absl::AsciiStrToLower(cat))));
 
       resp = Run({"ACL", "SETUSER", "kostas", absl::StrCat("-@", cat)});
       EXPECT_THAT(resp, "OK");
 
       resp = Run({"ACL", "LIST"});
-      EXPECT_THAT(resp.GetVec(),
-                  UnorderedElementsAre("user default on nopass ~* +@all",
-                                       absl::StrCat("user kostas off nopass -@all ", "-@",
-                                                    absl::AsciiStrToLower(cat))));
+      EXPECT_THAT(resp.GetVec(), UnorderedElementsAre("user default on nopass ~* +@all",
+                                                      absl::StrCat("user kostas off -@all ", "-@",
+                                                                   absl::AsciiStrToLower(cat))));
 
       resp = Run({"ACL", "DELUSER", "kostas"});
       EXPECT_THAT(resp, IntArg(1));
@@ -206,15 +255,15 @@ TEST_F(AclFamilyTest, TestAllCommands) {
       resp = Run({"ACL", "LIST"});
       EXPECT_THAT(resp.GetVec(),
                   UnorderedElementsAre("user default on nopass ~* +@all",
-                                       absl::StrCat("user kostas off nopass -@all ", "+",
+                                       absl::StrCat("user kostas off -@all ", "+",
                                                     absl::AsciiStrToLower(command_name))));
 
       resp = Run({"ACL", "SETUSER", "kostas", absl::StrCat("-", command_name)});
 
       resp = Run({"ACL", "LIST"});
       EXPECT_THAT(resp.GetVec(),
                   UnorderedElementsAre("user default on nopass ~* +@all",
-                                       absl::StrCat("user kostas off nopass ", "-@all ", "-",
+                                       absl::StrCat("user kostas off ", "-@all ", "-",
                                                     absl::AsciiStrToLower(command_name))));
 
       resp = Run({"ACL", "DELUSER", "kostas"});
@@ -272,7 +321,7 @@ TEST_F(AclFamilyTest, TestGetUser) {
   resp = Run({"ACL", "GETUSER", "kostas"});
   const auto& kvec = resp.GetVec();
   EXPECT_THAT(kvec[0], "flags");
-  EXPECT_THAT(kvec[1].GetVec(), UnorderedElementsAre("off", "nopass"));
+  EXPECT_THAT(kvec[1].GetVec(), UnorderedElementsAre("off"));
   EXPECT_THAT(kvec[2], "passwords");
   EXPECT_TRUE(kvec[3].GetVec().empty());
   EXPECT_THAT(kvec[4], "commands");

src/server/acl/helpers.cc

@@ -160,13 +160,22 @@ std::optional<ParseKeyResult> MaybeParseAclKey(std::string_view command) {
   return ParseKeyResult{std::string(key), op};
 }
 
-std::optional<std::string> MaybeParsePassword(std::string_view command, bool hashed) {
+std::optional<User::UpdatePass> MaybeParsePassword(std::string_view command, bool hashed) {
+  using UpPass = User::UpdatePass;
   if (command == "nopass") {
-    return std::string(command);
+    return UpPass{"", false, true};
+  }
+
+  if (command == "resetpass") {
+    return UpPass{"", false, false, true};
   }
 
   if (command[0] == '>' || (hashed && command[0] == '#')) {
-    return std::string(command.substr(1));
+    return UpPass{std::string(command.substr(1))};
+  }
+
+  if (command[0] == '<') {
+    return UpPass{std::string(command.substr(1)), true};
   }
 
   return {};
@@ -261,10 +270,8 @@ std::variant<User::UpdateRequest, ErrorReply> ParseAclSetUser(facade::ArgRange a
 
   for (std::string_view arg : args) {
     if (auto pass = MaybeParsePassword(facade::ToSV(arg), hashed); pass) {
-      if (req.password) {
-        return ErrorReply("Only one password is allowed");
-      }
-      req.password = std::move(pass);
+      req.passwords.push_back(std::move(*pass));
+
       if (hashed && absl::StartsWith(facade::ToSV(arg), "#")) {
         req.is_hashed = hashed;
       }
@@ -346,4 +353,16 @@ std::string AclKeysToString(const AclKeys& keys) {
   return result;
 }
 
+std::string PasswordsToString(const absl::flat_hash_set<std::string>& passwords, bool nopass,
+                              bool full_sha) {
+  if (nopass) {
+    return "nopass ";
+  }
+  std::string result;
+  for (const auto& pass : passwords) {
+    absl::StrAppend(&result, "#", PrettyPrintSha(pass, full_sha), " ");
+  }
+
+  return result;
+}
 }  // namespace dfly::acl

src/server/acl/helpers.h

@@ -10,6 +10,7 @@
 #include <string_view>
 #include <variant>
 
+#include "absl/container/flat_hash_set.h"
 #include "facade/facade_types.h"
 #include "server/acl/acl_log.h"
 #include "server/acl/user.h"
@@ -23,7 +24,7 @@ std::string AclCatAndCommandToString(const User::CategoryChanges& cat,
 std::string PrettyPrintSha(std::string_view pass, bool all = false);
 
 // When hashed is true, we allow passwords that start with both # and >
-std::optional<std::string> MaybeParsePassword(std::string_view command, bool hashed = false);
+std::optional<User::UpdatePass> MaybeParsePassword(std::string_view command, bool hashed = false);
 
 std::optional<bool> MaybeParseStatus(std::string_view command);
 
@@ -55,4 +56,8 @@ struct ParseKeyResult {
 std::optional<ParseKeyResult> MaybeParseAclKey(std::string_view command);
 
 std::string AclKeysToString(const AclKeys& keys);
+
+std::string PasswordsToString(const absl::flat_hash_set<std::string>& passwords, bool nopass,
+                              bool full_sha);
+
 }  // namespace dfly::acl

src/server/acl/user.cc

@@ -8,6 +8,7 @@
 
 #include <limits>
 
+#include "absl/container/flat_hash_set.h"
 #include "absl/strings/escaping.h"
 #include "core/overloaded.h"
 #include "server/acl/helpers.h"
@@ -30,8 +31,20 @@ User::User() {
 }
 
 void User::Update(UpdateRequest&& req) {
-  if (req.password) {
-    SetPasswordHash(*req.password, req.is_hashed);
+  for (auto& pass : req.passwords) {
+    if (pass.nopass) {
+      SetNopass();
+      continue;
+    }
+    if (pass.unset) {
+      UnsetPassword(pass.password);
+      continue;
+    }
+    if (pass.reset_password) {
+      password_hashes_.clear();
+      continue;
+    }
+    SetPasswordHash(pass.password, req.is_hashed);
   }
 
   auto cat_visitor = [this](UpdateRequest::CategoryValueType cat) {
@@ -68,23 +81,23 @@ void User::Update(UpdateRequest&& req) {
 }
 
 void User::SetPasswordHash(std::string_view password, bool is_hashed) {
-  if (password == "nopass") {
-    return;
-  }
-
+  nopass_ = false;
   if (is_hashed) {
-    password_hash_ = absl::HexStringToBytes(password);
+    password_hashes_.insert(absl::HexStringToBytes(password));
     return;
   }
-  password_hash_ = StringSHA256(password);
+  password_hashes_.insert(StringSHA256(password));
+}
+
+void User::UnsetPassword(std::string_view password) {
+  password_hashes_.erase(StringSHA256(password));
 }
 
 bool User::HasPassword(std::string_view password) const {
-  if (!password_hash_) {
+  if (nopass_) {
     return true;
   }
-  // hash password and compare
-  return *password_hash_ == StringSHA256(password);
+  return password_hashes_.contains(StringSHA256(password));
 }
 
 void User::SetAclCategoriesAndIncrSeq(uint32_t cat) {
@@ -174,10 +187,12 @@ bool User::IsActive() const {
   return is_active_;
 }
 
-static const std::string_view default_pass = "nopass";
+const absl::flat_hash_set<std::string>& User::Passwords() const {
+  return password_hashes_;
+}
 
-std::string_view User::Password() const {
-  return password_hash_ ? *password_hash_ : default_pass;
+bool User::HasNopass() const {
+  return nopass_;
 }
 
 const AclKeys& User::Keys() const {
@@ -206,4 +221,9 @@ void User::SetKeyGlobs(std::vector<UpdateKey> keys) {
   }
 }
 
+void User::SetNopass() {
+  nopass_ = true;
+  password_hashes_.clear();
+}
+
 }  // namespace dfly::acl