Tests/permission cases (vesoft-inc#234)

* Add the cases about permission. * Tune the permission cases. * Run in parallel. * Make the permission error message more clear. * Search the result. * Will not check space when not enable authorize or god user. * Don't return space id in error message. * Don't require user/password for create nebula client. * Only check permission when enable authorize. * Add authorize configuration to config file. * Remove the simpler autherticator. * Move the option about permission. Co-authored-by: Yee <[email protected]>
Shinji-IkariG · Oct 15, 2020 · 8a016f4 · 8a016f4
1 parent db6a6d7
commit 8a016f4
Show file tree

Hide file tree

Showing 29 changed files with 1,140 additions and 212 deletions.
diff --git a/conf/nebula-graphd.conf.default b/conf/nebula-graphd.conf.default
@@ -52,6 +52,7 @@
 --ws_http_port=13000
 # HTTP2 service port
 --ws_h2_port=13002
+
 # The default charset when a space is created
 --default_charset=utf8
 # The defaule collate when a space is created

diff --git a/src/executor/admin/ChangePasswordExecutor.cpp b/src/executor/admin/ChangePasswordExecutor.cpp
@@ -4,9 +4,11 @@
  * attached with Common Clause Condition 1.0, found in the LICENSES directory.
  */
 
+#include "common/encryption/MD5Utils.h"
+
+#include "context/QueryContext.h"
 #include "executor/admin/ChangePasswordExecutor.h"
 #include "planner/Admin.h"
-#include "context/QueryContext.h"
 
 namespace nebula {
 namespace graph {
@@ -18,8 +20,11 @@ folly::Future<Status> ChangePasswordExecutor::execute() {
 
 folly::Future<Status> ChangePasswordExecutor::changePassword() {
     auto *cpNode = asNode<ChangePassword>(node());
-    return qctx()->getMetaClient()->changePassword(
-            *cpNode->username(), *cpNode->newPassword(), *cpNode->password())
+    return qctx()
+        ->getMetaClient()
+        ->changePassword(*cpNode->username(),
+                         encryption::MD5Utils::md5Encode(*cpNode->newPassword()),
+                         encryption::MD5Utils::md5Encode(*cpNode->password()))
         .via(runner())
         .then([this](StatusOr<bool> &&resp) {
             SCOPED_TIMER(&execTime_);

diff --git a/src/executor/admin/CreateUserExecutor.cpp b/src/executor/admin/CreateUserExecutor.cpp
@@ -4,9 +4,11 @@
  * attached with Common Clause Condition 1.0, found in the LICENSES directory.
  */
 
+#include "common/encryption/MD5Utils.h"
+
+#include "context/QueryContext.h"
 #include "executor/admin/CreateUserExecutor.h"
 #include "planner/Admin.h"
-#include "context/QueryContext.h"
 
 namespace nebula {
 namespace graph {
@@ -18,8 +20,11 @@ folly::Future<Status> CreateUserExecutor::execute() {
 
 folly::Future<Status> CreateUserExecutor::createUser() {
     auto *cuNode = asNode<CreateUser>(node());
-    return qctx()->getMetaClient()->createUser(
-            *cuNode->username(), *cuNode->password(), cuNode->ifNotExist())
+    return qctx()
+        ->getMetaClient()
+        ->createUser(*cuNode->username(),
+                     encryption::MD5Utils::md5Encode(*cuNode->password()),
+                     cuNode->ifNotExist())
         .via(runner())
         .then([this](StatusOr<bool> resp) {
             SCOPED_TIMER(&execTime_);

diff --git a/src/executor/admin/GrantRoleExecutor.cpp b/src/executor/admin/GrantRoleExecutor.cpp
@@ -4,9 +4,12 @@
  * attached with Common Clause Condition 1.0, found in the LICENSES directory.
  */
 
+#include "common/interface/gen-cpp2/meta_types.h"
+
+#include "context/QueryContext.h"
 #include "executor/admin/GrantRoleExecutor.h"
 #include "planner/Admin.h"
-#include "context/QueryContext.h"
+#include "service/PermissionManager.h"
 
 namespace nebula {
 namespace graph {
@@ -21,15 +24,20 @@ folly::Future<Status> GrantRoleExecutor::grantRole() {
     auto *grNode = asNode<GrantRole>(node());
     const auto *spaceName = grNode->spaceName();
     auto spaceIdResult = qctx()->getMetaClient()->getSpaceIdByNameFromCache(*spaceName);
-    if (!spaceIdResult.ok()) {
-        return std::move(spaceIdResult).status();
-    }
+    NG_RETURN_IF_ERROR(spaceIdResult);
     auto spaceId = spaceIdResult.value();
+
+    auto *session = qctx_->rctx()->session();
+    NG_RETURN_IF_ERROR(
+        PermissionManager::canWriteRole(session, grNode->role(), spaceId, *grNode->username()));
+
     meta::cpp2::RoleItem item;
-    item.set_space_id(spaceId);  // TODO(shylock) pass space name directly
+    item.set_space_id(spaceId);   // TODO(shylock) pass space name directly
     item.set_user_id(*grNode->username());
     item.set_role_type(grNode->role());
-    return qctx()->getMetaClient()->grantToUser(std::move(item))
+    return qctx()
+        ->getMetaClient()
+        ->grantToUser(std::move(item))
         .via(runner())
         .then([this](StatusOr<bool> resp) {
             SCOPED_TIMER(&execTime_);

diff --git a/src/executor/admin/ListRolesExecutor.cpp b/src/executor/admin/ListRolesExecutor.cpp
@@ -5,8 +5,9 @@
  */
 
 #include "executor/admin/ListRolesExecutor.h"
-#include "planner/Admin.h"
 #include "context/QueryContext.h"
+#include "planner/Admin.h"
+#include "service/PermissionManager.h"
 
 namespace nebula {
 namespace graph {
@@ -18,7 +19,9 @@ folly::Future<Status> ListRolesExecutor::execute() {
 
 folly::Future<Status> ListRolesExecutor::listRoles() {
     auto *lrNode = asNode<ListRoles>(node());
-    return qctx()->getMetaClient()->listRoles(lrNode->space())
+    return qctx()
+        ->getMetaClient()
+        ->listRoles(lrNode->space())
         .via(runner())
         .then([this](StatusOr<std::vector<meta::cpp2::RoleItem>> &&resp) {
             SCOPED_TIMER(&execTime_);
@@ -27,12 +30,22 @@ folly::Future<Status> ListRolesExecutor::listRoles() {
             }
             nebula::DataSet v({"Account", "Role Type"});
             auto items = std::move(resp).value();
-            for (const auto &item : items) {
-                v.emplace_back(nebula::Row(
-                    {
-                        item.get_user_id(),
-                        meta::cpp2::_RoleType_VALUES_TO_NAMES.at(item.get_role_type())
-                    }));
+            // Only god and admin show all roles, other roles only show themselves
+            const auto &account = qctx_->rctx()->session()->user();
+            auto foundItem = std::find_if(items.begin(), items.end(), [&account](const auto &item) {
+                return item.get_user_id() == account;
+            });
+            if (foundItem != items.end() &&
+                foundItem->get_role_type() != meta::cpp2::RoleType::ADMIN) {
+                v.emplace_back(
+                    Row({foundItem->get_user_id(),
+                         meta::cpp2::_RoleType_VALUES_TO_NAMES.at(foundItem->get_role_type())}));
+            } else {
+                for (const auto &item : items) {
+                    v.emplace_back(nebula::Row(
+                        {item.get_user_id(),
+                         meta::cpp2::_RoleType_VALUES_TO_NAMES.at(item.get_role_type())}));
+                }
             }
             return finish(std::move(v));
         });

diff --git a/src/executor/admin/RevokeRoleExecutor.cpp b/src/executor/admin/RevokeRoleExecutor.cpp
@@ -4,9 +4,12 @@
  * attached with Common Clause Condition 1.0, found in the LICENSES directory.
  */
 
+#include "common/interface/gen-cpp2/meta_types.h"
+
+#include "context/QueryContext.h"
 #include "executor/admin/RevokeRoleExecutor.h"
 #include "planner/Admin.h"
-#include "context/QueryContext.h"
+#include "service/PermissionManager.h"
 
 namespace nebula {
 namespace graph {
@@ -20,15 +23,23 @@ folly::Future<Status> RevokeRoleExecutor::revokeRole() {
     auto *rrNode = asNode<RevokeRole>(node());
     const auto *spaceName = rrNode->spaceName();
     auto spaceIdResult = qctx()->getMetaClient()->getSpaceIdByNameFromCache(*spaceName);
-    if (!spaceIdResult.ok()) {
-        return std::move(spaceIdResult).status();
-    }
+    NG_RETURN_IF_ERROR(spaceIdResult);
     auto spaceId = spaceIdResult.value();
+
+    if (rrNode->role() == meta::cpp2::RoleType::GOD) {
+        return Status::PermissionError("Permission denied");
+    }
+    auto *session = qctx_->rctx()->session();
+    NG_RETURN_IF_ERROR(
+        PermissionManager::canWriteRole(session, rrNode->role(), spaceId, *rrNode->username()));
+
     meta::cpp2::RoleItem item;
     item.set_space_id(spaceId);
     item.set_user_id(*rrNode->username());
     item.set_role_type(rrNode->role());
-    return qctx()->getMetaClient()->revokeFromUser(std::move(item))
+    return qctx()
+        ->getMetaClient()
+        ->revokeFromUser(std::move(item))
         .via(runner())
         .then([this](StatusOr<bool> resp) {
             SCOPED_TIMER(&execTime_);

diff --git a/src/executor/admin/SpaceExecutor.cpp b/src/executor/admin/SpaceExecutor.cpp
@@ -6,6 +6,7 @@
 
 #include "executor/admin/SpaceExecutor.h"
 #include "context/QueryContext.h"
+#include "service/PermissionManager.h"
 #include "planner/Admin.h"
 #include "util/SchemaUtil.h"
 #include "util/ScopedTimer.h"
@@ -39,6 +40,14 @@ folly::Future<Status> DescSpaceExecutor::execute() {
                     LOG(ERROR) << resp.status();
                     return resp.status();
                 }
+                auto &spaceItem = resp.value();
+                auto &properties = spaceItem.get_properties();
+                auto spaceId = spaceItem.get_space_id();
+
+                // check permission
+                auto *session = qctx_->rctx()->session();
+                NG_RETURN_IF_ERROR(PermissionManager::canReadSpace(session, spaceId));
+
                 DataSet dataSet;
                 dataSet.colNames = {"ID",
                                     "Name",
@@ -47,10 +56,8 @@ folly::Future<Status> DescSpaceExecutor::execute() {
                                     "Charset",
                                     "Collate",
                                     "Vid Type"};
-                auto &spaceItem = resp.value();
-                auto &properties = spaceItem.get_properties();
                 Row row;
-                row.values.emplace_back(spaceItem.get_space_id());
+                row.values.emplace_back(spaceId);
                 row.values.emplace_back(properties.get_space_name());
                 row.values.emplace_back(properties.get_partition_num());
                 row.values.emplace_back(properties.get_replica_factor());
@@ -102,6 +109,10 @@ folly::Future<Status> ShowSpacesExecutor::execute() {
             DataSet dataSet({"Name"});
             std::set<std::string> orderSpaceNames;
             for (auto &space : spaceItems) {
+                if (!PermissionManager::canReadSpace(qctx_->rctx()->session(),
+                                                     space.first).ok()) {
+                    continue;
+                }
                 orderSpaceNames.emplace(space.second);
             }
             for (auto &name : orderSpaceNames) {

diff --git a/src/executor/admin/SwitchSpaceExecutor.cpp b/src/executor/admin/SwitchSpaceExecutor.cpp
@@ -6,6 +6,8 @@
 
 #include "executor/admin/SwitchSpaceExecutor.h"
 
+#include "service/PermissionManager.h"
+#include "planner/Query.h"
 #include "common/clients/meta/MetaClient.h"
 #include "common/interface/gen-cpp2/meta_types.h"
 #include "context/QueryContext.h"
@@ -29,6 +31,8 @@ folly::Future<Status> SwitchSpaceExecutor::execute() {
                 }
 
                 auto spaceId = resp.value().get_space_id();
+                auto *session = qctx_->rctx()->session();
+                NG_RETURN_IF_ERROR(PermissionManager::canReadSpace(session, spaceId));
                 const auto& properties = resp.value().get_properties();
 
                 SpaceInfo spaceInfo;

diff --git a/src/executor/admin/UpdateUserExecutor.cpp b/src/executor/admin/UpdateUserExecutor.cpp
@@ -4,9 +4,11 @@
  * attached with Common Clause Condition 1.0, found in the LICENSES directory.
  */
 
+#include "common/encryption/MD5Utils.h"
+
+#include "context/QueryContext.h"
 #include "executor/admin/UpdateUserExecutor.h"
 #include "planner/Admin.h"
-#include "context/QueryContext.h"
 
 namespace nebula {
 namespace graph {
@@ -18,7 +20,9 @@ folly::Future<Status> UpdateUserExecutor::execute() {
 
 folly::Future<Status> UpdateUserExecutor::updateUser() {
     auto *uuNode = asNode<UpdateUser>(node());
-    return qctx()->getMetaClient()->alterUser(*uuNode->username(), *uuNode->password())
+    return qctx()
+        ->getMetaClient()
+        ->alterUser(*uuNode->username(), encryption::MD5Utils::md5Encode(*uuNode->password()))
         .via(runner())
         .then([this](StatusOr<bool> resp) {
             SCOPED_TIMER(&execTime_);

diff --git a/src/service/GraphService.cpp b/src/service/GraphService.cpp
@@ -10,7 +10,6 @@
 #include "common/clients/storage/GraphStorageClient.h"
 #include "service/GraphService.h"
 #include "service/RequestContext.h"
-#include "service/SimpleAuthenticator.h"
 #include "service/GraphFlags.h"
 #include "service/PasswordAuthenticator.h"
 #include "service/CloudAuthenticator.h"
@@ -98,12 +97,28 @@ 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_SEMANTIC_ERROR:
+        return "Semantic error";
+    // TODO(shylock) fix the typo
+    case cpp2::ErrorCode::E_STATEMENT_EMTPY:
+        return "Statement emtpy";
+    case cpp2::ErrorCode::E_EXECUTION_ERROR:
+        return "Execution error";
+    case cpp2::ErrorCode::E_RPC_FAILURE:
+        return "RPC failure";
+    case cpp2::ErrorCode::E_DISCONNECTED:
+        return "Disconnected";
+    case cpp2::ErrorCode::E_FAIL_TO_CONNECT:
+        return "Fail to connect";
+    case cpp2::ErrorCode::E_BAD_PERMISSION:
+        return "Bad permission";
+    case cpp2::ErrorCode::E_USER_NOT_FOUND:
+        return "User not found";
+    }
     /**********************
      * Unknown error
      **********************/
-    default:
-        return "Unknown error";
-    }
+    return "Unknown error";
 }
 
 void GraphService::onHandle(RequestContext<cpp2::AuthResponse>& ctx, cpp2::ErrorCode code) {
@@ -117,16 +132,14 @@ void GraphService::onHandle(RequestContext<cpp2::AuthResponse>& ctx, cpp2::Error
 }
 
 bool GraphService::auth(const std::string& username, const std::string& password) {
-    std::string authType = FLAGS_auth_type;
-    folly::toLowerAscii(authType);
-    if (!authType.compare("password")) {
+    if (FLAGS_auth_type == "password") {
         auto authenticator = std::make_unique<PasswordAuthenticator>(queryEngine_->metaClient());
         return authenticator->auth(username, encryption::MD5Utils::md5Encode(password));
-    } else if (!authType.compare("cloud")) {
+    } else if (FLAGS_auth_type == "cloud") {
         auto authenticator = std::make_unique<CloudAuthenticator>(queryEngine_->metaClient());
         return authenticator->auth(username, password);
     }
-    LOG(WARNING) << "Unknown auth type: " << authType;
+    LOG(WARNING) << "Unknown auth type: " << FLAGS_auth_type;
     return false;
 }