Fix permission when use multi sentences with use space

src/service/PermissionCheck.cpp

@@ -33,6 +33,7 @@ namespace graph {
 // static
 Status PermissionCheck::permissionCheck(ClientSession *session,
  Sentence* sentence,
+ ValidateContext *vctx,
  GraphSpaceID targetSpace) {
  if (!FLAGS_enable_authorize) {
  return Status::OK();
@@ -92,7 +93,7 @@ Status PermissionCheck::permissionCheck(ClientSession *session,
  case Sentence::Kind::kDropFTIndex:
  case Sentence::Kind::kAddListener:
  case Sentence::Kind::kRemoveListener: {
- return PermissionManager::canWriteSchema(session);
+ return PermissionManager::canWriteSchema(session, vctx);
  }
  case Sentence::Kind::kCreateUser:
  case Sentence::Kind::kDropUser:
@@ -115,7 +116,7 @@ Status PermissionCheck::permissionCheck(ClientSession *session,
  case Sentence::Kind::kUpdateEdge :
  case Sentence::Kind::kDeleteVertices :
  case Sentence::Kind::kDeleteEdges : {
- return PermissionManager::canWriteData(session);
+ return PermissionManager::canWriteData(session, vctx);
  }
  case Sentence::Kind::kDescribeTag:
  case Sentence::Kind::kDescribeEdge:
@@ -136,7 +137,7 @@ Status PermissionCheck::permissionCheck(ClientSession *session,
  case Sentence::Kind::kLimit:
  case Sentence::Kind::kGroupBy:
  case Sentence::Kind::kReturn: {
- return PermissionManager::canReadSchemaOrData(session);
+ return PermissionManager::canReadSchemaOrData(session, vctx);
  }
  case Sentence::Kind::kShowParts:
  case Sentence::Kind::kShowTags:

src/service/PermissionCheck.h

@@ -20,6 +20,7 @@ class PermissionCheck final {
 
  static Status permissionCheck(ClientSession *session,
  Sentence* sentence,
+ ValidateContext *vctx,
  GraphSpaceID targetSpace = -1);
 };
 } // namespace graph

src/service/PermissionManager.cpp

@@ -36,19 +36,15 @@ Status PermissionManager::canReadSpace(ClientSession *session, GraphSpaceID spac
 }
 
 // static
-Status PermissionManager::canReadSchemaOrData(ClientSession *session) {
+Status PermissionManager::canReadSchemaOrData(ClientSession *session, ValidateContext *vctx) {
  if (!FLAGS_enable_authorize) {
  return Status::OK();
  }
  if (session->isGod()) {
  return Status::OK();
  }
- if (session->space().id == -1) {
- LOG(ERROR) << "No space selected";
- return Status::PermissionError("No space selected.");
- }
- auto roleResult = session->roleWithSpace(session->space().id);
- if (!roleResult.ok()) {
+ auto roleResult = checkRoleWithSpace(session, vctx);
+ if (!roleResult.ok()) {
  return Status::PermissionError("No permission to read schema/data.");
  }
  auto role = roleResult.value();
@@ -76,18 +72,14 @@ Status PermissionManager::canWriteSpace(ClientSession *session) {
 }
 
 // static
-Status PermissionManager::canWriteSchema(ClientSession *session) {
+Status PermissionManager::canWriteSchema(ClientSession *session, ValidateContext *vctx) {
  if (!FLAGS_enable_authorize) {
  return Status::OK();
  }
  if (session->isGod()) {
  return Status::OK();
  }
- if (session->space().id == -1) {
- LOG(ERROR) << "No space selected";
- return Status::PermissionError("No space selected.");
- }
- auto roleResult = session->roleWithSpace(session->space().id);
+ auto roleResult = checkRoleWithSpace(session, vctx);
  if (!roleResult.ok()) {
  return Status::PermissionError("No permission to write schema.");
  }
@@ -167,18 +159,14 @@ Status PermissionManager::canWriteRole(ClientSession *session,
 }
 
 // static
-Status PermissionManager::canWriteData(ClientSession *session) {
+Status PermissionManager::canWriteData(ClientSession *session, ValidateContext *vctx) {
  if (!FLAGS_enable_authorize) {
  return Status::OK();
  }
  if (session->isGod()) {
  return Status::OK();
  }
- if (session->space().id == -1) {
- LOG(ERROR) << "No space selected.";
- return Status::PermissionError("No space selected.");
- }
- auto roleResult = session->roleWithSpace(session->space().id);
+ auto roleResult = checkRoleWithSpace(session, vctx);
  if (!roleResult.ok()) {
  return Status::PermissionError("No permission to write data.");
  }
@@ -195,5 +183,21 @@ Status PermissionManager::canWriteData(ClientSession *session) {
  }
  return Status::PermissionError("No permission to write data.");
 }
+
+StatusOr<meta::cpp2::RoleType>
+PermissionManager::checkRoleWithSpace(ClientSession *session, ValidateContext *vctx) {
+ if (!vctx->spaceChosen()) {
+ return Status::Error("No space chosen.");
+ }
+ if (vctx->whichSpace().id <= 0) {
+ LOG(ERROR) << "Space id is invalid: " << vctx->whichSpace().id;
+ return Status::PermissionError("No space chosen.");
+ }
+ auto roleResult = session->roleWithSpace(vctx->whichSpace().id);
+ if (!roleResult.ok()) {
+ return Status::PermissionError("No permission with the space.");
+ }
+ return roleResult.value();
+}
 } // namespace graph
 } // namespace nebula

src/service/PermissionManager.h

@@ -12,6 +12,7 @@
 #include "parser/AdminSentences.h"
 #include "service/GraphFlags.h"
 #include "session/ClientSession.h"
+#include "context/ValidateContext.h"
 
 namespace nebula {
 namespace graph {
@@ -20,15 +21,19 @@ class PermissionManager final {
 public:
  PermissionManager() = delete;
  static Status canReadSpace(ClientSession *session, GraphSpaceID spaceId);
- static Status canReadSchemaOrData(ClientSession *session);
+ static Status canReadSchemaOrData(ClientSession *session, ValidateContext *vctx);
  static Status canWriteSpace(ClientSession *session);
- static Status canWriteSchema(ClientSession *session);
+ static Status canWriteSchema(ClientSession *session, ValidateContext *vctx);
  static Status canWriteUser(ClientSession *session);
  static Status canWriteRole(ClientSession *session,
  meta::cpp2::RoleType targetRole,
  GraphSpaceID spaceId,
  const std::string& targetUser);
- static Status canWriteData(ClientSession *session);
+ static Status canWriteData(ClientSession *session, ValidateContext *vctx);
+
+private:
+ static StatusOr<meta::cpp2::RoleType> checkRoleWithSpace(ClientSession *session,
+ ValidateContext *vctx);
 };
 } // namespace graph
 } // namespace nebula

src/validator/Validator.h

@@ -125,7 +125,8 @@ class Validator {
  // Do all permission checking in validator except which need execute
  // TODO(shylock) do all permission which don't need execute in here
  virtual Status checkPermission() {
- return PermissionCheck::permissionCheck(qctx_->rctx()->session(), sentence_, space_.id);
+ return PermissionCheck::permissionCheck(
+ qctx_->rctx()->session(), sentence_, qctx_->vctx(), space_.id);
  }
 
  /**

tests/admin/test_permission.py

@@ -352,14 +352,22 @@ def test_schema_and_data(self):
  self.check_resp_succeeded(resp)
  time.sleep(self.delay)
 
+ # dba use the not exist space to create tag
+ query = 'USE not_exist_space;CREATE TAG t11(t_c int);'
+ resp = self.dbaClient.execute(query)
+ self.check_resp_failed(resp)
+ resp.space_name() == ''
+
  # dba write schema test
- query = 'USE space2'
+ query = 'USE space2;CREATE TAG t1(t_c int);'
  resp = self.dbaClient.execute(query)
  self.check_resp_succeeded(resp)
 
- query = "CREATE TAG t1(t_c int)";
+ # dba use the not exist space to create tag
+ query = 'USE not_exist_space;CREATE TAG t11(t_c int);'
  resp = self.dbaClient.execute(query)
- self.check_resp_succeeded(resp)
+ self.check_resp_failed(resp)
+ resp.space_name() == 'space2'
 
  query = "CREATE EDGE e1(e_c int)";
  resp = self.dbaClient.execute(query)