From 4e8673c55cb105506a6ff1c146d829e097673228 Mon Sep 17 00:00:00 2001 From: bright-starry-sky Date: Wed, 4 Mar 2020 23:02:19 +0800 Subject: [PATCH 1/5] user permission manager --- etc/nebula-graphd.conf.default | 4 + etc/nebula-graphd.conf.production | 4 + src/common/CMakeLists.txt | 2 + src/common/base/Status.cpp | 3 + src/common/permission/CMakeLists.txt | 4 + src/common/permission/PermissionManager.cpp | 214 +++++ src/common/permission/PermissionManager.h | 41 + src/common/session/CMakeLists.txt | 4 + src/common/session/Session.cpp | 28 + src/common/session/Session.h | 130 +++ src/daemons/CMakeLists.txt | 2 + src/graph/CMakeLists.txt | 3 +- src/graph/ClientSession.cpp | 33 - src/graph/ClientSession.h | 75 -- src/graph/DescribeSpaceExecutor.cpp | 15 +- src/graph/DescribeSpaceExecutor.h | 2 + src/graph/ExecutionEngine.cpp | 31 +- src/graph/ExecutionEngine.h | 4 +- src/graph/ExecutionPlan.cpp | 2 + src/graph/GraphFlags.cpp | 3 +- src/graph/GraphFlags.h | 1 + src/graph/GraphService.cpp | 87 +- src/graph/GraphService.h | 11 +- src/graph/PasswordAuthenticator.cpp | 21 + src/graph/PasswordAuthenticator.h | 30 + src/graph/PermissionCheck.cpp | 139 +++ src/graph/PermissionCheck.h | 26 + src/graph/PrivilegeExecutor.cpp | 32 +- src/graph/PrivilegeExecutor.h | 1 + src/graph/RequestContext.h | 8 +- src/graph/SequentialExecutor.cpp | 10 + src/graph/SequentialExecutor.h | 3 +- src/graph/SessionManager.cpp | 8 +- src/graph/SessionManager.h | 4 +- src/graph/ShowExecutor.cpp | 41 +- src/graph/SimpleAuthenticator.h | 32 - src/graph/UseExecutor.cpp | 13 + src/graph/UseExecutor.h | 2 + src/graph/test/CMakeLists.txt | 22 + src/graph/test/PermissionTest.cpp | 811 ++++++++++++++++++ src/graph/test/TestEnv.cpp | 10 +- src/graph/test/TestEnv.h | 4 +- src/graph/test/UserTest.cpp | 12 +- src/interface/common.thrift | 14 +- src/interface/graph.thrift | 5 + src/interface/meta.thrift | 7 +- src/meta/MetaServiceHandler.cpp | 6 + src/meta/MetaServiceHandler.h | 3 + src/meta/MetaServiceUtils.cpp | 4 + src/meta/MetaServiceUtils.h | 2 + src/meta/client/MetaClient.cpp | 32 +- src/meta/client/MetaClient.h | 10 +- .../usersMan/AuthenticationProcessor.cpp | 53 +- .../usersMan/AuthenticationProcessor.h | 13 + src/meta/test/AuthProcessorTest.cpp | 12 +- src/parser/Sentence.h | 1 - 56 files changed, 1859 insertions(+), 235 deletions(-) create mode 100644 src/common/permission/CMakeLists.txt create mode 100644 src/common/permission/PermissionManager.cpp create mode 100644 src/common/permission/PermissionManager.h create mode 100644 src/common/session/CMakeLists.txt create mode 100644 src/common/session/Session.cpp create mode 100644 src/common/session/Session.h delete mode 100644 src/graph/ClientSession.cpp delete mode 100644 src/graph/ClientSession.h create mode 100644 src/graph/PasswordAuthenticator.cpp create mode 100644 src/graph/PasswordAuthenticator.h create mode 100644 src/graph/PermissionCheck.cpp create mode 100644 src/graph/PermissionCheck.h delete mode 100644 src/graph/SimpleAuthenticator.h create mode 100644 src/graph/test/PermissionTest.cpp diff --git a/etc/nebula-graphd.conf.default b/etc/nebula-graphd.conf.default index ba394ea1e42..6a6c2bd683f 100644 --- a/etc/nebula-graphd.conf.default +++ b/etc/nebula-graphd.conf.default @@ -54,3 +54,7 @@ --default_charset=utf8 # The defaule collate when a space is created --default_collate=utf8_bin + +########## authorization ########## +# Enable authorization +--enable_authorize=false diff --git a/etc/nebula-graphd.conf.production b/etc/nebula-graphd.conf.production index 9fe056ee6e1..de3625d7098 100644 --- a/etc/nebula-graphd.conf.production +++ b/etc/nebula-graphd.conf.production @@ -52,3 +52,7 @@ --ws_h2_port=13002 --heartbeat_interval_secs=10 + +########## authorization ########## +# Enable authorization +--enable_authorize=false \ No newline at end of file diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 43a4c887506..408b01fb7e7 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -14,4 +14,6 @@ nebula_add_subdirectory(test) nebula_add_subdirectory(charset) nebula_add_subdirectory(algorithm) nebula_add_subdirectory(encryption) +nebula_add_subdirectory(permission) +nebula_add_subdirectory(session) diff --git a/src/common/base/Status.cpp b/src/common/base/Status.cpp index 4c4198b6cb2..c8e7df6c2f9 100644 --- a/src/common/base/Status.cpp +++ b/src/common/base/Status.cpp @@ -33,6 +33,9 @@ std::string Status::toString() const { case kSyntaxError: str = "SyntaxError: "; break; + case kPermissionError: + str = "PermissionError: "; + break; default: snprintf(tmp, sizeof(tmp), "Unknown error(%hu): ", static_cast(code())); str = tmp; diff --git a/src/common/permission/CMakeLists.txt b/src/common/permission/CMakeLists.txt new file mode 100644 index 00000000000..1d2163366a0 --- /dev/null +++ b/src/common/permission/CMakeLists.txt @@ -0,0 +1,4 @@ +nebula_add_library( + permission_obj OBJECT + PermissionManager.cpp +) diff --git a/src/common/permission/PermissionManager.cpp b/src/common/permission/PermissionManager.cpp new file mode 100644 index 00000000000..c97cc7e91e5 --- /dev/null +++ b/src/common/permission/PermissionManager.cpp @@ -0,0 +1,214 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "permission/PermissionManager.h" + +namespace nebula { +namespace permission { + +// static +bool PermissionManager::canReadSpace(session::Session *session, GraphSpaceID spaceId) { + if (session->isGod()) { + return true; + } + bool havePermission = false; + switch (session->roleWithSpace(spaceId)) { + case session::Role::GOD : + case session::Role::ADMIN : + case session::Role::DBA : + case session::Role::USER : + case session::Role::GUEST : { + havePermission = true; + break; + } + case session::Role::INVALID_ROLE : { + break; + } + } + return havePermission; +} + +// static +bool PermissionManager::canReadSchemaData(session::Session *session) { + if (session->isGod()) { + return true; + } + if (session->space() == -1) { + LOG(ERROR) << "The space name is not set"; + return false; + } + bool havePermission = false; + switch (session->roleWithSpace(session->space())) { + case session::Role::GOD : + case session::Role::ADMIN : + case session::Role::DBA : + case session::Role::USER : + case session::Role::GUEST : { + havePermission = true; + break; + } + case session::Role::INVALID_ROLE : { + break; + } + } + return havePermission; +} + +// static +bool PermissionManager::canWriteSpace(session::Session *session) { + return session->isGod(); +} + +// static +bool PermissionManager::canWriteSchema(session::Session *session) { + if (session->isGod()) { + return true; + } + if (session->space() == -1) { + LOG(ERROR) << "The space name is not set"; + return false; + } + bool havePermission = false; + switch (session->roleWithSpace(session->space())) { + case session::Role::GOD : + case session::Role::ADMIN : + case session::Role::DBA : { + havePermission = true; + break; + } + case session::Role::USER : + case session::Role::GUEST : + case session::Role::INVALID_ROLE : { + break; + } + } + return havePermission; +} + +// static +bool PermissionManager::canWriteUser(session::Session *session) { + return session->isGod(); +} + +bool PermissionManager::canWriteRole(session::Session *session, + session::Role targetRole, + GraphSpaceID spaceId, + const std::string& targetUser) { + /** + * Reject grant or revoke to himself. + */ + if (session->user() == targetUser) { + return false; + } + /* + * Reject any user grant or revoke role to GOD + */ + if (targetRole == session::Role::GOD) { + return false; + } + /* + * God user can be grant or revoke any one. + */ + if (session->isGod()) { + return true; + } + /** + * Only allow ADMIN user grant or revoke other user to DBA, USER, GUEST. + */ + auto role = session->roleWithSpace(spaceId); + if (role == session::Role::ADMIN && targetRole != session::Role::ADMIN) { + return true; + } + return false; +} + +// static +bool PermissionManager::canWriteData(session::Session *session) { + if (session->isGod()) { + return true; + } + if (session->space() == -1) { + LOG(ERROR) << "The space name is not set"; + return false; + } + bool havePermission = false; + switch (session->roleWithSpace(session->space())) { + case session::Role::GOD : + case session::Role::ADMIN : + case session::Role::DBA : + case session::Role::USER : { + havePermission = true; + break; + } + case session::Role::GUEST : + case session::Role::INVALID_ROLE : { + break; + } + } + return havePermission; +} + +// static +bool PermissionManager::canShow(session::Session *session, + ShowSentence::ShowType type, + GraphSpaceID targetSpace) { + bool havePermission = false; + switch (type) { + case ShowSentence::ShowType::kShowParts: + case ShowSentence::ShowType::kShowTags: + case ShowSentence::ShowType::kShowEdges: + case ShowSentence::ShowType::kShowTagIndexes: + case ShowSentence::ShowType::kShowEdgeIndexes: + case ShowSentence::ShowType::kShowCreateTag: + case ShowSentence::ShowType::kShowCreateEdge: + case ShowSentence::ShowType::kShowCreateTagIndex: + case ShowSentence::ShowType::kShowCreateEdgeIndex: + case ShowSentence::ShowType::kShowTagIndexStatus: + case ShowSentence::ShowType::kShowEdgeIndexStatus: { + /** + * Above operations can get the space id via session, + * so the permission same with canReadSchemaData. + * They've been checked by "USE SPACE", so here skip the check. + */ + havePermission = true; + break; + } + case ShowSentence::ShowType::kShowCharset: + case ShowSentence::ShowType::kShowCollation: + case ShowSentence::ShowType::kShowHosts: { + /** + * all roles can be show for above operations. + */ + havePermission = true; + break; + } + case ShowSentence::ShowType::kShowSpaces: + case ShowSentence::ShowType::kShowCreateSpace: + case ShowSentence::ShowType::kShowRoles: { + /* + * Above operations are special operation. + * can not get the space id via session, + * Permission checking needs to be done in their executor. + */ + havePermission = canReadSpace(session, targetSpace); + break; + } + case ShowSentence::ShowType::kShowUsers: + case ShowSentence::ShowType::kShowSnapshots: { + /** + * Only GOD role can be show. + */ + havePermission = session->isGod(); + break; + } + case ShowSentence::ShowType::kUnknown: + break; + } + return havePermission; +} + +} // namespace permission +} // namespace nebula diff --git a/src/common/permission/PermissionManager.h b/src/common/permission/PermissionManager.h new file mode 100644 index 00000000000..67f82a40f26 --- /dev/null +++ b/src/common/permission/PermissionManager.h @@ -0,0 +1,41 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#ifndef COMMON_PERMISSION_PERMISSIONMANAGER_H_ +#define COMMON_PERMISSION_PERMISSIONMANAGER_H_ + +#include "base/Base.h" +#include "session/Session.h" +#include "meta/client/MetaClient.h" +#include "parser/Sentence.h" +#include "parser/UserSentences.h" +#include "parser/AdminSentences.h" + +namespace nebula { +namespace permission { + +class PermissionManager final { +public: + PermissionManager() = delete; + static bool canReadSpace(session::Session *session, GraphSpaceID spaceId); + static bool canReadSchemaData(session::Session *session); + static bool canWriteSpace(session::Session *session); + static bool canWriteSchema(session::Session *session); + static bool canWriteUser(session::Session *session); + static bool canWriteRole(session::Session *session, + session::Role targetRole, + GraphSpaceID spaceId, + const std::string& targetUser); + static bool canWriteData(session::Session *session); + static bool canShow(session::Session *session, + ShowSentence::ShowType type, + GraphSpaceID targetSpace = -1); +}; +} // namespace permission +} // namespace nebula + +#endif // COMMON_PERMISSION_PERMISSIONMANAGER_H_ + diff --git a/src/common/session/CMakeLists.txt b/src/common/session/CMakeLists.txt new file mode 100644 index 00000000000..74348bbc362 --- /dev/null +++ b/src/common/session/CMakeLists.txt @@ -0,0 +1,4 @@ +nebula_add_library( + session_obj OBJECT + Session.cpp +) diff --git a/src/common/session/Session.cpp b/src/common/session/Session.cpp new file mode 100644 index 00000000000..3c81ac5bf45 --- /dev/null +++ b/src/common/session/Session.cpp @@ -0,0 +1,28 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "session/Session.h" + +namespace nebula { +namespace session { + +Session::Session(int64_t id) { + id_ = id; +} + +std::shared_ptr Session::create(int64_t id) { + return std::shared_ptr(new Session(id)); +} + +void Session::charge() { + idleDuration_.reset(); +} + +uint64_t Session::idleSeconds() const { + return idleDuration_.elapsedInSec(); +} +} // namespace session +} // namespace nebula diff --git a/src/common/session/Session.h b/src/common/session/Session.h new file mode 100644 index 00000000000..769ed18bb7d --- /dev/null +++ b/src/common/session/Session.h @@ -0,0 +1,130 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ +#ifndef COMMON_SESSION_H_ +#define COMMON_SESSION_H_ + +#include "base/Base.h" +#include "time/Duration.h" +#include "interface/gen-cpp2/common_types.h" + +namespace nebula { +namespace session { + +enum class Role : char { + GOD = 1, + ADMIN = 2, + DBA = 3, + USER = 4, + GUEST = 5, + INVALID_ROLE = 6, +}; + +class Session final { +public: + static std::shared_ptr create(int64_t id); + + int64_t id() const { + return id_; + } + + void setId(int64_t id) { + id_ = id; + } + + GraphSpaceID space() const { + return space_; + } + + void setSpace(const std::string &name, GraphSpaceID space) { + spaceName_ = name; + space_ = space; + } + + const std::string& spaceName() const { + return spaceName_; + } + + const std::string& user() const { + return user_; + } + + void setUser(std::string user) { + user_ = std::move(user); + } + + std::unordered_map roles() const { + return roles_; + } + + Role roleWithSpace(GraphSpaceID space) const { + auto ret = roles_.find(space); + if (ret == roles_.end()) { + return Role::INVALID_ROLE; + } + return ret->second; + } + + Role toRole(nebula::cpp2::RoleType role) { + switch (role) { + case nebula::cpp2::RoleType::GOD : { + return Role::GOD; + } + case nebula::cpp2::RoleType::ADMIN : { + return Role::ADMIN; + } + case nebula::cpp2::RoleType::DBA : { + return Role::DBA; + } + case nebula::cpp2::RoleType::USER : { + return Role::USER; + } + case nebula::cpp2::RoleType::GUEST : { + return Role::GUEST; + } + } + return Role::INVALID_ROLE; + } + + bool isGod() const { + for (auto iter = roles_.begin(); iter != roles_.end(); iter++) { + if (iter->second == Role::GOD) { + return true; + } + } + return false; + } + + void setRole(GraphSpaceID space, Role role) { + roles_.emplace(space, role); + } + + uint64_t idleSeconds() const; + + void charge(); + +private: + Session() = default; + explicit Session(int64_t id); + + +private: + int64_t id_{0}; + GraphSpaceID space_{-1}; + std::string spaceName_; + std::string user_; + time::Duration idleDuration_; + /* + * map + * One user can have roles in multiple spaces + * But a user has only one role in one space + */ + std::unordered_map roles_; +}; + +} // namespace session +} // namespace nebula + +#endif // COMMON_SESSION_H_ diff --git a/src/daemons/CMakeLists.txt b/src/daemons/CMakeLists.txt index 4ebbe1f8e18..ca2964b49de 100644 --- a/src/daemons/CMakeLists.txt +++ b/src/daemons/CMakeLists.txt @@ -33,6 +33,8 @@ nebula_add_executable( $ $ $ + $ + $ LIBRARIES proxygenhttpserver proxygenlib diff --git a/src/graph/CMakeLists.txt b/src/graph/CMakeLists.txt index e4b501d75de..25ac1d87a3a 100644 --- a/src/graph/CMakeLists.txt +++ b/src/graph/CMakeLists.txt @@ -2,10 +2,11 @@ nebula_add_library( graph_obj OBJECT GraphFlags.cpp GraphService.cpp - ClientSession.cpp SessionManager.cpp + PasswordAuthenticator.cpp ExecutionEngine.cpp ExecutionContext.cpp + PermissionCheck.cpp ExecutionPlan.cpp Executor.cpp TraverseExecutor.cpp diff --git a/src/graph/ClientSession.cpp b/src/graph/ClientSession.cpp deleted file mode 100644 index daeaac35e49..00000000000 --- a/src/graph/ClientSession.cpp +++ /dev/null @@ -1,33 +0,0 @@ -/* Copyright (c) 2018 vesoft inc. All rights reserved. - * - * This source code is licensed under Apache 2.0 License, - * attached with Common Clause Condition 1.0, found in the LICENSES directory. - */ - -#include "base/Base.h" -#include "graph/ClientSession.h" - - -namespace nebula { -namespace graph { - -ClientSession::ClientSession(int64_t id) { - id_ = id; -} - -std::shared_ptr ClientSession::create(int64_t id) { - // return std::make_shared(id); - // `std::make_shared' cannot access ClientSession's construtor - return std::shared_ptr(new ClientSession(id)); -} - -void ClientSession::charge() { - idleDuration_.reset(); -} - -uint64_t ClientSession::idleSeconds() const { - return idleDuration_.elapsedInSec(); -} - -} // namespace graph -} // namespace nebula diff --git a/src/graph/ClientSession.h b/src/graph/ClientSession.h deleted file mode 100644 index 605e4433111..00000000000 --- a/src/graph/ClientSession.h +++ /dev/null @@ -1,75 +0,0 @@ -/* Copyright (c) 2018 vesoft inc. All rights reserved. - * - * This source code is licensed under Apache 2.0 License, - * attached with Common Clause Condition 1.0, found in the LICENSES directory. - */ - -#ifndef GRAPH_CLIENTSESSION_H_ -#define GRAPH_CLIENTSESSION_H_ - -#include "base/Base.h" -#include "time/Duration.h" - -/** - * A ClientSession holds the context informations of a session opened by a client. - */ - -namespace nebula { -namespace graph { - -class ClientSession final { -public: - int64_t id() const { - return id_; - } - - void setId(int64_t id) { - id_ = id; - } - - GraphSpaceID space() const { - return space_; - } - - void setSpace(const std::string &name, GraphSpaceID space) { - spaceName_ = name; - space_ = space; - } - - const std::string& spaceName() const { - return spaceName_; - } - - uint64_t idleSeconds() const; - - const std::string& user() const { - return user_; - } - - void setUser(std::string user) { - user_ = std::move(user); - } - - void charge(); - -private: - // ClientSession could only be created via SessionManager - friend class SessionManager; - ClientSession() = default; - explicit ClientSession(int64_t id); - - static std::shared_ptr create(int64_t id); - - -private: - int64_t id_{0}; - GraphSpaceID space_{-1}; - time::Duration idleDuration_; - std::string spaceName_; - std::string user_; -}; - -} // namespace graph -} // namespace nebula - -#endif // GRAPH_CLIENTSESSION_H_ diff --git a/src/graph/DescribeSpaceExecutor.cpp b/src/graph/DescribeSpaceExecutor.cpp index 9870cd549b8..0443f79d459 100644 --- a/src/graph/DescribeSpaceExecutor.cpp +++ b/src/graph/DescribeSpaceExecutor.cpp @@ -30,7 +30,18 @@ void DescribeSpaceExecutor::execute() { doError(Status::Error("Space not found")); return; } - + /** + * Permission check. + */ + auto spaceId = resp.value().get_space_id(); + if (FLAGS_enable_authorize) { + auto *session = ectx()->rctx()->session(); + auto rst = permission::PermissionManager::canReadSpace(session, spaceId); + if (!rst) { + doError(Status::PermissionError("Permission denied")); + return; + } + } resp_ = std::make_unique(); std::vector header{"ID", "Name", @@ -43,7 +54,7 @@ void DescribeSpaceExecutor::execute() { std::vector rows; std::vector row; row.resize(6); - row[0].set_integer(resp.value().get_space_id()); + row[0].set_integer(spaceId); auto properties = resp.value().get_properties(); row[1].set_str(properties.get_space_name()); diff --git a/src/graph/DescribeSpaceExecutor.h b/src/graph/DescribeSpaceExecutor.h index 31e8a37b14f..bea024c2c51 100644 --- a/src/graph/DescribeSpaceExecutor.h +++ b/src/graph/DescribeSpaceExecutor.h @@ -9,6 +9,8 @@ #include "base/Base.h" #include "graph/Executor.h" +#include "graph/GraphFlags.h" +#include "common/permission//PermissionManager.h" namespace nebula { namespace graph { diff --git a/src/graph/ExecutionEngine.cpp b/src/graph/ExecutionEngine.cpp index bbb640cea9f..ff8df3fdc95 100644 --- a/src/graph/ExecutionEngine.cpp +++ b/src/graph/ExecutionEngine.cpp @@ -10,13 +10,12 @@ #include "graph/ExecutionPlan.h" #include "storage/client/StorageClient.h" -DECLARE_string(meta_server_addrs); -DECLARE_bool(local_config); namespace nebula { namespace graph { -ExecutionEngine::ExecutionEngine() { +ExecutionEngine::ExecutionEngine(meta::MetaClient* client) { + metaClient_ = client; } @@ -25,31 +24,13 @@ ExecutionEngine::~ExecutionEngine() { Status ExecutionEngine::init(std::shared_ptr ioExecutor) { - auto addrs = network::NetworkUtils::toHosts(FLAGS_meta_server_addrs); - if (!addrs.ok()) { - return addrs.status(); - } - - meta::MetaClientOptions options; - options.serviceName_ = "graph"; - options.skipConfig_ = FLAGS_local_config; - metaClient_ = std::make_unique(ioExecutor, - std::move(addrs.value()), - options); - // load data try 3 time - bool loadDataOk = metaClient_->waitForMetadReady(3); - if (!loadDataOk) { - // Resort to retrying in the background - LOG(WARNING) << "Failed to synchronously wait for meta service ready"; - } - schemaManager_ = meta::SchemaManager::create(); - schemaManager_->init(metaClient_.get()); + schemaManager_->init(metaClient_); - gflagsManager_ = std::make_unique(metaClient_.get()); + gflagsManager_ = std::make_unique(metaClient_); storage_ = std::make_unique(ioExecutor, - metaClient_.get(), + metaClient_, "graph"); charsetInfo_ = CharsetInfo::instance(); @@ -61,7 +42,7 @@ void ExecutionEngine::execute(RequestContextPtr rctx) { schemaManager_.get(), gflagsManager_.get(), storage_.get(), - metaClient_.get(), + metaClient_, charsetInfo_); // TODO(dutor) add support to plan cache auto plan = new ExecutionPlan(std::move(ectx)); diff --git a/src/graph/ExecutionEngine.h b/src/graph/ExecutionEngine.h index 36833b5b41e..08223e64748 100644 --- a/src/graph/ExecutionEngine.h +++ b/src/graph/ExecutionEngine.h @@ -33,7 +33,7 @@ namespace graph { class ExecutionEngine final : public cpp::NonCopyable, public cpp::NonMovable { public: - ExecutionEngine(); + explicit ExecutionEngine(meta::MetaClient* client); ~ExecutionEngine(); Status init(std::shared_ptr ioExecutor); @@ -45,7 +45,7 @@ class ExecutionEngine final : public cpp::NonCopyable, public cpp::NonMovable { std::unique_ptr schemaManager_; std::unique_ptr gflagsManager_; std::unique_ptr storage_; - std::unique_ptr metaClient_; + meta::MetaClient* metaClient_; CharsetInfo* charsetInfo_{nullptr}; }; diff --git a/src/graph/ExecutionPlan.cpp b/src/graph/ExecutionPlan.cpp index fa2e6a50807..93700967ff6 100644 --- a/src/graph/ExecutionPlan.cpp +++ b/src/graph/ExecutionPlan.cpp @@ -79,6 +79,8 @@ void ExecutionPlan::onError(Status status) { rctx->resp().set_error_code(cpp2::ErrorCode::E_SYNTAX_ERROR); } else if (status.isStatementEmpty()) { rctx->resp().set_error_code(cpp2::ErrorCode::E_STATEMENT_EMTPY); + } else if (status.isPermissionError()) { + rctx->resp().set_error_code(cpp2::ErrorCode::E_BAD_PERMISSION); } else { rctx->resp().set_error_code(cpp2::ErrorCode::E_EXECUTION_ERROR); } diff --git a/src/graph/GraphFlags.cpp b/src/graph/GraphFlags.cpp index db63d4ce332..4f21e668c8f 100644 --- a/src/graph/GraphFlags.cpp +++ b/src/graph/GraphFlags.cpp @@ -5,7 +5,6 @@ */ #include "base/Base.h" -#include "graph/GraphFlags.h" DEFINE_int32(port, 3699, "Nebula Graph daemon's listen port"); DEFINE_int32(client_idle_timeout_secs, 0, @@ -32,3 +31,5 @@ DEFINE_bool(local_config, false, "meta client will not retrieve latest configura DEFINE_string(default_charset, "utf8", "The default charset when a space is created"); DEFINE_string(default_collate, "utf8_bin", "The default collate when a space is created"); + +DEFINE_bool(enable_authorize, false, "Enable authorization, default false"); diff --git a/src/graph/GraphFlags.h b/src/graph/GraphFlags.h index 543888ca1b7..aebf1da8fe0 100644 --- a/src/graph/GraphFlags.h +++ b/src/graph/GraphFlags.h @@ -29,5 +29,6 @@ DECLARE_string(meta_server_addrs); DECLARE_string(default_charset); DECLARE_string(default_collate); +DECLARE_bool(enable_authorize); #endif // GRAPH_GRAPHFLAGS_H_ diff --git a/src/graph/GraphService.cpp b/src/graph/GraphService.cpp index 918a5d1a143..bd29dbc858d 100644 --- a/src/graph/GraphService.cpp +++ b/src/graph/GraphService.cpp @@ -6,18 +6,38 @@ #include "base/Base.h" #include "graph/GraphService.h" -#include "time/Duration.h" #include "graph/RequestContext.h" -#include "graph/SimpleAuthenticator.h" #include "storage/client/StorageClient.h" +#include "graph/GraphFlags.h" +#include "common/encryption/MD5Utils.h" + +DECLARE_string(meta_server_addrs); +DECLARE_bool(local_config); namespace nebula { namespace graph { Status GraphService::init(std::shared_ptr ioExecutor) { + auto addrs = network::NetworkUtils::toHosts(FLAGS_meta_server_addrs); + if (!addrs.ok()) { + return addrs.status(); + } + + meta::MetaClientOptions options; + options.serviceName_ = "graph"; + options.skipConfig_ = FLAGS_local_config; + metaClient_ = std::make_unique(ioExecutor, + std::move(addrs.value()), + options); + // load data try 3 time + bool loadDataOk = metaClient_->waitForMetadReady(3); + if (!loadDataOk) { + // Resort to retrying in the background + LOG(WARNING) << "Failed to synchronously wait for meta service ready"; + } + sessionManager_ = std::make_unique(); - executionEngine_ = std::make_unique(); - authenticator_ = std::make_unique(); + executionEngine_ = std::make_unique(metaClient_.get()); return executionEngine_->init(std::move(ioExecutor)); } @@ -34,19 +54,62 @@ folly::Future GraphService::future_authenticate( session->setUser(username); ctx.setSession(std::move(session)); - if (authenticator_->auth(username, password)) { - ctx.resp().set_error_code(cpp2::ErrorCode::SUCCEEDED); - ctx.resp().set_session_id(ctx.session()->id()); + if (!FLAGS_enable_authorize) { + onHandle(ctx, cpp2::ErrorCode::SUCCEEDED); + } else if (auth(username, password)) { + auto ret = metaClient_->getRolesByUser(username); + if (ret.ok()) { + auto roles = std::move(ret).value(); + for (const auto& role : roles) { + ctx.session()->setRole(role.get_space_id(), toRole(role.get_role_type())); + } + onHandle(ctx, cpp2::ErrorCode::SUCCEEDED); + } else { + onHandle(ctx, cpp2::ErrorCode::E_EXECUTION_ERROR); + } } else { - sessionManager_->removeSession(ctx.session()->id()); - ctx.resp().set_error_code(cpp2::ErrorCode::E_BAD_USERNAME_PASSWORD); - ctx.resp().set_error_msg(getErrorStr(cpp2::ErrorCode::E_BAD_USERNAME_PASSWORD)); + onHandle(ctx, cpp2::ErrorCode::E_BAD_USERNAME_PASSWORD); } ctx.finish(); return ctx.future(); } +void GraphService::onHandle(RequestContext& ctx, cpp2::ErrorCode code) { + ctx.resp().set_error_code(code); + if (code != cpp2::ErrorCode::SUCCEEDED) { + sessionManager_->removeSession(ctx.session()->id()); + ctx.resp().set_error_msg(getErrorStr(code)); + } else { + ctx.resp().set_session_id(ctx.session()->id()); + } +} + +session::Role GraphService::toRole(nebula::cpp2::RoleType role) { + switch (role) { + case nebula::cpp2::RoleType::GOD : { + return session::Role::GOD; + } + case nebula::cpp2::RoleType::ADMIN : { + return session::Role::ADMIN; + } + case nebula::cpp2::RoleType::DBA : { + return session::Role::DBA; + } + case nebula::cpp2::RoleType::USER : { + return session::Role::USER; + } + case nebula::cpp2::RoleType::GUEST : { + return session::Role::GUEST; + } + } + return session::Role::INVALID_ROLE; +} + +bool GraphService::auth(const std::string& username, const std::string& password) { + auto authenticator = std::make_unique(metaClient_.get()); + return authenticator->auth(username, encryption::MD5Utils::md5Encode(password)); +} void GraphService::signout(int64_t sessionId) { VLOG(2) << "Sign out session " << sessionId; @@ -92,6 +155,10 @@ const char* GraphService::getErrorStr(cpp2::ErrorCode result) { return "The session timed out"; case cpp2::ErrorCode::E_SYNTAX_ERROR: return "Syntax error"; + case cpp2::ErrorCode::E_USER_NOT_FOUND: + return "User not exist"; + case cpp2::ErrorCode::E_BAD_PERMISSION: + return "Permission denied"; /********************** * Unknown error **********************/ diff --git a/src/graph/GraphService.h b/src/graph/GraphService.h index 804e49d284e..eff6ccf2eae 100644 --- a/src/graph/GraphService.h +++ b/src/graph/GraphService.h @@ -9,7 +9,7 @@ #include "base/Base.h" #include "gen-cpp2/GraphService.h" -#include "graph/Authenticator.h" +#include "graph/PasswordAuthenticator.h" #include "graph/ExecutionEngine.h" #include "graph/SessionManager.h" @@ -38,10 +38,17 @@ class GraphService final : public cpp2::GraphServiceSvIf { const char* getErrorStr(cpp2::ErrorCode result); +private: + void onHandle(RequestContext& ctx, cpp2::ErrorCode code); + + session::Role toRole(nebula::cpp2::RoleType role); + + bool auth(const std::string& username, const std::string& password); + private: std::unique_ptr sessionManager_; std::unique_ptr executionEngine_; - std::unique_ptr authenticator_; + std::unique_ptr metaClient_; }; } // namespace graph diff --git a/src/graph/PasswordAuthenticator.cpp b/src/graph/PasswordAuthenticator.cpp new file mode 100644 index 00000000000..b070d90ded5 --- /dev/null +++ b/src/graph/PasswordAuthenticator.cpp @@ -0,0 +1,21 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "graph/PasswordAuthenticator.h" + +namespace nebula { +namespace graph { + +PasswordAuthenticator::PasswordAuthenticator(meta::MetaClient* client) { + metaClient_ = client; +} + +bool PasswordAuthenticator::auth(const std::string& user, const std::string& password) { + return metaClient_->authenticationCheck(user, password); +} + +} // namespace graph +} // namespace nebula diff --git a/src/graph/PasswordAuthenticator.h b/src/graph/PasswordAuthenticator.h new file mode 100644 index 00000000000..2b4de8bac13 --- /dev/null +++ b/src/graph/PasswordAuthenticator.h @@ -0,0 +1,30 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#ifndef GRAPH_PASSWORDAUTHENTICATOR_H_ +#define GRAPH_PASSWORDAUTHENTICATOR_H_ + +#include "base/Base.h" +#include "graph/Authenticator.h" +#include "meta/client/MetaClient.h" + +namespace nebula { +namespace graph { + +class PasswordAuthenticator final : public Authenticator { +public: + explicit PasswordAuthenticator(meta::MetaClient* client); + + bool auth(const std::string& user, const std::string& password) override; + +private: + meta::MetaClient* metaClient_; +}; + +} // namespace graph +} // namespace nebula + +#endif // GRAPH_PASSWORDAUTHENTICATOR_H_ diff --git a/src/graph/PermissionCheck.cpp b/src/graph/PermissionCheck.cpp new file mode 100644 index 00000000000..37473b8e83b --- /dev/null +++ b/src/graph/PermissionCheck.cpp @@ -0,0 +1,139 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "graph/PermissionCheck.h" + +namespace nebula { +namespace graph { + +/** + * Read space : kUse, kDescribeSpace + * Write space : kCreateSpace, kDropSpace, kCreateSnapshot, kDropSnapshot + * kBalance, kAdmin, kConfig, kIngest, kDownload + * Read schema : kDescribeTag, kDescribeEdge, + * kDescribeTagIndex, kDescribeEdgeIndex + * Write schema : kCreateTag, kAlterTag, kCreateEdge, + * kAlterEdge, kDropTag, kDropEdge, + * kCreateTagIndex, kCreateEdgeIndex, kDropTagIndex, kDropEdgeIndex, + * Read user : + * Write user : kCreateUser, kDropUser, kAlterUser, + * Write role : kGrant, kRevoke, + * Read data : kGo , kSet, kPipe, kMatch, kAssignment, kLookup, + * kYield, kOrderBy, kFetchVertices, kFind + * kFetchEdges, kFindPath, kLimit, KGroupBy, kReturn + * Write data: kBuildTagIndex, kBuildEdgeIndex, + * kInsertVertex, kUpdateVertex, kInsertEdge, + * kUpdateEdge, kDeleteVertex, kDeleteEdges + * Special operation : kShow, kChangePassword + */ + +// static +bool PermissionCheck::permissionCheck(session::Session *session, Sentence* sentence) { + auto kind = sentence->kind(); + switch (kind) { + case Sentence::Kind::kUnknown : { + return false; + } + case Sentence::Kind::kUse : + case Sentence::Kind::kDescribeSpace : { + /** + * Use space and Describe space are special operations. + * Permission checking needs to be done in their executor. + * skip the check at here. + */ + return true; + } + case Sentence::Kind::kCreateSpace : + case Sentence::Kind::kDropSpace : + case Sentence::Kind::kCreateSnapshot : + case Sentence::Kind::kDropSnapshot : + case Sentence::Kind::kBalance : + case Sentence::Kind::kAdmin : + case Sentence::Kind::kConfig : + case Sentence::Kind::kIngest : + case Sentence::Kind::kDownload : { + return permission::PermissionManager::canWriteSpace(session); + } + case Sentence::Kind::kCreateTag : + case Sentence::Kind::kAlterTag : + case Sentence::Kind::kCreateEdge : + case Sentence::Kind::kAlterEdge : + case Sentence::Kind::kDropTag : + case Sentence::Kind::kDropEdge : + case Sentence::Kind::kCreateTagIndex : + case Sentence::Kind::kCreateEdgeIndex : + case Sentence::Kind::kDropTagIndex : + case Sentence::Kind::kDropEdgeIndex : { + return permission::PermissionManager::canWriteSchema(session); + } + case Sentence::Kind::kCreateUser : + case Sentence::Kind::kDropUser : + case Sentence::Kind::kAlterUser : { + return permission::PermissionManager::canWriteUser(session); + } + case Sentence::Kind::kRevoke : + case Sentence::Kind::kGrant : { + /** + * Use grant and revoke are special operations. + * Because have not found the target space id and target role + * so permission checking needs to be done in their executor. + * skip the check at here. + */ + return true; + } + case Sentence::Kind::kRebuildTagIndex : + case Sentence::Kind::kRebuildEdgeIndex : + case Sentence::Kind::kInsertVertex : + case Sentence::Kind::kUpdateVertex : + case Sentence::Kind::kInsertEdge : + case Sentence::Kind::kUpdateEdge : + case Sentence::Kind::kDeleteVertex : + case Sentence::Kind::kDeleteEdges : { + return permission::PermissionManager::canWriteData(session); + } + case Sentence::Kind::kDescribeTag : + case Sentence::Kind::kDescribeEdge : + case Sentence::Kind::kDescribeTagIndex : + case Sentence::Kind::kDescribeEdgeIndex : + case Sentence::Kind::kGo : + case Sentence::Kind::kSet : + case Sentence::Kind::kPipe : + case Sentence::Kind::kMatch : + case Sentence::Kind::kAssignment : + case Sentence::Kind::kLookup : + case Sentence::Kind::kYield : + case Sentence::Kind::kOrderBy : + case Sentence::Kind::kFetchVertices : + case Sentence::Kind::kFetchEdges : + case Sentence::Kind::kFindPath : + case Sentence::Kind::kLimit : + case Sentence::Kind::KGroupBy : + case Sentence::Kind::kReturn : { + return permission::PermissionManager::canReadSchemaData(session); + } + case Sentence::Kind::kShow : { + auto *stc = static_cast(sentence); + if (stc->showType() == ShowSentence::ShowType::kShowSpaces || + stc->showType() == ShowSentence::ShowType::kShowCreateSpace || + stc->showType() == ShowSentence::ShowType::kShowRoles) { + /* + * Above operations are special operation. + * can not get the space id via session, + * Permission checking needs to be done in their executor. + * here skip the permission check. + */ + return true; + } + return permission::PermissionManager::canShow(session, stc->showType()); + } + case Sentence::Kind::kChangePassword : { + return true; + } + } + return false; +} +} // namespace graph +} // namespace nebula diff --git a/src/graph/PermissionCheck.h b/src/graph/PermissionCheck.h new file mode 100644 index 00000000000..3422342502d --- /dev/null +++ b/src/graph/PermissionCheck.h @@ -0,0 +1,26 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ +#include "common/permission/PermissionManager.h" +#include "parser/Sentence.h" +#include "parser/TraverseSentences.h" +#include "parser/AdminSentences.h" + +#ifndef GRAPH_PERMISSIONCHECK_H_ +#define GRAPH_PERMISSIONCHECK_H_ + +namespace nebula { +namespace graph { + +class PermissionCheck final { +public: + PermissionCheck() = delete; + + static bool permissionCheck(session::Session *session, Sentence* sentence); +}; +} // namespace graph +} // namespace nebula + +#endif // GRAPH_PERMISSIONCHECK_H_ diff --git a/src/graph/PrivilegeExecutor.cpp b/src/graph/PrivilegeExecutor.cpp index 19ef5bed507..2d3d59fa38e 100644 --- a/src/graph/PrivilegeExecutor.cpp +++ b/src/graph/PrivilegeExecutor.cpp @@ -34,7 +34,21 @@ void GrantExecutor::execute() { doError(spaceRet.status()); return; } - + /** + * Permission check. + */ + if (FLAGS_enable_authorize) { + auto *session = ectx()->rctx()->session(); + auto role = session->toRole(PrivilegeUtils::toRoleType(aclItem->getRoleType())); + auto rst = permission::PermissionManager::canWriteRole(session, + role, + spaceRet.value(), + *account); + if (!rst) { + doError(Status::PermissionError("Permission denied")); + return; + } + } roleItem.set_user(*account); roleItem.set_space_id(spaceRet.value()); roleItem.set_role_type(PrivilegeUtils::toRoleType(aclItem->getRoleType())); @@ -86,6 +100,22 @@ void RevokeExecutor::execute() { return; } + /** + * Permission check. + */ + if (FLAGS_enable_authorize) { + auto *session = ectx()->rctx()->session(); + auto role = session->toRole(PrivilegeUtils::toRoleType(aclItem->getRoleType())); + auto rst = permission::PermissionManager::canWriteRole(session, + role, + spaceRet.value(), + *account); + if (!rst) { + doError(Status::PermissionError("Permission denied")); + return; + } + } + nebula::cpp2::RoleItem roleItem; roleItem.set_user(*account); roleItem.set_space_id(spaceRet.value()); diff --git a/src/graph/PrivilegeExecutor.h b/src/graph/PrivilegeExecutor.h index 06b6c4e9467..821ad357074 100644 --- a/src/graph/PrivilegeExecutor.h +++ b/src/graph/PrivilegeExecutor.h @@ -10,6 +10,7 @@ #include "graph/Executor.h" #include "meta/SchemaManager.h" #include "graph/GraphFlags.h" +#include "common/permission/PermissionManager.h" namespace nebula { namespace graph { diff --git a/src/graph/RequestContext.h b/src/graph/RequestContext.h index 4dbc34db646..85bb458adc5 100644 --- a/src/graph/RequestContext.h +++ b/src/graph/RequestContext.h @@ -11,7 +11,7 @@ #include "gen-cpp2/GraphService.h" #include "cpp/helpers.h" #include "time/Duration.h" -#include "graph/ClientSession.h" +#include "common/session/Session.h" /** * RequestContext holds context infos of a specific request from a client. @@ -52,7 +52,7 @@ class RequestContext final : public cpp::NonCopyable, public cpp::NonMovable { return promise_.getFuture(); } - void setSession(std::shared_ptr session) { + void setSession(std::shared_ptr session) { session_ = std::move(session); if (session_ != nullptr) { // keep the session active @@ -60,7 +60,7 @@ class RequestContext final : public cpp::NonCopyable, public cpp::NonMovable { } } - ClientSession* session() const { + session::Session* session() const { return session_.get(); } @@ -85,7 +85,7 @@ class RequestContext final : public cpp::NonCopyable, public cpp::NonMovable { std::string query_; Response resp_; folly::Promise promise_; - std::shared_ptr session_; + std::shared_ptr session_; folly::Executor *runner_{nullptr}; }; diff --git a/src/graph/SequentialExecutor.cpp b/src/graph/SequentialExecutor.cpp index 9071daeef0b..5a41f67d1a6 100644 --- a/src/graph/SequentialExecutor.cpp +++ b/src/graph/SequentialExecutor.cpp @@ -23,6 +23,16 @@ Status SequentialExecutor::prepare() { for (auto i = 0U; i < sentences_->sentences_.size(); i++) { auto *sentence = sentences_->sentences_[i].get(); auto executor = makeExecutor(sentence); + if (FLAGS_enable_authorize) { + auto *session = executor->ectx()->rctx()->session(); + /** + * Skip special operations check at here. they are : + * kUse, kDescribeSpace, kRevoke and kGrant. + */ + if (!PermissionCheck::permissionCheck(session, sentence)) { + return Status::PermissionError("Permission denied"); + } + } if (executor == nullptr) { return Status::Error("The statement has not been implemented"); } diff --git a/src/graph/SequentialExecutor.h b/src/graph/SequentialExecutor.h index 094328aa1b4..8d5055ac44b 100644 --- a/src/graph/SequentialExecutor.h +++ b/src/graph/SequentialExecutor.h @@ -9,7 +9,8 @@ #include "base/Base.h" #include "graph/Executor.h" - +#include "graph/GraphFlags.h" +#include "graph/PermissionCheck.h" namespace nebula { namespace graph { diff --git a/src/graph/SessionManager.cpp b/src/graph/SessionManager.cpp index d1ca38915fb..df629d1b1c4 100644 --- a/src/graph/SessionManager.cpp +++ b/src/graph/SessionManager.cpp @@ -29,7 +29,7 @@ SessionManager::~SessionManager() { } -StatusOr> +StatusOr> SessionManager::findSession(int64_t id) { folly::RWSpinLock::ReadHolder holder(rwlock_); auto iter = activeSessions_.find(id); @@ -40,7 +40,7 @@ SessionManager::findSession(int64_t id) { } -std::shared_ptr SessionManager::createSession() { +std::shared_ptr SessionManager::createSession() { folly::RWSpinLock::WriteHolder holder(rwlock_); auto sid = newSessionId(); while (true) { @@ -51,14 +51,14 @@ std::shared_ptr SessionManager::createSession() { sid = newSessionId(); } DCHECK_NE(sid, 0L); - auto session = ClientSession::create(sid); + auto session = session::Session::create(sid); activeSessions_[sid] = session; session->charge(); return session; } -std::shared_ptr SessionManager::removeSession(int64_t id) { +std::shared_ptr SessionManager::removeSession(int64_t id) { folly::RWSpinLock::WriteHolder holder(rwlock_); auto iter = activeSessions_.find(id); if (iter == activeSessions_.end()) { diff --git a/src/graph/SessionManager.h b/src/graph/SessionManager.h index d2dbe91c1b2..f174b536d9f 100644 --- a/src/graph/SessionManager.h +++ b/src/graph/SessionManager.h @@ -9,8 +9,8 @@ #include "base/Base.h" #include "base/StatusOr.h" -#include "graph/ClientSession.h" #include "thread/GenericWorker.h" +#include "common/session/Session.h" /** * SessionManager manages the client sessions, e.g. create new, find existing and drop expired. @@ -24,7 +24,7 @@ class SessionManager final { SessionManager(); ~SessionManager(); - using SessionPtr = std::shared_ptr; + using SessionPtr = std::shared_ptr; /** * Find an existing session */ diff --git a/src/graph/ShowExecutor.cpp b/src/graph/ShowExecutor.cpp index 3bb517d7657..a7d7098be9a 100644 --- a/src/graph/ShowExecutor.cpp +++ b/src/graph/ShowExecutor.cpp @@ -7,7 +7,8 @@ #include "graph/ShowExecutor.h" #include "network/NetworkUtils.h" #include "common/charset/Charset.h" - +#include "graph/GraphFlags.h" +#include "common/permission/PermissionManager.h" namespace nebula { namespace graph { @@ -243,6 +244,14 @@ void ShowExecutor::showSpaces() { resp_->set_column_names(std::move(header)); for (auto &space : retShowSpaces) { + if (FLAGS_enable_authorize) { + auto canShow = permission::PermissionManager::canShow(ectx()->rctx()->session(), + sentence_->showType(), + space.first); + if (!canShow) { + continue; + } + } std::vector row; row.emplace_back(); row.back().set_str(std::move(space.second)); @@ -526,7 +535,15 @@ void ShowExecutor::showCreateSpace() { sentence_->getName()->c_str(), resp.status().toString().c_str())); return; } - + if (FLAGS_enable_authorize) { + auto canShow = permission::PermissionManager::canShow(ectx()->rctx()->session(), + sentence_->showType(), + resp.value().get_space_id()); + if (!canShow) { + doError(Status::PermissionError()); + return; + } + } resp_ = std::make_unique(); std::vector header{"Space", "Create Space"}; resp_->set_column_names(std::move(header)); @@ -1090,7 +1107,25 @@ void ShowExecutor::showUsers() { void ShowExecutor::showRoles() { auto *space = sentence_->getName(); - auto future = ectx()->getMetaClient()->listRoles(*space); + auto *mc = ectx()->getMetaClient(); + + auto spaceRet = mc->getSpaceIdByNameFromCache(*space); + if (!spaceRet.ok()) { + doError(spaceRet.status()); + return; + } + + if (FLAGS_enable_authorize) { + auto canShow = permission::PermissionManager::canShow(ectx()->rctx()->session(), + sentence_->showType(), + spaceRet.value()); + if (!canShow) { + doError(Status::PermissionError()); + return; + } + } + + auto future = ectx()->getMetaClient()->listRoles(spaceRet.value()); auto *runner = ectx()->rctx()->runner(); auto cb = [this] (auto &&resp) { diff --git a/src/graph/SimpleAuthenticator.h b/src/graph/SimpleAuthenticator.h deleted file mode 100644 index 6ded4b242cc..00000000000 --- a/src/graph/SimpleAuthenticator.h +++ /dev/null @@ -1,32 +0,0 @@ -/* Copyright (c) 2018 vesoft inc. All rights reserved. - * - * This source code is licensed under Apache 2.0 License, - * attached with Common Clause Condition 1.0, found in the LICENSES directory. - */ - -#ifndef GRAPH_SIMPLEAUTHENTICATOR_H_ -#define GRAPH_SIMPLEAUTHENTICATOR_H_ - -#include "base/Base.h" -#include "graph/Authenticator.h" - -namespace nebula { -namespace graph { - -class SimpleAuthenticator final : public Authenticator { -public: - bool MUST_USE_RESULT auth(const std::string &user, - const std::string &password) override { - if (user == "user" && password == "password") { - return true; - } - return false; - } - -private: -}; - -} // namespace graph -} // namespace nebula - -#endif // GRAPH_SIMPLEAUTHENTICATOR_H_ diff --git a/src/graph/UseExecutor.cpp b/src/graph/UseExecutor.cpp index e43aa4f1472..fdb5a79dd02 100644 --- a/src/graph/UseExecutor.cpp +++ b/src/graph/UseExecutor.cpp @@ -32,7 +32,20 @@ void UseExecutor::execute() { } auto spaceId = resp.value().get_space_id(); + /** + * Permission check. + */ + if (FLAGS_enable_authorize) { + auto *session = ectx()->rctx()->session(); + auto rst = permission::PermissionManager::canReadSpace(session, spaceId); + if (!rst) { + doError(Status::PermissionError("Permission denied")); + return; + } + } + ectx()->rctx()->session()->setSpace(*sentence_->space(), spaceId); + FLOG_INFO("Graph space switched to `%s', space id: %d", sentence_->space()->c_str(), spaceId); diff --git a/src/graph/UseExecutor.h b/src/graph/UseExecutor.h index 66abd214b13..2e3491f7868 100644 --- a/src/graph/UseExecutor.h +++ b/src/graph/UseExecutor.h @@ -9,6 +9,8 @@ #include "base/Base.h" #include "graph/Executor.h" +#include "graph/GraphFlags.h" +#include "common/permission/PermissionManager.h" namespace nebula { namespace graph { diff --git a/src/graph/test/CMakeLists.txt b/src/graph/test/CMakeLists.txt index 0ff0a49010e..88c0fb9c633 100644 --- a/src/graph/test/CMakeLists.txt +++ b/src/graph/test/CMakeLists.txt @@ -33,6 +33,8 @@ set(GRAPH_TEST_LIBS $ $ $ + $ + $ ) set(GRAPH_TEST_CLIENT_LIBS @@ -372,3 +374,23 @@ nebula_add_test( wangle gtest ) + +nebula_add_test( + NAME + permission_test + SOURCES + PermissionTest.cpp + OBJECTS + $ + $ + $ + $ + ${GRAPH_TEST_LIBS} + LIBRARIES + ${THRIFT_LIBRARIES} + ${ROCKSDB_LIBRARIES} + proxygenlib + wangle + gtest +) + diff --git a/src/graph/test/PermissionTest.cpp b/src/graph/test/PermissionTest.cpp new file mode 100644 index 00000000000..7cde5417b50 --- /dev/null +++ b/src/graph/test/PermissionTest.cpp @@ -0,0 +1,811 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "base/Base.h" +#include "graph/test/TestEnv.h" +#include "graph/test/TestBase.h" +#include "meta/test/TestUtils.h" + +DECLARE_int32(heartbeat_interval_secs); + +namespace nebula { +namespace graph { + +class PermissionTest : public TestBase { +protected: + void SetUp() override { + TestBase::SetUp(); + // ... + } + + void TearDown() override { + // ... + TestBase::TearDown(); + } +}; + +TEST_F(PermissionTest, SimpleTest) { + FLAGS_enable_authorize = true; + /* + * test incorrect password. + */ + { + auto client = gEnv->getClient("root", "pwd"); + ASSERT_EQ(nullptr, client); + } + /* + * test incorrect user name. + */ + { + auto client = gEnv->getClient("user", "nebula"); + ASSERT_EQ(nullptr, client); + } + /* + * test root user password and use space. + */ + { + auto client = gEnv->getClient(); + ASSERT_NE(nullptr, client); + cpp2::ExecutionResponse resp; + std::string query = "CREATE SPACE my_space(partition_num=1, replica_factor=1)"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "USE my_space; CREATE TAG person(name string)"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + /* + * change root password, incorrect password. + */ + { + auto client = gEnv->getClient(); + ASSERT_NE(nullptr, client); + cpp2::ExecutionResponse resp; + std::string query = "CHANGE PASSWORD root FROM \"aa\" TO \"bb\""; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + } + /* + * change root password, correct password. + */ + { + auto client = gEnv->getClient(); + ASSERT_NE(nullptr, client); + cpp2::ExecutionResponse resp; + std::string query = "CHANGE PASSWORD root FROM \"nebula\" TO \"bb\""; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + /* + * verify password changed + */ + { + auto client = gEnv->getClient("root", "nebula"); + ASSERT_EQ(nullptr, client); + } + { + auto client = gEnv->getClient("root", "bb"); + ASSERT_NE(nullptr, client); + cpp2::ExecutionResponse resp; + std::string query = "CHANGE PASSWORD root FROM \"bb\" TO \"nebula\""; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } +} + +TEST_F(PermissionTest, UserWriteTest) { + FLAGS_enable_authorize = true; + auto client = gEnv->getClient(); + ASSERT_NE(nullptr, client); + { + cpp2::ExecutionResponse resp; + std::string query = "CREATE SPACE space1(partition_num=1, replica_factor=1)"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + sleep(FLAGS_heartbeat_interval_secs + 1); + { + cpp2::ExecutionResponse resp; + std::string query = "CREATE USER admin WITH PASSWORD \"admin\""; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "GRANT ROLE ADMIN ON space1 TO admin"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "GRANT ROLE GOD ON space1 TO admin"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "SHOW ROLES IN space1"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + ASSERT_EQ(1, resp.get_rows()->size()); + } + { + auto adminClient = gEnv->getClient("admin", "admin"); + ASSERT_NE(nullptr, adminClient); + cpp2::ExecutionResponse resp; + std::string query = "ALTER USER root WITH PASSWORD \"root\""; + auto code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + { + auto adminClient = gEnv->getClient("admin", "admin"); + ASSERT_NE(nullptr, adminClient); + cpp2::ExecutionResponse resp; + std::string query = "GRANT ROLE ADMIN ON space1 TO admin"; + auto code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + { + auto adminClient = gEnv->getClient("admin", "admin"); + ASSERT_NE(nullptr, adminClient); + cpp2::ExecutionResponse resp; + std::string query = "GRANT ROLE GOD ON space1 TO admin"; + auto code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + { + auto adminClient = gEnv->getClient("admin", "admin"); + ASSERT_NE(nullptr, adminClient); + cpp2::ExecutionResponse resp; + std::string query = "GRANT ROLE GOD ON space1 TO admin"; + auto code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + /** + * Reject the admin user grant or revoke to himself self + */ + { + auto adminClient = gEnv->getClient("admin", "admin"); + ASSERT_NE(nullptr, adminClient); + cpp2::ExecutionResponse resp; + std::string query = "GRANT ROLE GUEST ON space1 TO admin"; + auto code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + { + auto adminClient = gEnv->getClient("admin", "admin"); + ASSERT_NE(nullptr, adminClient); + cpp2::ExecutionResponse resp; + std::string query = "DROP USER admin"; + auto code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "DROP USER admin"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } +} + +TEST_F(PermissionTest, SchemaAndDataTest) { + FLAGS_enable_authorize = true; + auto client = gEnv->getClient(); + { + cpp2::ExecutionResponse resp; + std::string query = "CREATE SPACE space2(partition_num=1, replica_factor=1)"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + sleep(FLAGS_heartbeat_interval_secs + 1); + { + cpp2::ExecutionResponse resp; + std::string query = "CREATE USER admin WITH PASSWORD \"admin\""; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "GRANT ROLE ADMIN ON space2 TO admin"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "CREATE USER dba WITH PASSWORD \"dba\""; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "GRANT ROLE DBA ON space2 TO dba"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "CREATE USER user WITH PASSWORD \"user\""; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "GRANT ROLE USER ON space2 TO user"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "CREATE USER guest WITH PASSWORD \"guest\""; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "GRANT ROLE GUEST ON space2 TO guest"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + sleep(FLAGS_heartbeat_interval_secs + 1); + /** + * god write schema test + */ + { + cpp2::ExecutionResponse resp; + std::string query = "USE space2"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE TAG t1(t_c int)"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE EDGE e1(e_c int)"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE TAG INDEX tid1 ON t1(t_c)"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE EDGE INDEX eid1 ON e1(e_c)"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DESCRIBE TAG t1"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DESCRIBE EDGE e1"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DESCRIBE TAG INDEX tid1"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DESCRIBE EDGE INDEX eid1"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DROP TAG INDEX tid1"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DROP EDGE INDEX eid1"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "ALTER TAG t1 DROP (t_c)"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "ALTER EDGE e1 DROP (e_c)"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DROP TAG t1"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DROP EDGE e1"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + /** + * admin write schema test + */ + auto adminClient = gEnv->getClient("admin", "admin"); + ASSERT_NE(nullptr, adminClient); + { + cpp2::ExecutionResponse resp; + std::string query = "USE space2"; + auto code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE TAG t1(t_c int)"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE EDGE e1(e_c int)"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE TAG INDEX tid1 ON t1(t_c)"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE EDGE INDEX eid1 ON e1(e_c)"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DESCRIBE TAG t1"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DESCRIBE EDGE e1"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DESCRIBE TAG INDEX tid1"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DESCRIBE EDGE INDEX eid1"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DROP TAG INDEX tid1"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DROP EDGE INDEX eid1"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "ALTER TAG t1 DROP (t_c)"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "ALTER EDGE e1 DROP (e_c)"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DROP TAG t1"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DROP EDGE e1"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + + /** + * dba write schema test + */ + auto dbaClient = gEnv->getClient("dba", "dba"); + ASSERT_NE(nullptr, dbaClient); + { + cpp2::ExecutionResponse resp; + std::string query = "USE space2"; + auto code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE TAG t1(t_c int)"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE EDGE e1(e_c int)"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE TAG INDEX tid1 ON t1(t_c)"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE EDGE INDEX eid1 ON e1(e_c)"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DESCRIBE TAG t1"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DESCRIBE EDGE e1"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DESCRIBE TAG INDEX tid1"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DESCRIBE EDGE INDEX eid1"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DROP TAG INDEX tid1"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DROP EDGE INDEX eid1"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "ALTER TAG t1 DROP (t_c)"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "ALTER EDGE e1 DROP (e_c)"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DROP TAG t1"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "DROP EDGE e1"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + + /** + * user write schema test + */ + auto userClient = gEnv->getClient("user", "user"); + ASSERT_NE(nullptr, userClient); + { + cpp2::ExecutionResponse resp; + std::string query = "USE space2"; + auto code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE TAG t1(t_c int)"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "CREATE EDGE e1(e_c int)"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "CREATE TAG INDEX tid1 ON t1(t_c)"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "CREATE EDGE INDEX eid1 ON e1(e_c)"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "DESCRIBE TAG t1"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + + query = "DESCRIBE EDGE e1"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + + query = "DESCRIBE TAG INDEX tid1"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + + query = "DESCRIBE EDGE INDEX eid1"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + + query = "DROP TAG INDEX tid1"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "DROP EDGE INDEX eid1"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "ALTER TAG t1 DROP (t_c)"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "ALTER EDGE e1 DROP (e_c)"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "DROP TAG t1"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "DROP EDGE e1"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + + /** + * guest write schema test + */ + auto guestClient = gEnv->getClient("guest", "guest"); + ASSERT_NE(nullptr, guestClient); + { + cpp2::ExecutionResponse resp; + std::string query = "USE space2"; + auto code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE TAG t1(t_c int)"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "CREATE EDGE e1(e_c int)"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "CREATE TAG INDEX tid1 ON t1(t_c)"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "CREATE EDGE INDEX eid1 ON e1(e_c)"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "DESCRIBE TAG t1"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + + query = "DESCRIBE EDGE e1"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + + query = "DESCRIBE TAG INDEX tid1"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + + query = "DESCRIBE EDGE INDEX eid1"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + + query = "DROP TAG INDEX tid1"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "DROP EDGE INDEX eid1"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "ALTER TAG t1 DROP (t_c)"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "ALTER EDGE e1 DROP (e_c)"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "DROP TAG t1"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "DROP EDGE e1"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + /** + * god write data test + */ + { + cpp2::ExecutionResponse resp; + auto query = "CREATE TAG t1(t_c int)"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE EDGE e1(e_c int)"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + sleep(FLAGS_heartbeat_interval_secs + 1); + + query = "INSERT VERTEX t1(t_c) VALUES 1:(1)"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "INSERT EDGE e1(e_c) VALUES 1 -> 2:(95)"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "GO FROM 1 OVER e1"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + /** + * admin write data test + */ + { + cpp2::ExecutionResponse resp; + auto query = "INSERT VERTEX t1(t_c) VALUES 1:(1)"; + auto code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "INSERT EDGE e1(e_c) VALUES 1 -> 2:(95)"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "GO FROM 1 OVER e1"; + code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + /** + * dba write data test + */ + { + cpp2::ExecutionResponse resp; + auto query = "INSERT VERTEX t1(t_c) VALUES 1:(1)"; + auto code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "INSERT EDGE e1(e_c) VALUES 1 -> 2:(95)"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "GO FROM 1 OVER e1"; + code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + /** + * user write data test + */ + { + cpp2::ExecutionResponse resp; + auto query = "INSERT VERTEX t1(t_c) VALUES 1:(1)"; + auto code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "INSERT EDGE e1(e_c) VALUES 1 -> 2:(95)"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "GO FROM 1 OVER e1"; + code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + /** + * guest write data test + */ + { + cpp2::ExecutionResponse resp; + auto query = "INSERT VERTEX t1(t_c) VALUES 1:(1)"; + auto code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "INSERT EDGE e1(e_c) VALUES 1 -> 2:(95)"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "GO FROM 1 OVER e1"; + code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + /** + * use space test + */ + { + cpp2::ExecutionResponse resp; + std::string query = "CREATE SPACE space3(partition_num=1, replica_factor=1)"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "USE space3"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "USE space3"; + auto code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "USE space3"; + auto code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "USE space3"; + auto code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "USE space3"; + auto code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } +} +TEST_F(PermissionTest, ShowTest) { + auto client = gEnv->getClient(); + auto adminClient = gEnv->getClient("admin", "admin"); + auto dbaClient = gEnv->getClient("dba", "dba"); + auto userClient = gEnv->getClient("user", "user"); + auto guestClient = gEnv->getClient("guest", "guest"); + { + cpp2::ExecutionResponse resp; + std::string query = "CREATE SPACE space4(partition_num=1, replica_factor=1)"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + sleep(FLAGS_heartbeat_interval_secs + 1); + { + cpp2::ExecutionResponse resp; + std::string query = "SHOW SPACES"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + ASSERT_EQ(5, resp.get_rows()->size()); + } + { + cpp2::ExecutionResponse resp; + std::string query = "SHOW SPACES"; + auto code = adminClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + ASSERT_EQ(1, resp.get_rows()->size()); + } + { + cpp2::ExecutionResponse resp; + std::string query = "SHOW SPACES"; + auto code = dbaClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + ASSERT_EQ(1, resp.get_rows()->size()); + } + { + cpp2::ExecutionResponse resp; + std::string query = "SHOW SPACES"; + auto code = userClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + ASSERT_EQ(1, resp.get_rows()->size()); + } + { + cpp2::ExecutionResponse resp; + std::string query = "SHOW SPACES"; + auto code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + ASSERT_EQ(1, resp.get_rows()->size()); + } + { + cpp2::ExecutionResponse resp; + std::string query = "SHOW ROLES IN space1"; + auto code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "SHOW ROLES IN space2"; + auto code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "SHOW ROLES IN space1"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "SHOW CREATE SPACE space1"; + auto code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "SHOW CREATE SPACE space1"; + auto code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } + { + cpp2::ExecutionResponse resp; + std::string query = "SHOW CREATE SPACE space2"; + auto code = guestClient->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + } +} +} // namespace graph +} // namespace nebula diff --git a/src/graph/test/TestEnv.cpp b/src/graph/test/TestEnv.cpp index a2408f4a605..4a978294876 100644 --- a/src/graph/test/TestEnv.cpp +++ b/src/graph/test/TestEnv.cpp @@ -8,6 +8,7 @@ #include "graph/test/TestEnv.h" #include "meta/test/TestUtils.h" #include "storage/test/TestUtils.h" +#include "meta/RootUserMan.h" DECLARE_int32(heartbeat_interval_secs); DECLARE_string(meta_server_addrs); @@ -48,6 +49,10 @@ void TestEnv::SetUp() { } auto& localhost = hostRet.value(); + if (!nebula::meta::RootUserMan::initRootUser(metaServer_->kvStore_.get())) { + LOG(ERROR) << "Init root user failed"; + } + meta::MetaClientOptions options; options.localHost_ = localhost; options.clusterId_ = kClusterId; @@ -94,9 +99,10 @@ uint16_t TestEnv::storageServerPort() const { return storageServer_->port_; } -std::unique_ptr TestEnv::getClient() const { +std::unique_ptr TestEnv::getClient(const std::string& user, + const std::string& password) const { auto client = std::make_unique("127.0.0.1", graphServerPort()); - if (cpp2::ErrorCode::SUCCEEDED != client->connect("user", "password")) { + if (cpp2::ErrorCode::SUCCEEDED != client->connect(user, password)) { return nullptr; } return client; diff --git a/src/graph/test/TestEnv.h b/src/graph/test/TestEnv.h index c28ec6216c2..66c1ca55501 100644 --- a/src/graph/test/TestEnv.h +++ b/src/graph/test/TestEnv.h @@ -13,6 +13,7 @@ #include "test/ServerContext.h" #include #include "TestUtils.h" +#include "graph/GraphFlags.h" namespace nebula { namespace graph { @@ -31,7 +32,8 @@ class TestEnv : public ::testing::Environment { uint16_t metaServerPort() const; uint16_t storageServerPort() const; - std::unique_ptr getClient() const; + std::unique_ptr getClient(const std::string& user = "root", + const std::string& password = "nebula") const; meta::ClientBasedGflagsManager* gflagsManager(); diff --git a/src/graph/test/UserTest.cpp b/src/graph/test/UserTest.cpp index 7447f1068b3..89a67d38795 100644 --- a/src/graph/test/UserTest.cpp +++ b/src/graph/test/UserTest.cpp @@ -60,12 +60,12 @@ TEST_F(UserTest, CreateUser) { std::string query = "SHOW USERS"; auto code = client->execute(query, resp); ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); - ASSERT_EQ(2, resp.get_rows()->size()); + ASSERT_EQ(3, resp.get_rows()->size()); decltype(resp.column_names) colNames = {"Account"}; ASSERT_TRUE(verifyColNames(resp, colNames)); std::vector> - rows = { {"user1"}, {"user2"}, }; + rows = {{"root"}, {"user1"}, {"user2"}, }; ASSERT_TRUE(verifyResult(resp, rows)); } } @@ -119,7 +119,7 @@ TEST_F(UserTest, DropUser) { std::string query = "SHOW USERS"; auto code = client->execute(query, resp); ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); - ASSERT_EQ(2, resp.get_rows()->size()); + ASSERT_EQ(3, resp.get_rows()->size()); } } @@ -175,13 +175,13 @@ TEST_F(UserTest, GrantRevoke) { // user not exist. expect fail. { cpp2::ExecutionResponse resp; - std::string cmd = "GRANT DBA ON user_space TO user"; + std::string cmd = "GRANT DBA ON user_space TO user"; auto code = client->execute(cmd, resp); - ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); } { cpp2::ExecutionResponse resp; - std::string cmd = "GRANT GOD ON user_space TO user1"; + std::string cmd = "GRANT GOD ON user_space TO user1"; auto code = client->execute(cmd, resp); ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); } diff --git a/src/interface/common.thrift b/src/interface/common.thrift index 9156d6bff2b..8fe8bbff948 100644 --- a/src/interface/common.thrift +++ b/src/interface/common.thrift @@ -112,13 +112,13 @@ struct Pair { } /** -** GOD is A global senior administrator.like root of Linux systems. -** ADMIN is an administrator for a given Graph Space. -** DBA is an schema administrator for a given Graph Space. -** USER is a normal user for a given Graph Space. A User can access (read and write) the data in the Graph Space. -** GUEST is a read-only role for a given Graph Space. A Guest cannot modify the data in the Graph Space. -** Refer to header file src/graph/PermissionManager.h for details. -**/ + ** GOD is A global senior administrator.like root of Linux systems. + ** ADMIN is an administrator for a given Graph Space. + ** DBA is an schema administrator for a given Graph Space. + ** USER is a normal user for a given Graph Space. A User can access (read and write) the data in the Graph Space. + ** GUEST is a read-only role for a given Graph Space. A Guest cannot modify the data in the Graph Space. + ** Refer to header file src/graph/PermissionManager.h for details. + **/ enum RoleType { GOD = 0x01, diff --git a/src/interface/graph.thrift b/src/interface/graph.thrift index cd55f7d62e0..85f5b307ea9 100644 --- a/src/interface/graph.thrift +++ b/src/interface/graph.thrift @@ -29,6 +29,11 @@ enum ErrorCode { E_EXECUTION_ERROR = -8, // Nothing is executed When command is comment E_STATEMENT_EMTPY = -9, + + // User and permission error + E_USER_NOT_FOUND = -10, + E_BAD_PERMISSION = -11, + } (cpp.enum_strict) diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift index 8a6c696a453..62eb40082b8 100644 --- a/src/interface/meta.thrift +++ b/src/interface/meta.thrift @@ -540,7 +540,11 @@ struct ListUsersResp { } struct ListRolesReq { - 1: string space, + 1: common.GraphSpaceID space_id, +} + +struct GetUserRolesReq { + 1: string account, } struct ListRolesResp { @@ -737,6 +741,7 @@ service MetaService { ExecResp revokeRole(1: RevokeRoleReq req); ListUsersResp listUsers(1: ListUsersReq req); ListRolesResp listRoles(1: ListRolesReq req); + ListRolesResp getUserRoles(1: GetUserRolesReq req); ExecResp authCheck(1: AuthCheckReq req); ExecResp changePassword(1: ChangePasswordReq req); diff --git a/src/meta/MetaServiceHandler.cpp b/src/meta/MetaServiceHandler.cpp index 3d15cbe7a33..c6b0d0b6076 100644 --- a/src/meta/MetaServiceHandler.cpp +++ b/src/meta/MetaServiceHandler.cpp @@ -338,6 +338,12 @@ MetaServiceHandler::future_authCheck(const cpp2::AuthCheckReq& req) { RETURN_FUTURE(processor); } +folly::Future +MetaServiceHandler::future_getUserRoles(const cpp2::GetUserRolesReq& req) { + auto* processor = GetUserRolesProcessor::instance(kvstore_); + RETURN_FUTURE(processor); +} + folly::Future MetaServiceHandler::future_balance(const cpp2::BalanceReq& req) { auto* processor = BalanceProcessor::instance(kvstore_); diff --git a/src/meta/MetaServiceHandler.h b/src/meta/MetaServiceHandler.h index 990893d0aeb..333dbcbc3e8 100644 --- a/src/meta/MetaServiceHandler.h +++ b/src/meta/MetaServiceHandler.h @@ -172,6 +172,9 @@ class MetaServiceHandler final : public cpp2::MetaServiceSvIf { folly::Future future_authCheck(const cpp2::AuthCheckReq& req) override; + folly::Future + future_getUserRoles(const cpp2::GetUserRolesReq& req) override; + /** * HeartBeat * */ diff --git a/src/meta/MetaServiceUtils.cpp b/src/meta/MetaServiceUtils.cpp index db480225c5f..dd49d4f4ea4 100644 --- a/src/meta/MetaServiceUtils.cpp +++ b/src/meta/MetaServiceUtils.cpp @@ -556,6 +556,10 @@ std::string MetaServiceUtils::parseRoleUser(folly::StringPiece key) { return key.subpiece(offset, key.size() - offset).str(); } +GraphSpaceID MetaServiceUtils::parseRoleSpace(folly::StringPiece key) { + return *reinterpret_cast(key.data() + kRolesTable.size()); +} + std::string MetaServiceUtils::rolesPrefix() { return kRolesTable; } diff --git a/src/meta/MetaServiceUtils.h b/src/meta/MetaServiceUtils.h index 8e21d39bbfe..0f820be8a37 100644 --- a/src/meta/MetaServiceUtils.h +++ b/src/meta/MetaServiceUtils.h @@ -153,6 +153,8 @@ class MetaServiceUtils final { static std::string parseRoleUser(folly::StringPiece key); + static GraphSpaceID parseRoleSpace(folly::StringPiece key); + static std::string rolesPrefix(); static std::string roleSpacePrefix(GraphSpaceID spaceId); diff --git a/src/meta/client/MetaClient.cpp b/src/meta/client/MetaClient.cpp index 56f187c2048..2782746536a 100644 --- a/src/meta/client/MetaClient.cpp +++ b/src/meta/client/MetaClient.cpp @@ -1622,6 +1622,20 @@ const std::vector& MetaClient::getAddresses() { return addrs_; } +StatusOr> +MetaClient::getRolesByUser(const std::string& user) { + auto ret = getUserRoles(std::move(user)).get(); + if (ret.ok()) { + return std::move(ret).value(); + } + return ret.status(); +} + +bool MetaClient::authenticationCheck(std::string account, std::string password) { + auto ret = authCheck(std::move(account), std::move(password)).get(); + return ret.ok(); +} + StatusOr MetaClient::getLatestTagVersionFromCache(const GraphSpaceID& space, const TagID& tagId) { if (!ready_) { @@ -1786,9 +1800,9 @@ MetaClient::listUsers() { } folly::Future>> -MetaClient::listRoles(std::string space) { +MetaClient::listRoles(GraphSpaceID space) { cpp2::ListRolesReq req; - req.set_space(std::move(space)); + req.set_space_id(std::move(space)); folly::Promise>> promise; auto future = promise.getFuture(); getResponse(std::move(req), [] (auto client, auto request) { @@ -1832,6 +1846,20 @@ MetaClient::authCheck(std::string account, std::string password) { return future; } +folly::Future>> +MetaClient::getUserRoles(std::string account) { + cpp2::GetUserRolesReq req; + req.set_account(std::move(account)); + folly::Promise>> promise; + auto future = promise.getFuture(); + getResponse(std::move(req), [] (auto client, auto request) { + return client->future_getUserRoles(request); + }, [] (cpp2::ListRolesResp&& resp) -> decltype(auto) { + return std::move(resp).get_roles(); + }, std::move(promise)); + return future; +} + folly::Future> MetaClient::balance(std::vector hostDel, bool isStop) { cpp2::BalanceReq req; diff --git a/src/meta/client/MetaClient.h b/src/meta/client/MetaClient.h index a8c5e741c28..77664fb7fff 100644 --- a/src/meta/client/MetaClient.h +++ b/src/meta/client/MetaClient.h @@ -353,7 +353,7 @@ class MetaClient { listUsers(); folly::Future>> - listRoles(std::string space); + listRoles(GraphSpaceID space); folly::Future> changePassword(std::string account, std::string newPwd, std::string oldPwd); @@ -361,6 +361,9 @@ class MetaClient { folly::Future> authCheck(std::string account, std::string password); + folly::Future>> + getUserRoles(std::string account); + // Operations for admin folly::Future> balance(std::vector hostDel, bool isStop = false); @@ -472,6 +475,11 @@ class MetaClient { EdgeType edgeType, const std::string& field); + StatusOr> + getRolesByUser(const std::string& user); + + bool authenticationCheck(std::string account, std::string password); + Status refreshCache(); StatusOr loadLeader(); diff --git a/src/meta/processors/usersMan/AuthenticationProcessor.cpp b/src/meta/processors/usersMan/AuthenticationProcessor.cpp index d056bdd7b31..5b050fdc9f0 100644 --- a/src/meta/processors/usersMan/AuthenticationProcessor.cpp +++ b/src/meta/processors/usersMan/AuthenticationProcessor.cpp @@ -20,7 +20,7 @@ void CreateUserProcessor::process(const cpp2::CreateUserReq& req) { code = cpp2::ErrorCode::SUCCEEDED; } else { LOG(ERROR) << "Create User Failed : User " << account - << " have existed!"; + << " already existed!"; code = cpp2::ErrorCode::E_EXISTED; } handleErrorCode(code); @@ -198,19 +198,20 @@ void ListUsersProcessor::process(const cpp2::ListUsersReq& req) { void ListRolesProcessor::process(const cpp2::ListRolesReq& req) { - auto spaceRet = getSpaceId(req.get_space()); - if (!spaceRet.ok()) { - LOG(ERROR) << "space dose not found"; - handleErrorCode(MetaCommon::to(spaceRet.status())); - onFinished(); - return; + auto spaceId = req.get_space_id(); + /** + * Because the god role's space id is kDefaultSpaceId. + * skip the space check when space id is kDefaultSpaceId. + **/ + if (spaceId != kDefaultSpaceId) { + CHECK_SPACE_ID_AND_RETURN(spaceId); } folly::SharedMutex::ReadHolder rHolder(LockUtils::userLock()); - auto prefix = MetaServiceUtils::roleSpacePrefix(spaceRet.value()); + auto prefix = MetaServiceUtils::roleSpacePrefix(spaceId); std::unique_ptr iter; auto ret = kvstore_->prefix(kDefaultSpaceId, kDefaultPartId, prefix, &iter); if (ret != kvstore::ResultCode::SUCCEEDED) { - LOG(ERROR) << "Can't find any roles by space id " << spaceRet.value(); + LOG(ERROR) << "Can't find any roles by space id " << spaceId; handleErrorCode(cpp2::ErrorCode::E_NOT_FOUND); onFinished(); return; @@ -222,7 +223,7 @@ void ListRolesProcessor::process(const cpp2::ListRolesReq& req) { auto val = iter->val(); nebula::cpp2::RoleItem role; role.set_user(std::move(account)); - role.set_space_id(spaceRet.value()); + role.set_space_id(spaceId); role.set_role_type(*reinterpret_cast(val.begin())); roles.emplace_back(role); iter->next(); @@ -249,5 +250,37 @@ void AuthCheckProcessor::process(const cpp2::AuthCheckReq& req) { handleErrorCode(cpp2::ErrorCode::SUCCEEDED); onFinished(); } + +void GetUserRolesProcessor::process(const cpp2::GetUserRolesReq& req) { + folly::SharedMutex::WriteHolder wHolder(LockUtils::userLock()); + auto prefix = MetaServiceUtils::rolesPrefix(); + std::unique_ptr iter; + auto ret = kvstore_->prefix(kDefaultSpaceId, kDefaultPartId, prefix, &iter); + if (ret != kvstore::ResultCode::SUCCEEDED) { + LOG(ERROR) << "Can't find any roles by user " << req.get_account(); + handleErrorCode(cpp2::ErrorCode::E_NOT_FOUND); + onFinished(); + return; + } + + decltype(resp_.roles) roles; + while (iter->valid()) { + auto account = MetaServiceUtils::parseRoleUser(iter->key()); + auto spaceId = MetaServiceUtils::parseRoleSpace(iter->key()); + if (account == req.get_account()) { + auto val = iter->val(); + nebula::cpp2::RoleItem role; + role.set_user(std::move(account)); + role.set_space_id(spaceId); + role.set_role_type(*reinterpret_cast(val.begin())); + roles.emplace_back(role); + } + iter->next(); + } + resp_.set_roles(roles); + handleErrorCode(cpp2::ErrorCode::SUCCEEDED); + onFinished(); +} + } // namespace meta } // namespace nebula diff --git a/src/meta/processors/usersMan/AuthenticationProcessor.h b/src/meta/processors/usersMan/AuthenticationProcessor.h index 8370d441bec..3457cefa6aa 100644 --- a/src/meta/processors/usersMan/AuthenticationProcessor.h +++ b/src/meta/processors/usersMan/AuthenticationProcessor.h @@ -134,6 +134,19 @@ class AuthCheckProcessor : public BaseProcessor { explicit AuthCheckProcessor(kvstore::KVStore* kvstore) : BaseProcessor(kvstore) {} }; + +class GetUserRolesProcessor : public BaseProcessor { +public: + static GetUserRolesProcessor* instance(kvstore::KVStore* kvstore) { + return new GetUserRolesProcessor(kvstore); + } + + void process(const cpp2::GetUserRolesReq& req); + +private: + explicit GetUserRolesProcessor(kvstore::KVStore* kvstore) + : BaseProcessor(kvstore) {} +}; } // namespace meta } // namespace nebula #endif // META_AUTHENTICATIONPROCESSOR_H diff --git a/src/meta/test/AuthProcessorTest.cpp b/src/meta/test/AuthProcessorTest.cpp index 536d0bf1efa..bfd187bee79 100644 --- a/src/meta/test/AuthProcessorTest.cpp +++ b/src/meta/test/AuthProcessorTest.cpp @@ -355,7 +355,7 @@ TEST(AuthProcessorTest, GrantRevokeTest) { // list roles. { cpp2::ListRolesReq req; - req.set_space("space1"); + req.set_space_id(space1); auto* processor = ListRolesProcessor::instance(kv.get()); auto f = processor->getFuture(); processor->process(req); @@ -376,7 +376,7 @@ TEST(AuthProcessorTest, GrantRevokeTest) { // list roles. { cpp2::ListRolesReq req; - req.set_space("space2"); + req.set_space_id(space2); auto* processor = ListRolesProcessor::instance(kv.get()); auto f = processor->getFuture(); processor->process(req); @@ -460,7 +460,7 @@ TEST(AuthProcessorTest, GrantRevokeTest) { // list roles. { cpp2::ListRolesReq req; - req.set_space("space1"); + req.set_space_id(space1); auto* processor = ListRolesProcessor::instance(kv.get()); auto f = processor->getFuture(); processor->process(req); @@ -477,7 +477,7 @@ TEST(AuthProcessorTest, GrantRevokeTest) { // list roles. { cpp2::ListRolesReq req; - req.set_space("space2"); + req.set_space_id(space2); auto* processor = ListRolesProcessor::instance(kv.get()); auto f = processor->getFuture(); processor->process(req); @@ -528,7 +528,7 @@ TEST(AuthProcessorTest, GrantRevokeTest) { // list roles. { cpp2::ListRolesReq req; - req.set_space("space2"); + req.set_space_id(space2); auto* processor = ListRolesProcessor::instance(kv.get()); auto f = processor->getFuture(); processor->process(req); @@ -552,7 +552,7 @@ TEST(AuthProcessorTest, GrantRevokeTest) { } { cpp2::ListRolesReq req; - req.set_space("space1"); + req.set_space_id(space1); auto* processor = ListRolesProcessor::instance(kv.get()); auto f = processor->getFuture(); processor->process(req); diff --git a/src/parser/Sentence.h b/src/parser/Sentence.h index ad40b7a7661..02e7146e272 100644 --- a/src/parser/Sentence.h +++ b/src/parser/Sentence.h @@ -47,7 +47,6 @@ class Sentence { kShow, kDeleteVertex, kDeleteEdges, - kFind, kLookup, kCreateSpace, kDropSpace, From e61cb6a90285c5a6f5ffa70aa300f712b1f7d80c Mon Sep 17 00:00:00 2001 From: bright-starry-sky Date: Mon, 16 Mar 2020 22:15:45 +0800 Subject: [PATCH 2/5] Addressed dangleptr's comments --- src/common/permission/PermissionManager.cpp | 18 +++++----- src/common/session/Session.h | 8 ++--- src/graph/GraphService.cpp | 15 +++----- src/graph/SequentialExecutor.cpp | 1 + src/graph/test/PermissionTest.cpp | 14 ++++++++ src/graph/test/UserTest.cpp | 10 +++--- src/meta/client/MetaClient.cpp | 39 +++++++++++++++++---- src/meta/client/MetaClient.h | 10 ++++-- 8 files changed, 79 insertions(+), 36 deletions(-) diff --git a/src/common/permission/PermissionManager.cpp b/src/common/permission/PermissionManager.cpp index c97cc7e91e5..b5b0b327ac0 100644 --- a/src/common/permission/PermissionManager.cpp +++ b/src/common/permission/PermissionManager.cpp @@ -33,13 +33,13 @@ bool PermissionManager::canReadSpace(session::Session *session, GraphSpaceID spa // static bool PermissionManager::canReadSchemaData(session::Session *session) { - if (session->isGod()) { - return true; - } if (session->space() == -1) { LOG(ERROR) << "The space name is not set"; return false; } + if (session->isGod()) { + return true; + } bool havePermission = false; switch (session->roleWithSpace(session->space())) { case session::Role::GOD : @@ -64,13 +64,13 @@ bool PermissionManager::canWriteSpace(session::Session *session) { // static bool PermissionManager::canWriteSchema(session::Session *session) { - if (session->isGod()) { - return true; - } if (session->space() == -1) { LOG(ERROR) << "The space name is not set"; return false; } + if (session->isGod()) { + return true; + } bool havePermission = false; switch (session->roleWithSpace(session->space())) { case session::Role::GOD : @@ -127,13 +127,13 @@ bool PermissionManager::canWriteRole(session::Session *session, // static bool PermissionManager::canWriteData(session::Session *session) { - if (session->isGod()) { - return true; - } if (session->space() == -1) { LOG(ERROR) << "The space name is not set"; return false; } + if (session->isGod()) { + return true; + } bool havePermission = false; switch (session->roleWithSpace(session->space())) { case session::Role::GOD : diff --git a/src/common/session/Session.h b/src/common/session/Session.h index 769ed18bb7d..22e28e43284 100644 --- a/src/common/session/Session.h +++ b/src/common/session/Session.h @@ -48,11 +48,11 @@ class Session final { } const std::string& user() const { - return user_; + return account_; } - void setUser(std::string user) { - user_ = std::move(user); + void setAccount(std::string account) { + account_ = std::move(account); } std::unordered_map roles() const { @@ -114,7 +114,7 @@ class Session final { int64_t id_{0}; GraphSpaceID space_{-1}; std::string spaceName_; - std::string user_; + std::string account_; time::Duration idleDuration_; /* * map diff --git a/src/graph/GraphService.cpp b/src/graph/GraphService.cpp index bd29dbc858d..4817159b748 100644 --- a/src/graph/GraphService.cpp +++ b/src/graph/GraphService.cpp @@ -51,22 +51,17 @@ folly::Future GraphService::future_authenticate( RequestContext ctx; auto session = sessionManager_->createSession(); - session->setUser(username); + session->setAccount(username); ctx.setSession(std::move(session)); if (!FLAGS_enable_authorize) { onHandle(ctx, cpp2::ErrorCode::SUCCEEDED); } else if (auth(username, password)) { - auto ret = metaClient_->getRolesByUser(username); - if (ret.ok()) { - auto roles = std::move(ret).value(); - for (const auto& role : roles) { - ctx.session()->setRole(role.get_space_id(), toRole(role.get_role_type())); - } - onHandle(ctx, cpp2::ErrorCode::SUCCEEDED); - } else { - onHandle(ctx, cpp2::ErrorCode::E_EXECUTION_ERROR); + auto roles = metaClient_->getRolesByUserFromCache(username); + for (const auto& role : roles) { + ctx.session()->setRole(role.get_space_id(), toRole(role.get_role_type())); } + onHandle(ctx, cpp2::ErrorCode::SUCCEEDED); } else { onHandle(ctx, cpp2::ErrorCode::E_BAD_USERNAME_PASSWORD); } diff --git a/src/graph/SequentialExecutor.cpp b/src/graph/SequentialExecutor.cpp index 5a41f67d1a6..4b33a260c54 100644 --- a/src/graph/SequentialExecutor.cpp +++ b/src/graph/SequentialExecutor.cpp @@ -19,6 +19,7 @@ SequentialExecutor::SequentialExecutor(SequentialSentences *sentences, } + Status SequentialExecutor::prepare() { for (auto i = 0U; i < sentences_->sentences_.size(); i++) { auto *sentence = sentences_->sentences_[i].get(); diff --git a/src/graph/test/PermissionTest.cpp b/src/graph/test/PermissionTest.cpp index 7cde5417b50..5b08964f940 100644 --- a/src/graph/test/PermissionTest.cpp +++ b/src/graph/test/PermissionTest.cpp @@ -56,6 +56,14 @@ TEST_F(PermissionTest, SimpleTest) { query = "USE my_space; CREATE TAG person(name string)"; code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + + query = "USE my_space"; + code = client->execute(query, resp); + ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + + query = "CREATE TAG person(name string)"; + code = client->execute(query, resp); ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); } /* @@ -133,6 +141,7 @@ TEST_F(PermissionTest, UserWriteTest) { ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); ASSERT_EQ(1, resp.get_rows()->size()); } + sleep(FLAGS_heartbeat_interval_secs + 1); { auto adminClient = gEnv->getClient("admin", "admin"); ASSERT_NE(nullptr, adminClient); @@ -319,6 +328,7 @@ TEST_F(PermissionTest, SchemaAndDataTest) { /** * admin write schema test */ + sleep(FLAGS_heartbeat_interval_secs + 1); auto adminClient = gEnv->getClient("admin", "admin"); ASSERT_NE(nullptr, adminClient); { @@ -387,6 +397,7 @@ TEST_F(PermissionTest, SchemaAndDataTest) { /** * dba write schema test */ + sleep(FLAGS_heartbeat_interval_secs + 1); auto dbaClient = gEnv->getClient("dba", "dba"); ASSERT_NE(nullptr, dbaClient); { @@ -455,6 +466,7 @@ TEST_F(PermissionTest, SchemaAndDataTest) { /** * user write schema test */ + sleep(FLAGS_heartbeat_interval_secs + 1); auto userClient = gEnv->getClient("user", "user"); ASSERT_NE(nullptr, userClient); { @@ -523,6 +535,7 @@ TEST_F(PermissionTest, SchemaAndDataTest) { /** * guest write schema test */ + sleep(FLAGS_heartbeat_interval_secs + 1); auto guestClient = gEnv->getClient("guest", "guest"); ASSERT_NE(nullptr, guestClient); { @@ -691,6 +704,7 @@ TEST_F(PermissionTest, SchemaAndDataTest) { auto code = client->execute(query, resp); ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); } + sleep(FLAGS_heartbeat_interval_secs + 1); { cpp2::ExecutionResponse resp; std::string query = "USE space3"; diff --git a/src/graph/test/UserTest.cpp b/src/graph/test/UserTest.cpp index 89a67d38795..29a69712dda 100644 --- a/src/graph/test/UserTest.cpp +++ b/src/graph/test/UserTest.cpp @@ -8,7 +8,7 @@ #include "graph/test/TestEnv.h" #include "graph/test/TestBase.h" -DECLARE_uint32(raft_heartbeat_interval_secs); +DECLARE_int32(heartbeat_interval_secs); namespace nebula { namespace graph { @@ -164,7 +164,7 @@ TEST_F(UserTest, GrantRevoke) { auto code = client->execute(cmd, resp); ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); } - sleep(FLAGS_raft_heartbeat_interval_secs); + sleep(FLAGS_heartbeat_interval_secs + 1); // must set the space if is not god role. expect fail. { cpp2::ExecutionResponse resp; @@ -177,13 +177,13 @@ TEST_F(UserTest, GrantRevoke) { cpp2::ExecutionResponse resp; std::string cmd = "GRANT DBA ON user_space TO user"; auto code = client->execute(cmd, resp); - ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); + ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); } { cpp2::ExecutionResponse resp; std::string cmd = "GRANT GOD ON user_space TO user1"; auto code = client->execute(cmd, resp); - ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + ASSERT_EQ(cpp2::ErrorCode::E_BAD_PERMISSION, code); } // space not exists. expect fail. { @@ -197,7 +197,7 @@ TEST_F(UserTest, GrantRevoke) { std::string query = "SHOW USERS"; auto code = client->execute(query, resp); ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); - ASSERT_EQ(2, resp.get_rows()->size()); + ASSERT_EQ(3, resp.get_rows()->size()); } { cpp2::ExecutionResponse resp; diff --git a/src/meta/client/MetaClient.cpp b/src/meta/client/MetaClient.cpp index 2782746536a..917c4d6ed6d 100644 --- a/src/meta/client/MetaClient.cpp +++ b/src/meta/client/MetaClient.cpp @@ -132,12 +132,39 @@ void MetaClient::heartBeatThreadFunc() { } } +bool MetaClient::loadRoles() { + auto userRoleRet = listUsers().get(); + if (!userRoleRet.ok()) { + LOG(ERROR) << "List users failed, status:" << userRoleRet.status(); + return false; + } + decltype(userRolesMap_) userRolesMap; + for (auto& user : userRoleRet.value()) { + auto rolesRet = getUserRoles(user).get(); + if (!rolesRet.ok()) { + LOG(ERROR) << "List role by user failed, user : " << user; + return false; + } + userRolesMap[user] = rolesRet.value(); + } + { + folly::RWSpinLock::WriteHolder holder(localCacheLock_); + userRolesMap_ = std::move(userRolesMap); + } + return true; +} + bool MetaClient::loadData() { if (ioThreadPool_->numThreads() <= 0) { LOG(ERROR) << "The threads number in ioThreadPool should be greater than 0"; return false; } + if (!loadRoles()) { + LOG(ERROR) << "Load roles Failed"; + return false; + } + auto ret = listSpaces().get(); if (!ret.ok()) { LOG(ERROR) << "List space failed, status:" << ret.status(); @@ -1622,13 +1649,13 @@ const std::vector& MetaClient::getAddresses() { return addrs_; } -StatusOr> -MetaClient::getRolesByUser(const std::string& user) { - auto ret = getUserRoles(std::move(user)).get(); - if (ret.ok()) { - return std::move(ret).value(); +std::vector +MetaClient::getRolesByUserFromCache(const std::string& user) { + auto iter = userRolesMap_.find(user); + if (iter == userRolesMap_.end()) { + return std::vector(0); } - return ret.status(); + return iter->second; } bool MetaClient::authenticationCheck(std::string account, std::string password) { diff --git a/src/meta/client/MetaClient.h b/src/meta/client/MetaClient.h index 77664fb7fff..e978360c64c 100644 --- a/src/meta/client/MetaClient.h +++ b/src/meta/client/MetaClient.h @@ -83,6 +83,9 @@ using LeaderMap = std::unordered_map, HostA using IndexStatus = std::tuple; +// get user roles by account +using UserRolesMap = std::unordered_map>; + struct ConfigItem { ConfigItem() {} @@ -475,8 +478,7 @@ class MetaClient { EdgeType edgeType, const std::string& field); - StatusOr> - getRolesByUser(const std::string& user); + std::vector getRolesByUserFromCache(const std::string& user); bool authenticationCheck(std::string account, std::string password); @@ -504,6 +506,8 @@ class MetaClient { SpaceNewestEdgeVerMap &newestEdgeVerMap, SpaceAllEdgeMap &allEdgemap); + bool loadRoles(); + bool loadIndexes(GraphSpaceID spaceId, std::shared_ptr cache); @@ -585,6 +589,8 @@ class MetaClient { SpaceNewestEdgeVerMap spaceNewestEdgeVerMap_; SpaceAllEdgeMap spaceAllEdgeMap_; + UserRolesMap userRolesMap_; + NameIndexMap tagNameIndexMap_; NameIndexMap edgeNameIndexMap_; From d57fff89d08022619efd05027a3efc1e68fda3ca Mon Sep 17 00:00:00 2001 From: bright-starry-sky Date: Tue, 17 Mar 2020 16:21:14 +0800 Subject: [PATCH 3/5] 1,cache user pwd; 2,check god by user name root;3,move FLAGS_enable_authorize to PermissionManager;4,skip space check for kDefaultSpaceId when list roles --- src/common/permission/PermissionManager.cpp | 9 +++ src/common/permission/PermissionManager.h | 1 + src/common/session/Session.h | 10 ++-- src/graph/DescribeSpaceExecutor.cpp | 14 ++--- src/graph/PasswordAuthenticator.cpp | 2 +- src/graph/PrivilegeExecutor.cpp | 41 ++++++------- src/graph/PrivilegeExecutor.h | 1 - src/graph/ShowExecutor.cpp | 46 ++++++--------- src/graph/UseExecutor.cpp | 13 ++-- src/graph/test/PermissionTest.cpp | 9 +++ src/interface/meta.thrift | 13 ++-- src/meta/MetaServiceHandler.cpp | 6 -- src/meta/MetaServiceHandler.h | 3 - src/meta/MetaServiceUtils.cpp | 14 +++++ src/meta/MetaServiceUtils.h | 4 ++ src/meta/RootUserMan.h | 4 +- src/meta/client/MetaClient.cpp | 41 +++++-------- src/meta/client/MetaClient.h | 12 ++-- src/meta/processors/BaseProcessor.inl | 2 +- .../usersMan/AuthenticationProcessor.cpp | 59 +++++-------------- .../usersMan/AuthenticationProcessor.h | 13 ---- src/meta/test/AuthProcessorTest.cpp | 54 ----------------- 22 files changed, 139 insertions(+), 232 deletions(-) diff --git a/src/common/permission/PermissionManager.cpp b/src/common/permission/PermissionManager.cpp index b5b0b327ac0..e00715450ba 100644 --- a/src/common/permission/PermissionManager.cpp +++ b/src/common/permission/PermissionManager.cpp @@ -11,6 +11,9 @@ namespace permission { // static bool PermissionManager::canReadSpace(session::Session *session, GraphSpaceID spaceId) { + if (!FLAGS_enable_authorize) { + return true; + } if (session->isGod()) { return true; } @@ -97,6 +100,9 @@ bool PermissionManager::canWriteRole(session::Session *session, session::Role targetRole, GraphSpaceID spaceId, const std::string& targetUser) { + if (!FLAGS_enable_authorize) { + return true; + } /** * Reject grant or revoke to himself. */ @@ -155,6 +161,9 @@ bool PermissionManager::canWriteData(session::Session *session) { bool PermissionManager::canShow(session::Session *session, ShowSentence::ShowType type, GraphSpaceID targetSpace) { + if (!FLAGS_enable_authorize) { + return true; + } bool havePermission = false; switch (type) { case ShowSentence::ShowType::kShowParts: diff --git a/src/common/permission/PermissionManager.h b/src/common/permission/PermissionManager.h index 67f82a40f26..9079cf8f4d3 100644 --- a/src/common/permission/PermissionManager.h +++ b/src/common/permission/PermissionManager.h @@ -13,6 +13,7 @@ #include "parser/Sentence.h" #include "parser/UserSentences.h" #include "parser/AdminSentences.h" +#include "graph/GraphFlags.h" namespace nebula { namespace permission { diff --git a/src/common/session/Session.h b/src/common/session/Session.h index 22e28e43284..c7bfe0b055c 100644 --- a/src/common/session/Session.h +++ b/src/common/session/Session.h @@ -89,12 +89,10 @@ class Session final { } bool isGod() const { - for (auto iter = roles_.begin(); iter != roles_.end(); iter++) { - if (iter->second == Role::GOD) { - return true; - } - } - return false; + /** + * Only have one user as GOD, the user name is "root". + */ + return user() == "root"; } void setRole(GraphSpaceID space, Role role) { diff --git a/src/graph/DescribeSpaceExecutor.cpp b/src/graph/DescribeSpaceExecutor.cpp index 0443f79d459..5734b4d98c2 100644 --- a/src/graph/DescribeSpaceExecutor.cpp +++ b/src/graph/DescribeSpaceExecutor.cpp @@ -34,14 +34,14 @@ void DescribeSpaceExecutor::execute() { * Permission check. */ auto spaceId = resp.value().get_space_id(); - if (FLAGS_enable_authorize) { - auto *session = ectx()->rctx()->session(); - auto rst = permission::PermissionManager::canReadSpace(session, spaceId); - if (!rst) { - doError(Status::PermissionError("Permission denied")); - return; - } + + auto *session = ectx()->rctx()->session(); + auto rst = permission::PermissionManager::canReadSpace(session, spaceId); + if (!rst) { + doError(Status::PermissionError("Permission denied")); + return; } + resp_ = std::make_unique(); std::vector header{"ID", "Name", diff --git a/src/graph/PasswordAuthenticator.cpp b/src/graph/PasswordAuthenticator.cpp index b070d90ded5..1e56041db1e 100644 --- a/src/graph/PasswordAuthenticator.cpp +++ b/src/graph/PasswordAuthenticator.cpp @@ -14,7 +14,7 @@ PasswordAuthenticator::PasswordAuthenticator(meta::MetaClient* client) { } bool PasswordAuthenticator::auth(const std::string& user, const std::string& password) { - return metaClient_->authenticationCheck(user, password); + return metaClient_->authCheckFromCache(user, password); } } // namespace graph diff --git a/src/graph/PrivilegeExecutor.cpp b/src/graph/PrivilegeExecutor.cpp index 2d3d59fa38e..6dbde0a0225 100644 --- a/src/graph/PrivilegeExecutor.cpp +++ b/src/graph/PrivilegeExecutor.cpp @@ -37,18 +37,17 @@ void GrantExecutor::execute() { /** * Permission check. */ - if (FLAGS_enable_authorize) { - auto *session = ectx()->rctx()->session(); - auto role = session->toRole(PrivilegeUtils::toRoleType(aclItem->getRoleType())); - auto rst = permission::PermissionManager::canWriteRole(session, - role, - spaceRet.value(), - *account); - if (!rst) { - doError(Status::PermissionError("Permission denied")); - return; - } + auto *session = ectx()->rctx()->session(); + auto role = session->toRole(PrivilegeUtils::toRoleType(aclItem->getRoleType())); + auto rst = permission::PermissionManager::canWriteRole(session, + role, + spaceRet.value(), + *account); + if (!rst) { + doError(Status::PermissionError("Permission denied")); + return; } + roleItem.set_user(*account); roleItem.set_space_id(spaceRet.value()); roleItem.set_role_type(PrivilegeUtils::toRoleType(aclItem->getRoleType())); @@ -103,17 +102,15 @@ void RevokeExecutor::execute() { /** * Permission check. */ - if (FLAGS_enable_authorize) { - auto *session = ectx()->rctx()->session(); - auto role = session->toRole(PrivilegeUtils::toRoleType(aclItem->getRoleType())); - auto rst = permission::PermissionManager::canWriteRole(session, - role, - spaceRet.value(), - *account); - if (!rst) { - doError(Status::PermissionError("Permission denied")); - return; - } + auto *session = ectx()->rctx()->session(); + auto role = session->toRole(PrivilegeUtils::toRoleType(aclItem->getRoleType())); + auto rst = permission::PermissionManager::canWriteRole(session, + role, + spaceRet.value(), + *account); + if (!rst) { + doError(Status::PermissionError("Permission denied")); + return; } nebula::cpp2::RoleItem roleItem; diff --git a/src/graph/PrivilegeExecutor.h b/src/graph/PrivilegeExecutor.h index 821ad357074..98048479aa8 100644 --- a/src/graph/PrivilegeExecutor.h +++ b/src/graph/PrivilegeExecutor.h @@ -9,7 +9,6 @@ #include "base/Base.h" #include "graph/Executor.h" #include "meta/SchemaManager.h" -#include "graph/GraphFlags.h" #include "common/permission/PermissionManager.h" namespace nebula { diff --git a/src/graph/ShowExecutor.cpp b/src/graph/ShowExecutor.cpp index a7d7098be9a..ddd326a607a 100644 --- a/src/graph/ShowExecutor.cpp +++ b/src/graph/ShowExecutor.cpp @@ -6,8 +6,6 @@ #include "graph/ShowExecutor.h" #include "network/NetworkUtils.h" -#include "common/charset/Charset.h" -#include "graph/GraphFlags.h" #include "common/permission/PermissionManager.h" namespace nebula { @@ -244,14 +242,13 @@ void ShowExecutor::showSpaces() { resp_->set_column_names(std::move(header)); for (auto &space : retShowSpaces) { - if (FLAGS_enable_authorize) { - auto canShow = permission::PermissionManager::canShow(ectx()->rctx()->session(), - sentence_->showType(), - space.first); - if (!canShow) { - continue; - } + auto canShow = permission::PermissionManager::canShow(ectx()->rctx()->session(), + sentence_->showType(), + space.first); + if (!canShow) { + continue; } + std::vector row; row.emplace_back(); row.back().set_str(std::move(space.second)); @@ -535,14 +532,12 @@ void ShowExecutor::showCreateSpace() { sentence_->getName()->c_str(), resp.status().toString().c_str())); return; } - if (FLAGS_enable_authorize) { - auto canShow = permission::PermissionManager::canShow(ectx()->rctx()->session(), - sentence_->showType(), - resp.value().get_space_id()); - if (!canShow) { - doError(Status::PermissionError()); - return; - } + auto canShow = permission::PermissionManager::canShow(ectx()->rctx()->session(), + sentence_->showType(), + resp.value().get_space_id()); + if (!canShow) { + doError(Status::PermissionError()); + return; } resp_ = std::make_unique(); std::vector header{"Space", "Create Space"}; @@ -1088,7 +1083,7 @@ void ShowExecutor::showUsers() { for (auto& user : value) { std::vector row; row.resize(1); - row[0].set_str(user); + row[0].set_str(user.first); rows.emplace_back(); rows.back().set_columns(std::move(row)); } @@ -1114,15 +1109,12 @@ void ShowExecutor::showRoles() { doError(spaceRet.status()); return; } - - if (FLAGS_enable_authorize) { - auto canShow = permission::PermissionManager::canShow(ectx()->rctx()->session(), - sentence_->showType(), - spaceRet.value()); - if (!canShow) { - doError(Status::PermissionError()); - return; - } + auto canShow = permission::PermissionManager::canShow(ectx()->rctx()->session(), + sentence_->showType(), + spaceRet.value()); + if (!canShow) { + doError(Status::PermissionError()); + return; } auto future = ectx()->getMetaClient()->listRoles(spaceRet.value()); diff --git a/src/graph/UseExecutor.cpp b/src/graph/UseExecutor.cpp index fdb5a79dd02..9f2d0b992e0 100644 --- a/src/graph/UseExecutor.cpp +++ b/src/graph/UseExecutor.cpp @@ -32,16 +32,15 @@ void UseExecutor::execute() { } auto spaceId = resp.value().get_space_id(); + /** * Permission check. */ - if (FLAGS_enable_authorize) { - auto *session = ectx()->rctx()->session(); - auto rst = permission::PermissionManager::canReadSpace(session, spaceId); - if (!rst) { - doError(Status::PermissionError("Permission denied")); - return; - } + auto *session = ectx()->rctx()->session(); + auto rst = permission::PermissionManager::canReadSpace(session, spaceId); + if (!rst) { + doError(Status::PermissionError("Permission denied")); + return; } ectx()->rctx()->session()->setSpace(*sentence_->space(), spaceId); diff --git a/src/graph/test/PermissionTest.cpp b/src/graph/test/PermissionTest.cpp index 5b08964f940..918abd32d88 100644 --- a/src/graph/test/PermissionTest.cpp +++ b/src/graph/test/PermissionTest.cpp @@ -28,6 +28,7 @@ class PermissionTest : public TestBase { }; TEST_F(PermissionTest, SimpleTest) { + FLAGS_heartbeat_interval_secs = 1; FLAGS_enable_authorize = true; /* * test incorrect password. @@ -65,6 +66,7 @@ TEST_F(PermissionTest, SimpleTest) { query = "CREATE TAG person(name string)"; code = client->execute(query, resp); ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + client->disconnect(); } /* * change root password, incorrect password. @@ -76,6 +78,7 @@ TEST_F(PermissionTest, SimpleTest) { std::string query = "CHANGE PASSWORD root FROM \"aa\" TO \"bb\""; auto code = client->execute(query, resp); ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + client->disconnect(); } /* * change root password, correct password. @@ -87,10 +90,12 @@ TEST_F(PermissionTest, SimpleTest) { std::string query = "CHANGE PASSWORD root FROM \"nebula\" TO \"bb\""; auto code = client->execute(query, resp); ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + client->disconnect(); } /* * verify password changed */ + sleep(FLAGS_heartbeat_interval_secs + 1); { auto client = gEnv->getClient("root", "nebula"); ASSERT_EQ(nullptr, client); @@ -102,11 +107,15 @@ TEST_F(PermissionTest, SimpleTest) { std::string query = "CHANGE PASSWORD root FROM \"bb\" TO \"nebula\""; auto code = client->execute(query, resp); ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code); + client->disconnect(); } + sleep(FLAGS_heartbeat_interval_secs + 1); } TEST_F(PermissionTest, UserWriteTest) { + FLAGS_heartbeat_interval_secs = 1; FLAGS_enable_authorize = true; + sleep(FLAGS_heartbeat_interval_secs + 1); auto client = gEnv->getClient(); ASSERT_NE(nullptr, client); { diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift index 62eb40082b8..67ba0864710 100644 --- a/src/interface/meta.thrift +++ b/src/interface/meta.thrift @@ -524,19 +524,15 @@ struct RevokeRoleReq { 1: common.RoleItem role_item, } -struct AuthCheckReq { - 1: string account, - 2: string encoded_pwd, -} - struct ListUsersReq { } struct ListUsersResp { - 1: ErrorCode code, + 1: ErrorCode code, // Valid if ret equals E_LEADER_CHANGED. - 2: common.HostAddr leader, - 3: list users, + 2: common.HostAddr leader, + // map + 3: map users, } struct ListRolesReq { @@ -742,7 +738,6 @@ service MetaService { ListUsersResp listUsers(1: ListUsersReq req); ListRolesResp listRoles(1: ListRolesReq req); ListRolesResp getUserRoles(1: GetUserRolesReq req); - ExecResp authCheck(1: AuthCheckReq req); ExecResp changePassword(1: ChangePasswordReq req); HBResp heartBeat(1: HBReq req); diff --git a/src/meta/MetaServiceHandler.cpp b/src/meta/MetaServiceHandler.cpp index c6b0d0b6076..13293c16709 100644 --- a/src/meta/MetaServiceHandler.cpp +++ b/src/meta/MetaServiceHandler.cpp @@ -332,12 +332,6 @@ MetaServiceHandler::future_changePassword(const cpp2::ChangePasswordReq& req) { RETURN_FUTURE(processor); } -folly::Future -MetaServiceHandler::future_authCheck(const cpp2::AuthCheckReq& req) { - auto* processor = AuthCheckProcessor::instance(kvstore_); - RETURN_FUTURE(processor); -} - folly::Future MetaServiceHandler::future_getUserRoles(const cpp2::GetUserRolesReq& req) { auto* processor = GetUserRolesProcessor::instance(kvstore_); diff --git a/src/meta/MetaServiceHandler.h b/src/meta/MetaServiceHandler.h index 333dbcbc3e8..aa7a774cd15 100644 --- a/src/meta/MetaServiceHandler.h +++ b/src/meta/MetaServiceHandler.h @@ -169,9 +169,6 @@ class MetaServiceHandler final : public cpp2::MetaServiceSvIf { folly::Future future_changePassword(const cpp2::ChangePasswordReq& req) override; - folly::Future - future_authCheck(const cpp2::AuthCheckReq& req) override; - folly::Future future_getUserRoles(const cpp2::GetUserRolesReq& req) override; diff --git a/src/meta/MetaServiceUtils.cpp b/src/meta/MetaServiceUtils.cpp index dd49d4f4ea4..8351f146bdb 100644 --- a/src/meta/MetaServiceUtils.cpp +++ b/src/meta/MetaServiceUtils.cpp @@ -531,10 +531,24 @@ std::string MetaServiceUtils::userKey(const std::string& account) { return key; } +std::string MetaServiceUtils::userVal(const std::string& val) { + std::string key; + auto pwdLen = val.size(); + key.reserve(sizeof(int64_t) + pwdLen); + key.append(reinterpret_cast(&pwdLen), sizeof(size_t)) + .append(val); + return key; +} + std::string MetaServiceUtils::parseUser(folly::StringPiece key) { return key.subpiece(kUsersTable.size(), key.size() - kUsersTable.size()).str(); } +std::string MetaServiceUtils::parseUserPwd(folly::StringPiece val) { + auto len = *reinterpret_cast(val.data()); + return val.subpiece(sizeof(size_t), len).str(); +} + std::string MetaServiceUtils::roleKey(GraphSpaceID spaceId, const std::string& account) { std::string key; key.reserve(kRolesTable.size() + sizeof(GraphSpaceID) + account.size()); diff --git a/src/meta/MetaServiceUtils.h b/src/meta/MetaServiceUtils.h index 0f820be8a37..dd81f957637 100644 --- a/src/meta/MetaServiceUtils.h +++ b/src/meta/MetaServiceUtils.h @@ -145,8 +145,12 @@ class MetaServiceUtils final { static std::string userKey(const std::string& account); + static std::string userVal(const std::string& val); + static std::string parseUser(folly::StringPiece key); + static std::string parseUserPwd(folly::StringPiece val); + static std::string roleKey(GraphSpaceID spaceId, const std::string& account); static std::string roleVal(nebula::cpp2::RoleType roleType); diff --git a/src/meta/RootUserMan.h b/src/meta/RootUserMan.h index e88fe6d7688..23dba19d68c 100644 --- a/src/meta/RootUserMan.h +++ b/src/meta/RootUserMan.h @@ -35,12 +35,14 @@ class RootUserMan { static bool initRootUser(kvstore::KVStore* kv) { LOG(INFO) << "Init root user"; + auto encodedPwd = encryption::MD5Utils::md5Encode("nebula"); auto userKey = MetaServiceUtils::userKey("root"); + auto userVal = MetaServiceUtils::userVal(std::move(encodedPwd)); auto roleKey = MetaServiceUtils::roleKey(kDefaultSpaceId, "root"); auto roleVal = MetaServiceUtils::roleVal(nebula::cpp2::RoleType::GOD); std::vector data; - data.emplace_back(std::move(userKey), encryption::MD5Utils::md5Encode("nebula")); + data.emplace_back(std::move(userKey), std::move(userVal)); data.emplace_back(std::move(roleKey), std::move(roleVal)); bool ret = true; folly::Baton baton; diff --git a/src/meta/client/MetaClient.cpp b/src/meta/client/MetaClient.cpp index 917c4d6ed6d..37bb43fddfd 100644 --- a/src/meta/client/MetaClient.cpp +++ b/src/meta/client/MetaClient.cpp @@ -132,24 +132,27 @@ void MetaClient::heartBeatThreadFunc() { } } -bool MetaClient::loadRoles() { +bool MetaClient::loadUsersAndRoles() { auto userRoleRet = listUsers().get(); if (!userRoleRet.ok()) { LOG(ERROR) << "List users failed, status:" << userRoleRet.status(); return false; } decltype(userRolesMap_) userRolesMap; + decltype(userPasswordMap_) userPasswordMap; for (auto& user : userRoleRet.value()) { - auto rolesRet = getUserRoles(user).get(); + auto rolesRet = getUserRoles(user.first).get(); if (!rolesRet.ok()) { - LOG(ERROR) << "List role by user failed, user : " << user; + LOG(ERROR) << "List role by user failed, user : " << user.first; return false; } - userRolesMap[user] = rolesRet.value(); + userRolesMap[user.first] = rolesRet.value(); + userPasswordMap[user.first] = user.second; } { folly::RWSpinLock::WriteHolder holder(localCacheLock_); userRolesMap_ = std::move(userRolesMap); + userPasswordMap_ = std::move(userPasswordMap); } return true; } @@ -160,7 +163,7 @@ bool MetaClient::loadData() { return false; } - if (!loadRoles()) { + if (!loadUsersAndRoles()) { LOG(ERROR) << "Load roles Failed"; return false; } @@ -1658,9 +1661,12 @@ MetaClient::getRolesByUserFromCache(const std::string& user) { return iter->second; } -bool MetaClient::authenticationCheck(std::string account, std::string password) { - auto ret = authCheck(std::move(account), std::move(password)).get(); - return ret.ok(); +bool MetaClient::authCheckFromCache(const std::string& account, const std::string& password) { + auto iter = userPasswordMap_.find(account); + if (iter == userPasswordMap_.end()) { + return false; + } + return iter->second == password; } StatusOr MetaClient::getLatestTagVersionFromCache(const GraphSpaceID& space, @@ -1813,10 +1819,10 @@ MetaClient::revokeFromUser(nebula::cpp2::RoleItem roleItem) { return future; } -folly::Future>> +folly::Future>> MetaClient::listUsers() { cpp2::ListUsersReq req; - folly::Promise>> promise; + folly::Promise>> promise; auto future = promise.getFuture(); getResponse(std::move(req), [] (auto client, auto request) { return client->future_listUsers(request); @@ -1858,21 +1864,6 @@ MetaClient::changePassword(std::string account, return future; } -folly::Future> -MetaClient::authCheck(std::string account, std::string password) { - cpp2::AuthCheckReq req; - req.set_account(std::move(account)); - req.set_encoded_pwd(std::move(password)); - folly::Promise> promise; - auto future = promise.getFuture(); - getResponse(std::move(req), [] (auto client, auto request) { - return client->future_authCheck(request); - }, [] (cpp2::ExecResp&& resp) -> bool { - return resp.code == cpp2::ErrorCode::SUCCEEDED; - }, std::move(promise), true); - return future; -} - folly::Future>> MetaClient::getUserRoles(std::string account) { cpp2::GetUserRolesReq req; diff --git a/src/meta/client/MetaClient.h b/src/meta/client/MetaClient.h index e978360c64c..e5cecb74278 100644 --- a/src/meta/client/MetaClient.h +++ b/src/meta/client/MetaClient.h @@ -85,6 +85,8 @@ using IndexStatus = std::tuple; // get user roles by account using UserRolesMap = std::unordered_map>; +// get user password by account +using UserPasswordMap = std::unordered_map; struct ConfigItem { ConfigItem() {} @@ -352,7 +354,7 @@ class MetaClient { folly::Future> revokeFromUser(nebula::cpp2::RoleItem roleItem); - folly::Future>> + folly::Future>> listUsers(); folly::Future>> @@ -361,9 +363,6 @@ class MetaClient { folly::Future> changePassword(std::string account, std::string newPwd, std::string oldPwd); - folly::Future> - authCheck(std::string account, std::string password); - folly::Future>> getUserRoles(std::string account); @@ -480,7 +479,7 @@ class MetaClient { std::vector getRolesByUserFromCache(const std::string& user); - bool authenticationCheck(std::string account, std::string password); + bool authCheckFromCache(const std::string& account, const std::string& password); Status refreshCache(); @@ -506,7 +505,7 @@ class MetaClient { SpaceNewestEdgeVerMap &newestEdgeVerMap, SpaceAllEdgeMap &allEdgemap); - bool loadRoles(); + bool loadUsersAndRoles(); bool loadIndexes(GraphSpaceID spaceId, std::shared_ptr cache); @@ -590,6 +589,7 @@ class MetaClient { SpaceAllEdgeMap spaceAllEdgeMap_; UserRolesMap userRolesMap_; + UserPasswordMap userPasswordMap_; NameIndexMap tagNameIndexMap_; NameIndexMap edgeNameIndexMap_; diff --git a/src/meta/processors/BaseProcessor.inl b/src/meta/processors/BaseProcessor.inl index c72cd7cb35e..48992baccaa 100644 --- a/src/meta/processors/BaseProcessor.inl +++ b/src/meta/processors/BaseProcessor.inl @@ -331,7 +331,7 @@ template bool BaseProcessor::checkPassword(const std::string& account, const std::string& password) { auto userKey = MetaServiceUtils::userKey(account); auto ret = doGet(userKey); - return ret.value() == password; + return MetaServiceUtils::parseUserPwd(ret.value()) == password; } template diff --git a/src/meta/processors/usersMan/AuthenticationProcessor.cpp b/src/meta/processors/usersMan/AuthenticationProcessor.cpp index 5b050fdc9f0..33815ed7406 100644 --- a/src/meta/processors/usersMan/AuthenticationProcessor.cpp +++ b/src/meta/processors/usersMan/AuthenticationProcessor.cpp @@ -29,9 +29,10 @@ void CreateUserProcessor::process(const cpp2::CreateUserReq& req) { } std::vector data; - data.emplace_back(MetaServiceUtils::userKey(account), password); + data.emplace_back(MetaServiceUtils::userKey(account), + MetaServiceUtils::userVal(password)); handleErrorCode(cpp2::ErrorCode::SUCCEEDED); - doPut(std::move(data)); + doSyncPutAndUpdate(std::move(data)); } @@ -40,6 +41,7 @@ void AlterUserProcessor::process(const cpp2::AlterUserReq& req) { const auto& account = req.get_account(); const auto& password = req.get_encoded_pwd(); auto userKey = MetaServiceUtils::userKey(account); + auto userVal = MetaServiceUtils::userVal(password); std::string val; auto result = kvstore_->get(kDefaultSpaceId, kDefaultPartId, userKey, &val); if (result != kvstore::ResultCode::SUCCEEDED) { @@ -48,9 +50,9 @@ void AlterUserProcessor::process(const cpp2::AlterUserReq& req) { return; } std::vector data; - data.emplace_back(std::move(userKey), password); + data.emplace_back(std::move(userKey), std::move(userVal)); handleErrorCode(cpp2::ErrorCode::SUCCEEDED); - doPut(std::move(data)); + doSyncPutAndUpdate(std::move(data)); } @@ -88,7 +90,7 @@ void DropUserProcessor::process(const cpp2::DropUserReq& req) { handleErrorCode(cpp2::ErrorCode::SUCCEEDED); LOG(INFO) << "Drop User " << req.get_account(); - doMultiRemove(std::move(keys)); + doSyncMultiRemoveAndUpdate({std::move(keys)}); } @@ -108,7 +110,7 @@ void GrantProcessor::process(const cpp2::GrantRoleReq& req) { data.emplace_back(MetaServiceUtils::roleKey(spaceId, roleItem.get_user()), MetaServiceUtils::roleVal(roleItem.get_role_type())); handleErrorCode(cpp2::ErrorCode::SUCCEEDED); - doPut(std::move(data)); + doSyncPutAndUpdate(std::move(data)); } @@ -139,7 +141,7 @@ void RevokeProcessor::process(const cpp2::RevokeRoleReq& req) { return; } handleErrorCode(cpp2::ErrorCode::SUCCEEDED); - doRemove(roleKey); + doSyncMultiRemoveAndUpdate({std::move(roleKey)}); } @@ -159,17 +161,11 @@ void ChangePasswordProcessor::process(const cpp2::ChangePasswordReq& req) { } auto userKey = MetaServiceUtils::userKey(req.get_account()); - std::string val; - auto result = kvstore_->get(kDefaultSpaceId, kDefaultPartId, userKey, &val); - if (result != kvstore::ResultCode::SUCCEEDED) { - handleErrorCode(cpp2::ErrorCode::E_NOT_FOUND); - onFinished(); - return; - } + auto userVal = MetaServiceUtils::userVal(req.get_new_encoded_pwd()); std::vector data; - data.emplace_back(std::move(userKey), req.get_new_encoded_pwd()); + data.emplace_back(std::move(userKey), std::move(userVal)); handleErrorCode(cpp2::ErrorCode::SUCCEEDED); - doPut(std::move(data)); + doSyncPutAndUpdate(std::move(data)); } @@ -187,8 +183,9 @@ void ListUsersProcessor::process(const cpp2::ListUsersReq& req) { } decltype(resp_.users) users; while (iter->valid()) { - auto user = MetaServiceUtils::parseUser(iter->key()); - users.emplace_back(std::move(user)); + auto account = MetaServiceUtils::parseUser(iter->key()); + auto password = MetaServiceUtils::parseUserPwd(iter->val()); + users.emplace(std::pair(std::move(account), std::move(password))); iter->next(); } resp_.set_users(users); @@ -199,13 +196,7 @@ void ListUsersProcessor::process(const cpp2::ListUsersReq& req) { void ListRolesProcessor::process(const cpp2::ListRolesReq& req) { auto spaceId = req.get_space_id(); - /** - * Because the god role's space id is kDefaultSpaceId. - * skip the space check when space id is kDefaultSpaceId. - **/ - if (spaceId != kDefaultSpaceId) { - CHECK_SPACE_ID_AND_RETURN(spaceId); - } + CHECK_SPACE_ID_AND_RETURN(spaceId); folly::SharedMutex::ReadHolder rHolder(LockUtils::userLock()); auto prefix = MetaServiceUtils::roleSpacePrefix(spaceId); std::unique_ptr iter; @@ -233,24 +224,6 @@ void ListRolesProcessor::process(const cpp2::ListRolesReq& req) { onFinished(); } -void AuthCheckProcessor::process(const cpp2::AuthCheckReq& req) { - folly::SharedMutex::WriteHolder wHolder(LockUtils::userLock()); - auto userRet = userExist(req.get_account()); - if (!userRet.ok()) { - handleErrorCode(MetaCommon::to(userRet)); - onFinished(); - return; - } - - if (!checkPassword(req.get_account(), req.get_encoded_pwd())) { - handleErrorCode(cpp2::ErrorCode::E_INVALID_PASSWORD); - onFinished(); - return; - } - handleErrorCode(cpp2::ErrorCode::SUCCEEDED); - onFinished(); -} - void GetUserRolesProcessor::process(const cpp2::GetUserRolesReq& req) { folly::SharedMutex::WriteHolder wHolder(LockUtils::userLock()); auto prefix = MetaServiceUtils::rolesPrefix(); diff --git a/src/meta/processors/usersMan/AuthenticationProcessor.h b/src/meta/processors/usersMan/AuthenticationProcessor.h index 3457cefa6aa..9d738124b86 100644 --- a/src/meta/processors/usersMan/AuthenticationProcessor.h +++ b/src/meta/processors/usersMan/AuthenticationProcessor.h @@ -122,19 +122,6 @@ class ListRolesProcessor : public BaseProcessor { : BaseProcessor(kvstore) {} }; -class AuthCheckProcessor : public BaseProcessor { -public: - static AuthCheckProcessor* instance(kvstore::KVStore* kvstore) { - return new AuthCheckProcessor(kvstore); - } - - void process(const cpp2::AuthCheckReq& req); - -private: - explicit AuthCheckProcessor(kvstore::KVStore* kvstore) - : BaseProcessor(kvstore) {} -}; - class GetUserRolesProcessor : public BaseProcessor { public: static GetUserRolesProcessor* instance(kvstore::KVStore* kvstore) { diff --git a/src/meta/test/AuthProcessorTest.cpp b/src/meta/test/AuthProcessorTest.cpp index bfd187bee79..1bb75cde76e 100644 --- a/src/meta/test/AuthProcessorTest.cpp +++ b/src/meta/test/AuthProcessorTest.cpp @@ -64,39 +64,6 @@ TEST(AuthProcessorTest, CreateUserTest) { auto resp = std::move(f).get(); ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, resp.get_code()); } - // authCheck - { - cpp2::AuthCheckReq req; - req.set_account("user1"); - req.set_encoded_pwd("password"); - auto* processor = AuthCheckProcessor::instance(kv.get()); - auto f = processor->getFuture(); - processor->process(req); - auto resp = std::move(f).get(); - ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, resp.get_code()); - } - // authCheck, user not exists. - { - cpp2::AuthCheckReq req; - req.set_account("user4"); - req.set_encoded_pwd("password"); - auto* processor = AuthCheckProcessor::instance(kv.get()); - auto f = processor->getFuture(); - processor->process(req); - auto resp = std::move(f).get(); - ASSERT_EQ(cpp2::ErrorCode::E_NOT_FOUND, resp.get_code()); - } - // authCheck, password invalid. - { - cpp2::AuthCheckReq req; - req.set_account("user1"); - req.set_encoded_pwd("passwordd"); - auto* processor = AuthCheckProcessor::instance(kv.get()); - auto f = processor->getFuture(); - processor->process(req); - auto resp = std::move(f).get(); - ASSERT_EQ(cpp2::ErrorCode::E_INVALID_PASSWORD, resp.get_code()); - } } TEST(AuthProcessorTest, AlterUserTest) { @@ -125,16 +92,6 @@ TEST(AuthProcessorTest, AlterUserTest) { auto resp = std::move(f).get(); ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, resp.get_code()); } - { - cpp2::AuthCheckReq req; - req.set_account("user1"); - req.set_encoded_pwd("password_1"); - auto* processor = AuthCheckProcessor::instance(kv.get()); - auto f = processor->getFuture(); - processor->process(req); - auto resp = std::move(f).get(); - ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, resp.get_code()); - } // If user not exists { cpp2::AlterUserReq req; @@ -646,17 +603,6 @@ TEST(AuthProcessorTest, ChangePasswordTest) { auto resp = std::move(f).get(); ASSERT_EQ(cpp2::ErrorCode::E_INVALID_PASSWORD, resp.get_code()); } - // authCheck - { - cpp2::AuthCheckReq req; - req.set_account("user1"); - req.set_encoded_pwd("pwd1"); - auto* processor = AuthCheckProcessor::instance(kv.get()); - auto f = processor->getFuture(); - processor->process(req); - auto resp = std::move(f).get(); - ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, resp.get_code()); - } } } // namespace meta From a9436b8ba3c7a8616102be87d42eaf2c8e31a8a8 Mon Sep 17 00:00:00 2001 From: bright-starry-sky Date: Tue, 17 Mar 2020 20:02:25 +0800 Subject: [PATCH 4/5] Changed the method name from canReadSchemaData to canReadSchemaOrData --- src/common/permission/PermissionManager.cpp | 4 ++-- src/common/permission/PermissionManager.h | 2 +- src/graph/PermissionCheck.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/permission/PermissionManager.cpp b/src/common/permission/PermissionManager.cpp index e00715450ba..795b982bfa4 100644 --- a/src/common/permission/PermissionManager.cpp +++ b/src/common/permission/PermissionManager.cpp @@ -35,7 +35,7 @@ bool PermissionManager::canReadSpace(session::Session *session, GraphSpaceID spa } // static -bool PermissionManager::canReadSchemaData(session::Session *session) { +bool PermissionManager::canReadSchemaOrData(session::Session *session) { if (session->space() == -1) { LOG(ERROR) << "The space name is not set"; return false; @@ -179,7 +179,7 @@ bool PermissionManager::canShow(session::Session *session, case ShowSentence::ShowType::kShowEdgeIndexStatus: { /** * Above operations can get the space id via session, - * so the permission same with canReadSchemaData. + * so the permission same with canReadSchemaOrData. * They've been checked by "USE SPACE", so here skip the check. */ havePermission = true; diff --git a/src/common/permission/PermissionManager.h b/src/common/permission/PermissionManager.h index 9079cf8f4d3..e155e15268c 100644 --- a/src/common/permission/PermissionManager.h +++ b/src/common/permission/PermissionManager.h @@ -22,7 +22,7 @@ class PermissionManager final { public: PermissionManager() = delete; static bool canReadSpace(session::Session *session, GraphSpaceID spaceId); - static bool canReadSchemaData(session::Session *session); + static bool canReadSchemaOrData(session::Session *session); static bool canWriteSpace(session::Session *session); static bool canWriteSchema(session::Session *session); static bool canWriteUser(session::Session *session); diff --git a/src/graph/PermissionCheck.cpp b/src/graph/PermissionCheck.cpp index 37473b8e83b..3f83af701a5 100644 --- a/src/graph/PermissionCheck.cpp +++ b/src/graph/PermissionCheck.cpp @@ -112,7 +112,7 @@ bool PermissionCheck::permissionCheck(session::Session *session, Sentence* sente case Sentence::Kind::kLimit : case Sentence::Kind::KGroupBy : case Sentence::Kind::kReturn : { - return permission::PermissionManager::canReadSchemaData(session); + return permission::PermissionManager::canReadSchemaOrData(session); } case Sentence::Kind::kShow : { auto *stc = static_cast(sentence); From 03a20e02bd0862d6fb6735c68a9ece1fb8715f0a Mon Sep 17 00:00:00 2001 From: bright-starry-sky Date: Thu, 19 Mar 2020 11:59:22 +0800 Subject: [PATCH 5/5] fixed comment typo error --- src/common/session/Session.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/session/Session.h b/src/common/session/Session.h index c7bfe0b055c..1c68868dab9 100644 --- a/src/common/session/Session.h +++ b/src/common/session/Session.h @@ -115,7 +115,7 @@ class Session final { std::string account_; time::Duration idleDuration_; /* - * map + * map * One user can have roles in multiple spaces * But a user has only one role in one space */