From 0c96fcebf007e4bbb5abdc51bf96b9f78e63f150 Mon Sep 17 00:00:00 2001
From: Kurdakov Alexander <kusancho12@gmail.com>
Date: Fri, 24 Nov 2023 18:00:00 +0300
Subject: [PATCH] feat: add default privileges to _pico_privilege for new users

---
 src/bootstrap_entries.rs |  9 ++-----
 src/schema.rs            | 29 +++++++++++++++++----
 src/storage.rs           | 54 ++++++++++++++++++++++++++++++++++++++--
 test/int/test_acl.py     | 51 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 128 insertions(+), 15 deletions(-)

diff --git a/src/bootstrap_entries.rs b/src/bootstrap_entries.rs
index 56a81bcf69..e858b4aa10 100644
--- a/src/bootstrap_entries.rs
+++ b/src/bootstrap_entries.rs
@@ -3,13 +3,13 @@ use protobuf::Message;
 use tarantool::auth::AuthData;
 use tarantool::auth::AuthDef;
 use tarantool::auth::AuthMethod;
-use tarantool::session::UserId;
 
 use crate::cli::args;
 use crate::instance::Instance;
 use crate::schema::PrivilegeDef;
 use crate::schema::RoleDef;
 use crate::schema::UserDef;
+use crate::schema::{ADMIN_ID, GUEST_ID, PUBLIC_ID, SUPER_ID};
 use crate::sql::pgproto;
 use crate::storage;
 use crate::storage::ClusterwideTable;
@@ -19,11 +19,6 @@ use crate::traft;
 use crate::traft::op;
 use crate::traft::LogicalClock;
 
-pub const GUEST_ID: UserId = 0;
-pub const ADMIN_ID: UserId = 1;
-pub const PUBLIC_ID: UserId = 2;
-pub const SUPER_ID: UserId = 31;
-
 pub(super) fn prepare(args: &args::Run, instance: &Instance, tiers: &[Tier]) -> Vec<raft::Entry> {
     let mut lc = LogicalClock::new(instance.raft_id, 0);
     let mut init_entries = Vec::new();
@@ -175,7 +170,7 @@ pub(super) fn prepare(args: &args::Run, instance: &Instance, tiers: &[Tier]) ->
 
     // equivalent SQL expressions under 'admin' user:
     // GRANT <'usage', 'session'> ON 'universe' TO 'guest'
-    // GRANT 'execute' ON <'public', 'super'> TO 'guest'
+    // GRANT <'public', 'super'> TO 'guest'
     // GRANT 'all privileges' ON 'universe' TO 'admin'
     // GRANT 'all privileges' ON 'universe' TO 'super'
     for priv_def in PrivilegeDef::get_default_privileges() {
diff --git a/src/schema.rs b/src/schema.rs
index 89f00e569d..9fbf723c77 100644
--- a/src/schema.rs
+++ b/src/schema.rs
@@ -24,7 +24,6 @@ use tarantool::{
 
 use serde::{Deserialize, Serialize};
 
-use crate::bootstrap_entries::{ADMIN_ID, GUEST_ID, PUBLIC_ID, SUPER_ID};
 use crate::cas::{self, compare_and_swap};
 use crate::storage::SPACE_ID_INTERNAL_MAX;
 use crate::storage::{ClusterwideTable, PropertyName};
@@ -327,6 +326,17 @@ impl RoleDef {
 // PrivilegeDef
 ////////////////////////////////////////////////////////////////////////////////
 
+// default users
+pub const GUEST_ID: UserId = 0;
+pub const ADMIN_ID: UserId = 1;
+
+// default roles
+pub const PUBLIC_ID: UserId = 2;
+pub const SUPER_ID: UserId = 31;
+
+// `universe` has object_id 0
+pub const UNIVERSE_ID: i64 = 0;
+
 tarantool::define_str_enum! {
     pub enum SchemaObjectType {
         Table = "table",
@@ -444,8 +454,7 @@ impl PrivilegeDef {
                     grantor_id: ADMIN_ID,
                     grantee_id: GUEST_ID,
                     object_type: SchemaObjectType::Universe,
-                    // `universe` has object_id 0
-                    object_id: 0,
+                    object_id: UNIVERSE_ID,
                     privilege,
                     schema_version: 0,
                 });
@@ -472,7 +481,7 @@ impl PrivilegeDef {
                     grantor_id: ADMIN_ID,
                     grantee_id: ADMIN_ID,
                     object_type: SchemaObjectType::Universe,
-                    object_id: 0,
+                    object_id: UNIVERSE_ID,
                     privilege: *privilege,
                     schema_version: 0,
                 });
@@ -485,7 +494,7 @@ impl PrivilegeDef {
                     grantor_id: ADMIN_ID,
                     grantee_id: SUPER_ID,
                     object_type: SchemaObjectType::Universe,
-                    object_id: 0,
+                    object_id: UNIVERSE_ID,
                     privilege: *privilege,
                     schema_version: 0,
                 });
@@ -494,6 +503,16 @@ impl PrivilegeDef {
             v
         })
     }
+
+    pub fn is_default_privilege(&self) -> bool {
+        Self::get_default_privileges().contains(self)
+            // default user permissions
+            || (self.object_id == PUBLIC_ID as i64 && self.privilege == PrivilegeType::Execute)
+            || (self.object_type == SchemaObjectType::Universe && self.privilege == PrivilegeType::Session)
+            || (self.object_type == SchemaObjectType::Universe && self.privilege == PrivilegeType::Usage)
+            // alter on himself
+            || (self.object_type == SchemaObjectType::User && self.privilege == PrivilegeType::Alter && self.grantee_id as i64 == self.object_id)
+    }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/src/storage.rs b/src/storage.rs
index 9014537d6b..dfc299ae6f 100644
--- a/src/storage.rs
+++ b/src/storage.rs
@@ -20,6 +20,7 @@ use crate::replicaset::Replicaset;
 use crate::schema::{Distribution, PrivilegeType, SchemaObjectType};
 use crate::schema::{IndexDef, TableDef};
 use crate::schema::{PrivilegeDef, RoleDef, UserDef};
+use crate::schema::{ADMIN_ID, PUBLIC_ID, UNIVERSE_ID};
 use crate::sql::pgproto::DEFAULT_MAX_PG_PORTALS;
 use crate::tier::Tier;
 use crate::tlog;
@@ -2713,7 +2714,7 @@ impl SchemaDef for PrivilegeDef {
     #[inline(always)]
     fn on_insert(&self, storage: &Clusterwide) -> traft::Result<()> {
         _ = storage;
-        if Self::get_default_privileges().contains(self) {
+        if self.is_default_privilege() {
             return Ok(());
         }
 
@@ -2757,11 +2758,55 @@ pub mod acl {
         }
     }
 
+    fn get_default_privileges_for_user(
+        user_def: &UserDef,
+        grantor_id: UserId,
+    ) -> [PrivilegeDef; 4] {
+        [
+            // SQL: GRANT 'public' TO <user_name>
+            PrivilegeDef {
+                privilege: PrivilegeType::Execute,
+                object_type: SchemaObjectType::Role,
+                object_id: PUBLIC_ID as _,
+                grantee_id: user_def.id,
+                grantor_id,
+                schema_version: user_def.schema_version,
+            },
+            // ALTER USER <user_name> LOGIN
+            PrivilegeDef {
+                privilege: PrivilegeType::Session,
+                object_type: SchemaObjectType::Universe,
+                object_id: UNIVERSE_ID,
+                grantee_id: user_def.id,
+                grantor_id: ADMIN_ID,
+                schema_version: user_def.schema_version,
+            },
+            // ALTER USER <user_name> ENABLE
+            PrivilegeDef {
+                privilege: PrivilegeType::Usage,
+                object_type: SchemaObjectType::Universe,
+                object_id: UNIVERSE_ID,
+                grantee_id: user_def.id,
+                grantor_id: ADMIN_ID,
+                schema_version: user_def.schema_version,
+            },
+            // SQL: GRANT ALTER ON <user_name> TO <user_name>
+            PrivilegeDef {
+                privilege: PrivilegeType::Alter,
+                object_type: SchemaObjectType::User,
+                object_id: user_def.id as _,
+                grantee_id: user_def.id,
+                grantor_id,
+                schema_version: user_def.schema_version,
+            },
+        ]
+    }
+
     ////////////////////////////////////////////////////////////////////////////
     // acl in global storage
     ////////////////////////////////////////////////////////////////////////////
 
-    /// Persist a user definition in the internal clusterwide storage.
+    /// Persist a user definition with it's default privileges in the internal clusterwide storage.
     pub fn global_create_user(storage: &Clusterwide, user_def: &UserDef) -> tarantool::Result<()> {
         storage.users.insert(user_def)?;
 
@@ -2774,6 +2819,11 @@ pub mod acl {
             user: user,
         );
 
+        let grantor_id = ::tarantool::session::euid()?; // TODO: should be replaced by owner of user
+        for user_priv in get_default_privileges_for_user(user_def, grantor_id) {
+            global_grant_privilege(storage, &user_priv)?;
+        }
+
         Ok(())
     }
 
diff --git a/test/int/test_acl.py b/test/int/test_acl.py
index 884360fdd9..88db16da3c 100644
--- a/test/int/test_acl.py
+++ b/test/int/test_acl.py
@@ -838,7 +838,7 @@ def test_acl_from_snapshot(cluster: Cluster):
 
 def test_acl_drop_table_with_privileges(cluster: Cluster):
     i1, *_ = cluster.deploy(instance_count=1)
-    number_of_privileges_since_bootstrap = 24
+    number_of_privileges_since_bootstrap = 28
 
     # Check that we can drop a table with privileges granted on it.
     index = i1.call("pico.create_user", "Dave", VALID_PASSWORD)
@@ -918,5 +918,54 @@ def test_create_space_smoke(cluster: Cluster):
     assert ddl["row_count"] == 1
 
 
+def test_grant_and_revoke_default_users_privileges(cluster: Cluster):
+    i1, *_ = cluster.deploy(instance_count=1)
+
+    index = i1.call("pico.create_user", "Dave", VALID_PASSWORD)
+
+    # granting default privilege does not raise an error
+    new_index = i1.grant_privilege(
+        "Dave",
+        "execute",
+        "role",
+        "public",
+    )
+    assert index == new_index
+
+    # granting default privilege does not raise an error
+    new_index = i1.grant_privilege(
+        "Dave",
+        "session",
+        "universe",
+    )
+    assert index == new_index
+
+    # granting default privilege does not raise an error
+    new_index = i1.grant_privilege(
+        "Dave",
+        "usage",
+        "universe",
+    )
+    assert index == new_index
+
+    # granting default privilege does not raise an error
+    new_index = i1.grant_privilege(
+        "Dave",
+        "alter",
+        "user",
+        "Dave",
+    )
+    assert index == new_index
+
+    # revoke default privilege, so raft_index should change
+    new_index = i1.revoke_privilege("Dave", "usage", "universe")
+    assert new_index != index
+
+    index = new_index
+    # already revoked, so it should be idempotent
+    new_index = i1.revoke_privilege("Dave", "usage", "universe")
+    assert new_index == index
+
+
 # TODO: test acl get denied when there's an unfinished ddl
 # TODO: check various retryable cas outcomes when doing schema change requests
-- 
GitLab