Skip to content

Commit

Permalink
fix: fix user ERR NOAUTH Authentication required (#2939)
Browse files Browse the repository at this point in the history
* fix authrequired bug
  • Loading branch information
buzhimingyonghu authored Nov 22, 2024
1 parent 77177c7 commit 8095602
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 47 deletions.
2 changes: 1 addition & 1 deletion conf/pika.conf
Original file line number Diff line number Diff line change
Expand Up @@ -691,4 +691,4 @@ dont-compact-sst-created-in-seconds : 20
# For OBD_Compact
# According to the number of sst files in rocksdb,
# compact every `compact-every-num-of-files` file.
best-delete-min-ratio : 10
best-delete-min-ratio : 10
7 changes: 3 additions & 4 deletions include/acl.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class AclSelector {
friend User;

public:
explicit AclSelector() : AclSelector(0){};
explicit AclSelector() : AclSelector(0) {};
explicit AclSelector(uint32_t flag);
explicit AclSelector(const AclSelector& selector);
~AclSelector() = default;
Expand All @@ -138,7 +138,7 @@ class AclSelector {
inline bool HasFlags(uint32_t flag) const { return flags_ & flag; };
inline void AddFlags(uint32_t flag) { flags_ |= flag; };
inline void DecFlags(uint32_t flag) { flags_ &= ~flag; };
bool EqualChannel(const std::vector<std::string> &allChannel);
bool EqualChannel(const std::vector<std::string>& allChannel);

private:
pstd::Status SetSelector(const std::string& op);
Expand Down Expand Up @@ -224,8 +224,7 @@ class User {
~User() = default;

std::string Name() const;

// inline uint32_t Flags() const { return flags_; };
// inline uint32_t Flags() const { return flags_; };
inline bool HasFlags(uint32_t flag) const { return flags_ & flag; };
inline void AddFlags(uint32_t flag) { flags_ |= flag; };
inline void DecFlags(uint32_t flag) { flags_ &= ~flag; };
Expand Down
23 changes: 7 additions & 16 deletions include/pika_client_conn.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,15 @@ struct TimeStat {
before_queue_ts_ = 0;
}

uint64_t start_ts() const {
return enqueue_ts_;
}
uint64_t start_ts() const { return enqueue_ts_; }

uint64_t total_time() const {
return process_done_ts_ > enqueue_ts_ ? process_done_ts_ - enqueue_ts_ : 0;
}
uint64_t total_time() const { return process_done_ts_ > enqueue_ts_ ? process_done_ts_ - enqueue_ts_ : 0; }

uint64_t queue_time() const {
return dequeue_ts_ > enqueue_ts_ ? dequeue_ts_ - enqueue_ts_ : 0;
}
uint64_t queue_time() const { return dequeue_ts_ > enqueue_ts_ ? dequeue_ts_ - enqueue_ts_ : 0; }

uint64_t process_time() const {
return process_done_ts_ > dequeue_ts_ ? process_done_ts_ - dequeue_ts_ : 0;
}
uint64_t process_time() const { return process_done_ts_ > dequeue_ts_ ? process_done_ts_ - dequeue_ts_ : 0; }

uint64_t before_queue_time() const {
return process_done_ts_ > dequeue_ts_ ? before_queue_ts_ - enqueue_ts_ : 0;
}
uint64_t before_queue_time() const { return process_done_ts_ > dequeue_ts_ ? before_queue_ts_ - enqueue_ts_ : 0; }

uint64_t enqueue_ts_;
uint64_t dequeue_ts_;
Expand Down Expand Up @@ -94,7 +84,7 @@ class PikaClientConn : public net::RedisConn {
void UnAuth(const std::shared_ptr<User>& user);

bool IsAuthed() const;

void InitUser();
bool AuthRequired() const;

std::string UserName() const;
Expand Down Expand Up @@ -123,6 +113,7 @@ class PikaClientConn : public net::RedisConn {
std::vector<std::shared_ptr<std::string>> resp_array;

std::shared_ptr<TimeStat> time_stat_;

private:
net::ServerThread* const server_thread_;
std::string current_db_;
Expand Down
14 changes: 9 additions & 5 deletions src/acl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,11 @@ void Acl::UpdateDefaultUserPassword(const std::string& pass) {
if (pass.empty()) {
u->SetUser("nopass");
} else {
u->SetUser(">" + pass);
if (g_pika_conf->userpass().empty()) {
u->SetUser("nopass");
} else {
u->SetUser(">" + pass);
}
}
}

Expand All @@ -489,27 +493,27 @@ void Acl::InitLimitUser(const std::string& bl, bool limit_exist) {
auto u = GetUser(DefaultLimitUser);
if (limit_exist) {
if (!bl.empty()) {
for(auto& cmd : blacklist) {
for (auto& cmd : blacklist) {
cmd = pstd::StringTrim(cmd, " ");
u->SetUser("-" + cmd);
}
u->SetUser("on");
}
if (!pass.empty()) {
u->SetUser(">"+pass);
u->SetUser(">" + pass);
}
} else {
if (pass.empty()) {
u->SetUser("nopass");
} else {
u->SetUser(">"+pass);
u->SetUser(">" + pass);
}
u->SetUser("on");
u->SetUser("+@all");
u->SetUser("~*");
u->SetUser("&*");

for(auto& cmd : blacklist) {
for (auto& cmd : blacklist) {
cmd = pstd::StringTrim(cmd, " ");
u->SetUser("-" + cmd);
}
Expand Down
50 changes: 29 additions & 21 deletions src/pika_client_conn.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ PikaClientConn::PikaClientConn(int fd, const std::string& ip_port, net::Thread*
: RedisConn(fd, ip_port, thread, mpx, handle_type, max_conn_rbuf_size),
server_thread_(reinterpret_cast<net::ServerThread*>(thread)),
current_db_(g_pika_conf->default_db()) {
// client init, set client user is default, and authenticated = false
UnAuth(g_pika_server->Acl()->GetUserLock(Acl::DefaultUser));
InitUser();
time_stat_.reset(new TimeStat());
}

Expand Down Expand Up @@ -259,7 +258,7 @@ void PikaClientConn::ProcessMonitor(const PikaCmdArgsType& argv) {
}

bool PikaClientConn::IsInterceptedByRTC(std::string& opt) {
//currently we only Intercept: Get, HGet
// currently we only Intercept: Get, HGet
if (opt == kCmdNameGet && g_pika_conf->GetCacheString()) {
return true;
}
Expand Down Expand Up @@ -289,11 +288,9 @@ void PikaClientConn::ProcessRedisCmds(const std::vector<net::RedisCmdArgsType>&
bool is_slow_cmd = g_pika_conf->is_slow_cmd(opt);
bool is_admin_cmd = g_pika_conf->is_admin_cmd(opt);

//we don't intercept pipeline batch (argvs.size() > 1)
if (g_pika_conf->rtc_cache_read_enabled() &&
argvs.size() == 1 && IsInterceptedByRTC(opt) &&
PIKA_CACHE_NONE != g_pika_conf->cache_mode() &&
!IsInTxn()) {
// we don't intercept pipeline batch (argvs.size() > 1)
if (g_pika_conf->rtc_cache_read_enabled() && argvs.size() == 1 && IsInterceptedByRTC(opt) &&
PIKA_CACHE_NONE != g_pika_conf->cache_mode() && !IsInTxn()) {
// read in cache
if (ReadCmdInCache(argvs[0], opt)) {
delete arg;
Expand Down Expand Up @@ -358,21 +355,17 @@ bool PikaClientConn::ReadCmdInCache(const net::RedisCmdArgsType& argv, const std
resp_num--;
return false;
}
//acl check
// acl check
int8_t subCmdIndex = -1;
std::string errKey;
auto checkRes = user_->CheckUserPermission(c_ptr, argv, subCmdIndex, &errKey);
std::string object;
if (checkRes == AclDeniedCmd::CMD ||
checkRes == AclDeniedCmd::KEY ||
checkRes == AclDeniedCmd::CHANNEL ||
checkRes == AclDeniedCmd::NO_SUB_CMD ||
checkRes == AclDeniedCmd::NO_AUTH
) {
//acl check failed
if (checkRes == AclDeniedCmd::CMD || checkRes == AclDeniedCmd::KEY || checkRes == AclDeniedCmd::CHANNEL ||
checkRes == AclDeniedCmd::NO_SUB_CMD || checkRes == AclDeniedCmd::NO_AUTH) {
// acl check failed
return false;
}
//only read command(Get, HGet) will reach here, no need of record lock
// only read command(Get, HGet) will reach here, no need of record lock
bool read_status = c_ptr->DoReadCommandInCache();
auto cmdstat_map = g_pika_cmd_table_manager->GetCommandStatMap();
resp_num--;
Expand Down Expand Up @@ -547,13 +540,28 @@ void PikaClientConn::UnAuth(const std::shared_ptr<User>& user) {
}

bool PikaClientConn::IsAuthed() const { return authenticated_; }

void PikaClientConn::InitUser() {
if (!g_pika_conf->GetUserBlackList().empty()) {
user_ = g_pika_server->Acl()->GetUserLock(Acl::DefaultLimitUser);
} else {
user_ = g_pika_server->Acl()->GetUserLock(Acl::DefaultUser);
}
authenticated_ = user_->HasFlags(static_cast<uint32_t>(AclUserFlag::NO_PASS)) &&
!user_->HasFlags(static_cast<uint32_t>(AclUserFlag::DISABLED));
}
bool PikaClientConn::AuthRequired() const {
// If the user does not have a password, and the user is valid, then the user does not need authentication
// Otherwise, you need to determine whether go has been authenticated
return (!user_->HasFlags(static_cast<uint32_t>(AclUserFlag::NO_PASS)) ||
user_->HasFlags(static_cast<uint32_t>(AclUserFlag::DISABLED))) &&
!IsAuthed();
if (IsAuthed()) {
return false;
}
if (user_->HasFlags(static_cast<uint32_t>(AclUserFlag::DISABLED))) {
return true;
}
if (user_->HasFlags(static_cast<uint32_t>(AclUserFlag::NO_PASS))) {
return false;
}
return true;
}
std::string PikaClientConn::UserName() const { return user_->Name(); }

Expand Down

0 comments on commit 8095602

Please sign in to comment.