Skip to content

Commit

Permalink
fix(acl): loading interleaved plain and hashed passwords (#3297)
Browse files Browse the repository at this point in the history
* fix a bug of rehashing hashed passwords while loading plan/hashed passwords from acl file

Signed-off-by: kostas <[email protected]>
  • Loading branch information
kostasrim authored Jul 10, 2024
1 parent 038d081 commit 9782eb2
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/server/acl/helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ std::variant<User::UpdateRequest, ErrorReply> ParseAclSetUser(facade::ArgRange a
req.passwords.push_back(std::move(*pass));

if (hashed && absl::StartsWith(facade::ToSV(arg), "#")) {
req.is_hashed = hashed;
req.passwords.back().is_hashed = true;
}
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion src/server/acl/user.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ void User::Update(UpdateRequest&& req) {
password_hashes_.clear();
continue;
}
SetPasswordHash(pass.password, req.is_hashed);
SetPasswordHash(pass.password, pass.is_hashed);
}

auto cat_visitor = [this](UpdateRequest::CategoryValueType cat) {
Expand Down
1 change: 1 addition & 0 deletions src/server/acl/user.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class User final {
bool unset{false};
bool nopass{false};
bool reset_password{false};
bool is_hashed{false};
};

struct UpdateRequest {
Expand Down
16 changes: 14 additions & 2 deletions tests/dragonfly/acl_family_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,11 @@ async def test_bad_acl_file(df_factory, tmp_dir):
@pytest.mark.asyncio
@dfly_args({"port": 1111})
async def test_good_acl_file(df_factory, tmp_dir):
acl = create_temp_file("USER MrFoo ON >mypass", tmp_dir)
# The hash below is password temp
acl = create_temp_file(
"USER MrFoo ON #a6864eb339b0e1f6e00d75293a8840abf069a2c0fe82e6e53af6ac099793c1d5 >mypass",
tmp_dir,
)
df = df_factory.create(aclfile=acl)

df.start()
Expand All @@ -332,8 +336,16 @@ async def test_good_acl_file(df_factory, tmp_dir):
await client.execute_command("ACL LOAD")
result = await client.execute_command("ACL list")
assert 2 == len(result)
assert "user MrFoo on #ea71c25a7a60224 -@all" in result
assert (
"user MrFoo on #ea71c25a7a60224 #a6864eb339b0e1f -@all" in result
or "user MrFoo on #a6864eb339b0e1f #ea71c25a7a60224 -@all" in result
)
assert "user default on nopass ~* +@all" in result
await client.execute_command("ACL SETUSER MrFoo +@all")
# Check multiple passwords work
assert "OK" == await client.execute_command("AUTH mypass")
assert "OK" == await client.execute_command("AUTH temp")
assert "OK" == await client.execute_command("AUTH default")
await client.execute_command("ACL DELUSER MrFoo")

await client.execute_command("ACL SETUSER roy ON >mypass +@string +hset")
Expand Down

0 comments on commit 9782eb2

Please sign in to comment.