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

fix: fix user ERR NOAUTH Authentication required #2962

Merged
merged 9 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
8 changes: 4 additions & 4 deletions src/acl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -489,27 +489,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
52 changes: 31 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,30 @@ 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();
LOG(INFO) << "IsAuthed: " << IsAuthed();
LOG(INFO) << "user_->Flags(): " << user_->Flags();
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
Loading