From 950c3d11e954a9e8595bbde2874051cebdac33c1 Mon Sep 17 00:00:00 2001
From: Georgy Moshkin <gmoshkin@picodata.io>
Date: Fri, 9 Jun 2023 17:12:35 +0300
Subject: [PATCH] feat: acl drop user

---
 src/storage.rs       | 14 ++++++++++++++
 src/traft/node.rs    | 29 ++++++++++++++++++++++++++++-
 src/traft/op.rs      | 11 ++++++++++-
 test/int/test_acl.py | 37 ++++++++++++++++++++++++++++++++++---
 4 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/src/storage.rs b/src/storage.rs
index 8e04f898d7..f113a6a75b 100644
--- a/src/storage.rs
+++ b/src/storage.rs
@@ -1982,6 +1982,12 @@ impl Users {
         self.space.insert(user_def)?;
         Ok(())
     }
+
+    #[inline]
+    pub fn delete(&self, user_id: UserId) -> tarantool::Result<()> {
+        self.space.delete(&[user_id])?;
+        Ok(())
+    }
 }
 
 impl ToEntryIter for Users {
@@ -2003,6 +2009,14 @@ pub fn acl_global_create_user(storage: &Clusterwide, user_def: &UserDef) -> tara
     Ok(())
 }
 
+/// Remove a user definition and any entities owned by it from the internal
+/// clusterwide storage.
+pub fn acl_global_drop_user(storage: &Clusterwide, user_id: UserId) -> tarantool::Result<()> {
+    // TODO: delete privilege records
+    storage.users.delete(user_id)?;
+    Ok(())
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // acl
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/src/traft/node.rs b/src/traft/node.rs
index aeb270efc2..ff02a67091 100644
--- a/src/traft/node.rs
+++ b/src/traft/node.rs
@@ -13,12 +13,12 @@ use crate::loop_start;
 use crate::r#loop::FlowControl;
 use crate::rpc;
 use crate::schema::{Distribution, IndexDef, SpaceDef};
-use crate::storage::acl_global_create_user;
 use crate::storage::ddl_meta_drop_space;
 use crate::storage::SnapshotData;
 use crate::storage::ToEntryIter as _;
 use crate::storage::ToEntryIter as _;
 use crate::storage::{acl_create_user_on_master, acl_drop_user_on_master};
+use crate::storage::{acl_global_create_user, acl_global_drop_user};
 use crate::storage::{ddl_abort_on_master, ddl_meta_space_update_operable};
 use crate::storage::{local_schema_version, set_local_schema_version};
 use crate::storage::{Clusterwide, ClusterwideSpaceId, PropertyName};
@@ -1000,6 +1000,33 @@ impl NodeImpl {
                     .put(PropertyName::NextSchemaVersion, &(v_pending + 1))
                     .expect("storage error");
             }
+
+            Op::Acl {
+                schema_version,
+                acl: Acl::DropUser { user_id },
+            } => {
+                let v_local = local_schema_version().expect("storage error");
+                let v_pending = schema_version;
+                if v_local < v_pending {
+                    if self.is_readonly() {
+                        // Wait for tarantool replication with master to progress.
+                        return SleepAndRetry;
+                    } else {
+                        acl_drop_user_on_master(user_id).expect("creating user shouldn't fail");
+                        set_local_schema_version(v_pending).expect("storage error");
+                    }
+                }
+
+                acl_global_drop_user(&self.storage, user_id)
+                    .expect("droping a user definition shouldn't fail");
+
+                storage_properties
+                    .put(PropertyName::GlobalSchemaVersion, &v_pending)
+                    .expect("storage error");
+                storage_properties
+                    .put(PropertyName::NextSchemaVersion, &(v_pending + 1))
+                    .expect("storage error");
+            }
         }
 
         if let Some(lc) = &lc {
diff --git a/src/traft/op.rs b/src/traft/op.rs
index 36682223fe..dfcd0c6f9f 100644
--- a/src/traft/op.rs
+++ b/src/traft/op.rs
@@ -1,4 +1,4 @@
-use crate::schema::{Distribution, UserDef};
+use crate::schema::{Distribution, UserDef, UserId};
 use crate::storage::space_by_name;
 use crate::storage::Clusterwide;
 use ::tarantool::index::{IndexId, Part};
@@ -121,6 +121,12 @@ impl std::fmt::Display for Op {
                     user_def.id, user_def.name,
                 )
             }
+            Self::Acl {
+                schema_version,
+                acl: Acl::DropUser { user_id },
+            } => {
+                write!(f, "DropUser({schema_version}, {user_id})")
+            }
         };
 
         struct DisplayAsJson<T>(pub T);
@@ -417,6 +423,9 @@ impl DdlBuilder {
 pub enum Acl {
     /// Create a tarantool user. Grant it default privileges.
     CreateUser { user_def: UserDef },
+
+    /// Drop a tarantool user and any entities owned by it.
+    DropUser { user_id: UserId },
 }
 
 mod vec_of_raw_byte_buf {
diff --git a/test/int/test_acl.py b/test/int/test_acl.py
index 6186ae0b1a..12a69891b2 100644
--- a/test/int/test_acl.py
+++ b/test/int/test_acl.py
@@ -6,7 +6,6 @@ def propose_create_user(
     id: int,
     name: str,
     password: str,
-    wait_index: bool = True,
     timeout: int = 3,
 ) -> int:
     digest = instance.call("box.internal.prepare_auth", "chap-sha1", password)
@@ -25,14 +24,46 @@ def propose_create_user(
     return instance.call("pico.raft_propose", op, timeout=timeout)
 
 
-def test_acl_create_user_basic(cluster: Cluster):
+def propose_drop_user(
+    instance: Instance,
+    id: int,
+    timeout: int = 3,
+) -> int:
+    schema_version = instance.next_schema_version()
+    op = dict(
+        kind="acl",
+        op_kind="drop_user",
+        user_id=id,
+        schema_version=schema_version,
+    )
+    # TODO: use pico.cas
+    return instance.call("pico.raft_propose", op, timeout=timeout)
+
+
+def test_acl_basic(cluster: Cluster):
     i1, *_ = cluster.deploy(instance_count=4, init_replication_factor=2)
 
     username = "Bobby"
-    index = propose_create_user(i1, id=314, name=username, password="s3cr3t")
+
+    for i in cluster.instances:
+        assert i.call("box.space._pico_property:get", "global_schema_version")[1] == 0
+        assert i.call("box.space._user.index.name:get", username) is None
+
+    user_id = 314
+    index = propose_create_user(i1, id=user_id, name=username, password="s3cr3t")
 
     for i in cluster.instances:
         i.raft_wait_index(index)
 
     for i in cluster.instances:
+        assert i.call("box.space._pico_property:get", "global_schema_version")[1] == 1
         assert i.call("box.space._user.index.name:get", username) is not None
+
+    index = propose_drop_user(i1, id=user_id)
+
+    for i in cluster.instances:
+        i.raft_wait_index(index)
+
+    for i in cluster.instances:
+        assert i.call("box.space._pico_property:get", "global_schema_version")[1] == 2
+        assert i.call("box.space._user.index.name:get", username) is None
-- 
GitLab