chore(Http): change authed username to default and skip auth on metrics (#1950)

* Update Http auth with username default instead of user
* skip auth for /metrics page
* add/improve tests
* fix a bug with admin port requiring auth on http even if nopass was set
* update helio ref
* update listener class to contain its respective Role
* fix http init to only include admin and main listener
This commit is contained in:
Kostas Kyrimis 2023-10-03 10:45:37 +03:00 committed by GitHub
parent 38c4e3b588
commit 945d3a39e4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 144 additions and 99 deletions

2
helio

@ -1 +1 @@
Subproject commit 308aa229f1866a39e833755648687b0415963591
Subproject commit 1a2a1fc5ce2ad09d4dbe310470e06776f2c527ab

View file

@ -335,7 +335,7 @@ void Connection::HandleRequests() {
#ifdef DFLY_USE_SSL
if (ctx_) {
const bool no_tls_on_admin_port = absl::GetFlag(FLAGS_no_tls_on_admin_port);
if (!(IsAdmin() && no_tls_on_admin_port)) {
if (!(IsPrivileged() && no_tls_on_admin_port)) {
unique_ptr<tls::TlsSocket> tls_sock = make_unique<tls::TlsSocket>(std::move(socket_));
tls_sock->InitSSL(ctx_);
FiberSocketBase::AcceptResult aresult = tls_sock->Accept();
@ -465,14 +465,21 @@ uint32_t Connection::GetClientId() const {
return id_;
}
bool Connection::IsAdmin() const {
return static_cast<Listener*>(owner())->IsAdminInterface();
bool Connection::IsPrivileged() const {
return static_cast<Listener*>(owner())->IsPrivilegedInterface();
}
bool Connection::IsMain() const {
return static_cast<Listener*>(owner())->IsMainInterface();
}
io::Result<bool> Connection::CheckForHttpProto(FiberSocketBase* peer) {
bool primary_port_enabled = absl::GetFlag(FLAGS_primary_port_http_enabled);
bool admin = IsAdmin();
if (!primary_port_enabled && !admin) {
if (!IsPrivileged() && !IsMain()) {
return false;
}
const bool primary_port_enabled = absl::GetFlag(FLAGS_primary_port_http_enabled);
if (!primary_port_enabled && !IsPrivileged()) {
return false;
}

View file

@ -163,7 +163,9 @@ class Connection : public util::Connection {
uint32_t GetClientId() const;
// Virtual because behavior is overridden in test_utils.
virtual bool IsAdmin() const;
virtual bool IsPrivileged() const;
bool IsMain() const;
Protocol protocol() const {
return protocol_;

View file

@ -6,6 +6,8 @@
#include <openssl/err.h>
#include <memory>
#include "facade/tls_error.h"
#ifdef DFLY_USE_SSL
@ -34,6 +36,8 @@ ABSL_FLAG(uint32_t, tcp_keepalive, 300,
"the period in seconds of inactivity after which keep-alives are triggerred,"
"the duration until an inactive connection is terminated is twice the specified time");
ABSL_DECLARE_FLAG(bool, primary_port_http_enabled);
#if 0
enum TlsClientAuth {
CL_AUTH_NO = 0,
@ -138,17 +142,24 @@ bool ConfigureKeepAlive(int fd) {
} // namespace
Listener::Listener(Protocol protocol, ServiceInterface* si) : service_(si), protocol_(protocol) {
Listener::Listener(Protocol protocol, ServiceInterface* si, Role role)
: service_(si), protocol_(protocol) {
#ifdef DFLY_USE_SSL
if (GetFlag(FLAGS_tls)) {
OPENSSL_init_ssl(OPENSSL_INIT_SSL_DEFAULT, NULL);
ctx_ = CreateSslServerCntx();
}
#endif
http_base_.reset(new HttpListener<>);
http_base_->set_resource_prefix("http://static.dragonflydb.io/data-plane");
si->ConfigureHttpHandlers(http_base_.get());
role_ = role;
// We only set the HTTP interface for:
// 1. Privileged users (on privileged listener)
// 2. Main listener (if enabled)
const bool is_main_enabled = GetFlag(FLAGS_primary_port_http_enabled);
if (IsPrivilegedInterface() || (IsMainInterface() && is_main_enabled)) {
http_base_ = std::make_unique<HttpListener<>>();
http_base_->set_resource_prefix("http://static.dragonflydb.io/data-plane");
si->ConfigureHttpHandlers(http_base_.get(), IsPrivilegedInterface());
}
}
Listener::~Listener() {
@ -213,12 +224,12 @@ bool Listener::AwaitDispatches(absl::Duration timeout,
return false;
}
bool Listener::IsAdminInterface() const {
return is_admin_;
bool Listener::IsPrivilegedInterface() const {
return role_ == Role::PRIVILEGED;
}
void Listener::SetAdminInterface(bool is_admin) {
is_admin_ = is_admin;
bool Listener::IsMainInterface() const {
return role_ == Role::MAIN;
}
void Listener::PreShutdown() {

View file

@ -25,7 +25,11 @@ class ServiceInterface;
class Listener : public util::ListenerInterface {
public:
Listener(Protocol protocol, ServiceInterface*);
// The Role PRIVILEGED is for admin port/listener
// The Role MAIN is for the main listener on main port
// The Role OTHER is for all the other listeners
enum class Role { PRIVILEGED, MAIN, OTHER };
Listener(Protocol protocol, ServiceInterface*, Role role = Role::OTHER);
~Listener();
std::error_code ConfigureServerSocket(int fd) final;
@ -35,8 +39,8 @@ class Listener : public util::ListenerInterface {
bool AwaitDispatches(absl::Duration timeout,
const std::function<bool(util::Connection*)>& filter);
bool IsAdminInterface() const;
void SetAdminInterface(bool is_admin = true);
bool IsPrivilegedInterface() const;
bool IsMainInterface() const;
private:
util::Connection* NewConnection(ProactorBase* proactor) final;
@ -62,7 +66,7 @@ class Listener : public util::ListenerInterface {
std::atomic_uint32_t next_id_{0};
bool is_admin_ = false;
Role role_;
uint32_t conn_cnt_{0};
uint32_t min_cnt_thread_id_{0};

View file

@ -36,7 +36,7 @@ class ServiceInterface {
virtual ConnectionStats* GetThreadLocalConnectionStats() = 0;
virtual void ConfigureHttpHandlers(util::HttpListenerBase* base) {
virtual void ConfigureHttpHandlers(util::HttpListenerBase* base, bool is_privileged) {
}
virtual void OnClose(ConnectionContext* cntx) {

View file

@ -378,7 +378,7 @@ void ClusterFamily::DflyCluster(CmdArgList args, ConnectionContext* cntx) {
return (*cntx)->SendError(kClusterDisabled);
}
if (cntx->conn() && !cntx->conn()->IsAdmin()) {
if (cntx->conn() && !cntx->conn()->IsPrivileged()) {
return (*cntx)->SendError(kDflyClusterCmdPort);
}

View file

@ -34,7 +34,7 @@ class ClusterFamilyTest : public BaseFamilyTest {
static constexpr string_view kInvalidConfiguration = "Invalid cluster configuration";
string GetMyId() {
return RunAdmin({"dflycluster", "myid"}).GetString();
return RunPrivileged({"dflycluster", "myid"}).GetString();
}
};
@ -56,13 +56,13 @@ TEST_F(ClusterFamilyTest, DflyClusterOnlyOnAdminPort) {
"replicas": []
}
])json";
EXPECT_EQ(RunAdmin({"dflycluster", "config", config}), "OK");
EXPECT_EQ(RunPrivileged({"dflycluster", "config", config}), "OK");
EXPECT_THAT(Run({"dflycluster", "config", config}),
ErrArg("DflyCluster command allowed only under admin port"));
}
TEST_F(ClusterFamilyTest, ClusterConfigInvalidJSON) {
EXPECT_THAT(RunAdmin({"dflycluster", "config", "invalid JSON"}),
EXPECT_THAT(RunPrivileged({"dflycluster", "config", "invalid JSON"}),
ErrArg("Invalid JSON cluster config"));
string cluster_info = Run({"cluster", "info"}).GetString();
@ -78,7 +78,7 @@ TEST_F(ClusterFamilyTest, ClusterConfigInvalidJSON) {
}
TEST_F(ClusterFamilyTest, ClusterConfigInvalidConfig) {
EXPECT_THAT(RunAdmin({"dflycluster", "config", "[]"}), ErrArg(kInvalidConfiguration));
EXPECT_THAT(RunPrivileged({"dflycluster", "config", "[]"}), ErrArg(kInvalidConfiguration));
string cluster_info = Run({"cluster", "info"}).GetString();
EXPECT_THAT(cluster_info, HasSubstr("cluster_state:fail"));
@ -89,7 +89,7 @@ TEST_F(ClusterFamilyTest, ClusterConfigInvalidConfig) {
}
TEST_F(ClusterFamilyTest, ClusterConfigInvalidMissingSlots) {
EXPECT_THAT(RunAdmin({"dflycluster", "config", R"json(
EXPECT_THAT(RunPrivileged({"dflycluster", "config", R"json(
[
{
"slot_ranges": [
@ -117,7 +117,7 @@ TEST_F(ClusterFamilyTest, ClusterConfigInvalidMissingSlots) {
}
TEST_F(ClusterFamilyTest, ClusterConfigInvalidOverlappingSlots) {
EXPECT_THAT(RunAdmin({"dflycluster", "config", R"json(
EXPECT_THAT(RunPrivileged({"dflycluster", "config", R"json(
[
{
"slot_ranges": [
@ -159,7 +159,7 @@ TEST_F(ClusterFamilyTest, ClusterConfigInvalidOverlappingSlots) {
}
TEST_F(ClusterFamilyTest, ClusterConfigNoReplicas) {
EXPECT_EQ(RunAdmin({"dflycluster", "config", R"json(
EXPECT_EQ(RunPrivileged({"dflycluster", "config", R"json(
[
{
"slot_ranges": [
@ -215,7 +215,7 @@ TEST_F(ClusterFamilyTest, ClusterConfigNoReplicas) {
}
TEST_F(ClusterFamilyTest, ClusterConfigFull) {
EXPECT_EQ(RunAdmin({"dflycluster", "config", R"json(
EXPECT_EQ(RunPrivileged({"dflycluster", "config", R"json(
[
{
"slot_ranges": [
@ -287,7 +287,7 @@ TEST_F(ClusterFamilyTest, ClusterConfigFull) {
}
TEST_F(ClusterFamilyTest, ClusterConfigFullMultipleInstances) {
EXPECT_EQ(RunAdmin({"dflycluster", "config", R"json(
EXPECT_EQ(RunPrivileged({"dflycluster", "config", R"json(
[
{
"slot_ranges": [
@ -440,9 +440,9 @@ TEST_F(ClusterFamilyTest, ClusterConfigFullMultipleInstances) {
TEST_F(ClusterFamilyTest, ClusterGetSlotInfoInvalid) {
constexpr string_view kTooFewArgs =
"ERR wrong number of arguments for 'dflycluster getslotinfo' command";
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo"}), ErrArg(kTooFewArgs));
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "s"}), ErrArg(kTooFewArgs));
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots"}), ErrArg(kTooFewArgs));
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo"}), ErrArg(kTooFewArgs));
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "s"}), ErrArg(kTooFewArgs));
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots"}), ErrArg(kTooFewArgs));
}
TEST_F(ClusterFamilyTest, ClusterGetSlotInfo) {
@ -465,7 +465,7 @@ TEST_F(ClusterFamilyTest, ClusterGetSlotInfo) {
])json";
string config = absl::Substitute(config_template, GetMyId());
EXPECT_EQ(RunAdmin({"dflycluster", "config", config}), "OK");
EXPECT_EQ(RunPrivileged({"dflycluster", "config", config}), "OK");
constexpr string_view kKey = "some-key";
const SlotId slot = ClusterConfig::KeySlot(kKey);
@ -473,7 +473,7 @@ TEST_F(ClusterFamilyTest, ClusterGetSlotInfo) {
EXPECT_EQ(Run({"SET", kKey, "value"}), "OK");
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots", "0", absl::StrCat(slot)}),
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots", "0", absl::StrCat(slot)}),
RespArray(ElementsAre(
RespArray(ElementsAre(IntArg(0), "key_count", IntArg(0), "total_reads", IntArg(0),
"total_writes", IntArg(0))),
@ -482,7 +482,7 @@ TEST_F(ClusterFamilyTest, ClusterGetSlotInfo) {
EXPECT_EQ(Run({"GET", kKey}), "value");
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots", "0", absl::StrCat(slot)}),
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots", "0", absl::StrCat(slot)}),
RespArray(ElementsAre(
RespArray(ElementsAre(IntArg(0), "key_count", IntArg(0), "total_reads", IntArg(0),
"total_writes", IntArg(0))),
@ -491,7 +491,7 @@ TEST_F(ClusterFamilyTest, ClusterGetSlotInfo) {
EXPECT_EQ(Run({"SET", kKey, "value2"}), "OK");
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots", "0", absl::StrCat(slot)}),
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots", "0", absl::StrCat(slot)}),
RespArray(ElementsAre(
RespArray(ElementsAre(IntArg(0), "key_count", IntArg(0), "total_reads", IntArg(0),
"total_writes", IntArg(0))),
@ -519,17 +519,17 @@ TEST_F(ClusterFamilyTest, ClusterSlotsPopulate) {
])json";
string config = absl::Substitute(config_template, GetMyId());
EXPECT_EQ(RunAdmin({"dflycluster", "config", config}), "OK");
EXPECT_EQ(RunPrivileged({"dflycluster", "config", config}), "OK");
Run({"debug", "populate", "10000", "key", "4", "SLOTS", "0", "1000"});
for (int i = 0; i <= 1'000; ++i) {
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots", absl::StrCat(i)}),
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots", absl::StrCat(i)}),
RespArray(ElementsAre(IntArg(i), "key_count", Not(IntArg(0)), _, _, _, _)));
}
for (int i = 1'001; i <= 16'383; ++i) {
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots", absl::StrCat(i)}),
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots", absl::StrCat(i)}),
RespArray(ElementsAre(IntArg(i), "key_count", IntArg(0), _, _, _, _)));
}
}
@ -554,11 +554,11 @@ TEST_F(ClusterFamilyTest, ClusterConfigDeleteSlots) {
])json";
string config = absl::Substitute(config_template, GetMyId());
EXPECT_EQ(RunAdmin({"dflycluster", "config", config}), "OK");
EXPECT_EQ(RunPrivileged({"dflycluster", "config", config}), "OK");
Run({"debug", "populate", "100000"});
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots", "1", "2"}),
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots", "1", "2"}),
RespArray(ElementsAre(
RespArray(ElementsAre(IntArg(1), "key_count", Not(IntArg(0)), "total_reads",
IntArg(0), "total_writes", Not(IntArg(0)))),
@ -566,12 +566,12 @@ TEST_F(ClusterFamilyTest, ClusterConfigDeleteSlots) {
IntArg(0), "total_writes", Not(IntArg(0)))))));
config = absl::Substitute(config_template, "abc");
EXPECT_EQ(RunAdmin({"dflycluster", "config", config}), "OK");
EXPECT_EQ(RunPrivileged({"dflycluster", "config", config}), "OK");
ExpectConditionWithinTimeout([&]() { return CheckedInt({"dbsize"}) == 0; });
EXPECT_THAT(
RunAdmin({"dflycluster", "getslotinfo", "slots", "1", "2"}),
RunPrivileged({"dflycluster", "getslotinfo", "slots", "1", "2"}),
RespArray(ElementsAre(RespArray(ElementsAre(IntArg(1), "key_count", IntArg(0), "total_reads",
IntArg(0), "total_writes", Not(IntArg(0)))),
RespArray(ElementsAre(IntArg(2), "key_count", IntArg(0), "total_reads",
@ -599,11 +599,11 @@ TEST_F(ClusterFamilyTest, ClusterConfigDeleteSlotsNoCrashOnShutdown) {
])json";
string config = absl::Substitute(config_template, GetMyId());
EXPECT_EQ(RunAdmin({"dflycluster", "config", config}), "OK");
EXPECT_EQ(RunPrivileged({"dflycluster", "config", config}), "OK");
Run({"debug", "populate", "100000"});
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots", "1", "2"}),
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots", "1", "2"}),
RespArray(ElementsAre(
RespArray(ElementsAre(IntArg(1), "key_count", Not(IntArg(0)), "total_reads",
IntArg(0), "total_writes", Not(IntArg(0)))),
@ -613,7 +613,7 @@ TEST_F(ClusterFamilyTest, ClusterConfigDeleteSlotsNoCrashOnShutdown) {
config = absl::Substitute(config_template, "abc");
// After running the new config we start a fiber that removes all slots from current instance
// we immediately shut down to test that we do not crash.
EXPECT_EQ(RunAdmin({"dflycluster", "config", config}), "OK");
EXPECT_EQ(RunPrivileged({"dflycluster", "config", config}), "OK");
}
TEST_F(ClusterFamilyTest, ClusterConfigDeleteSomeSlots) {
@ -650,12 +650,12 @@ TEST_F(ClusterFamilyTest, ClusterConfigDeleteSomeSlots) {
])json";
string config = absl::Substitute(config_template, GetMyId());
EXPECT_EQ(RunAdmin({"dflycluster", "config", config}), "OK");
EXPECT_EQ(RunPrivileged({"dflycluster", "config", config}), "OK");
Run({"debug", "populate", "1", "key", "4", "SLOTS", "7999", "7999"});
Run({"debug", "populate", "2", "key", "4", "SLOTS", "8000", "8000"});
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots", "7999", "8000"}),
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots", "7999", "8000"}),
RespArray(ElementsAre(
RespArray(ElementsAre(IntArg(7999), "key_count", IntArg(1), _, _, _, _)),
RespArray(ElementsAre(IntArg(8000), "key_count", IntArg(2), _, _, _, _)))));
@ -663,12 +663,12 @@ TEST_F(ClusterFamilyTest, ClusterConfigDeleteSomeSlots) {
// Move ownership over 8000 to other master
config = absl::StrReplaceAll(config, {{"8000", "7999"}, {"8001", "8000"}});
EXPECT_EQ(RunAdmin({"dflycluster", "config", config}), "OK");
EXPECT_EQ(RunPrivileged({"dflycluster", "config", config}), "OK");
// Verify that keys for slot 8000 were deleted, while key for slot 7999 was kept
ExpectConditionWithinTimeout([&]() { return CheckedInt({"dbsize"}) == 1; });
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots", "7999", "8000"}),
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots", "7999", "8000"}),
RespArray(ElementsAre(
RespArray(ElementsAre(IntArg(7999), "key_count", IntArg(1), _, _, _, _)),
RespArray(ElementsAre(IntArg(8000), "key_count", IntArg(0), _, _, _, _)))));
@ -688,7 +688,7 @@ TEST_F(ClusterFamilyTest, ClusterFirstConfigCallDropsEntriesNotOwnedByNode) {
EXPECT_EQ(Run({"debug", "load", save_info->file_name}), "OK");
EXPECT_EQ(CheckedInt({"dbsize"}), 50000);
EXPECT_EQ(RunAdmin({"dflycluster", "config", R"json(
EXPECT_EQ(RunPrivileged({"dflycluster", "config", R"json(
[
{
"slot_ranges": [
@ -725,25 +725,25 @@ TEST_F(ClusterFamilyTest, Keyslot) {
TEST_F(ClusterFamilyTest, FlushSlots) {
EXPECT_EQ(Run({"debug", "populate", "100", "key", "4", "slots", "0", "1"}), "OK");
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots", "0", "1"}),
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots", "0", "1"}),
RespArray(ElementsAre(RespArray(ElementsAre(IntArg(0), "key_count", Not(IntArg(0)),
"total_reads", _, "total_writes", _)),
RespArray(ElementsAre(IntArg(1), "key_count", Not(IntArg(0)),
"total_reads", _, "total_writes", _)))));
ExpectConditionWithinTimeout([&]() {
return RunAdmin({"dflycluster", "flushslots", "0"}) == "OK";
return RunPrivileged({"dflycluster", "flushslots", "0"}) == "OK";
});
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots", "0", "1"}),
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots", "0", "1"}),
RespArray(ElementsAre(RespArray(ElementsAre(IntArg(0), "key_count", IntArg(0),
"total_reads", _, "total_writes", _)),
RespArray(ElementsAre(IntArg(1), "key_count", Not(IntArg(0)),
"total_reads", _, "total_writes", _)))));
EXPECT_EQ(RunAdmin({"dflycluster", "flushslots", "0", "1"}), "OK");
EXPECT_EQ(RunPrivileged({"dflycluster", "flushslots", "0", "1"}), "OK");
EXPECT_THAT(RunAdmin({"dflycluster", "getslotinfo", "slots", "0", "1"}),
EXPECT_THAT(RunPrivileged({"dflycluster", "getslotinfo", "slots", "0", "1"}),
RespArray(ElementsAre(RespArray(ElementsAre(IntArg(0), "key_count", IntArg(0),
"total_reads", _, "total_writes", _)),
RespArray(ElementsAre(IntArg(1), "key_count", IntArg(0),
@ -770,7 +770,7 @@ TEST_F(ClusterFamilyTest, ClusterCrossSlot) {
])json";
string config = absl::Substitute(config_template, GetMyId());
EXPECT_EQ(RunAdmin({"dflycluster", "config", config}), "OK");
EXPECT_EQ(RunPrivileged({"dflycluster", "config", config}), "OK");
EXPECT_EQ(Run({"SET", "key", "value"}), "OK");
EXPECT_EQ(Run({"GET", "key"}), "value");

View file

@ -348,7 +348,7 @@ bool RunEngine(ProactorPool* pool, AcceptServer* acceptor) {
// we depend on tcp listener to be at the front since we later
// need to pass it to the AclFamily::Init
if (!tcp_disabled) {
main_listener = new Listener{Protocol::REDIS, &service};
main_listener = new Listener{Protocol::REDIS, &service, Listener::Role::MAIN};
listeners.push_back(main_listener);
}
@ -427,8 +427,8 @@ bool RunEngine(ProactorPool* pool, AcceptServer* acceptor) {
const char* interface_addr = admin_bind.empty() ? nullptr : admin_bind.c_str();
const std::string printable_addr =
absl::StrCat("admin socket ", interface_addr ? interface_addr : "any", ":", admin_port);
auto admin_listener = std::make_unique<Listener>(Protocol::REDIS, &service);
admin_listener->SetAdminInterface();
auto admin_listener =
std::make_unique<Listener>(Protocol::REDIS, &service, Listener::Role::PRIVILEGED);
error_code ec = acceptor->AddListener(interface_addr, admin_port, admin_listener.get());
if (ec) {

View file

@ -82,8 +82,8 @@ ABSL_FLAG(uint32_t, multi_eval_squash_buffer, 4_KB, "Max buffer for squashed com
ABSL_DECLARE_FLAG(bool, primary_port_http_enabled);
ABSL_FLAG(bool, admin_nopass, false,
"If set, would enable open admin access to console on the assigned port, without auth "
"token needed.");
"If set, would enable open admin access to console on the assigned port, without "
"authorization needed.");
ABSL_FLAG(MaxMemoryFlag, maxmemory, MaxMemoryFlag{},
"Limit on maximum-memory that is used by the database. "
@ -823,7 +823,7 @@ std::optional<ErrorReply> Service::VerifyCommandState(const CommandId* cid, CmdA
// If there is no connection owner, it means the command it being called
// from another command or used internally, therefore is always permitted.
if (dfly_cntx.conn() != nullptr && !dfly_cntx.conn()->IsAdmin() && cid->IsRestricted()) {
if (dfly_cntx.conn() != nullptr && !dfly_cntx.conn()->IsPrivileged() && cid->IsRestricted()) {
return ErrorReply{"Cannot execute restricted command (admin only)"};
}
@ -1204,7 +1204,7 @@ ErrorReply Service::ReportUnknownCmd(string_view cmd_name) {
return ErrorReply{StrCat("unknown command `", cmd_name, "`"), "unknown_cmd"};
}
bool RequireAdminAuth() {
bool RequirePrivilegedAuth() {
return !GetFlag(FLAGS_admin_nopass);
}
@ -1212,7 +1212,7 @@ facade::ConnectionContext* Service::CreateContext(util::FiberSocketBase* peer,
facade::Connection* owner) {
ConnectionContext* res = new ConnectionContext{peer, owner};
if (owner->IsAdmin() && !RequireAdminAuth()) {
if (owner->IsPrivileged() && !RequirePrivilegedAuth()) {
res->req_auth = false;
} else {
res->req_auth = !GetPassword().empty();
@ -2070,11 +2070,17 @@ GlobalState Service::GetGlobalState() const {
return global_state_;
}
void Service::ConfigureHttpHandlers(util::HttpListenerBase* base) {
// We set the password for the HTTP service unless it is only enabled on the
// admin port and the admin port is password-less.
if (GetFlag(FLAGS_primary_port_http_enabled) || !GetFlag(FLAGS_admin_nopass)) {
base->SetPassword(GetPassword());
void Service::ConfigureHttpHandlers(util::HttpListenerBase* base, bool is_privileged) {
// We skip authentication on privileged listener if the flag admin_nopass is set
const bool should_skip_auth = is_privileged && !RequirePrivilegedAuth();
if (!should_skip_auth) {
base->SetAuthFunctor([pass = GetPassword()](std::string_view path, std::string_view username,
std::string_view password) {
if (path == "/metrics")
return true;
const bool pass_verified = pass.empty() ? true : password == pass;
return username == "default" && pass_verified;
});
}
server_family_.ConfigureMetrics(base);
base->RegisterCb("/txz", TxTable);

View file

@ -91,7 +91,7 @@ class Service : public facade::ServiceInterface {
GlobalState GetGlobalState() const;
void ConfigureHttpHandlers(util::HttpListenerBase* base) final;
void ConfigureHttpHandlers(util::HttpListenerBase* base, bool is_privileged) final;
void OnClose(facade::ConnectionContext* cntx) final;
std::string GetContextInfo(facade::ConnectionContext* cntx) final;

View file

@ -374,7 +374,7 @@ ServerFamily::~ServerFamily() {
void SetMaxClients(std::vector<facade::Listener*>& listeners, uint32_t maxclients) {
for (auto* listener : listeners) {
if (!listener->IsAdminInterface()) {
if (!listener->IsPrivilegedInterface()) {
listener->SetMaxClients(maxclients);
}
}

View file

@ -269,19 +269,19 @@ RespExpr BaseFamilyTest::Run(ArgSlice list) {
return Run(GetId(), list);
}
RespExpr BaseFamilyTest::RunAdmin(std::initializer_list<const std::string_view> list) {
RespExpr BaseFamilyTest::RunPrivileged(std::initializer_list<const std::string_view> list) {
if (!ProactorBase::IsProactorThread()) {
return pp_->at(0)->Await([&] { return this->RunAdmin(list); });
return pp_->at(0)->Await([&] { return this->RunPrivileged(list); });
}
string id = GetId();
TestConnWrapper* conn_wrapper = AddFindConn(Protocol::REDIS, id);
// Before running the command set the connection as admin connection
conn_wrapper->conn()->SetAdmin(true);
conn_wrapper->conn()->SetPrivileged(true);
auto res = Run(id, ArgSlice{list.begin(), list.size()});
// After running the command set the connection as non admin connection
// because the connction is returned to the poll. This way the next call to Run from the same
// thread will not have the connection set as admin.
conn_wrapper->conn()->SetAdmin(false);
conn_wrapper->conn()->SetPrivileged(false);
return res;
}

View file

@ -24,18 +24,18 @@ class TestConnection : public facade::Connection {
void SendPubMessageAsync(PubMessage pmsg) final;
bool IsAdmin() const override {
return is_admin_;
bool IsPrivileged() const override {
return is_privileged_;
}
void SetAdmin(bool is_admin) {
is_admin_ = is_admin;
void SetPrivileged(bool is_privileged) {
is_privileged_ = is_privileged;
}
std::vector<PubMessage> messages;
private:
io::StringSink* sink_;
bool is_admin_ = false;
bool is_privileged_ = false;
};
// The TransactionSuspension class is designed to facilitate the temporary suspension of commands
@ -68,9 +68,9 @@ class BaseFamilyTest : public ::testing::Test {
return Run(ArgSlice{list.begin(), list.size()});
}
// Runs the command in a mocked admin connection
// Runs the command in a mocked privileged connection
// Use for running commands which are allowed only when using admin connection.
RespExpr RunAdmin(std::initializer_list<const std::string_view> list);
RespExpr RunPrivileged(std::initializer_list<const std::string_view> list);
RespExpr Run(ArgSlice list);
RespExpr Run(absl::Span<std::string> list);

View file

@ -2,46 +2,61 @@ import aiohttp
async def test_password(df_factory):
# Needs a private key and certificate.
with df_factory.create(port=1112, requirepass="XXX") as server:
async with aiohttp.ClientSession() as session:
resp = await session.get(f"http://localhost:{server.port}/")
assert resp.status == 401
async with aiohttp.ClientSession(
auth=aiohttp.BasicAuth("user", "wrongpassword")
auth=aiohttp.BasicAuth("default", "wrongpassword")
) as session:
resp = await session.get(f"http://localhost:{server.port}/")
assert resp.status == 401
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("user", "XXX")) as session:
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("default", "XXX")) as session:
resp = await session.get(f"http://localhost:{server.port}/")
assert resp.status == 200
async def test_skip_metrics(df_factory):
with df_factory.create(port=1112, admin_port=1113, requirepass="XXX") as server:
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("whoops", "whoops")) as session:
resp = await session.get(f"http://localhost:{server.port}/metrics")
assert resp.status == 200
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("whoops", "whoops")) as session:
resp = await session.get(f"http://localhost:{server.admin_port}/metrics")
assert resp.status == 200
async def test_no_password_on_admin(df_factory):
# Needs a private key and certificate.
with df_factory.create(
port=1112,
admin_port=1113,
requirepass="XXX",
noprimary_port_http_enabled=None,
admin_nopass=None,
primary_port_http_enabled=True,
admin_nopass=True,
) as server:
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("user", "XXX")) as session:
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("default", "XXX")) as session:
resp = await session.get(f"http://localhost:{server.admin_port}/")
assert resp.status == 200
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("random")) as session:
resp = await session.get(f"http://localhost:{server.admin_port}/")
assert resp.status == 200
async with aiohttp.ClientSession() as session:
resp = await session.get(f"http://localhost:{server.admin_port}/")
assert resp.status == 200
async def test_password_on_admin(df_factory):
# Needs a private key and certificate.
with df_factory.create(
port=1112,
admin_port=1113,
requirepass="XXX",
admin_nopass=None,
) as server:
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("user", "badpass")) as session:
resp = await session.get(f"http://localhost:{server.port}/")
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("default", "badpass")) as session:
resp = await session.get(f"http://localhost:{server.admin_port}/")
assert resp.status == 401
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("user", "XXX")) as session:
resp = await session.get(f"http://localhost:{server.port}/")
async with aiohttp.ClientSession() as session:
resp = await session.get(f"http://localhost:{server.admin_port}/")
assert resp.status == 401
async with aiohttp.ClientSession(auth=aiohttp.BasicAuth("default", "XXX")) as session:
resp = await session.get(f"http://localhost:{server.admin_port}/")
assert resp.status == 200