From 8d82581c969afccbf3bd40883d5ba9478a76b400 Mon Sep 17 00:00:00 2001 From: Kostas Kyrimis Date: Mon, 17 Jun 2024 09:40:41 +0300 Subject: [PATCH] chore: small acl compat changes (#3177) * print categories and commands in lower case instead of capital case * fix a bug of default user inheriting the wrong acl rules on new connections * move keys position to be after password when printed from an acl command --- src/facade/acl_commands_def.h | 6 ++ src/facade/conn_context.h | 1 - src/server/acl/acl_family.cc | 16 +++-- src/server/acl/acl_family_test.cc | 43 ++++++------ src/server/acl/helpers.cc | 8 +-- src/server/acl/user_registry.cc | 2 +- src/server/acl/user_registry.h | 6 -- src/server/conn_context.cc | 20 +++++- src/server/conn_context.h | 3 +- src/server/main_service.cc | 4 +- src/server/replica.cc | 2 +- src/server/server_family.cc | 3 +- src/server/test_utils.cc | 3 +- tests/dragonfly/acl_family_test.py | 103 +++++++++++++++++------------ 14 files changed, 130 insertions(+), 90 deletions(-) diff --git a/src/facade/acl_commands_def.h b/src/facade/acl_commands_def.h index 0e0f0bf0f..62e131fdd 100644 --- a/src/facade/acl_commands_def.h +++ b/src/facade/acl_commands_def.h @@ -24,4 +24,10 @@ struct AclKeys { bool all_keys = false; }; +struct UserCredentials { + uint32_t acl_categories{0}; + std::vector acl_commands; + AclKeys keys; +}; + } // namespace dfly::acl diff --git a/src/facade/conn_context.h b/src/facade/conn_context.h index b58bef253..97d6a39e4 100644 --- a/src/facade/conn_context.h +++ b/src/facade/conn_context.h @@ -110,7 +110,6 @@ class ConnectionContext { // TODO fix inherit actual values from default std::string authed_username{"default"}; - uint32_t acl_categories{dfly::acl::ALL}; std::vector acl_commands; // keys dfly::acl::AclKeys keys{{}, true}; diff --git a/src/server/acl/acl_family.cc b/src/server/acl/acl_family.cc index 1feab44be..62c4290db 100644 --- a/src/server/acl/acl_family.cc +++ b/src/server/acl/acl_family.cc @@ -67,16 +67,16 @@ void AclFamily::List(CmdArgList args, ConnectionContext* cntx) { const std::string_view pass = user.Password(); const std::string password = pass == "nopass" ? "nopass" : PrettyPrintSha(pass); - const std::string acl_cat_and_commands = - AclCatAndCommandToString(user.CatChanges(), user.CmdChanges()); - const std::string acl_keys = AclKeysToString(user.Keys()); const std::string maybe_space_com = acl_keys.empty() ? "" : " "; + const std::string acl_cat_and_commands = + AclCatAndCommandToString(user.CatChanges(), user.CmdChanges()); + using namespace std::string_view_literals; absl::StrAppend(&buffer, username, " ", user.IsActive() ? "on "sv : "off "sv, password, " ", - acl_cat_and_commands, maybe_space_com, acl_keys); + acl_keys, maybe_space_com, acl_cat_and_commands); cntx->SendSimpleString(buffer); } @@ -199,15 +199,17 @@ std::string AclFamily::RegistryToString() const { const std::string_view pass = user.Password(); const std::string password = pass == "nopass" ? "nopass " : absl::StrCat("#", PrettyPrintSha(pass, true), " "); - const std::string acl_cat_and_commands = - AclCatAndCommandToString(user.CatChanges(), user.CmdChanges()); + const std::string acl_keys = AclKeysToString(user.Keys()); const std::string maybe_space = acl_keys.empty() ? "" : " "; + const std::string acl_cat_and_commands = + AclCatAndCommandToString(user.CatChanges(), user.CmdChanges()); + using namespace std::string_view_literals; absl::StrAppend(&result, command, username, " ", user.IsActive() ? "ON "sv : "OFF "sv, password, - acl_cat_and_commands, maybe_space, acl_keys, "\n"); + acl_keys, maybe_space, acl_cat_and_commands, "\n"); } return result; diff --git a/src/server/acl/acl_family_test.cc b/src/server/acl/acl_family_test.cc index c8354e440..0aac28dd3 100644 --- a/src/server/acl/acl_family_test.cc +++ b/src/server/acl/acl_family_test.cc @@ -6,6 +6,7 @@ #include "absl/container/flat_hash_map.h" #include "absl/flags/internal/flag.h" +#include "absl/strings/ascii.h" #include "absl/strings/str_cat.h" #include "base/gtest.h" #include "base/logging.h" @@ -47,15 +48,15 @@ TEST_F(AclFamilyTest, AclSetUser) { resp = Run({"ACL", "LIST"}); auto vec = resp.GetVec(); EXPECT_THAT( - vec, UnorderedElementsAre("user default on nopass +@ALL ~*", "user vlad off nopass -@ALL")); + vec, UnorderedElementsAre("user default on nopass ~* +@all", "user vlad off nopass -@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 nopass -@all +acl")); } TEST_F(AclFamilyTest, AclDelUser) { @@ -82,7 +83,7 @@ TEST_F(AclFamilyTest, AclDelUser) { EXPECT_THAT(resp, IntArg(0)); resp = Run({"ACL", "LIST"}); - EXPECT_THAT(resp.GetString(), "user default on nopass +@ALL ~*"); + EXPECT_THAT(resp.GetString(), "user default on nopass ~* +@all"); Run({"ACL", "SETUSER", "michael", "ON"}); Run({"ACL", "SETUSER", "kobe", "ON"}); @@ -103,9 +104,9 @@ 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")); + EXPECT_THAT(vec, UnorderedElementsAre("user default on nopass ~* +@all", + "user kostas off d74ff0ee8da3b98 -@all +@admin", + "user adi off d74ff0ee8da3b98 -@all +@fast")); } TEST_F(AclFamilyTest, AclAuth) { @@ -154,16 +155,18 @@ TEST_F(AclFamilyTest, TestAllCategories) { resp = Run({"ACL", "LIST"}); EXPECT_THAT(resp.GetVec(), - UnorderedElementsAre("user default on nopass +@ALL ~*", - absl::StrCat("user kostas off nopass -@ALL ", "+@", cat))); + UnorderedElementsAre("user default on nopass ~* +@all", + absl::StrCat("user kostas off nopass -@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 ", "-@", cat))); + UnorderedElementsAre("user default on nopass ~* +@all", + absl::StrCat("user kostas off nopass -@all ", "-@", + absl::AsciiStrToLower(cat)))); resp = Run({"ACL", "DELUSER", "kostas"}); EXPECT_THAT(resp, IntArg(1)); @@ -201,16 +204,18 @@ TEST_F(AclFamilyTest, TestAllCommands) { 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 ", - "+", command_name))); + EXPECT_THAT(resp.GetVec(), + UnorderedElementsAre("user default on nopass ~* +@all", + absl::StrCat("user kostas off nopass -@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 ", "-", command_name))); + EXPECT_THAT(resp.GetVec(), + UnorderedElementsAre("user default on nopass ~* +@all", + absl::StrCat("user kostas off nopass ", "-@all ", "-", + absl::AsciiStrToLower(command_name)))); resp = Run({"ACL", "DELUSER", "kostas"}); EXPECT_THAT(resp, IntArg(1)); @@ -259,7 +264,7 @@ TEST_F(AclFamilyTest, TestGetUser) { EXPECT_THAT(vec[2], "passwords"); EXPECT_TRUE(vec[3].GetVec().empty()); EXPECT_THAT(vec[4], "commands"); - EXPECT_THAT(vec[5], "+@ALL"); + EXPECT_THAT(vec[5], "+@all"); EXPECT_THAT(vec[6], "keys"); EXPECT_THAT(vec[7], "~*"); @@ -271,7 +276,7 @@ TEST_F(AclFamilyTest, TestGetUser) { EXPECT_THAT(kvec[2], "passwords"); EXPECT_TRUE(kvec[3].GetVec().empty()); EXPECT_THAT(kvec[4], "commands"); - EXPECT_THAT(kvec[5], "-@ALL +@STRING +HSET"); + EXPECT_THAT(kvec[5], "-@all +@string +hset"); } TEST_F(AclFamilyTest, TestDryRun) { diff --git a/src/server/acl/helpers.cc b/src/server/acl/helpers.cc index fde332539..d54f4980b 100644 --- a/src/server/acl/helpers.cc +++ b/src/server/acl/helpers.cc @@ -25,12 +25,12 @@ namespace { std::string AclCatToString(uint32_t acl_category, User::Sign sign) { std::string res = sign == User::Sign::PLUS ? "+@" : "-@"; if (acl_category == acl::ALL) { - absl::StrAppend(&res, "ALL"); + absl::StrAppend(&res, "all"); return res; } const auto& index = CategoryToIdx().at(acl_category); - absl::StrAppend(&res, REVERSE_CATEGORY_INDEX_TABLE[index]); + absl::StrAppend(&res, absl::AsciiStrToLower(REVERSE_CATEGORY_INDEX_TABLE[index])); return res; } @@ -41,7 +41,7 @@ std::string AclCommandToString(size_t family, uint64_t mask, User::Sign sign) { std::string prefix = (sign == User::Sign::PLUS) ? "+" : "-"; if (mask == ALL_COMMANDS) { for (const auto& cmd : rev_index[family]) { - absl::StrAppend(&res, prefix, cmd, " "); + absl::StrAppend(&res, prefix, absl::AsciiStrToLower(cmd), " "); } res.pop_back(); return res; @@ -53,7 +53,7 @@ std::string AclCommandToString(size_t family, uint64_t mask, User::Sign sign) { mask = mask >> 1; } --pos; - absl::StrAppend(&res, prefix, rev_index[family][pos]); + absl::StrAppend(&res, prefix, absl::AsciiStrToLower(rev_index[family][pos])); return res; } diff --git a/src/server/acl/user_registry.cc b/src/server/acl/user_registry.cc index 42d452550..54510344e 100644 --- a/src/server/acl/user_registry.cc +++ b/src/server/acl/user_registry.cc @@ -29,7 +29,7 @@ bool UserRegistry::RemoveUser(std::string_view username) { return registry_.erase(username); } -UserRegistry::UserCredentials UserRegistry::GetCredentials(std::string_view username) const { +UserCredentials UserRegistry::GetCredentials(std::string_view username) const { std::shared_lock lock(mu_); auto it = registry_.find(username); if (it == registry_.end()) { diff --git a/src/server/acl/user_registry.h b/src/server/acl/user_registry.h index 336a561d5..fef3fefd8 100644 --- a/src/server/acl/user_registry.h +++ b/src/server/acl/user_registry.h @@ -42,12 +42,6 @@ class UserRegistry { // kills already existing connections from the removed user bool RemoveUser(std::string_view username); - struct UserCredentials { - uint32_t acl_categories{0}; - std::vector acl_commands; - AclKeys keys; - }; - // Acquires a read lock UserCredentials GetCredentials(std::string_view username) const; diff --git a/src/server/conn_context.cc b/src/server/conn_context.cc index 231089134..e521e150d 100644 --- a/src/server/conn_context.cc +++ b/src/server/conn_context.cc @@ -6,6 +6,7 @@ #include "base/logging.h" #include "core/heap_size.h" +#include "facade/acl_commands_def.h" #include "server/acl/acl_commands_def.h" #include "server/command_registry.h" #include "server/engine_shard_set.h" @@ -80,18 +81,31 @@ const CommandId* StoredCmd::Cid() const { return cid_; } -ConnectionContext::ConnectionContext(::io::Sink* stream, facade::Connection* owner) +ConnectionContext::ConnectionContext(::io::Sink* stream, facade::Connection* owner, + acl::UserCredentials cred) : facade::ConnectionContext(stream, owner) { if (owner) { skip_acl_validation = owner->IsPrivileged(); } - acl_commands = std::vector(acl::NumberOfFamilies(), acl::ALL_COMMANDS); + + keys = std::move(cred.keys); + if (cred.acl_commands.empty()) { + acl_commands = std::vector(acl::NumberOfFamilies(), acl::NONE_COMMANDS); + } else { + acl_commands = std::move(cred.acl_commands); + } } ConnectionContext::ConnectionContext(const ConnectionContext* owner, Transaction* tx, facade::CapturingReplyBuilder* crb) : facade::ConnectionContext(nullptr, nullptr), transaction{tx} { - acl_commands = std::vector(acl::NumberOfFamilies(), acl::ALL_COMMANDS); + if (owner) { + acl_commands = owner->acl_commands; + keys = owner->keys; + skip_acl_validation = owner->skip_acl_validation; + } else { + acl_commands = std::vector(acl::NumberOfFamilies(), acl::NONE_COMMANDS); + } if (tx) { // If we have a carrier transaction, this context is used for squashing DCHECK(owner); conn_state.db_index = owner->conn_state.db_index; diff --git a/src/server/conn_context.h b/src/server/conn_context.h index 6d3c65857..cfaacad50 100644 --- a/src/server/conn_context.h +++ b/src/server/conn_context.h @@ -8,6 +8,7 @@ #include #include "acl/acl_commands_def.h" +#include "facade/acl_commands_def.h" #include "facade/conn_context.h" #include "facade/reply_capture.h" #include "server/common.h" @@ -265,7 +266,7 @@ struct ConnectionState { class ConnectionContext : public facade::ConnectionContext { public: - ConnectionContext(::io::Sink* stream, facade::Connection* owner); + ConnectionContext(::io::Sink* stream, facade::Connection* owner, dfly::acl::UserCredentials cred); ConnectionContext(const ConnectionContext* owner, Transaction* tx, facade::CapturingReplyBuilder* crb); diff --git a/src/server/main_service.cc b/src/server/main_service.cc index 0bafdfa7c..7f0a4d92e 100644 --- a/src/server/main_service.cc +++ b/src/server/main_service.cc @@ -1568,10 +1568,12 @@ bool RequirePrivilegedAuth() { facade::ConnectionContext* Service::CreateContext(util::FiberSocketBase* peer, facade::Connection* owner) { - ConnectionContext* res = new ConnectionContext{peer, owner}; + auto cred = user_registry_.GetCredentials("default"); + ConnectionContext* res = new ConnectionContext{peer, owner, std::move(cred)}; if (peer->IsUDS()) { res->req_auth = false; + res->skip_acl_validation = true; } else if (owner->IsPrivileged() && RequirePrivilegedAuth()) { res->req_auth = !GetPassword().empty(); } else if (!owner->IsPrivileged()) { diff --git a/src/server/replica.cc b/src/server/replica.cc index 3e8b7d395..ac85a88ad 100644 --- a/src/server/replica.cc +++ b/src/server/replica.cc @@ -557,7 +557,7 @@ error_code Replica::InitiateDflySync() { error_code Replica::ConsumeRedisStream() { base::IoBuf io_buf(16_KB); io::NullSink null_sink; // we never reply back on the commands. - ConnectionContext conn_context{&null_sink, nullptr}; + ConnectionContext conn_context{&null_sink, nullptr, {}}; conn_context.is_replicating = true; conn_context.journal_emulated = true; conn_context.skip_acl_validation = true; diff --git a/src/server/server_family.cc b/src/server/server_family.cc index 7fc3b60e2..ac2a16afd 100644 --- a/src/server/server_family.cc +++ b/src/server/server_family.cc @@ -1644,7 +1644,6 @@ void ServerFamily::Auth(CmdArgList args, ConnectionContext* cntx) { if (is_authorized) { cntx->authed_username = username; auto cred = registry->GetCredentials(username); - cntx->acl_categories = cred.acl_categories; cntx->acl_commands = cred.acl_commands; cntx->keys = std::move(cred.keys); cntx->authenticated = true; @@ -2586,7 +2585,7 @@ void ServerFamily::ReplicaOf(CmdArgList args, ConnectionContext* cntx) { void ServerFamily::Replicate(string_view host, string_view port) { io::NullSink sink; - ConnectionContext ctxt{&sink, nullptr}; + ConnectionContext ctxt{&sink, nullptr, {}}; ctxt.skip_acl_validation = true; StringVec replicaof_params{string(host), string(port)}; diff --git a/src/server/test_utils.cc b/src/server/test_utils.cc index 2df43cdbe..39a18dbdf 100644 --- a/src/server/test_utils.cc +++ b/src/server/test_utils.cc @@ -57,7 +57,8 @@ static vector SplitLines(const std::string& src) { TestConnection::TestConnection(Protocol protocol, io::StringSink* sink) : facade::Connection(protocol, nullptr, nullptr, nullptr), sink_(sink) { - cc_.reset(new dfly::ConnectionContext(sink_, this)); + cc_.reset(new dfly::ConnectionContext(sink_, this, {})); + cc_->skip_acl_validation = true; SetSocket(ProactorBase::me()->CreateSocket()); OnConnectionStart(); } diff --git a/tests/dragonfly/acl_family_test.py b/tests/dragonfly/acl_family_test.py index ab8c4616e..f044269e9 100644 --- a/tests/dragonfly/acl_family_test.py +++ b/tests/dragonfly/acl_family_test.py @@ -12,61 +12,61 @@ from . import dfly_args @pytest.mark.asyncio async def test_acl_setuser(async_client): await async_client.execute_command("ACL SETUSER kostas") - result = await async_client.execute_command("ACL LIST") + result = await async_client.execute_command("ACL list") assert 2 == len(result) - assert "user kostas off nopass -@ALL" in result + assert "user kostas off nopass -@all" in result await async_client.execute_command("ACL SETUSER kostas ON") - result = await async_client.execute_command("ACL LIST") - assert "user kostas on nopass -@ALL" in result + result = await async_client.execute_command("ACL list") + assert "user kostas on nopass -@all" in result await async_client.execute_command("ACL SETUSER kostas +@list +@string +@admin") - result = await async_client.execute_command("ACL LIST") + result = await async_client.execute_command("ACL list") # TODO consider printing to lowercase - assert "user kostas on nopass -@ALL +@LIST +@STRING +@ADMIN" in result + assert "user kostas on nopass -@all +@list +@string +@admin" in result await async_client.execute_command("ACL SETUSER kostas -@list -@admin") - result = await async_client.execute_command("ACL LIST") - assert "user kostas on nopass -@ALL +@STRING -@LIST -@ADMIN" in result + result = await async_client.execute_command("ACL list") + assert "user kostas on nopass -@all +@string -@list -@admin" in result # mix and match await async_client.execute_command("ACL SETUSER kostas +@list -@string") - result = await async_client.execute_command("ACL LIST") - assert "user kostas on nopass -@ALL -@ADMIN +@LIST -@STRING" in result + result = await async_client.execute_command("ACL list") + assert "user kostas on nopass -@all -@admin +@list -@string" in result # mix and match interleaved await async_client.execute_command("ACL SETUSER kostas +@set -@set +@set") - result = await async_client.execute_command("ACL LIST") - assert "user kostas on nopass -@ALL -@ADMIN +@LIST -@STRING +@SET" in result + result = await async_client.execute_command("ACL list") + assert "user kostas on nopass -@all -@admin +@list -@string +@set" in result await async_client.execute_command("ACL SETUSER kostas +@all") - result = await async_client.execute_command("ACL LIST") - assert "user kostas on nopass -@ADMIN +@LIST -@STRING +@SET +@ALL" in result + result = await async_client.execute_command("ACL list") + assert "user kostas on nopass -@admin +@list -@string +@set +@all" in result # commands await async_client.execute_command("ACL SETUSER kostas +set +get +hset") - result = await async_client.execute_command("ACL LIST") - assert "user kostas on nopass -@ADMIN +@LIST -@STRING +@SET +@ALL +SET +GET +HSET" in result + result = await async_client.execute_command("ACL list") + assert "user kostas on nopass -@admin +@list -@string +@set +@all +set +get +hset" in result await async_client.execute_command("ACL SETUSER kostas -set -get +hset") - result = await async_client.execute_command("ACL LIST") - assert "user kostas on nopass -@ADMIN +@LIST -@STRING +@SET +@ALL -SET -GET +HSET" in result + result = await async_client.execute_command("ACL list") + assert "user kostas on nopass -@admin +@list -@string +@set +@all -set -get +hset" in result # interleaved await async_client.execute_command("ACL SETUSER kostas -hset +get -get -@all") - result = await async_client.execute_command("ACL LIST") - assert "user kostas on nopass -@ADMIN +@LIST -@STRING +@SET -SET -HSET -GET -@ALL" in result + result = await async_client.execute_command("ACL list") + assert "user kostas on nopass -@admin +@list -@string +@set -set -hset -get -@all" in result # interleaved with categories await async_client.execute_command("ACL SETUSER kostas +@string +get -get +set") - result = await async_client.execute_command("ACL LIST") - assert "user kostas on nopass -@ADMIN +@LIST +@SET -HSET -@ALL +@STRING -GET +SET" in result + result = await async_client.execute_command("ACL list") + assert "user kostas on nopass -@admin +@list +@set -hset -@all +@string -get +set" in result @pytest.mark.asyncio async def test_acl_categories(async_client): await async_client.execute_command( - "ACL SETUSER vlad ON >mypass -@ALL +@string +@list +@connection ~*" + "ACL SETUSER vlad ON >mypass -@all +@string +@list +@connection ~*" ) result = await async_client.execute_command("AUTH vlad mypass") @@ -116,7 +116,7 @@ async def test_acl_categories(async_client): @pytest.mark.asyncio async def test_acl_commands(async_client): - await async_client.execute_command("ACL SETUSER random ON >mypass -@ALL +set +get ~*") + await async_client.execute_command("ACL SETUSER random ON >mypass -@all +set +get ~*") result = await async_client.execute_command("AUTH random mypass") assert result == "OK" @@ -177,7 +177,7 @@ async def test_acl_cat_commands_multi_exec_squash(df_local_factory): for x in range(33): await client.execute_command(f"SET x{x} {x}") - # ADMIN revokes permissions + # admin revokes permissions res = await admin_client.execute_command("ACL SETUSER kk -@string") assert res == b"OK" @@ -330,22 +330,22 @@ async def test_good_acl_file(df_local_factory, tmp_dir): client = df.client() await client.execute_command("ACL LOAD") - result = await client.execute_command("ACL LIST") + result = await client.execute_command("ACL list") assert 2 == len(result) - assert "user MrFoo on ea71c25a7a60224 -@ALL" in result - assert "user default on nopass +@ALL ~*" in result + assert "user MrFoo on ea71c25a7a60224 -@all" in result + assert "user default on nopass ~* +@all" in result await client.execute_command("ACL DELUSER MrFoo") - await client.execute_command("ACL SETUSER roy ON >mypass +@STRING +HSET") - await client.execute_command("ACL SETUSER shahar >mypass +@SET") - await client.execute_command("ACL SETUSER vlad +@STRING ~foo ~bar*") + await client.execute_command("ACL SETUSER roy ON >mypass +@string +hset") + await client.execute_command("ACL SETUSER shahar >mypass +@set") + await client.execute_command("ACL SETUSER vlad ~foo ~bar* +@string") - result = await client.execute_command("ACL LIST") + result = await client.execute_command("ACL list") assert 4 == len(result) - assert "user roy on ea71c25a7a60224 -@ALL +@STRING +HSET" in result - assert "user shahar off ea71c25a7a60224 -@ALL +@SET" in result - assert "user vlad off nopass -@ALL +@STRING ~foo ~bar*" in result - assert "user default on nopass +@ALL ~*" in result + assert "user roy on ea71c25a7a60224 -@all +@string +hset" in result + assert "user shahar off ea71c25a7a60224 -@all +@set" in result + assert "user vlad off nopass ~foo ~bar* -@all +@string" in result + assert "user default on nopass ~* +@all" in result result = await client.execute_command("ACL DELUSER shahar") assert result == 1 @@ -354,11 +354,11 @@ async def test_good_acl_file(df_local_factory, tmp_dir): result = await client.execute_command("ACL LOAD") - result = await client.execute_command("ACL LIST") + result = await client.execute_command("ACL list") assert 3 == len(result) - assert "user roy on ea71c25a7a60224 -@ALL +@STRING +HSET" in result - assert "user vlad off nopass -@ALL +@STRING ~foo ~bar*" in result - assert "user default on nopass +@ALL ~*" in result + assert "user roy on ea71c25a7a60224 -@all +@string +hset" in result + assert "user vlad off nopass ~foo ~bar* -@all +@string" in result + assert "user default on nopass ~* +@all" in result await client.close() @@ -387,7 +387,7 @@ async def test_acl_log(async_client): res = await async_client.execute_command("SET mykey 22") with pytest.raises(redis.exceptions.ResponseError): - await async_client.execute_command("HSET mk kk 22") + await async_client.execute_command("hset mk kk 22") res = await async_client.execute_command("ACL LOG") assert 1 == len(res) @@ -447,7 +447,7 @@ async def test_require_pass(df_local_factory): @pytest.mark.asyncio async def test_set_acl_file(async_client: aioredis.Redis, tmp_dir): - acl_file_content = "USER roy ON #ea71c25a7a602246b4c39824b855678894a96f43bb9b71319c39700a1e045222 +@STRING +@FAST +HSET\nUSER john on nopass +@string" + acl_file_content = "USER roy ON #ea71c25a7a602246b4c39824b855678894a96f43bb9b71319c39700a1e045222 +@string +@fast +hset\nUSER john on nopass +@string" acl = create_temp_file(acl_file_content, tmp_dir) @@ -455,7 +455,7 @@ async def test_set_acl_file(async_client: aioredis.Redis, tmp_dir): await async_client.execute_command("ACL LOAD") - result = await async_client.execute_command("ACL LIST") + result = await async_client.execute_command("ACL list") assert 3 == len(result) result = await async_client.execute_command("AUTH roy mypass") @@ -536,3 +536,20 @@ async def test_acl_keys(async_client): # reject because bonus key does not match with pytest.raises(redis.exceptions.ResponseError): await async_client.execute_command("ZUNIONSTORE destkey 2 barz1 barz2") + + +@pytest.mark.asyncio +async def default_user_bug(df_local_factory): + df.start() + + client = aioredis.Redis(port=df.port) + + await async_client.execute_command("ACL SETUSER default -@all") + await client.close() + + client = aioredis.Redis(port=df.port) + + with pytest.raises(redis.exceptions.ResponseError): + await client.execute_command("SET foo bar") + + await client.close()