Fix the error the not attached user list all roles of space (#2778)

* Fix the error the not attached user list all roles of space. * Remove the unused code. Co-authored-by: Yee <[email protected]>
vesoft-inc · Sep 23, 2021 · bdda650 · bdda650
1 parent 020dbd4
commit bdda650
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 17 deletions.
diff --git a/src/graph/executor/admin/ListRolesExecutor.cpp b/src/graph/executor/admin/ListRolesExecutor.cpp
@@ -38,10 +38,17 @@ folly::Future<Status> ListRolesExecutor::listRoles() {
         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(),
-                              apache::thrift::util::enumNameSafe(foundItem->get_role_type())}));
-        } else {
+        if (foundItem != items.end()) {
+          if (foundItem->get_role_type() != meta::cpp2::RoleType::ADMIN) {
+            v.emplace_back(Row({foundItem->get_user_id(),
+                                apache::thrift::util::enumNameSafe(foundItem->get_role_type())}));
+          } else {
+            for (const auto &item : items) {
+              v.emplace_back(nebula::Row(
+                  {item.get_user_id(), apache::thrift::util::enumNameSafe(item.get_role_type())}));
+            }
+          }
+        } else if (qctx_->rctx()->session()->isGod()) {
           for (const auto &item : items) {
             v.emplace_back(nebula::Row(
                 {item.get_user_id(), apache::thrift::util::enumNameSafe(item.get_role_type())}));

diff --git a/src/graph/session/ClientSession.cpp b/src/graph/session/ClientSession.cpp
@@ -55,7 +55,7 @@ void ClientSession::deleteQuery(QueryContext* qctx) {
   session_.queries_ref()->erase(epId);
 }
 
-bool ClientSession::findQuery(nebula::ExecutionPlanID epId) {
+bool ClientSession::findQuery(nebula::ExecutionPlanID epId) const {
   folly::RWSpinLock::ReadHolder rHolder(rwSpinLock_);
   auto context = contexts_.find(epId);
   if (context != contexts_.end()) {

diff --git a/src/graph/session/ClientSession.h b/src/graph/session/ClientSession.h
@@ -29,12 +29,12 @@ class ClientSession final {
   static std::shared_ptr<ClientSession> create(meta::cpp2::Session&& session,
                                                meta::MetaClient* metaClient);
 
-  int64_t id() {
+  int64_t id() const {
     folly::RWSpinLock::ReadHolder rHolder(rwSpinLock_);
     return session_.get_session_id();
   }
 
-  const SpaceInfo space() {
+  const SpaceInfo& space() const {
     folly::RWSpinLock::ReadHolder rHolder(rwSpinLock_);
     return space_;
   }
@@ -47,22 +47,22 @@ class ClientSession final {
     }
   }
 
-  const std::string spaceName() {
+  const std::string& spaceName() const {
     folly::RWSpinLock::ReadHolder rHolder(rwSpinLock_);
     return session_.get_space_name();
   }
 
-  const std::string user() {
+  const std::string& user() const {
     folly::RWSpinLock::ReadHolder rHolder(rwSpinLock_);
     return session_.get_user_name();
   }
 
-  std::unordered_map<GraphSpaceID, meta::cpp2::RoleType> roles() {
+  const std::unordered_map<GraphSpaceID, meta::cpp2::RoleType>& roles() const {
     folly::RWSpinLock::ReadHolder rHolder(rwSpinLock_);
     return roles_;
   }
 
-  StatusOr<meta::cpp2::RoleType> roleWithSpace(GraphSpaceID space) {
+  StatusOr<meta::cpp2::RoleType> roleWithSpace(GraphSpaceID space) const {
     folly::RWSpinLock::ReadHolder rHolder(rwSpinLock_);
     auto ret = roles_.find(space);
     if (ret == roles_.end()) {
@@ -71,7 +71,7 @@ class ClientSession final {
     return ret->second;
   }
 
-  bool isGod() {
+  bool isGod() const {
     folly::RWSpinLock::ReadHolder rHolder(rwSpinLock_);
     // Cloud may have multiple God accounts
     for (auto& role : roles_) {
@@ -91,12 +91,12 @@ class ClientSession final {
 
   void charge();
 
-  int32_t getTimezone() {
+  int32_t getTimezone() const {
     folly::RWSpinLock::ReadHolder rHolder(rwSpinLock_);
     return session_.get_timezone();
   }
 
-  HostAddr getGraphAddr() {
+  const HostAddr& getGraphAddr() const {
     folly::RWSpinLock::ReadHolder rHolder(rwSpinLock_);
     return session_.get_graph_addr();
   }
@@ -120,7 +120,7 @@ class ClientSession final {
     }
   }
 
-  const meta::cpp2::Session getSession() {
+  const meta::cpp2::Session& getSession() const {
     folly::RWSpinLock::ReadHolder rHolder(rwSpinLock_);
     return session_;
   }
@@ -134,7 +134,7 @@ class ClientSession final {
 
   void deleteQuery(QueryContext* qctx);
 
-  bool findQuery(nebula::ExecutionPlanID epId);
+  bool findQuery(nebula::ExecutionPlanID epId) const;
 
   void markQueryKilled(nebula::ExecutionPlanID epId);
 
@@ -150,7 +150,7 @@ class ClientSession final {
   time::Duration idleDuration_;
   meta::cpp2::Session session_;
   meta::MetaClient* metaClient_{nullptr};
-  folly::RWSpinLock rwSpinLock_;
+  mutable folly::RWSpinLock rwSpinLock_;
   /*
    * map<spaceId, role>
    * One user can have roles in multiple spaces

diff --git a/tests/admin/test_permission.py b/tests/admin/test_permission.py
@@ -762,6 +762,8 @@ def test_show_roles(self):
         self.check_resp_succeeded(resp)
         time.sleep(self.delay)
 
+        ret, self.testClient = self.spawn_nebula_client_and_auth('test', 'test')
+        assert ret
         ret, self.adminClient = self.spawn_nebula_client_and_auth('admin', 'admin')
         assert ret
         ret, self.dbaClient = self.spawn_nebula_client_and_auth('dba', 'dba')
@@ -771,6 +773,11 @@ def test_show_roles(self):
         ret, self.guestClient = self.spawn_nebula_client_and_auth('guest', 'guest')
         assert ret
 
+        query = 'SHOW ROLES IN space5'
+        expected_result = []
+        resp = self.testClient.execute(query)
+        self.check_resp_failed(resp, ttypes.ErrorCode.E_BAD_PERMISSION)
+
         query = 'SHOW ROLES IN space5'
         expected_result = [['guest', 'GUEST'],
                            ['user', 'USER'],