From 360710635df01710329ebe9769482dd184cbbdde Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Mon, 8 Nov 2021 16:04:34 +0800 Subject: [PATCH 01/14] feat: describe the user --- src/clients/meta/MetaClient.cpp | 14 +++++ src/clients/meta/MetaClient.h | 2 + .../executor/admin/ListUsersExecutor.cpp | 34 +++++++++-- src/interface/meta.thrift | 14 +++++ src/meta/MetaServiceHandler.cpp | 6 ++ src/meta/MetaServiceHandler.h | 3 + .../user/AuthenticationProcessor.cpp | 59 +++++++++++++++++++ .../processors/user/AuthenticationProcessor.h | 13 ++++ src/meta/test/AuthProcessorTest.cpp | 2 +- 9 files changed, 140 insertions(+), 7 deletions(-) diff --git a/src/clients/meta/MetaClient.cpp b/src/clients/meta/MetaClient.cpp index ecc8ca83d81..75b63acfbf1 100644 --- a/src/clients/meta/MetaClient.cpp +++ b/src/clients/meta/MetaClient.cpp @@ -2481,6 +2481,20 @@ folly::Future>> MetaClient return future; } +folly::Future>> MetaClient::listUsersWithDesc() { + cpp2::ListUsersReq req; + folly::Promise>> promise; + auto future = promise.getFuture(); + getResponse( + std::move(req), + [](auto client, auto request) { return client->future_listUsersWithDesc(request); }, + [](cpp2::ListUsersWithDescResp&& resp) -> decltype(auto) { + return std::move(resp.get_users()); + }, + std::move(promise)); + return future; +} + folly::Future>> MetaClient::listRoles(GraphSpaceID space) { cpp2::ListRolesReq req; req.set_space_id(std::move(space)); diff --git a/src/clients/meta/MetaClient.h b/src/clients/meta/MetaClient.h index 6605404ca53..e4c786c7675 100644 --- a/src/clients/meta/MetaClient.h +++ b/src/clients/meta/MetaClient.h @@ -360,6 +360,8 @@ class MetaClient { folly::Future>> listUsers(); + folly::Future>> listUsersWithDesc(); + folly::Future>> listRoles(GraphSpaceID space); folly::Future> changePassword(std::string account, diff --git a/src/graph/executor/admin/ListUsersExecutor.cpp b/src/graph/executor/admin/ListUsersExecutor.cpp index b196d660ad3..c98a8acb718 100644 --- a/src/graph/executor/admin/ListUsersExecutor.cpp +++ b/src/graph/executor/admin/ListUsersExecutor.cpp @@ -5,8 +5,11 @@ #include "graph/executor/admin/ListUsersExecutor.h" +#include + #include "graph/context/QueryContext.h" #include "graph/planner/plan/Admin.h" +#include "interface/gen-cpp2/meta_types.h" namespace nebula { namespace graph { @@ -17,18 +20,37 @@ folly::Future ListUsersExecutor::execute() { } folly::Future ListUsersExecutor::listUsers() { - return qctx()->getMetaClient()->listUsers().via(runner()).thenValue( - [this](StatusOr> &&resp) { + return qctx()->getMetaClient()->listUsersWithDesc().via(runner()).thenValue( + [this](StatusOr> &&resp) { SCOPED_TIMER(&execTime_); if (!resp.ok()) { return std::move(resp).status(); } - nebula::DataSet v({"Account"}); + + nebula::DataSet v({"Account", "Roles in spaces"}); auto items = std::move(resp).value(); for (const auto &item : items) { - v.emplace_back(nebula::Row({ - std::move(item).first, - })); + auto spaceRoleMap = item.get_space_role_map(); + std::vector rolesInSpacesStrVector; + for (auto iter = spaceRoleMap.begin(); iter != spaceRoleMap.end(); iter++) { + auto spaceNameResult = qctx_->schemaMng()->toGraphSpaceName(iter->first); + if (!spaceNameResult.ok()) { + if (iter->first == 0) { + rolesInSpacesStrVector.emplace_back(folly::sformat( + "{} in ALL_SPACE", apache::thrift::util::enumNameSafe(iter->second))); + } else { + LOG(ERROR) << "Space name of " << iter->first << " no found"; + return Status::Error("Space not found"); + } + } else { + rolesInSpacesStrVector.emplace_back( + folly::sformat("{} in {}", + apache::thrift::util::enumNameSafe(iter->second), + spaceNameResult.value())); + } + } + v.emplace_back( + nebula::Row({item.get_account(), folly::join(',', rolesInSpacesStrVector)})); } return finish(std::move(v)); }); diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift index 9418a9d7a6b..1b918322570 100644 --- a/src/interface/meta.thrift +++ b/src/interface/meta.thrift @@ -682,6 +682,19 @@ struct ListUsersResp { 3: map (cpp.template = "std::unordered_map") users, } +struct ListUsersWithDescResp { + 1: common.ErrorCode code, + // Valid if ret equals E_LEADER_CHANGED. + 2: common.HostAddr leader, + 3: list users, +} + +struct UserDescItem { + 1: binary account, + // map + 2: map (cpp.template = "std::map") space_role_map, +} + struct ListRolesReq { 1: common.GraphSpaceID space_id, } @@ -1217,6 +1230,7 @@ service MetaService { ExecResp grantRole(1: GrantRoleReq req); ExecResp revokeRole(1: RevokeRoleReq req); ListUsersResp listUsers(1: ListUsersReq req); + ListUsersWithDescResp listUsersWithDesc(1: ListUsersReq req); ListRolesResp listRoles(1: ListRolesReq req); ListRolesResp getUserRoles(1: GetUserRolesReq req); ExecResp changePassword(1: ChangePasswordReq req); diff --git a/src/meta/MetaServiceHandler.cpp b/src/meta/MetaServiceHandler.cpp index a706e4e2ae5..c8a2614b34b 100644 --- a/src/meta/MetaServiceHandler.cpp +++ b/src/meta/MetaServiceHandler.cpp @@ -360,6 +360,12 @@ folly::Future MetaServiceHandler::future_listUsers( RETURN_FUTURE(processor); } +folly::Future MetaServiceHandler::future_listUsersWithDesc( + const cpp2::ListUsersReq& req) { + auto* processor = ListUsersWithDescProcessor::instance(kvstore_); + RETURN_FUTURE(processor); +} + folly::Future MetaServiceHandler::future_listRoles( const cpp2::ListRolesReq& req) { auto* processor = ListRolesProcessor::instance(kvstore_); diff --git a/src/meta/MetaServiceHandler.h b/src/meta/MetaServiceHandler.h index a66a01ecdee..b2ed51327a6 100644 --- a/src/meta/MetaServiceHandler.h +++ b/src/meta/MetaServiceHandler.h @@ -143,6 +143,9 @@ class MetaServiceHandler final : public cpp2::MetaServiceSvIf { folly::Future future_listUsers(const cpp2::ListUsersReq& req) override; + folly::Future future_listUsersWithDesc( + const cpp2::ListUsersReq& req) override; + folly::Future future_listRoles(const cpp2::ListRolesReq& req) override; folly::Future future_changePassword(const cpp2::ChangePasswordReq& req) override; diff --git a/src/meta/processors/user/AuthenticationProcessor.cpp b/src/meta/processors/user/AuthenticationProcessor.cpp index c2e6a70a223..65deb61e200 100644 --- a/src/meta/processors/user/AuthenticationProcessor.cpp +++ b/src/meta/processors/user/AuthenticationProcessor.cpp @@ -252,6 +252,65 @@ void ListUsersProcessor::process(const cpp2::ListUsersReq& req) { onFinished(); } +void ListUsersWithDescProcessor::process(const cpp2::ListUsersReq& req) { + UNUSED(req); + folly::SharedMutex::ReadHolder rHolder1(LockUtils::userLock()); + folly::SharedMutex::ReadHolder rHolder2(LockUtils::spaceLock()); + // list users + std::string userPrefix = "__users__"; + auto usersRet = doPrefix(userPrefix); + if (!nebula::ok(usersRet)) { + auto retCode = nebula::error(usersRet); + LOG(ERROR) << "List User failed, error: " << apache::thrift::util::enumNameSafe(retCode); + handleErrorCode(retCode); + onFinished(); + return; + } + std::unordered_map usersMap; + // trans users to map + auto iter1 = nebula::value(usersRet).get(); + while (iter1->valid()) { + auto account = MetaKeyUtils::parseUser(iter1->key()); + cpp2::UserDescItem user; + user.set_account(account); + std::map spaceRoleMap; + user.set_space_role_map(spaceRoleMap); + usersMap[std::move(account)] = std::move(user); + iter1->next(); + } + // add desc + auto rolePrefix = MetaKeyUtils::rolesPrefix(); + auto roleRet = doPrefix(rolePrefix); + if (!nebula::ok(roleRet)) { + auto retCode = nebula::error(roleRet); + LOG(ERROR) << "Prefix roles failed, error: " << apache::thrift::util::enumNameSafe(retCode); + handleErrorCode(retCode); + onFinished(); + return; + } + auto iter2 = nebula::value(roleRet).get(); + while (iter2->valid()) { + auto account = MetaKeyUtils::parseRoleUser(iter2->key()); + auto userDesc = usersMap.find(account); + if (userDesc != usersMap.end()) { + auto spaceId = MetaKeyUtils::parseRoleSpace(iter2->key()); + auto val = iter2->val(); + auto map = userDesc->second.get_space_role_map(); + map[spaceId] = *reinterpret_cast(val.begin()); + userDesc->second.set_space_role_map(std::move(map)); + } + iter2->next(); + } + // trans to vector + std::vector users; + for (auto iter = usersMap.begin(); iter != usersMap.end(); iter++) { + users.emplace_back(iter->second); + } + resp_.set_users(std::move(users)); + handleErrorCode(nebula::cpp2::ErrorCode::SUCCEEDED); + onFinished(); +} + void ListRolesProcessor::process(const cpp2::ListRolesReq& req) { auto spaceId = req.get_space_id(); CHECK_SPACE_ID_AND_RETURN(spaceId); diff --git a/src/meta/processors/user/AuthenticationProcessor.h b/src/meta/processors/user/AuthenticationProcessor.h index a4be35dc4c5..deabb4f7290 100644 --- a/src/meta/processors/user/AuthenticationProcessor.h +++ b/src/meta/processors/user/AuthenticationProcessor.h @@ -96,6 +96,19 @@ class ListUsersProcessor : public BaseProcessor { : BaseProcessor(kvstore) {} }; +class ListUsersWithDescProcessor : public BaseProcessor { + public: + static ListUsersWithDescProcessor* instance(kvstore::KVStore* kvstore) { + return new ListUsersWithDescProcessor(kvstore); + } + + void process(const cpp2::ListUsersReq& req); + + private: + explicit ListUsersWithDescProcessor(kvstore::KVStore* kvstore) + : BaseProcessor(kvstore) {} +}; + class ListRolesProcessor : public BaseProcessor { public: static ListRolesProcessor* instance(kvstore::KVStore* kvstore) { diff --git a/src/meta/test/AuthProcessorTest.cpp b/src/meta/test/AuthProcessorTest.cpp index 3c0d95493d4..ab431433efe 100644 --- a/src/meta/test/AuthProcessorTest.cpp +++ b/src/meta/test/AuthProcessorTest.cpp @@ -458,7 +458,7 @@ TEST(AuthProcessorTest, GrantRevokeTest) { // list users { cpp2::ListUsersReq req; - auto* processor = ListUsersProcessor::instance(kv.get()); + auto* processor = ListUsersWithDescProcessor::instance(kv.get()); auto f = processor->getFuture(); processor->process(req); auto resp = std::move(f).get(); From 72baa0b10be994f7948c21ae97562d338d4c7cd5 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Wed, 10 Nov 2021 17:31:55 +0800 Subject: [PATCH 02/14] refactor - add syntax desc user xxx --- src/clients/meta/MetaClient.cpp | 13 ++-- src/clients/meta/MetaClient.h | 2 +- src/graph/executor/CMakeLists.txt | 1 + src/graph/executor/Executor.cpp | 4 ++ .../executor/admin/DescribeUserExecutor.cpp | 61 +++++++++++++++++++ .../executor/admin/DescribeUserExecutor.h | 28 +++++++++ .../executor/admin/ListUsersExecutor.cpp | 34 ++--------- src/graph/planner/plan/Admin.cpp | 6 ++ src/graph/planner/plan/Admin.h | 17 ++++++ src/graph/planner/plan/PlanNode.cpp | 2 + src/graph/planner/plan/PlanNode.h | 1 + src/graph/service/PermissionCheck.cpp | 1 + src/graph/validator/ACLValidator.cpp | 15 +++++ src/graph/validator/ACLValidator.h | 12 ++++ src/graph/validator/Validator.cpp | 2 + src/interface/meta.thrift | 10 ++- src/meta/MetaServiceHandler.cpp | 6 +- src/meta/MetaServiceHandler.h | 4 +- .../user/AuthenticationProcessor.cpp | 55 +++++------------ .../processors/user/AuthenticationProcessor.h | 12 ++-- src/meta/test/AuthProcessorTest.cpp | 2 +- src/parser/AdminSentences.cpp | 4 ++ src/parser/AdminSentences.h | 15 +++++ src/parser/Sentence.h | 1 + src/parser/parser.yy | 12 +++- src/parser/scanner.lex | 2 +- 26 files changed, 228 insertions(+), 94 deletions(-) create mode 100644 src/graph/executor/admin/DescribeUserExecutor.cpp create mode 100644 src/graph/executor/admin/DescribeUserExecutor.h diff --git a/src/clients/meta/MetaClient.cpp b/src/clients/meta/MetaClient.cpp index 75b63acfbf1..e0c473ee355 100644 --- a/src/clients/meta/MetaClient.cpp +++ b/src/clients/meta/MetaClient.cpp @@ -2481,16 +2481,15 @@ folly::Future>> MetaClient return future; } -folly::Future>> MetaClient::listUsersWithDesc() { - cpp2::ListUsersReq req; - folly::Promise>> promise; +folly::Future> MetaClient::describeUser(std::string account) { + cpp2::DescribeUserReq req; + req.set_account(account); + folly::Promise> promise; auto future = promise.getFuture(); getResponse( std::move(req), - [](auto client, auto request) { return client->future_listUsersWithDesc(request); }, - [](cpp2::ListUsersWithDescResp&& resp) -> decltype(auto) { - return std::move(resp.get_users()); - }, + [](auto client, auto request) { return client->future_describeUser(request); }, + [](cpp2::DescribeUserResp&& resp) -> decltype(auto) { return std::move(resp.get_user()); }, std::move(promise)); return future; } diff --git a/src/clients/meta/MetaClient.h b/src/clients/meta/MetaClient.h index e4c786c7675..fe3c4f7a5a3 100644 --- a/src/clients/meta/MetaClient.h +++ b/src/clients/meta/MetaClient.h @@ -360,7 +360,7 @@ class MetaClient { folly::Future>> listUsers(); - folly::Future>> listUsersWithDesc(); + folly::Future> describeUser(std::string account); folly::Future>> listRoles(GraphSpaceID space); diff --git a/src/graph/executor/CMakeLists.txt b/src/graph/executor/CMakeLists.txt index a37e82e309f..e8ee5bb562e 100644 --- a/src/graph/executor/CMakeLists.txt +++ b/src/graph/executor/CMakeLists.txt @@ -49,6 +49,7 @@ nebula_add_library( admin/ChangePasswordExecutor.cpp admin/ListUserRolesExecutor.cpp admin/ListUsersExecutor.cpp + admin/DescribeUserExecutor.cpp admin/ListRolesExecutor.cpp admin/SubmitJobExecutor.cpp admin/BalanceExecutor.cpp diff --git a/src/graph/executor/Executor.cpp b/src/graph/executor/Executor.cpp index fa515f26212..ef5b7321e1b 100644 --- a/src/graph/executor/Executor.cpp +++ b/src/graph/executor/Executor.cpp @@ -22,6 +22,7 @@ #include "graph/executor/admin/CharsetExecutor.h" #include "graph/executor/admin/ConfigExecutor.h" #include "graph/executor/admin/CreateUserExecutor.h" +#include "graph/executor/admin/DescribeUserExecutor.h" #include "graph/executor/admin/DownloadExecutor.h" #include "graph/executor/admin/DropUserExecutor.h" #include "graph/executor/admin/GrantRoleExecutor.h" @@ -389,6 +390,9 @@ Executor *Executor::makeExecutor(QueryContext *qctx, const PlanNode *node) { case PlanNode::Kind::kListRoles: { return pool->add(new ListRolesExecutor(node, qctx)); } + case PlanNode::Kind::kDescribeUser: { + return pool->add(new DescribeUserExecutor(node, qctx)); + } case PlanNode::Kind::kBalanceLeaders: { return pool->add(new BalanceLeadersExecutor(node, qctx)); } diff --git a/src/graph/executor/admin/DescribeUserExecutor.cpp b/src/graph/executor/admin/DescribeUserExecutor.cpp new file mode 100644 index 00000000000..e27e9e8971e --- /dev/null +++ b/src/graph/executor/admin/DescribeUserExecutor.cpp @@ -0,0 +1,61 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License. + */ + +#include "graph/executor/admin/DescribeUserExecutor.h" + +#include + +#include "graph/context/QueryContext.h" +#include "graph/planner/plan/Admin.h" +#include "interface/gen-cpp2/meta_types.h" + +namespace nebula { +namespace graph { + +folly::Future DescribeUserExecutor::execute() { + SCOPED_TIMER(&execTime_); + return describeUser(); +} + +folly::Future DescribeUserExecutor::describeUser() { + auto* duNode = asNode(node()); + return qctx() + ->getMetaClient() + ->describeUser(*duNode->username()) + .via(runner()) + .thenValue([this](StatusOr&& resp) { + SCOPED_TIMER(&execTime_); + if (!resp.ok()) { + return std::move(resp).status(); + } + + nebula::DataSet v({"Account", "Roles in spaces"}); + auto user = std::move(resp).value(); + auto spaceRoleMap = user.get_space_role_map(); + std::vector rolesInSpacesStrVector; + for (auto& item : spaceRoleMap) { + auto spaceNameResult = qctx_->schemaMng()->toGraphSpaceName(item.first); + if (!spaceNameResult.ok()) { + if (item.first == 0) { + rolesInSpacesStrVector.emplace_back(folly::sformat( + " {} in ALL_SPACE", apache::thrift::util::enumNameSafe(item.second))); + } else { + LOG(ERROR) << " Space name of " << item.first << " no found"; + return Status::Error("Space not found"); + } + } else { + rolesInSpacesStrVector.emplace_back( + folly::sformat(" {} in {}", + apache::thrift::util::enumNameSafe(item.second), + spaceNameResult.value())); + } + } + v.emplace_back(nebula::Row({user.get_account(), folly::join(',', rolesInSpacesStrVector)})); + return finish(std::move(v)); + }); +} + +} // namespace graph +} // namespace nebula diff --git a/src/graph/executor/admin/DescribeUserExecutor.h b/src/graph/executor/admin/DescribeUserExecutor.h new file mode 100644 index 00000000000..629e1cb7854 --- /dev/null +++ b/src/graph/executor/admin/DescribeUserExecutor.h @@ -0,0 +1,28 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License. + */ + +#ifndef GRAPH_EXECUTOR_ADMIN_DESCRIBEUSEREXECUTOR_H_ +#define GRAPH_EXECUTOR_ADMIN_DESCRIBEUSEREXECUTOR_H_ + +#include "graph/executor/Executor.h" + +namespace nebula { +namespace graph { + +class DescribeUserExecutor final : public Executor { + public: + DescribeUserExecutor(const PlanNode *node, QueryContext *qctx) + : Executor("DescribeUsersExecutor", node, qctx) {} + + folly::Future execute() override; + + private: + folly::Future describeUser(); +}; + +} // namespace graph +} // namespace nebula + +#endif // GRAPH_EXECUTOR_ADMIN_LISTUSERSEXECUTOR_H_ diff --git a/src/graph/executor/admin/ListUsersExecutor.cpp b/src/graph/executor/admin/ListUsersExecutor.cpp index c98a8acb718..b196d660ad3 100644 --- a/src/graph/executor/admin/ListUsersExecutor.cpp +++ b/src/graph/executor/admin/ListUsersExecutor.cpp @@ -5,11 +5,8 @@ #include "graph/executor/admin/ListUsersExecutor.h" -#include - #include "graph/context/QueryContext.h" #include "graph/planner/plan/Admin.h" -#include "interface/gen-cpp2/meta_types.h" namespace nebula { namespace graph { @@ -20,37 +17,18 @@ folly::Future ListUsersExecutor::execute() { } folly::Future ListUsersExecutor::listUsers() { - return qctx()->getMetaClient()->listUsersWithDesc().via(runner()).thenValue( - [this](StatusOr> &&resp) { + return qctx()->getMetaClient()->listUsers().via(runner()).thenValue( + [this](StatusOr> &&resp) { SCOPED_TIMER(&execTime_); if (!resp.ok()) { return std::move(resp).status(); } - - nebula::DataSet v({"Account", "Roles in spaces"}); + nebula::DataSet v({"Account"}); auto items = std::move(resp).value(); for (const auto &item : items) { - auto spaceRoleMap = item.get_space_role_map(); - std::vector rolesInSpacesStrVector; - for (auto iter = spaceRoleMap.begin(); iter != spaceRoleMap.end(); iter++) { - auto spaceNameResult = qctx_->schemaMng()->toGraphSpaceName(iter->first); - if (!spaceNameResult.ok()) { - if (iter->first == 0) { - rolesInSpacesStrVector.emplace_back(folly::sformat( - "{} in ALL_SPACE", apache::thrift::util::enumNameSafe(iter->second))); - } else { - LOG(ERROR) << "Space name of " << iter->first << " no found"; - return Status::Error("Space not found"); - } - } else { - rolesInSpacesStrVector.emplace_back( - folly::sformat("{} in {}", - apache::thrift::util::enumNameSafe(iter->second), - spaceNameResult.value())); - } - } - v.emplace_back( - nebula::Row({item.get_account(), folly::join(',', rolesInSpacesStrVector)})); + v.emplace_back(nebula::Row({ + std::move(item).first, + })); } return finish(std::move(v)); }); diff --git a/src/graph/planner/plan/Admin.cpp b/src/graph/planner/plan/Admin.cpp index 5400ff50100..1154314a761 100644 --- a/src/graph/planner/plan/Admin.cpp +++ b/src/graph/planner/plan/Admin.cpp @@ -136,6 +136,12 @@ std::unique_ptr ChangePassword::explain() const { return desc; } +std::unique_ptr DescribeUser::explain() const { + auto desc = SingleDependencyNode::explain(); + addDescription("username", *username_, desc.get()); + return desc; +} + std::unique_ptr ListUserRoles::explain() const { auto desc = SingleDependencyNode::explain(); addDescription("username", *username_, desc.get()); diff --git a/src/graph/planner/plan/Admin.h b/src/graph/planner/plan/Admin.h index 8caac08f917..62f3bca3fb7 100644 --- a/src/graph/planner/plan/Admin.h +++ b/src/graph/planner/plan/Admin.h @@ -653,6 +653,23 @@ class ListUsers final : public SingleDependencyNode { : SingleDependencyNode(qctx, Kind::kListUsers, dep) {} }; +class DescribeUser final : public SingleDependencyNode { + public: + static DescribeUser* make(QueryContext* qctx, PlanNode* dep, const std::string* username) { + return qctx->objPool()->add(new DescribeUser(qctx, dep, username)); + } + + std::unique_ptr explain() const override; + + const std::string* username() const { return username_; } + + private: + explicit DescribeUser(QueryContext* qctx, PlanNode* dep, const std::string* username) + : SingleDependencyNode(qctx, Kind::kDescribeUser, dep), username_(username) {} + + const std::string* username_; +}; + class ListRoles final : public SingleDependencyNode { public: static ListRoles* make(QueryContext* qctx, PlanNode* dep, GraphSpaceID space) { diff --git a/src/graph/planner/plan/PlanNode.cpp b/src/graph/planner/plan/PlanNode.cpp index 77dc04b5df9..0dc4502ea68 100644 --- a/src/graph/planner/plan/PlanNode.cpp +++ b/src/graph/planner/plan/PlanNode.cpp @@ -148,6 +148,8 @@ const char* PlanNode::toString(PlanNode::Kind kind) { return "ListUserRoles"; case Kind::kListUsers: return "ListUsers"; + case Kind::kDescribeUser: + return "DescribeUser"; case Kind::kListRoles: return "ListRoles"; case Kind::kShowCreateSpace: diff --git a/src/graph/planner/plan/PlanNode.h b/src/graph/planner/plan/PlanNode.h index 7ad7d670f30..4859ad3ca32 100644 --- a/src/graph/planner/plan/PlanNode.h +++ b/src/graph/planner/plan/PlanNode.h @@ -123,6 +123,7 @@ class PlanNode { kListUserRoles, kListUsers, kListRoles, + kDescribeUser, // Snapshot kCreateSnapshot, diff --git a/src/graph/service/PermissionCheck.cpp b/src/graph/service/PermissionCheck.cpp index f23bac7386b..47182ebe76a 100644 --- a/src/graph/service/PermissionCheck.cpp +++ b/src/graph/service/PermissionCheck.cpp @@ -184,6 +184,7 @@ Status PermissionCheck::permissionCheck(ClientSession *session, case Sentence::Kind::kShowRoles: { return PermissionManager::canReadSpace(session, targetSpace); } + case Sentence::Kind::kDescribeUser: case Sentence::Kind::kShowUsers: case Sentence::Kind::kShowSnapshots: case Sentence::Kind::kShowTSClients: diff --git a/src/graph/validator/ACLValidator.cpp b/src/graph/validator/ACLValidator.cpp index 9fd45da9523..a4f647557eb 100644 --- a/src/graph/validator/ACLValidator.cpp +++ b/src/graph/validator/ACLValidator.cpp @@ -131,6 +131,21 @@ Status RevokeRoleValidator::toPlan() { sentence->getAclItemClause()->getRoleType()); } +// describe user +Status DescribeUserValidator::validateImpl() { + auto sentence = static_cast(sentence_); + if (sentence->account()->size() > kUsernameMaxLength) { + return Status::SemanticError("Username exceed maximum length %ld characters.", + kUsernameMaxLength); + } + return Status::OK(); +} + +Status DescribeUserValidator::toPlan() { + auto sentence = static_cast(sentence_); + return genSingleNodePlan(sentence->account()); +} + // show roles in space Status ShowRolesInSpaceValidator::validateImpl() { auto sentence = static_cast(sentence_); diff --git a/src/graph/validator/ACLValidator.h b/src/graph/validator/ACLValidator.h index bf88439383c..7bf805a6544 100644 --- a/src/graph/validator/ACLValidator.h +++ b/src/graph/validator/ACLValidator.h @@ -97,6 +97,18 @@ class RevokeRoleValidator final : public Validator { Status toPlan() override; }; +class DescribeUserValidator final : public Validator { + public: + DescribeUserValidator(Sentence* sentence, QueryContext* context) : Validator(sentence, context) { + setNoSpaceRequired(); + } + + private: + Status validateImpl() override; + + Status toPlan() override; +}; + class ShowRolesInSpaceValidator final : public Validator { public: ShowRolesInSpaceValidator(Sentence* sentence, QueryContext* context) diff --git a/src/graph/validator/Validator.cpp b/src/graph/validator/Validator.cpp index 4d3c0830eeb..29701ba1685 100644 --- a/src/graph/validator/Validator.cpp +++ b/src/graph/validator/Validator.cpp @@ -132,6 +132,8 @@ std::unique_ptr Validator::makeValidator(Sentence* sentence, QueryCon return std::make_unique(sentence, context); case Sentence::Kind::kShowRoles: return std::make_unique(sentence, context); + case Sentence::Kind::kDescribeUser: + return std::make_unique(sentence, context); case Sentence::Kind::kBalance: return std::make_unique(sentence, context); case Sentence::Kind::kAdminJob: diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift index 1b918322570..c47506942af 100644 --- a/src/interface/meta.thrift +++ b/src/interface/meta.thrift @@ -682,11 +682,15 @@ struct ListUsersResp { 3: map (cpp.template = "std::unordered_map") users, } -struct ListUsersWithDescResp { +struct DescribeUserReq { + 1: binary account, +} + +struct DescribeUserResp { 1: common.ErrorCode code, // Valid if ret equals E_LEADER_CHANGED. 2: common.HostAddr leader, - 3: list users, + 3: UserDescItem user, } struct UserDescItem { @@ -1230,7 +1234,7 @@ service MetaService { ExecResp grantRole(1: GrantRoleReq req); ExecResp revokeRole(1: RevokeRoleReq req); ListUsersResp listUsers(1: ListUsersReq req); - ListUsersWithDescResp listUsersWithDesc(1: ListUsersReq req); + DescribeUserResp describeUser(1: DescribeUserReq req); ListRolesResp listRoles(1: ListRolesReq req); ListRolesResp getUserRoles(1: GetUserRolesReq req); ExecResp changePassword(1: ChangePasswordReq req); diff --git a/src/meta/MetaServiceHandler.cpp b/src/meta/MetaServiceHandler.cpp index c8a2614b34b..70d78d17f03 100644 --- a/src/meta/MetaServiceHandler.cpp +++ b/src/meta/MetaServiceHandler.cpp @@ -360,9 +360,9 @@ folly::Future MetaServiceHandler::future_listUsers( RETURN_FUTURE(processor); } -folly::Future MetaServiceHandler::future_listUsersWithDesc( - const cpp2::ListUsersReq& req) { - auto* processor = ListUsersWithDescProcessor::instance(kvstore_); +folly::Future MetaServiceHandler::future_describeUser( + const cpp2::DescribeUserReq& req) { + auto* processor = DescribeUserProcessor::instance(kvstore_); RETURN_FUTURE(processor); } diff --git a/src/meta/MetaServiceHandler.h b/src/meta/MetaServiceHandler.h index b2ed51327a6..76a28b0e30d 100644 --- a/src/meta/MetaServiceHandler.h +++ b/src/meta/MetaServiceHandler.h @@ -143,8 +143,8 @@ class MetaServiceHandler final : public cpp2::MetaServiceSvIf { folly::Future future_listUsers(const cpp2::ListUsersReq& req) override; - folly::Future future_listUsersWithDesc( - const cpp2::ListUsersReq& req) override; + folly::Future future_describeUser( + const cpp2::DescribeUserReq& req) override; folly::Future future_listRoles(const cpp2::ListRolesReq& req) override; diff --git a/src/meta/processors/user/AuthenticationProcessor.cpp b/src/meta/processors/user/AuthenticationProcessor.cpp index 65deb61e200..72708f9caa1 100644 --- a/src/meta/processors/user/AuthenticationProcessor.cpp +++ b/src/meta/processors/user/AuthenticationProcessor.cpp @@ -252,33 +252,9 @@ void ListUsersProcessor::process(const cpp2::ListUsersReq& req) { onFinished(); } -void ListUsersWithDescProcessor::process(const cpp2::ListUsersReq& req) { +void DescribeUserProcessor::process(const cpp2::DescribeUserReq& req) { UNUSED(req); folly::SharedMutex::ReadHolder rHolder1(LockUtils::userLock()); - folly::SharedMutex::ReadHolder rHolder2(LockUtils::spaceLock()); - // list users - std::string userPrefix = "__users__"; - auto usersRet = doPrefix(userPrefix); - if (!nebula::ok(usersRet)) { - auto retCode = nebula::error(usersRet); - LOG(ERROR) << "List User failed, error: " << apache::thrift::util::enumNameSafe(retCode); - handleErrorCode(retCode); - onFinished(); - return; - } - std::unordered_map usersMap; - // trans users to map - auto iter1 = nebula::value(usersRet).get(); - while (iter1->valid()) { - auto account = MetaKeyUtils::parseUser(iter1->key()); - cpp2::UserDescItem user; - user.set_account(account); - std::map spaceRoleMap; - user.set_space_role_map(spaceRoleMap); - usersMap[std::move(account)] = std::move(user); - iter1->next(); - } - // add desc auto rolePrefix = MetaKeyUtils::rolesPrefix(); auto roleRet = doPrefix(rolePrefix); if (!nebula::ok(roleRet)) { @@ -288,25 +264,22 @@ void ListUsersWithDescProcessor::process(const cpp2::ListUsersReq& req) { onFinished(); return; } - auto iter2 = nebula::value(roleRet).get(); - while (iter2->valid()) { - auto account = MetaKeyUtils::parseRoleUser(iter2->key()); - auto userDesc = usersMap.find(account); - if (userDesc != usersMap.end()) { - auto spaceId = MetaKeyUtils::parseRoleSpace(iter2->key()); - auto val = iter2->val(); - auto map = userDesc->second.get_space_role_map(); + auto& act = req.get_account(); + std::map<::nebula::cpp2::GraphSpaceID, ::nebula::meta::cpp2::RoleType> map; + auto iter = nebula::value(roleRet).get(); + while (iter->valid()) { + auto account = MetaKeyUtils::parseRoleUser(iter->key()); + if (account == act) { + auto spaceId = MetaKeyUtils::parseRoleSpace(iter->key()); + auto val = iter->val(); map[spaceId] = *reinterpret_cast(val.begin()); - userDesc->second.set_space_role_map(std::move(map)); } - iter2->next(); - } - // trans to vector - std::vector users; - for (auto iter = usersMap.begin(); iter != usersMap.end(); iter++) { - users.emplace_back(iter->second); + iter->next(); } - resp_.set_users(std::move(users)); + nebula::meta::cpp2::UserDescItem user; + user.set_account(std::move(act)); + user.set_space_role_map(std::move(map)); + resp_.set_user(std::move(user)); handleErrorCode(nebula::cpp2::ErrorCode::SUCCEEDED); onFinished(); } diff --git a/src/meta/processors/user/AuthenticationProcessor.h b/src/meta/processors/user/AuthenticationProcessor.h index deabb4f7290..6a4dc9821db 100644 --- a/src/meta/processors/user/AuthenticationProcessor.h +++ b/src/meta/processors/user/AuthenticationProcessor.h @@ -96,17 +96,17 @@ class ListUsersProcessor : public BaseProcessor { : BaseProcessor(kvstore) {} }; -class ListUsersWithDescProcessor : public BaseProcessor { +class DescribeUserProcessor : public BaseProcessor { public: - static ListUsersWithDescProcessor* instance(kvstore::KVStore* kvstore) { - return new ListUsersWithDescProcessor(kvstore); + static DescribeUserProcessor* instance(kvstore::KVStore* kvstore) { + return new DescribeUserProcessor(kvstore); } - void process(const cpp2::ListUsersReq& req); + void process(const cpp2::DescribeUserReq& req); private: - explicit ListUsersWithDescProcessor(kvstore::KVStore* kvstore) - : BaseProcessor(kvstore) {} + explicit DescribeUserProcessor(kvstore::KVStore* kvstore) + : BaseProcessor(kvstore) {} }; class ListRolesProcessor : public BaseProcessor { diff --git a/src/meta/test/AuthProcessorTest.cpp b/src/meta/test/AuthProcessorTest.cpp index ab431433efe..3c0d95493d4 100644 --- a/src/meta/test/AuthProcessorTest.cpp +++ b/src/meta/test/AuthProcessorTest.cpp @@ -458,7 +458,7 @@ TEST(AuthProcessorTest, GrantRevokeTest) { // list users { cpp2::ListUsersReq req; - auto* processor = ListUsersWithDescProcessor::instance(kv.get()); + auto* processor = ListUsersProcessor::instance(kv.get()); auto f = processor->getFuture(); processor->process(req); auto resp = std::move(f).get(); diff --git a/src/parser/AdminSentences.cpp b/src/parser/AdminSentences.cpp index 1e1dd237f65..62f687a240a 100644 --- a/src/parser/AdminSentences.cpp +++ b/src/parser/AdminSentences.cpp @@ -27,6 +27,10 @@ std::string ShowPartsSentence::toString() const { return std::string("SHOW PARTS std::string ShowUsersSentence::toString() const { return std::string("SHOW USERS"); } +std::string DescribeUserSentence::toString() const { + return folly::stringPrintf("DESCRIBE USER %s", account_.get()->c_str()); +} + std::string ShowRolesSentence::toString() const { return folly::stringPrintf("SHOW ROLES IN %s", name_.get()->c_str()); } diff --git a/src/parser/AdminSentences.h b/src/parser/AdminSentences.h index 71a3e994569..7ebb2005ebe 100644 --- a/src/parser/AdminSentences.h +++ b/src/parser/AdminSentences.h @@ -80,6 +80,21 @@ class ShowUsersSentence : public Sentence { std::string toString() const override; }; +class DescribeUserSentence : public Sentence { + public: + explicit DescribeUserSentence(std::string* account) { + account_.reset(account); + kind_ = Kind::kDescribeUser; + } + + std::string toString() const override; + + const std::string* account() const { return account_.get(); } + + private: + std::unique_ptr account_; +}; + class ShowRolesSentence : public Sentence { public: explicit ShowRolesSentence(std::string* name) { diff --git a/src/parser/Sentence.h b/src/parser/Sentence.h index 05552da6b89..fcfca4727fd 100644 --- a/src/parser/Sentence.h +++ b/src/parser/Sentence.h @@ -79,6 +79,7 @@ class Sentence { kShowStats, kShowTSClients, kShowFTIndexes, + kDescribeUser, kDeleteVertices, kDeleteTags, kDeleteEdges, diff --git a/src/parser/parser.yy b/src/parser/parser.yy index 7b5259a7eaf..638abd89035 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -360,7 +360,7 @@ static constexpr size_t kCommentLengthLimit = 256; %type add_listener_sentence remove_listener_sentence list_listener_sentence %type admin_job_sentence -%type create_user_sentence alter_user_sentence drop_user_sentence change_password_sentence +%type create_user_sentence alter_user_sentence drop_user_sentence change_password_sentence describe_user_sentence %type show_queries_sentence kill_query_sentence %type show_sentence @@ -2357,6 +2357,15 @@ column_property } ; +describe_user_sentence + : KW_DESCRIBE KW_USER name_label { + $$ = new DescribeUserSentence($3); + } + | KW_DESC KW_USER name_label { + $$ = new DescribeUserSentence($3); + } + ; + describe_tag_sentence : KW_DESCRIBE KW_TAG name_label { $$ = new DescribeTagSentence($3); @@ -3527,6 +3536,7 @@ maintain_sentence | show_sentence { $$ = $1; } | create_user_sentence { $$ = $1; } | alter_user_sentence { $$ = $1; } + | describe_user_sentence { $$ = $1; } | drop_user_sentence { $$ = $1; } | change_password_sentence { $$ = $1; } | grant_sentence { $$ = $1; } diff --git a/src/parser/scanner.lex b/src/parser/scanner.lex index 6c316e4b2bf..da804d09927 100644 --- a/src/parser/scanner.lex +++ b/src/parser/scanner.lex @@ -62,7 +62,6 @@ IP_OCTET ([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]) "INSERT" { return TokenType::KW_INSERT; } "YIELD" { return TokenType::KW_YIELD; } "RETURN" { return TokenType::KW_RETURN; } -"DESCRIBE" { return TokenType::KW_DESCRIBE; } "DESC" { return TokenType::KW_DESC; } "VERTEX" { return TokenType::KW_VERTEX; } "VERTICES" { return TokenType::KW_VERTICES; } @@ -105,6 +104,7 @@ IP_OCTET ([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]) "NO" { return TokenType::KW_NO; } "OVERWRITE" { return TokenType::KW_OVERWRITE; } "SHOW" { return TokenType::KW_SHOW; } +"DESCRIBE" { return TokenType::KW_DESCRIBE; } "ADD" { return TokenType::KW_ADD; } "CREATE" { return TokenType::KW_CREATE;} "DROP" { return TokenType::KW_DROP; } From 2c81457b5d081f0611e77fb72f03d384287625f8 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Thu, 11 Nov 2021 12:07:24 +0800 Subject: [PATCH 03/14] bug - add permission check --- src/graph/service/PermissionManager.cpp | 20 ++++++++++++++++++++ src/graph/service/PermissionManager.h | 1 + src/graph/validator/ACLValidator.cpp | 5 +++++ src/graph/validator/ACLValidator.h | 2 ++ src/parser/scanner.lex | 2 +- 5 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/graph/service/PermissionManager.cpp b/src/graph/service/PermissionManager.cpp index a0ea92b5229..1888cf837d7 100644 --- a/src/graph/service/PermissionManager.cpp +++ b/src/graph/service/PermissionManager.cpp @@ -113,6 +113,26 @@ Status PermissionManager::canWriteUser(ClientSession *session) { } } +// static +Status PermissionManager::canReadUser(ClientSession *session, const std::string &targetUser) { + if (!FLAGS_enable_authorize) { + return Status::OK(); + } + // Cloud auth user cannot create user + if (FLAGS_auth_type == "cloud") { + return Status::PermissionError("Cloud authenticate user can't write user."); + } + if (session->isGod()) { + return Status::OK(); + } + + if (session->user() == targetUser) { + return Status::OK(); + } + + return Status::PermissionError("No permission to read user."); +} + Status PermissionManager::canWriteRole(ClientSession *session, meta::cpp2::RoleType targetRole, GraphSpaceID spaceId, diff --git a/src/graph/service/PermissionManager.h b/src/graph/service/PermissionManager.h index 335e8d7fdaf..655b8dd420f 100644 --- a/src/graph/service/PermissionManager.h +++ b/src/graph/service/PermissionManager.h @@ -24,6 +24,7 @@ class PermissionManager final { static Status canWriteSpace(ClientSession *session); static Status canWriteSchema(ClientSession *session, ValidateContext *vctx); static Status canWriteUser(ClientSession *session); + static Status canReadUser(ClientSession *session, const std::string &targetUser); static Status canWriteRole(ClientSession *session, meta::cpp2::RoleType targetRole, GraphSpaceID spaceId, diff --git a/src/graph/validator/ACLValidator.cpp b/src/graph/validator/ACLValidator.cpp index a4f647557eb..157b425ecff 100644 --- a/src/graph/validator/ACLValidator.cpp +++ b/src/graph/validator/ACLValidator.cpp @@ -141,6 +141,11 @@ Status DescribeUserValidator::validateImpl() { return Status::OK(); } +Status DescribeUserValidator::checkPermission() { + auto sentence = static_cast(sentence_); + return PermissionManager::canReadUser(qctx_->rctx()->session(), *sentence->account()); +} + Status DescribeUserValidator::toPlan() { auto sentence = static_cast(sentence_); return genSingleNodePlan(sentence->account()); diff --git a/src/graph/validator/ACLValidator.h b/src/graph/validator/ACLValidator.h index 7bf805a6544..616d6bc4eed 100644 --- a/src/graph/validator/ACLValidator.h +++ b/src/graph/validator/ACLValidator.h @@ -106,6 +106,8 @@ class DescribeUserValidator final : public Validator { private: Status validateImpl() override; + Status checkPermission() override; + Status toPlan() override; }; diff --git a/src/parser/scanner.lex b/src/parser/scanner.lex index da804d09927..6c316e4b2bf 100644 --- a/src/parser/scanner.lex +++ b/src/parser/scanner.lex @@ -62,6 +62,7 @@ IP_OCTET ([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]) "INSERT" { return TokenType::KW_INSERT; } "YIELD" { return TokenType::KW_YIELD; } "RETURN" { return TokenType::KW_RETURN; } +"DESCRIBE" { return TokenType::KW_DESCRIBE; } "DESC" { return TokenType::KW_DESC; } "VERTEX" { return TokenType::KW_VERTEX; } "VERTICES" { return TokenType::KW_VERTICES; } @@ -104,7 +105,6 @@ IP_OCTET ([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]) "NO" { return TokenType::KW_NO; } "OVERWRITE" { return TokenType::KW_OVERWRITE; } "SHOW" { return TokenType::KW_SHOW; } -"DESCRIBE" { return TokenType::KW_DESCRIBE; } "ADD" { return TokenType::KW_ADD; } "CREATE" { return TokenType::KW_CREATE;} "DROP" { return TokenType::KW_DROP; } From c2fdd3ac7dd86e25c7b2ac01f7b795dff2adbb78 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Thu, 11 Nov 2021 14:37:34 +0800 Subject: [PATCH 04/14] style - permissionCheck --- src/graph/service/PermissionManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/graph/service/PermissionManager.cpp b/src/graph/service/PermissionManager.cpp index 1888cf837d7..fa85a2f1757 100644 --- a/src/graph/service/PermissionManager.cpp +++ b/src/graph/service/PermissionManager.cpp @@ -130,7 +130,7 @@ Status PermissionManager::canReadUser(ClientSession *session, const std::string return Status::OK(); } - return Status::PermissionError("No permission to read user."); + return Status::PermissionError("No permission to read user `%s'.", targetUser.c_str()); } Status PermissionManager::canWriteRole(ClientSession *session, From 5f86501874594944f61fb05f830e85f072ebe6bd Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Thu, 11 Nov 2021 16:18:35 +0800 Subject: [PATCH 05/14] fix - user not exist --- src/meta/processors/user/AuthenticationProcessor.cpp | 11 ++++++++++- tests/tck/features/user/User.feature | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/meta/processors/user/AuthenticationProcessor.cpp b/src/meta/processors/user/AuthenticationProcessor.cpp index 72708f9caa1..e100a7aaaf9 100644 --- a/src/meta/processors/user/AuthenticationProcessor.cpp +++ b/src/meta/processors/user/AuthenticationProcessor.cpp @@ -255,6 +255,16 @@ void ListUsersProcessor::process(const cpp2::ListUsersReq& req) { void DescribeUserProcessor::process(const cpp2::DescribeUserReq& req) { UNUSED(req); folly::SharedMutex::ReadHolder rHolder1(LockUtils::userLock()); + auto& act = req.get_account(); + auto userRet = userExist(act); + if (userRet != nebula::cpp2::ErrorCode::SUCCEEDED) { + LOG(ERROR) << "Describe user Failed, get user " << act << " failed, " + << " error: " << apache::thrift::util::enumNameSafe(userRet); + handleErrorCode(userRet); + onFinished(); + return; + } + auto rolePrefix = MetaKeyUtils::rolesPrefix(); auto roleRet = doPrefix(rolePrefix); if (!nebula::ok(roleRet)) { @@ -264,7 +274,6 @@ void DescribeUserProcessor::process(const cpp2::DescribeUserReq& req) { onFinished(); return; } - auto& act = req.get_account(); std::map<::nebula::cpp2::GraphSpaceID, ::nebula::meta::cpp2::RoleType> map; auto iter = nebula::value(roleRet).get(); while (iter->valid()) { diff --git a/tests/tck/features/user/User.feature b/tests/tck/features/user/User.feature index 5be998fb107..0dce85b3e20 100644 --- a/tests/tck/features/user/User.feature +++ b/tests/tck/features/user/User.feature @@ -1,3 +1,4 @@ +@li Feature: User & privilege Test Background: From 65f7ddc544e3603cad52581e77a91490c0f28fcc Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Fri, 12 Nov 2021 11:51:02 +0800 Subject: [PATCH 06/14] test - add tck session and cases --- tests/conftest.py | 19 +++++++ tests/tck/conftest.py | 8 ++- tests/tck/features/user/User.feature | 82 ++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index a635ad1a201..92f458f6e6d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -109,6 +109,17 @@ def conn_pool_to_second_graph_service(pytestconfig): yield pool pool.close() + +@pytest.fixture(scope="session") +def conn_pool_to_third_graph_service(pytestconfig): + addr = pytestconfig.getoption("address") + host_addr = ["localhost", get_ports()[1]] + assert len(host_addr) == 2 + pool = get_conn_pool(host_addr[0], host_addr[1]) + yield pool + pool.close() + + @pytest.fixture(scope="session") def conn_pool(conn_pool_to_first_graph_service): return conn_pool_to_first_graph_service @@ -129,6 +140,14 @@ def session_from_second_conn_pool(conn_pool_to_second_graph_service, pytestconfi yield sess sess.release() +@pytest.fixture(scope="class") +def session_from_third_conn_pool(conn_pool_to_third_graph_service, pytestconfig): + user = "user1" + password = "pwd1" + sess = conn_pool_to_third_graph_service.get_session(user, password) + yield sess + sess.release() + @pytest.fixture(scope="class") def session(session_from_first_conn_pool): return session_from_first_conn_pool diff --git a/tests/tck/conftest.py b/tests/tck/conftest.py index 5ddcaddfcd4..3d5cc9f1120 100644 --- a/tests/tck/conftest.py +++ b/tests/tck/conftest.py @@ -496,13 +496,15 @@ def check_plan(plan, graph_spaces): @when(parse("executing query via graph {index:d}:\n{query}")) -def executing_query(query, index, graph_spaces, session_from_first_conn_pool, session_from_second_conn_pool, request): - assert index < 2, "There exists only 0,1 graph: {}".format(index) +def executing_query(query, index, graph_spaces, session_from_first_conn_pool, session_from_second_conn_pool, session_from_third_conn_pool,request): + assert index < 3, "There exists only 0,1,2 graph: {}".format(index) ngql = combine_query(query) if index == 0: exec_query(request, ngql, session_from_first_conn_pool, graph_spaces) - else: + if index == 1: exec_query(request, ngql, session_from_second_conn_pool, graph_spaces) + else: + exec_query(request, ngql, session_from_third_conn_pool, graph_spaces) @then(parse("the result should be, the first {n:d} records in order, and register {column_name} as a list named {key}:\n{result}")) diff --git a/tests/tck/features/user/User.feature b/tests/tck/features/user/User.feature index 0dce85b3e20..93f3c469a90 100644 --- a/tests/tck/features/user/User.feature +++ b/tests/tck/features/user/User.feature @@ -281,3 +281,85 @@ Feature: User & privilege Test DROP SPACE user_tmp_space_3; """ Then the execution should be successful + + Scenario: Describe User + When executing query: + """ + CREATE SPACE IF NOT EXISTS user_tmp_space_4(partition_num=1, replica_factor=1, vid_type=FIXED_STRING(8)) + """ + Then the execution should be successful + And wait 10 seconds + When executing query: + """ + CREATE USER IF NOT EXISTS user1 WITH PASSWORD "pwd1" + """ + Then the execution should be successful + When executing query: + """ + GRANT ROLE ADMIN ON user_tmp_space_4 TO user1 + """ + Then the execution should be successful + When executing query: + """ + CREATE USER IF NOT EXISTS user2 WITH PASSWORD "pwd2" + """ + Then the execution should be successful + When executing query: + """ + GRANT ROLE ADMIN ON user_tmp_space_4 TO user2 + """ + Then the execution should be successful + When executing query: + """ + SHOW USERS + """ + Then the result should contain: + | Account | + | "root" | + | "user1" | + | "user2" | + When executing query: + """ + DESC USER root + """ + Then the result should be, in any order, with relax comparison: + | Account | Roles in spaces | + | "root" | " GOD in ALL_SPACE" | + When executing query: + """ + DESC USER user1 + """ + Then the result should be, in any order, with relax comparison: + | Account | Roles in spaces | + | "user1" | " ADMIN in user_tmp_space_4" | + When executing query: + """ + DESC USER user_not_exist + """ + Then a ExecutionError should be raised at runtime: + When executing query via graph 2: + """ + DESC USER user1 + """ + Then the result should be, in any order, with relax comparison: + | Account | Roles in spaces | + | "user1" | " ADMIN in user_tmp_space_4" | + When executing query via graph 2: + """ + DESC USER user2 + """ + Then a PermissionError should be raised at runtime: + When executing query: + """ + GRANT ROLE GUEST ON user_tmp_space_4 TO user1 + """ + Then the execution should be successful + When executing query via graph 2: + """ + DESC USER root + """ + Then a PermissionError should be raised at runtime: + + + + From c1c8106dd14a5f5bf7b390c3ef25c99a34eaaaec Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Fri, 12 Nov 2021 14:30:12 +0800 Subject: [PATCH 07/14] style - remove annotation --- tests/tck/features/user/User.feature | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/tck/features/user/User.feature b/tests/tck/features/user/User.feature index 93f3c469a90..c1e3bf2fe0b 100644 --- a/tests/tck/features/user/User.feature +++ b/tests/tck/features/user/User.feature @@ -1,4 +1,3 @@ -@li Feature: User & privilege Test Background: From cb29ca5090135d0b98acc9759046383b6a68acfa Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Mon, 15 Nov 2021 16:56:53 +0800 Subject: [PATCH 08/14] refact - table style --- src/graph/executor/admin/DescribeUserExecutor.cpp | 12 ++++-------- tests/tck/features/user/User.feature | 12 ++++++------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/graph/executor/admin/DescribeUserExecutor.cpp b/src/graph/executor/admin/DescribeUserExecutor.cpp index e27e9e8971e..0b684230db3 100644 --- a/src/graph/executor/admin/DescribeUserExecutor.cpp +++ b/src/graph/executor/admin/DescribeUserExecutor.cpp @@ -31,7 +31,7 @@ folly::Future DescribeUserExecutor::describeUser() { return std::move(resp).status(); } - nebula::DataSet v({"Account", "Roles in spaces"}); + nebula::DataSet v({"Role", "Space"}); auto user = std::move(resp).value(); auto spaceRoleMap = user.get_space_role_map(); std::vector rolesInSpacesStrVector; @@ -39,20 +39,16 @@ folly::Future DescribeUserExecutor::describeUser() { auto spaceNameResult = qctx_->schemaMng()->toGraphSpaceName(item.first); if (!spaceNameResult.ok()) { if (item.first == 0) { - rolesInSpacesStrVector.emplace_back(folly::sformat( - " {} in ALL_SPACE", apache::thrift::util::enumNameSafe(item.second))); + v.emplace_back(nebula::Row({apache::thrift::util::enumNameSafe(item.second), ""})); } else { LOG(ERROR) << " Space name of " << item.first << " no found"; return Status::Error("Space not found"); } } else { - rolesInSpacesStrVector.emplace_back( - folly::sformat(" {} in {}", - apache::thrift::util::enumNameSafe(item.second), - spaceNameResult.value())); + v.emplace_back(nebula::Row( + {apache::thrift::util::enumNameSafe(item.second), spaceNameResult.value()})); } } - v.emplace_back(nebula::Row({user.get_account(), folly::join(',', rolesInSpacesStrVector)})); return finish(std::move(v)); }); } diff --git a/tests/tck/features/user/User.feature b/tests/tck/features/user/User.feature index c1e3bf2fe0b..788e0e95e47 100644 --- a/tests/tck/features/user/User.feature +++ b/tests/tck/features/user/User.feature @@ -322,15 +322,15 @@ Feature: User & privilege Test DESC USER root """ Then the result should be, in any order, with relax comparison: - | Account | Roles in spaces | - | "root" | " GOD in ALL_SPACE" | + | Role | Space | + | "GOD" | "" | When executing query: """ DESC USER user1 """ Then the result should be, in any order, with relax comparison: - | Account | Roles in spaces | - | "user1" | " ADMIN in user_tmp_space_4" | + | Role | Space | + | "ADMIN" | "user_tmp_space_4" | When executing query: """ DESC USER user_not_exist @@ -341,8 +341,8 @@ Feature: User & privilege Test DESC USER user1 """ Then the result should be, in any order, with relax comparison: - | Account | Roles in spaces | - | "user1" | " ADMIN in user_tmp_space_4" | + | Role | Space | + | "ADMIN" | "user_tmp_space_4" | When executing query via graph 2: """ DESC USER user2 From 062754ca67350cc445024a5379b2dac5dc159215 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Mon, 15 Nov 2021 18:38:00 +0800 Subject: [PATCH 09/14] feat - compatible with pipe --- src/graph/executor/admin/DescribeUserExecutor.cpp | 5 +++-- src/graph/validator/ACLValidator.cpp | 5 +++++ src/parser/parser.yy | 2 +- tests/tck/features/user/User.feature | 13 ++++++++++--- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/graph/executor/admin/DescribeUserExecutor.cpp b/src/graph/executor/admin/DescribeUserExecutor.cpp index 0b684230db3..ff19047ddd6 100644 --- a/src/graph/executor/admin/DescribeUserExecutor.cpp +++ b/src/graph/executor/admin/DescribeUserExecutor.cpp @@ -31,7 +31,7 @@ folly::Future DescribeUserExecutor::describeUser() { return std::move(resp).status(); } - nebula::DataSet v({"Role", "Space"}); + DataSet v({"role", "space"}); auto user = std::move(resp).value(); auto spaceRoleMap = user.get_space_role_map(); std::vector rolesInSpacesStrVector; @@ -49,7 +49,8 @@ folly::Future DescribeUserExecutor::describeUser() { {apache::thrift::util::enumNameSafe(item.second), spaceNameResult.value()})); } } - return finish(std::move(v)); + return finish( + ResultBuilder().value(Value(std::move(v))).iter(Iterator::Kind::kSequential).build()); }); } diff --git a/src/graph/validator/ACLValidator.cpp b/src/graph/validator/ACLValidator.cpp index 157b425ecff..3f570f2e67a 100644 --- a/src/graph/validator/ACLValidator.cpp +++ b/src/graph/validator/ACLValidator.cpp @@ -138,6 +138,11 @@ Status DescribeUserValidator::validateImpl() { return Status::SemanticError("Username exceed maximum length %ld characters.", kUsernameMaxLength); } + if (!inputs_.empty()) { + return Status::SemanticError("Show queries sentence do not support input"); + } + outputs_.emplace_back("role", Value::Type::STRING); + outputs_.emplace_back("space", Value::Type::STRING); return Status::OK(); } diff --git a/src/parser/parser.yy b/src/parser/parser.yy index 638abd89035..1aa2a6f42f7 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -2647,6 +2647,7 @@ traverse_sentence | delete_edge_sentence { $$ = $1; } | show_queries_sentence { $$ = $1; } | kill_query_sentence { $$ = $1; } + | describe_user_sentence { $$ = $1; } ; piped_sentence @@ -3536,7 +3537,6 @@ maintain_sentence | show_sentence { $$ = $1; } | create_user_sentence { $$ = $1; } | alter_user_sentence { $$ = $1; } - | describe_user_sentence { $$ = $1; } | drop_user_sentence { $$ = $1; } | change_password_sentence { $$ = $1; } | grant_sentence { $$ = $1; } diff --git a/tests/tck/features/user/User.feature b/tests/tck/features/user/User.feature index 788e0e95e47..57819e5d7f7 100644 --- a/tests/tck/features/user/User.feature +++ b/tests/tck/features/user/User.feature @@ -322,15 +322,22 @@ Feature: User & privilege Test DESC USER root """ Then the result should be, in any order, with relax comparison: - | Role | Space | + | role | space | | "GOD" | "" | When executing query: """ DESC USER user1 """ Then the result should be, in any order, with relax comparison: - | Role | Space | + | role | space | | "ADMIN" | "user_tmp_space_4" | + When executing query: + """ + DESC USER user1 | YIELD $-.space as sp + """ + Then the result should be, in any order, with relax comparison: + | sp | + | "user_tmp_space_4" | When executing query: """ DESC USER user_not_exist @@ -341,7 +348,7 @@ Feature: User & privilege Test DESC USER user1 """ Then the result should be, in any order, with relax comparison: - | Role | Space | + | role | space | | "ADMIN" | "user_tmp_space_4" | When executing query via graph 2: """ From 8716a436bbf396ff51be924797e8ea4dd7c92a26 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Wed, 17 Nov 2021 12:53:31 +0800 Subject: [PATCH 10/14] fix - merge conflict --- src/graph/executor/Executor.cpp | 15 --------------- src/graph/validator/Validator.cpp | 2 -- 2 files changed, 17 deletions(-) diff --git a/src/graph/executor/Executor.cpp b/src/graph/executor/Executor.cpp index 2dd27ed2512..d7eb30f1759 100644 --- a/src/graph/executor/Executor.cpp +++ b/src/graph/executor/Executor.cpp @@ -388,21 +388,6 @@ Executor *Executor::makeExecutor(QueryContext *qctx, const PlanNode *node) { case PlanNode::Kind::kDescribeUser: { return pool->add(new DescribeUserExecutor(node, qctx)); } - case PlanNode::Kind::kBalanceLeaders: { - return pool->add(new BalanceLeadersExecutor(node, qctx)); - } - case PlanNode::Kind::kBalance: { - return pool->add(new BalanceExecutor(node, qctx)); - } - case PlanNode::Kind::kStopBalance: { - return pool->add(new StopBalanceExecutor(node, qctx)); - } - case PlanNode::Kind::kResetBalance: { - return pool->add(new ResetBalanceExecutor(node, qctx)); - } - case PlanNode::Kind::kShowBalance: { - return pool->add(new ShowBalanceExecutor(node, qctx)); - } case PlanNode::Kind::kShowConfigs: { return pool->add(new ShowConfigsExecutor(node, qctx)); } diff --git a/src/graph/validator/Validator.cpp b/src/graph/validator/Validator.cpp index b49346ff0c2..7513a7ac93a 100644 --- a/src/graph/validator/Validator.cpp +++ b/src/graph/validator/Validator.cpp @@ -133,8 +133,6 @@ std::unique_ptr Validator::makeValidator(Sentence* sentence, QueryCon return std::make_unique(sentence, context); case Sentence::Kind::kDescribeUser: return std::make_unique(sentence, context); - case Sentence::Kind::kBalance: - return std::make_unique(sentence, context); case Sentence::Kind::kAdminJob: case Sentence::Kind::kAdminShowJobs: return std::make_unique(sentence, context); From 114c1e417da1f555f548851a777c0426f81f4172 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Wed, 17 Nov 2021 13:39:17 +0800 Subject: [PATCH 11/14] style - format --- tests/tck/features/user/User.feature | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/tck/features/user/User.feature b/tests/tck/features/user/User.feature index 57819e5d7f7..2b4887b0636 100644 --- a/tests/tck/features/user/User.feature +++ b/tests/tck/features/user/User.feature @@ -365,7 +365,3 @@ Feature: User & privilege Test DESC USER root """ Then a PermissionError should be raised at runtime: - - - - From 76dabc76b62f894b550be7a4957b56efb199fed2 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Tue, 23 Nov 2021 17:08:09 +0800 Subject: [PATCH 12/14] feat - tck query by common user --- tests/conftest.py | 19 ------------------- tests/tck/conftest.py | 14 +++++++++----- tests/tck/features/user/User.feature | 6 +++--- 3 files changed, 12 insertions(+), 27 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 92f458f6e6d..a635ad1a201 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -109,17 +109,6 @@ def conn_pool_to_second_graph_service(pytestconfig): yield pool pool.close() - -@pytest.fixture(scope="session") -def conn_pool_to_third_graph_service(pytestconfig): - addr = pytestconfig.getoption("address") - host_addr = ["localhost", get_ports()[1]] - assert len(host_addr) == 2 - pool = get_conn_pool(host_addr[0], host_addr[1]) - yield pool - pool.close() - - @pytest.fixture(scope="session") def conn_pool(conn_pool_to_first_graph_service): return conn_pool_to_first_graph_service @@ -140,14 +129,6 @@ def session_from_second_conn_pool(conn_pool_to_second_graph_service, pytestconfi yield sess sess.release() -@pytest.fixture(scope="class") -def session_from_third_conn_pool(conn_pool_to_third_graph_service, pytestconfig): - user = "user1" - password = "pwd1" - sess = conn_pool_to_third_graph_service.get_session(user, password) - yield sess - sess.release() - @pytest.fixture(scope="class") def session(session_from_first_conn_pool): return session_from_first_conn_pool diff --git a/tests/tck/conftest.py b/tests/tck/conftest.py index 3d5cc9f1120..637e1e1db32 100644 --- a/tests/tck/conftest.py +++ b/tests/tck/conftest.py @@ -213,6 +213,12 @@ def executing_query(query, graph_spaces, session, request): ngql = combine_query(query) exec_query(request, ngql, session, graph_spaces) +@when(parse("executing query with user {username} with password {password}:\n{query}")) +def executing_query(username, password, conn_pool_to_first_graph_service, query, graph_spaces, request): + sess = conn_pool_to_first_graph_service.get_session(username, password) + ngql = combine_query(query) + exec_query(request, ngql, sess, graph_spaces) + sess.release() @when(parse("profiling query:\n{query}")) def profiling_query(query, graph_spaces, session, request): @@ -496,15 +502,13 @@ def check_plan(plan, graph_spaces): @when(parse("executing query via graph {index:d}:\n{query}")) -def executing_query(query, index, graph_spaces, session_from_first_conn_pool, session_from_second_conn_pool, session_from_third_conn_pool,request): - assert index < 3, "There exists only 0,1,2 graph: {}".format(index) +def executing_query(query, index, graph_spaces, session_from_first_conn_pool, session_from_second_conn_pool, request): + assert index < 2, "There exists only 0,1 graph: {}".format(index) ngql = combine_query(query) if index == 0: exec_query(request, ngql, session_from_first_conn_pool, graph_spaces) - if index == 1: - exec_query(request, ngql, session_from_second_conn_pool, graph_spaces) else: - exec_query(request, ngql, session_from_third_conn_pool, graph_spaces) + exec_query(request, ngql, session_from_second_conn_pool, graph_spaces) @then(parse("the result should be, the first {n:d} records in order, and register {column_name} as a list named {key}:\n{result}")) diff --git a/tests/tck/features/user/User.feature b/tests/tck/features/user/User.feature index 2b4887b0636..e2a364f5b10 100644 --- a/tests/tck/features/user/User.feature +++ b/tests/tck/features/user/User.feature @@ -343,14 +343,14 @@ Feature: User & privilege Test DESC USER user_not_exist """ Then a ExecutionError should be raised at runtime: - When executing query via graph 2: + When executing query with user user1 with password pwd1: """ DESC USER user1 """ Then the result should be, in any order, with relax comparison: | role | space | | "ADMIN" | "user_tmp_space_4" | - When executing query via graph 2: + When executing query with user user1 with password pwd1: """ DESC USER user2 """ @@ -360,7 +360,7 @@ Feature: User & privilege Test GRANT ROLE GUEST ON user_tmp_space_4 TO user1 """ Then the execution should be successful - When executing query via graph 2: + When executing query with user user1 with password pwd1: """ DESC USER root """ From 4681dd0a7a2cf48d98c10aa028ce53a5d16b9ed2 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Wed, 24 Nov 2021 10:07:05 +0800 Subject: [PATCH 13/14] refact - remove duplicate interfaces --- src/clients/meta/MetaClient.cpp | 13 ------ src/clients/meta/MetaClient.h | 2 - .../executor/admin/DescribeUserExecutor.cpp | 27 ++++++------ src/interface/meta.thrift | 18 -------- src/meta/MetaServiceHandler.cpp | 6 --- src/meta/MetaServiceHandler.h | 3 -- .../user/AuthenticationProcessor.cpp | 41 ------------------- .../processors/user/AuthenticationProcessor.h | 13 ------ tests/tck/features/user/User.feature | 3 +- 9 files changed, 15 insertions(+), 111 deletions(-) diff --git a/src/clients/meta/MetaClient.cpp b/src/clients/meta/MetaClient.cpp index d3446a8b1ad..21880d788ae 100644 --- a/src/clients/meta/MetaClient.cpp +++ b/src/clients/meta/MetaClient.cpp @@ -2534,19 +2534,6 @@ folly::Future>> MetaClient return future; } -folly::Future> MetaClient::describeUser(std::string account) { - cpp2::DescribeUserReq req; - req.set_account(account); - folly::Promise> promise; - auto future = promise.getFuture(); - getResponse( - std::move(req), - [](auto client, auto request) { return client->future_describeUser(request); }, - [](cpp2::DescribeUserResp&& resp) -> decltype(auto) { return std::move(resp.get_user()); }, - std::move(promise)); - return future; -} - folly::Future>> MetaClient::listRoles(GraphSpaceID space) { cpp2::ListRolesReq req; req.set_space_id(std::move(space)); diff --git a/src/clients/meta/MetaClient.h b/src/clients/meta/MetaClient.h index ea5fc30c890..4e19f2e79ee 100644 --- a/src/clients/meta/MetaClient.h +++ b/src/clients/meta/MetaClient.h @@ -383,8 +383,6 @@ class MetaClient { folly::Future>> listUsers(); - folly::Future> describeUser(std::string account); - folly::Future>> listRoles(GraphSpaceID space); folly::Future> changePassword(std::string account, diff --git a/src/graph/executor/admin/DescribeUserExecutor.cpp b/src/graph/executor/admin/DescribeUserExecutor.cpp index ff19047ddd6..9b2d893b7f8 100644 --- a/src/graph/executor/admin/DescribeUserExecutor.cpp +++ b/src/graph/executor/admin/DescribeUserExecutor.cpp @@ -23,30 +23,29 @@ folly::Future DescribeUserExecutor::describeUser() { auto* duNode = asNode(node()); return qctx() ->getMetaClient() - ->describeUser(*duNode->username()) + ->getUserRoles(*duNode->username()) .via(runner()) - .thenValue([this](StatusOr&& resp) { + .thenValue([this](StatusOr>&& resp) { SCOPED_TIMER(&execTime_); if (!resp.ok()) { return std::move(resp).status(); } DataSet v({"role", "space"}); - auto user = std::move(resp).value(); - auto spaceRoleMap = user.get_space_role_map(); - std::vector rolesInSpacesStrVector; - for (auto& item : spaceRoleMap) { - auto spaceNameResult = qctx_->schemaMng()->toGraphSpaceName(item.first); - if (!spaceNameResult.ok()) { - if (item.first == 0) { - v.emplace_back(nebula::Row({apache::thrift::util::enumNameSafe(item.second), ""})); + auto roleItemList = std::move(resp).value(); + for (auto& item : roleItemList) { + if (item.get_space_id() == 0) { + v.emplace_back( + nebula::Row({apache::thrift::util::enumNameSafe(item.get_role_type()), ""})); + } else { + auto spaceNameResult = qctx_->schemaMng()->toGraphSpaceName(item.get_space_id()); + if (spaceNameResult.ok()) { + v.emplace_back(nebula::Row({apache::thrift::util::enumNameSafe(item.get_role_type()), + spaceNameResult.value()})); } else { - LOG(ERROR) << " Space name of " << item.first << " no found"; + LOG(ERROR) << " Space name of " << item.get_space_id() << " no found"; return Status::Error("Space not found"); } - } else { - v.emplace_back(nebula::Row( - {apache::thrift::util::enumNameSafe(item.second), spaceNameResult.value()})); } } return finish( diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift index 8a72bbf124e..f75e5751bf1 100644 --- a/src/interface/meta.thrift +++ b/src/interface/meta.thrift @@ -683,23 +683,6 @@ struct ListUsersResp { 3: map (cpp.template = "std::unordered_map") users, } -struct DescribeUserReq { - 1: binary account, -} - -struct DescribeUserResp { - 1: common.ErrorCode code, - // Valid if ret equals E_LEADER_CHANGED. - 2: common.HostAddr leader, - 3: UserDescItem user, -} - -struct UserDescItem { - 1: binary account, - // map - 2: map (cpp.template = "std::map") space_role_map, -} - struct ListRolesReq { 1: common.GraphSpaceID space_id, } @@ -1218,7 +1201,6 @@ service MetaService { ExecResp grantRole(1: GrantRoleReq req); ExecResp revokeRole(1: RevokeRoleReq req); ListUsersResp listUsers(1: ListUsersReq req); - DescribeUserResp describeUser(1: DescribeUserReq req); ListRolesResp listRoles(1: ListRolesReq req); ListRolesResp getUserRoles(1: GetUserRolesReq req); ExecResp changePassword(1: ChangePasswordReq req); diff --git a/src/meta/MetaServiceHandler.cpp b/src/meta/MetaServiceHandler.cpp index c9a344c90e8..4e58544af0b 100644 --- a/src/meta/MetaServiceHandler.cpp +++ b/src/meta/MetaServiceHandler.cpp @@ -358,12 +358,6 @@ folly::Future MetaServiceHandler::future_listUsers( RETURN_FUTURE(processor); } -folly::Future MetaServiceHandler::future_describeUser( - const cpp2::DescribeUserReq& req) { - auto* processor = DescribeUserProcessor::instance(kvstore_); - RETURN_FUTURE(processor); -} - folly::Future MetaServiceHandler::future_listRoles( const cpp2::ListRolesReq& req) { auto* processor = ListRolesProcessor::instance(kvstore_); diff --git a/src/meta/MetaServiceHandler.h b/src/meta/MetaServiceHandler.h index 5124225fc9a..2ea463e7c89 100644 --- a/src/meta/MetaServiceHandler.h +++ b/src/meta/MetaServiceHandler.h @@ -143,9 +143,6 @@ class MetaServiceHandler final : public cpp2::MetaServiceSvIf { folly::Future future_listUsers(const cpp2::ListUsersReq& req) override; - folly::Future future_describeUser( - const cpp2::DescribeUserReq& req) override; - folly::Future future_listRoles(const cpp2::ListRolesReq& req) override; folly::Future future_changePassword(const cpp2::ChangePasswordReq& req) override; diff --git a/src/meta/processors/user/AuthenticationProcessor.cpp b/src/meta/processors/user/AuthenticationProcessor.cpp index e100a7aaaf9..c2e6a70a223 100644 --- a/src/meta/processors/user/AuthenticationProcessor.cpp +++ b/src/meta/processors/user/AuthenticationProcessor.cpp @@ -252,47 +252,6 @@ void ListUsersProcessor::process(const cpp2::ListUsersReq& req) { onFinished(); } -void DescribeUserProcessor::process(const cpp2::DescribeUserReq& req) { - UNUSED(req); - folly::SharedMutex::ReadHolder rHolder1(LockUtils::userLock()); - auto& act = req.get_account(); - auto userRet = userExist(act); - if (userRet != nebula::cpp2::ErrorCode::SUCCEEDED) { - LOG(ERROR) << "Describe user Failed, get user " << act << " failed, " - << " error: " << apache::thrift::util::enumNameSafe(userRet); - handleErrorCode(userRet); - onFinished(); - return; - } - - auto rolePrefix = MetaKeyUtils::rolesPrefix(); - auto roleRet = doPrefix(rolePrefix); - if (!nebula::ok(roleRet)) { - auto retCode = nebula::error(roleRet); - LOG(ERROR) << "Prefix roles failed, error: " << apache::thrift::util::enumNameSafe(retCode); - handleErrorCode(retCode); - onFinished(); - return; - } - std::map<::nebula::cpp2::GraphSpaceID, ::nebula::meta::cpp2::RoleType> map; - auto iter = nebula::value(roleRet).get(); - while (iter->valid()) { - auto account = MetaKeyUtils::parseRoleUser(iter->key()); - if (account == act) { - auto spaceId = MetaKeyUtils::parseRoleSpace(iter->key()); - auto val = iter->val(); - map[spaceId] = *reinterpret_cast(val.begin()); - } - iter->next(); - } - nebula::meta::cpp2::UserDescItem user; - user.set_account(std::move(act)); - user.set_space_role_map(std::move(map)); - resp_.set_user(std::move(user)); - handleErrorCode(nebula::cpp2::ErrorCode::SUCCEEDED); - onFinished(); -} - void ListRolesProcessor::process(const cpp2::ListRolesReq& req) { auto spaceId = req.get_space_id(); CHECK_SPACE_ID_AND_RETURN(spaceId); diff --git a/src/meta/processors/user/AuthenticationProcessor.h b/src/meta/processors/user/AuthenticationProcessor.h index 6a4dc9821db..a4be35dc4c5 100644 --- a/src/meta/processors/user/AuthenticationProcessor.h +++ b/src/meta/processors/user/AuthenticationProcessor.h @@ -96,19 +96,6 @@ class ListUsersProcessor : public BaseProcessor { : BaseProcessor(kvstore) {} }; -class DescribeUserProcessor : public BaseProcessor { - public: - static DescribeUserProcessor* instance(kvstore::KVStore* kvstore) { - return new DescribeUserProcessor(kvstore); - } - - void process(const cpp2::DescribeUserReq& req); - - private: - explicit DescribeUserProcessor(kvstore::KVStore* kvstore) - : BaseProcessor(kvstore) {} -}; - class ListRolesProcessor : public BaseProcessor { public: static ListRolesProcessor* instance(kvstore::KVStore* kvstore) { diff --git a/tests/tck/features/user/User.feature b/tests/tck/features/user/User.feature index e2a364f5b10..e66bb795bfa 100644 --- a/tests/tck/features/user/User.feature +++ b/tests/tck/features/user/User.feature @@ -342,7 +342,8 @@ Feature: User & privilege Test """ DESC USER user_not_exist """ - Then a ExecutionError should be raised at runtime: + Then the result should be, in any order, with relax comparison: + | role | space | When executing query with user user1 with password pwd1: """ DESC USER user1 From b7e5e509c1402e7b0d86e9c2257b9d1e5fca2118 Mon Sep 17 00:00:00 2001 From: heroicNeZha <25311962+heroicNeZha@users.noreply.github.com> Date: Wed, 24 Nov 2021 12:00:32 +0800 Subject: [PATCH 14/14] fix comment --- src/graph/service/PermissionManager.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/graph/service/PermissionManager.cpp b/src/graph/service/PermissionManager.cpp index fa85a2f1757..80ce3153a53 100644 --- a/src/graph/service/PermissionManager.cpp +++ b/src/graph/service/PermissionManager.cpp @@ -118,9 +118,8 @@ Status PermissionManager::canReadUser(ClientSession *session, const std::string if (!FLAGS_enable_authorize) { return Status::OK(); } - // Cloud auth user cannot create user if (FLAGS_auth_type == "cloud") { - return Status::PermissionError("Cloud authenticate user can't write user."); + return Status::PermissionError("Cloud authenticate user can't read user."); } if (session->isGod()) { return Status::OK();