Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(acl): add acl keys to acl log command #2274

Merged
merged 9 commits into from
Dec 12, 2023
18 changes: 15 additions & 3 deletions src/server/acl/acl_family.cc
Original file line number Diff line number Diff line change
Expand Up @@ -359,13 +359,22 @@ void AclFamily::Log(CmdArgList args, ConnectionContext* cntx) {
rb->StartArray(12);
rb->SendSimpleString("reason");
using Reason = AclLog::Reason;
std::string_view reason = entry.reason == Reason::COMMAND ? "COMMAND" : "AUTH";
std::string reason;
if (entry.reason == Reason::COMMAND) {
reason = "COMMAND";
} else if (entry.reason == Reason::KEY) {
reason = "KEY";
} else {
reason = "AUTH";
}

rb->SendSimpleString(reason);
rb->SendSimpleString("object");
rb->SendSimpleString(entry.object);
rb->SendSimpleString("username");
rb->SendSimpleString(entry.username);
rb->SendSimpleString("age-seconds");

auto now_diff = std::chrono::system_clock::now() - entry.entry_creation;
auto secs = std::chrono::duration_cast<std::chrono::seconds>(now_diff);
auto left_over = now_diff - std::chrono::duration_cast<std::chrono::microseconds>(secs);
Expand Down Expand Up @@ -491,6 +500,7 @@ void AclFamily::GetUser(CmdArgList args, ConnectionContext* cntx) {

std::string acl = absl::StrCat(AclCatToString(user.AclCategory()), " ",
AclCommandToString(user.AclCommandsRef()));

rb->SendSimpleString(acl);

rb->SendSimpleString("keys");
Expand Down Expand Up @@ -550,8 +560,10 @@ void AclFamily::DryRun(CmdArgList args, ConnectionContext* cntx) {
}

const auto& user = registry.find(username)->second;
if (IsUserAllowedToInvokeCommandGeneric(user.AclCategory(), user.AclCommandsRef(), {{}, true}, {},
*cid)) {
const bool is_allowed = IsUserAllowedToInvokeCommandGeneric(
user.AclCategory(), user.AclCommandsRef(), {{}, true}, {}, *cid)
.first;
if (is_allowed) {
cntx->SendOk();
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/server/acl/acl_log.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class AclLog {
public:
explicit AclLog();

enum class Reason { COMMAND, AUTH };
enum class Reason { COMMAND, AUTH, KEY };

struct LogEntry {
std::string username;
Expand Down
18 changes: 8 additions & 10 deletions src/server/acl/validator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,12 @@ namespace dfly::acl {
return true;
}

const auto is_authed = IsUserAllowedToInvokeCommandGeneric(cntx.acl_categories, cntx.acl_commands,
cntx.keys, tail_args, id);
const auto [is_authed, reason] = IsUserAllowedToInvokeCommandGeneric(
cntx.acl_categories, cntx.acl_commands, cntx.keys, tail_args, id);

if (!is_authed) {
auto& log = ServerState::tlocal()->acl_log;
using Reason = acl::AclLog::Reason;
log.Add(cntx, std::string(id.name()), Reason::COMMAND);
log.Add(cntx, std::string(id.name()), reason);
}

return is_authed;
Expand All @@ -39,10 +38,9 @@ namespace dfly::acl {
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"

[[nodiscard]] bool IsUserAllowedToInvokeCommandGeneric(uint32_t acl_cat,
const std::vector<uint64_t>& acl_commands,
const AclKeys& keys, CmdArgList tail_args,
const CommandId& id) {
[[nodiscard]] std::pair<bool, AclLog::Reason> IsUserAllowedToInvokeCommandGeneric(
uint32_t acl_cat, const std::vector<uint64_t>& acl_commands, const AclKeys& keys,
CmdArgList tail_args, const CommandId& id) {
const auto cat_credentials = id.acl_categories();
const size_t index = id.GetFamily();
const uint64_t command_mask = id.GetBitIndex();
Expand All @@ -52,7 +50,7 @@ namespace dfly::acl {
(acl_cat & cat_credentials) != 0 || (acl_commands[index] & command_mask) != 0;

if (!command) {
return false;
return {false, AclLog::Reason::COMMAND};
}

auto match = [](const auto& pattern, const auto& target) {
Expand Down Expand Up @@ -97,7 +95,7 @@ namespace dfly::acl {
}
}

return keys_allowed;
return {keys_allowed, AclLog::Reason::KEY};
}

#pragma GCC diagnostic pop
Expand Down
9 changes: 4 additions & 5 deletions src/server/acl/validator.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,15 @@

#include <utility>

#include "facade/command_id.h"
#include "server/acl/acl_log.h"
#include "server/command_registry.h"
#include "server/conn_context.h"

namespace dfly::acl {

bool IsUserAllowedToInvokeCommandGeneric(uint32_t acl_cat,
const std::vector<uint64_t>& acl_commands,
const AclKeys& keys, CmdArgList tail_args,
const CommandId& id);
std::pair<bool, AclLog::Reason> IsUserAllowedToInvokeCommandGeneric(
uint32_t acl_cat, const std::vector<uint64_t>& acl_commands, const AclKeys& keys,
CmdArgList tail_args, const CommandId& id);

bool IsUserAllowedToInvokeCommand(const ConnectionContext& cntx, const CommandId& id,
CmdArgList tail_args);
Expand Down
12 changes: 12 additions & 0 deletions tests/dragonfly/acl_family_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,18 @@ async def test_acl_log(async_client):
res = await async_client.execute_command("ACL LOG")
assert 2 == len(res)

res = await async_client.execute_command("ACL LOG RESET")
await async_client.execute_command("ACL SETUSER elon resetkeys ~foo")

with pytest.raises(redis.exceptions.ResponseError):
await async_client.execute_command("SET bar val")

res = await async_client.execute_command("ACL LOG")
assert 1 == len(res)
assert res[0]["reason"] == "KEY"
assert res[0]["object"] == "SET"
assert res[0]["username"] == "elon"


@pytest.mark.asyncio
@dfly_args({"port": 1111, "admin_port": 1112, "requirepass": "mypass"})
Expand Down
Loading