Add privilege commands for foreign server

Implements:

* GRANT CREATE SERVER ON DATABASE <dbName> TO <entityList>;
* GRANT DROP SERVER ON DATABASE <dbName> TO <entityList>;
* GRANT DROP ON SERVER <serverName> TO <entityList>;
* REVOKE CREATE SERVER ON DATABASE <dbName> FROM <entityList>;
* REVOKE DROP SERVER ON DATABASE <dbName> FROM <entityList>;
* REVOKE DROP ON SERVER <serverName> FROM <entityList>;

Author: mattgara

Catalog/Catalog.cpp

@@ -614,6 +614,7 @@ void Catalog::createFsiSchemasAndDefaultServers() {
         "id integer primary key, "
         "name text unique, "
         "data_wrapper_type text, "
+        "owner_user_id integer, "
         "options text)");
     createDefaultServersIfNotExists();
     sqliteConnector_.query(
@@ -721,7 +722,8 @@ void Catalog::recordOwnershipOfObjectsInObjectPermissions() {
             {DatabaseDBObjectType, AccessPrivileges::ALL_DATABASE},
             {TableDBObjectType, AccessPrivileges::ALL_TABLE},
             {DashboardDBObjectType, AccessPrivileges::ALL_DASHBOARD},
-            {ViewDBObjectType, AccessPrivileges::ALL_VIEW}};
+            {ViewDBObjectType, AccessPrivileges::ALL_VIEW},
+            {ServerDBObjectType, AccessPrivileges::ALL_SERVER}};
 
     // grant owner all permissions on DB
     DBObjectKey key;
@@ -2396,10 +2398,12 @@ void Catalog::createForeignServerNoLocks(
 
   if (sqliteConnector_.getNumRows() == 0) {
     sqliteConnector_.query_with_text_params(
-        "INSERT INTO omnisci_foreign_servers (name, data_wrapper_type, options) "
-        "VALUES (?, ?, ?)",
+        "INSERT INTO omnisci_foreign_servers (name, data_wrapper_type, owner_user_id, "
+        "options) "
+        "VALUES (?, ?, ?, ?)",
         std::vector<std::string>{foreign_server->name,
                                  foreign_server->data_wrapper.name,
+                                 std::to_string(foreign_server->user_id),
                                  foreign_server->getOptionsAsJsonString()});
     sqliteConnector_.query_with_text_params(
         "SELECT id from omnisci_foreign_servers where name = ?",
@@ -2417,37 +2421,40 @@ void Catalog::createForeignServerNoLocks(
   foreignServerMapById_[foreign_server_shared->id] = foreign_server_shared;
 }
 
-foreign_storage::ForeignServer* Catalog::getForeignServer(const std::string& server_name,
-                                                          const bool skip_cache) {
+foreign_storage::ForeignServer* Catalog::getForeignServer(
+    const std::string& server_name) const {
   foreign_storage::ForeignServer* foreign_server = nullptr;
-  if (!skip_cache) {
-    cat_read_lock read_lock(this);
-    if (foreignServerMap_.find(server_name) != foreignServerMap_.end()) {
-      foreign_server = foreignServerMap_.find(server_name)->second.get();
-    }
-  } else {
-    cat_write_lock write_lock(this);
-    cat_sqlite_lock sqlite_lock(this);
+  cat_read_lock read_lock(this);
+  if (foreignServerMap_.find(server_name) != foreignServerMap_.end()) {
+    foreign_server = foreignServerMap_.find(server_name)->second.get();
+  }
+  return foreign_server;
+}
 
-    sqliteConnector_.query_with_text_params(
-        "SELECT id, name, data_wrapper_type, options "
-        "FROM omnisci_foreign_servers WHERE name = ?",
-        std::vector<std::string>{server_name});
-    if (sqliteConnector_.getNumRows() > 0) {
-      auto server = std::make_shared<foreign_storage::ForeignServer>(
-          foreign_storage::DataWrapper{sqliteConnector_.getData<std::string>(0, 2)});
-      server->id = sqliteConnector_.getData<int>(0, 0);
-      server->name = sqliteConnector_.getData<std::string>(0, 1);
-      server->populateOptionsMap(sqliteConnector_.getData<std::string>(0, 3));
-      foreign_server = server.get();
-      foreignServerMap_[server->name] = server;
-      foreignServerMapById_[server->id] = server;
-    }
+foreign_storage::ForeignServer* Catalog::getForeignServerSkipCache(
+    const std::string& server_name) {
+  foreign_storage::ForeignServer* foreign_server = nullptr;
+  cat_write_lock write_lock(this);
+  cat_sqlite_lock sqlite_lock(this);
+  sqliteConnector_.query_with_text_params(
+      "SELECT id, name, data_wrapper_type, options, owner_user_id "
+      "FROM omnisci_foreign_servers WHERE name = ?",
+      std::vector<std::string>{server_name});
+  if (sqliteConnector_.getNumRows() > 0) {
+    auto server = std::make_shared<foreign_storage::ForeignServer>(
+        foreign_storage::DataWrapper{sqliteConnector_.getData<std::string>(0, 2)});
+    server->id = sqliteConnector_.getData<int>(0, 0);
+    server->name = sqliteConnector_.getData<std::string>(0, 1);
+    server->user_id = sqliteConnector_.getData<std::int32_t>(0, 4);
+    server->populateOptionsMap(sqliteConnector_.getData<std::string>(0, 3));
+    foreign_server = server.get();
+    foreignServerMap_[server->name] = server;
+    foreignServerMapById_[server->id] = server;
   }
   return foreign_server;
 }
 
-void Catalog::dropForeignServer(const std::string& server_name, bool if_exists) {
+void Catalog::dropForeignServer(const std::string& server_name) {
   cat_write_lock write_lock(this);
   cat_sqlite_lock sqlite_lock(this);
 
@@ -2471,9 +2478,6 @@ void Catalog::dropForeignServer(const std::string& server_name, bool if_exists)
         std::vector<std::string>{server_name});
     foreignServerMap_.erase(server_name);
     foreignServerMapById_.erase(server_id);
-  } else if (!if_exists) {
-    throw std::runtime_error{"Foreign server with name \"" + server_name +
-                             "\" does not exist."};
   }
 }
 
@@ -3451,14 +3455,16 @@ void Catalog::vacuumDeletedRows(const TableDescriptor* td) const {
 
 void Catalog::buildForeignServerMap() {
   sqliteConnector_.query(
-      "SELECT id, name, data_wrapper_type, options FROM omnisci_foreign_servers");
+      "SELECT id, name, data_wrapper_type, options, owner_user_id FROM "
+      "omnisci_foreign_servers");
   auto num_rows = sqliteConnector_.getNumRows();
   for (size_t row = 0; row < num_rows; row++) {
     auto foreign_server = std::make_shared<foreign_storage::ForeignServer>(
         foreign_storage::DataWrapper{sqliteConnector_.getData<std::string>(row, 2)});
     foreign_server->id = sqliteConnector_.getData<int>(row, 0);
     foreign_server->name = sqliteConnector_.getData<std::string>(row, 1);
     foreign_server->populateOptionsMap(sqliteConnector_.getData<std::string>(row, 3));
+    foreign_server->user_id = sqliteConnector_.getData<std::int32_t>(row, 4);
     foreignServerMap_[foreign_server->name] = foreign_server;
     foreignServerMapById_[foreign_server->id] = foreign_server;
   }
@@ -3490,6 +3496,7 @@ void Catalog::createDefaultServersIfNotExists() {
   local_csv_server
       ->options[std::string{foreign_storage::ForeignServer::STORAGE_TYPE_KEY}] =
       foreign_storage::ForeignServer::LOCAL_FILE_STORAGE_TYPE;
+  local_csv_server->user_id = OMNISCI_ROOT_USER_ID;
   local_csv_server->options[std::string{foreign_storage::ForeignServer::BASE_PATH_KEY}] =
       "/";
   local_csv_server->validate();
@@ -3501,6 +3508,7 @@ void Catalog::createDefaultServersIfNotExists() {
   local_parquet_server
       ->options[std::string{foreign_storage::ForeignServer::STORAGE_TYPE_KEY}] =
       foreign_storage::ForeignServer::LOCAL_FILE_STORAGE_TYPE;
+  local_parquet_server->user_id = OMNISCI_ROOT_USER_ID;
   local_parquet_server
       ->options[std::string{foreign_storage::ForeignServer::BASE_PATH_KEY}] = "/";
   local_parquet_server->validate();

Catalog/Catalog.h

@@ -248,25 +248,29 @@ class Catalog final {
    * Gets a pointer to a struct containing foreign server details.
    *
    * @param server_name - Name of foreign server whose details will be fetched
-   * @param skip_cache - flag indicating whether or not to skip in-memory cache of foreign
-   * server struct when attempting to fetch foreign server details. This flag is mainly
-   * used for testing
    * @return pointer to a struct containing foreign server details. nullptr is returned if
    * no foreign server exists with the given name
    */
-  foreign_storage::ForeignServer* getForeignServer(const std::string& server_name,
-                                                   const bool skip_cache = false);
+  foreign_storage::ForeignServer* getForeignServer(const std::string& server_name) const;
+
+  /**
+   * Gets a pointer to a struct containing foreign server details.
+   * Skip in-memory cache of foreign server struct when attempting to fetch foreign server
+   * details. This is mainly used for testing.
+   *
+   * @param server_name - Name of foreign server whose details will be fetched
+   * @return pointer to a struct containing foreign server details. nullptr is returned if
+   * no foreign server exists with the given name
+   */
+  foreign_storage::ForeignServer* getForeignServerSkipCache(
+      const std::string& server_name);
 
   /**
    * Drops/deletes a foreign server DB object.
    *
    * @param server_name - Name of foreign server that will be deleted
-   * @param if_exists - flag indicating whether or not an attempt to delete a foreign
-   * server should occur if a server with the same name does not exists. An exception is
-   * thrown if this flag is set to "false" and an attempt is made to delete a nonexistent
-   * foreign server
    */
-  void dropForeignServer(const std::string& server_name, bool if_exists);
+  void dropForeignServer(const std::string& server_name);
 
   /**
    * Creates default local file servers (if they don't already exist).

Catalog/DBObject.cpp

@@ -69,6 +69,13 @@ const AccessPrivileges AccessPrivileges::DELETE_FROM_VIEW =
 const AccessPrivileges AccessPrivileges::TRUNCATE_VIEW =
     AccessPrivileges(ViewPrivileges::TRUNCATE_VIEW);
 
+const AccessPrivileges AccessPrivileges::ALL_SERVER =
+    AccessPrivileges(ServerPrivileges::ALL);
+const AccessPrivileges AccessPrivileges::CREATE_SERVER =
+    AccessPrivileges(ServerPrivileges::CREATE_SERVER);
+const AccessPrivileges AccessPrivileges::DROP_SERVER =
+    AccessPrivileges(ServerPrivileges::DROP_SERVER);
+
 std::string ObjectPermissionTypeToString(DBObjectType type) {
   switch (type) {
     case DatabaseDBObjectType:
@@ -79,6 +86,8 @@ std::string ObjectPermissionTypeToString(DBObjectType type) {
       return "DASHBOARD";
     case ViewDBObjectType:
       return "VIEW";
+    case ServerDBObjectType:
+      return "SERVER";
     default:
       CHECK(false);
   }
@@ -94,6 +103,8 @@ DBObjectType DBObjectTypeFromString(const std::string& type) {
     return DashboardDBObjectType;
   } else if (type.compare("VIEW") == 0) {
     return ViewDBObjectType;
+  } else if (type.compare("SERVER") == 0) {
+    return ServerDBObjectType;
   } else {
     throw std::runtime_error("DB object type " + type + " is not supported.");
   }
@@ -151,6 +162,7 @@ std::vector<std::string> DBObject::toString() const {
     case TableDBObjectType:
     case DashboardDBObjectType:
     case ViewDBObjectType:
+    case ServerDBObjectType:
       objectKey.push_back(std::to_string(objectKey_.permissionType));
       objectKey.push_back(std::to_string(objectKey_.dbId));
       objectKey.push_back(std::to_string(objectKey_.objectId));
@@ -183,6 +195,23 @@ void DBObject::loadKey(const Catalog_Namespace::Catalog& catalog) {
       loadKey();
       break;
     }
+    case ServerDBObjectType: {
+      objectKey_.dbId = catalog.getCurrentDB().dbId;
+
+      if (!getName().empty()) {
+        auto server = catalog.getForeignServer(getName());
+        if (!server) {
+          throw std::runtime_error("Failure generating DB object key. Server " +
+                                   getName() + " does not exist.");
+        }
+        objectKey_.objectId = server->id;
+        ownerId_ = server->user_id;
+      } else {
+        ownerId_ = catalog.getCurrentDB().dbOwner;
+      }
+
+      break;
+    }
     case ViewDBObjectType:
     case TableDBObjectType: {
       objectKey_.dbId = catalog.getCurrentDB().dbId;
@@ -233,6 +262,7 @@ DBObjectKey DBObjectKey::fromString(const std::vector<std::string>& key,
       objectKey.permissionType = std::stoi(key[0]);
       objectKey.dbId = std::stoi(key[1]);
       break;
+    case ServerDBObjectType:
     case TableDBObjectType:
     case ViewDBObjectType:
     case DashboardDBObjectType:

Catalog/DBObject.h

@@ -44,7 +44,8 @@ enum DBObjectType {
   DatabaseDBObjectType,
   TableDBObjectType,
   DashboardDBObjectType,
-  ViewDBObjectType
+  ViewDBObjectType,
+  ServerDBObjectType
 };
 
 std::string DBObjectTypeToString(DBObjectType type);
@@ -122,6 +123,12 @@ struct ViewPrivileges {
       CREATE_VIEW | DROP_VIEW | SELECT_FROM_VIEW | INSERT_INTO_VIEW;
 };
 
+struct ServerPrivileges {
+  static const int32_t ALL = -1;
+  static const int32_t CREATE_SERVER = 1 << 0;
+  static const int32_t DROP_SERVER = 1 << 1;
+};
+
 struct AccessPrivileges {
   int64_t privileges;
 
@@ -175,6 +182,11 @@ struct AccessPrivileges {
   static const AccessPrivileges UPDATE_IN_VIEW;
   static const AccessPrivileges DELETE_FROM_VIEW;
   static const AccessPrivileges TRUNCATE_VIEW;
+
+  // server permissions
+  static const AccessPrivileges ALL_SERVER;
+  static const AccessPrivileges CREATE_SERVER;
+  static const AccessPrivileges DROP_SERVER;
 };
 
 class DBObject {

Catalog/DdlCommandExecutor.cpp

@@ -19,6 +19,7 @@
 #include <boost/algorithm/string/predicate.hpp>
 
 #include "Catalog/Catalog.h"
+#include "Catalog/SysCatalog.h"
 #include "LockMgr/LockMgr.h"
 #include "Parser/ParserNode.h"
 #include "Shared/StringTransform.h"
@@ -69,20 +70,39 @@ CreateForeignServerCommand::CreateForeignServerCommand(
 }
 
 void CreateForeignServerCommand::execute(TQueryResult& _return) {
-  std::string_view server_name = ddl_payload["serverName"].GetString();
+  std::string server_name = ddl_payload["serverName"].GetString();
   if (boost::iequals(server_name.substr(0, 7), "omnisci")) {
     throw std::runtime_error{"Server names cannot start with \"omnisci\"."};
   }
+  bool if_not_exists = ddl_payload["ifNotExists"].GetBool();
+  if (session_ptr->getCatalog().getForeignServer(server_name)) {
+    if (if_not_exists) {
+      return;
+    } else {
+      throw std::runtime_error{"A foreign server with name \"" + server_name +
+                               "\" already exists."};
+    }
+  }
+  // check access privileges
+  if (!session_ptr->checkDBAccessPrivileges(DBObjectType::ServerDBObjectType,
+                                            AccessPrivileges::CREATE_SERVER)) {
+    throw std::runtime_error("Server " + std::string(server_name) +
+                             " will not be created. User has no create privileges.");
+  }
 
-  // TODO: add permissions check and ownership
+  auto& current_user = session_ptr->get_currentUser();
   auto foreign_server = std::make_unique<foreign_storage::ForeignServer>(
       foreign_storage::DataWrapper{ddl_payload["dataWrapper"].GetString()});
   foreign_server->name = server_name;
+  foreign_server->user_id = current_user.userId;
   foreign_server->populateOptionsMap(ddl_payload["options"]);
   foreign_server->validate();
 
-  session_ptr->getCatalog().createForeignServer(std::move(foreign_server),
-                                                ddl_payload["ifNotExists"].GetBool());
+  auto& catalog = session_ptr->getCatalog();
+  catalog.createForeignServer(std::move(foreign_server),
+                              ddl_payload["ifNotExists"].GetBool());
+  Catalog_Namespace::SysCatalog::instance().createDBObject(
+      current_user, server_name, ServerDBObjectType, catalog);
 }
 
 DropForeignServerCommand::DropForeignServerCommand(
@@ -96,14 +116,27 @@ DropForeignServerCommand::DropForeignServerCommand(
 }
 
 void DropForeignServerCommand::execute(TQueryResult& _return) {
-  std::string_view server_name = ddl_payload["serverName"].GetString();
+  std::string server_name = ddl_payload["serverName"].GetString();
   if (boost::iequals(server_name.substr(0, 7), "omnisci")) {
     throw std::runtime_error{"OmniSci default servers cannot be dropped."};
   }
-
-  // TODO: add permissions check
-  session_ptr->getCatalog().dropForeignServer(ddl_payload["serverName"].GetString(),
-                                              ddl_payload["ifExists"].GetBool());
+  bool if_exists = ddl_payload["ifExists"].GetBool();
+  if (!session_ptr->getCatalog().getForeignServer(server_name)) {
+    if (if_exists) {
+      return;
+    } else {
+      throw std::runtime_error{"Foreign server with name \"" + server_name +
+                               "\" does not exist."};
+    }
+  }
+  // check access privileges
+  if (!session_ptr->checkDBAccessPrivileges(DBObjectType::ServerDBObjectType,
+                                            AccessPrivileges::DROP_SERVER,
+                                            server_name.data())) {
+    throw std::runtime_error("Server " + server_name +
+                             " will not be dropped. User has no DROP SERVER privileges.");
+  }
+  session_ptr->getCatalog().dropForeignServer(ddl_payload["serverName"].GetString());
 }
 
 SQLTypes JsonColumnSqlType::getSqlType(const rapidjson::Value& data_type) {

Catalog/ForeignServer.h

@@ -56,6 +56,7 @@ struct ForeignServer : public OptionsContainer {
   int id;
   std::string name;
   DataWrapper data_wrapper;
+  int32_t user_id;
 
   ForeignServer(const DataWrapper& data_wrapper) : data_wrapper(data_wrapper) {}