diff --git a/src/cas.rs b/src/cas.rs
index 89a291daa7e6895e248c2adbd18c83d86d38b3e8..13b8adc32fca128d4e6c46fec0a9e6e8d9a8f250 100644
--- a/src/cas.rs
+++ b/src/cas.rs
@@ -375,7 +375,7 @@ fn proc_cas_local(req: &Request) -> Result<Response> {
// We can't use a `do_acl` inside of a transaction here similarly to `do_dml`,
// as acl should be applied only on replicaset leader. Raft leader is not always
// a replicaset leader.
- Op::Acl(acl) => acl.validate()?,
+ Op::Acl(acl) => acl.validate(storage)?,
_ => (),
}
diff --git a/src/schema.rs b/src/schema.rs
index a834a02d41cf0484f814b0ab92222531521fcbf5..9cccde3c45a4762c00e43cd69fe74ace292bfbf1 100644
--- a/src/schema.rs
+++ b/src/schema.rs
@@ -856,6 +856,53 @@ impl UserDef {
ty: UserMetadataKind::User,
}
}
+
+ pub fn ensure_no_dependent_objects(&self, storage: &Clusterwide) -> traft::Result<()> {
+ let tables: Vec<_> = storage
+ .tables
+ .by_owner_id(self.id)?
+ .map(|def| def.name)
+ .collect();
+
+ let routines: Vec<_> = storage
+ .routines
+ .by_owner_id(self.id)?
+ .map(|def| def.name)
+ .collect();
+
+ let users: Vec<_> = storage
+ .users
+ .by_owner_id(self.id)?
+ .filter(|def| def.ty == UserMetadataKind::User)
+ .map(|def| def.name)
+ .collect();
+
+ let roles: Vec<_> = storage
+ .users
+ .by_owner_id(self.id)?
+ .filter(|def| def.ty == UserMetadataKind::Role)
+ .map(|def| def.name)
+ .collect();
+
+ if tables.is_empty() && routines.is_empty() && roles.is_empty() && users.is_empty() {
+ return Ok(());
+ }
+
+ let mut err = "user cannot be dropped because some objects depend on it\n".to_string();
+ if !tables.is_empty() {
+ err.push_str(&format!("owner of tables {}\n", tables.join(", ")));
+ }
+ if !routines.is_empty() {
+ err.push_str(&format!("owner of procedures {}\n", routines.join(", ")));
+ }
+ if !users.is_empty() {
+ err.push_str(&format!("owner of users {}\n", users.join(", ")));
+ }
+ if !roles.is_empty() {
+ err.push_str(&format!("owner of roles {}\n", roles.join(", ")));
+ }
+ Err(traft::error::Error::Other(err.into()))
+ }
}
////////////////////////////////////////////////////////////////////////////////
diff --git a/src/storage.rs b/src/storage.rs
index 1e7ef8bc2d4706a7d0686472c4c0f5adefa1d6f1..6356679e64f9d18ce40d6376304c7af0a1a56c98 100644
--- a/src/storage.rs
+++ b/src/storage.rs
@@ -319,8 +319,9 @@ define_clusterwide_tables! {
pub struct Tables {
space: Space,
#[primary]
- index_id: Index => "_pico_table_id",
- index_name: Index => "_pico_table_name",
+ index_id: Index => "_pico_table_id",
+ index_name: Index => "_pico_table_name",
+ index_owner_id: Index => "_pico_table_owner_id",
}
}
Index = 513, "_pico_index" => {
@@ -387,6 +388,7 @@ define_clusterwide_tables! {
#[primary]
index_id: Index => "_pico_user_id",
index_name: Index => "_pico_user_name",
+ index_owner_id: Index => "_pico_user_owner_id",
}
}
Privilege = 521, "_pico_privilege" => {
@@ -418,8 +420,9 @@ define_clusterwide_tables! {
pub struct Routines {
space: Space,
#[primary]
- index_id: Index => "_pico_routine_id",
- index_name: Index => "_pico_routine_name",
+ index_id: Index => "_pico_routine_id",
+ index_name: Index => "_pico_routine_name",
+ index_owner_id: Index => "_pico_routine_owner_id",
}
}
Plugin = 526, "_pico_plugin" => {
@@ -2379,10 +2382,18 @@ impl Tables {
.if_not_exists(true)
.create()?;
+ let index_owner_id = space
+ .index_builder("_pico_table_owner_id")
+ .unique(false)
+ .part("owner")
+ .if_not_exists(true)
+ .create()?;
+
Ok(Self {
space,
index_id,
index_name,
+ index_owner_id,
})
}
@@ -2417,6 +2428,17 @@ impl Tables {
// This means the local schema is already up to date and main loop doesn't need to do anything
schema_version: INITIAL_SCHEMA_VERSION,
},
+ IndexDef {
+ table_id: Self::TABLE_ID,
+ id: 2,
+ name: "_pico_table_owner_id".into(),
+ ty: IndexType::Tree,
+ opts: vec![IndexOption::Unique(false)],
+ parts: vec![Part::from(("owner", IndexFieldType::Unsigned)).is_nullable(false)],
+ operable: true,
+ // This means the local schema is already up to date and main loop doesn't need to do anything
+ schema_version: INITIAL_SCHEMA_VERSION,
+ },
]
}
@@ -2475,6 +2497,14 @@ impl Tables {
Ok(None)
}
}
+
+ pub fn by_owner_id(
+ &self,
+ owner_id: UserId,
+ ) -> tarantool::Result<EntryIter<TableDef, MP_CUSTOM>> {
+ let iter = self.index_owner_id.select(IteratorType::Eq, &[owner_id])?;
+ Ok(EntryIter::new(iter))
+ }
}
impl ToEntryIter<MP_CUSTOM> for Tables {
@@ -3052,10 +3082,18 @@ impl Users {
.if_not_exists(true)
.create()?;
+ let index_owner_id = space
+ .index_builder("_pico_user_owner_id")
+ .unique(false)
+ .part("owner")
+ .if_not_exists(true)
+ .create()?;
+
Ok(Self {
space,
index_id,
index_name,
+ index_owner_id,
})
}
@@ -3090,6 +3128,17 @@ impl Users {
// This means the local schema is already up to date and main loop doesn't need to do anything
schema_version: INITIAL_SCHEMA_VERSION,
},
+ IndexDef {
+ table_id: Self::TABLE_ID,
+ id: 2,
+ name: "_pico_user_owner_id".into(),
+ ty: IndexType::Tree,
+ opts: vec![IndexOption::Unique(false)],
+ parts: vec![Part::from(("owner", IndexFieldType::Unsigned)).is_nullable(false)],
+ operable: true,
+ // This means the local schema is already up to date and main loop doesn't need to do anything
+ schema_version: INITIAL_SCHEMA_VERSION,
+ },
]
}
@@ -3105,6 +3154,12 @@ impl Users {
tuple.as_ref().map(Tuple::decode).transpose()
}
+ #[inline]
+ pub fn by_owner_id(&self, owner_id: UserId) -> tarantool::Result<EntryIter<UserDef, MP_SERDE>> {
+ let iter = self.index_owner_id.select(IteratorType::Eq, &[owner_id])?;
+ Ok(EntryIter::new(iter))
+ }
+
#[inline]
pub fn max_user_id(&self) -> tarantool::Result<Option<UserId>> {
match self.index_id.max(&())? {
@@ -3455,10 +3510,18 @@ impl Routines {
.if_not_exists(true)
.create()?;
+ let index_owner_id = space
+ .index_builder("_pico_routine_owner_id")
+ .unique(false)
+ .part("owner")
+ .if_not_exists(true)
+ .create()?;
+
Ok(Self {
space,
index_id,
index_name,
+ index_owner_id,
})
}
@@ -3493,6 +3556,17 @@ impl Routines {
// This means the local schema is already up to date and main loop doesn't need to do anything
schema_version: INITIAL_SCHEMA_VERSION,
},
+ IndexDef {
+ table_id: Self::TABLE_ID,
+ id: 2,
+ name: "_pico_routine_owner_id".into(),
+ ty: IndexType::Tree,
+ opts: vec![IndexOption::Unique(false)],
+ parts: vec![Part::from(("owner", IndexFieldType::Unsigned)).is_nullable(false)],
+ operable: true,
+ // This means the local schema is already up to date and main loop doesn't need to do anything
+ schema_version: INITIAL_SCHEMA_VERSION,
+ },
]
}
@@ -3535,6 +3609,14 @@ impl Routines {
self.space.update(&[routine_id], ops)?;
Ok(())
}
+
+ pub fn by_owner_id(
+ &self,
+ owner_id: UserId,
+ ) -> tarantool::Result<EntryIter<RoutineDef, MP_SERDE>> {
+ let iter = self.index_owner_id.select(IteratorType::Eq, &[owner_id])?;
+ Ok(EntryIter::new(iter))
+ }
}
impl ToEntryIter<MP_SERDE> for Routines {
@@ -4443,14 +4525,17 @@ pub mod acl {
Ok(())
}
- /// Remove a user definition and any entities owned by it from the internal
- /// clusterwide storage.
+ /// Remove a user with no dependent objects.
pub fn global_drop_user(
storage: &Clusterwide,
user_id: UserId,
initiator: UserId,
- ) -> tarantool::Result<()> {
- let user_def = storage.users.by_id(user_id)?.expect("failed to get user");
+ ) -> traft::Result<()> {
+ let user_def = storage.users.by_id(user_id)?.ok_or_else(|| {
+ BoxError::new(TntErrorCode::NoSuchUser, format!("no such user #{user_id}"))
+ })?;
+ user_def.ensure_no_dependent_objects(storage)?;
+
storage.privileges.delete_all_by_grantee_id(user_id)?;
storage.users.delete(user_id)?;
@@ -4720,7 +4805,6 @@ pub mod acl {
let lua = ::tarantool::lua_state();
lua.exec_with("box.schema.user.drop(...)", user_id)
.map_err(LuaError::from)?;
-
Ok(())
}
diff --git a/src/traft/op.rs b/src/traft/op.rs
index 9b767f7d1f81491aa0a3da99d2f60acf75d026c5..569fe0f57a466c929dd3efa451899d02551aafb9 100644
--- a/src/traft/op.rs
+++ b/src/traft/op.rs
@@ -13,6 +13,7 @@ use ::tarantool::space::{Field, SpaceId};
use ::tarantool::tlua;
use ::tarantool::tuple::{ToTupleBuffer, TupleBuffer};
use serde::{Deserialize, Serialize};
+use tarantool::error::{TarantoolError, TarantoolErrorCode};
use tarantool::index::IndexType;
use tarantool::session::UserId;
use tarantool::space::SpaceEngineType;
@@ -203,7 +204,7 @@ impl std::fmt::Display for Op {
initiator,
schema_version,
}) => {
- write!(f, "DropUser({schema_version}, {user_id} {initiator})")
+ write!(f, "DropUser({schema_version}, {user_id}, {initiator})")
}
Self::Acl(Acl::CreateRole { role_def }) => {
let UserDef {
@@ -824,7 +825,7 @@ impl Acl {
/// Performs preliminary checks on acl so that it will not fail when applied.
/// These checks do not include authorization checks, which are done separately in
/// [`crate::access_control::access_check_op`].
- pub fn validate(&self) -> Result<(), TRaftError> {
+ pub fn validate(&self, storage: &Clusterwide) -> Result<(), TRaftError> {
// THOUGHT: should we move access_check_* fns here as it's done in tarantool?
match self {
Self::ChangeAuth { user_id, .. } => {
@@ -840,6 +841,15 @@ impl Acl {
if *user_id == GUEST_ID || *user_id == ADMIN_ID {
return Err(TRaftError::other("dropping system user is not allowed"));
}
+
+ let user_def = storage.users.by_id(*user_id)?.ok_or_else(|| {
+ TarantoolError::new(
+ TarantoolErrorCode::NoSuchUser,
+ format!("no such user #{user_id}"),
+ )
+ })?;
+ user_def.ensure_no_dependent_objects(storage)?;
+
// user_has_data will be successful in any case https://git.picodata.io/picodata/tarantool/-/blob/da5ad0fa3ab8940f524cfa9bf3d582347c01fc4a/src/box/alter.cc#L2846
// as box.schema.user.drop(..) deletes all the related spaces/priveleges/etc.
}
diff --git a/test/int/test_basics.py b/test/int/test_basics.py
index ed4ba1a789e7a3f8f152a340423efae8580c4530..6872adcefa01588c8413c6f882f85ab059587af3 100644
--- a/test/int/test_basics.py
+++ b/test/int/test_basics.py
@@ -424,6 +424,7 @@ Insert(_pico_user, [3,"replication",0,null,1,"role"]))|
Insert(_pico_table, [{_pico_table},"_pico_table",{{"Global":null}},[{{"field_type":"unsigned","is_nullable":false,"name":"id"}},{{"field_type":"string","is_nullable":false,"name":"name"}},{{"field_type":"map","is_nullable":false,"name":"distribution"}},{{"field_type":"array","is_nullable":false,"name":"format"}},{{"field_type":"unsigned","is_nullable":false,"name":"schema_version"}},{{"field_type":"boolean","is_nullable":false,"name":"operable"}},{{"field_type":"string","is_nullable":false,"name":"engine"}},{{"field_type":"unsigned","is_nullable":false,"name":"owner"}},{{"field_type":"string","is_nullable":false,"name":"description"}}],0,true,"memtx",1,"Stores metadata of all the cluster tables in picodata."]),
Insert(_pico_index, [{_pico_table},0,"_pico_table_id","tree",[{{"unique":true}}],[["id","unsigned",null,false,null]],true,0]),
Insert(_pico_index, [{_pico_table},1,"_pico_table_name","tree",[{{"unique":true}}],[["name","string",null,false,null]],true,0]),
+Insert(_pico_index, [{_pico_table},2,"_pico_table_owner_id","tree",[{{"unique":false}}],[["owner","unsigned",null,false,null]],true,0]),
Insert(_pico_table, [{_pico_index},"_pico_index",{{"Global":null}},[{{"field_type":"unsigned","is_nullable":false,"name":"table_id"}},{{"field_type":"unsigned","is_nullable":false,"name":"id"}},{{"field_type":"string","is_nullable":false,"name":"name"}},{{"field_type":"string","is_nullable":false,"name":"type"}},{{"field_type":"array","is_nullable":false,"name":"opts"}},{{"field_type":"array","is_nullable":false,"name":"parts"}},{{"field_type":"boolean","is_nullable":false,"name":"operable"}},{{"field_type":"unsigned","is_nullable":false,"name":"schema_version"}}],0,true,"memtx",1,""]),
Insert(_pico_index, [{_pico_index},0,"_pico_index_id","tree",[{{"unique":true}}],[["table_id","unsigned",null,false,null],["id","unsigned",null,false,null]],true,0]),
Insert(_pico_index, [{_pico_index},1,"_pico_index_name","tree",[{{"unique":true}}],[["name","string",null,false,null]],true,0]),
@@ -441,6 +442,7 @@ Insert(_pico_index, [{_pico_replicaset},1,"_pico_replicaset_uuid","tree",[{{"uni
Insert(_pico_table, [{_pico_user},"_pico_user",{{"Global":null}},[{{"field_type":"unsigned","is_nullable":false,"name":"id"}},{{"field_type":"string","is_nullable":false,"name":"name"}},{{"field_type":"unsigned","is_nullable":false,"name":"schema_version"}},{{"field_type":"array","is_nullable":true,"name":"auth"}},{{"field_type":"unsigned","is_nullable":false,"name":"owner"}},{{"field_type":"string","is_nullable":false,"name":"type"}}],0,true,"memtx",1,""]),
Insert(_pico_index, [{_pico_user},0,"_pico_user_id","tree",[{{"unique":true}}],[["id","unsigned",null,false,null]],true,0]),
Insert(_pico_index, [{_pico_user},1,"_pico_user_name","tree",[{{"unique":true}}],[["name","string",null,false,null]],true,0]),
+Insert(_pico_index, [{_pico_user},2,"_pico_user_owner_id","tree",[{{"unique":false}}],[["owner","unsigned",null,false,null]],true,0]),
Insert(_pico_table, [{_pico_privilege},"_pico_privilege",{{"Global":null}},[{{"field_type":"unsigned","is_nullable":false,"name":"grantor_id"}},{{"field_type":"unsigned","is_nullable":false,"name":"grantee_id"}},{{"field_type":"string","is_nullable":false,"name":"privilege"}},{{"field_type":"string","is_nullable":false,"name":"object_type"}},{{"field_type":"integer","is_nullable":false,"name":"object_id"}},{{"field_type":"unsigned","is_nullable":false,"name":"schema_version"}}],0,true,"memtx",1,""]),
Insert(_pico_index, [{_pico_privilege},0,"_pico_privilege_primary","tree",[{{"unique":true}}],[["grantee_id","unsigned",null,false,null],["object_type","string",null,false,null],["object_id","integer",null,false,null],["privilege","string",null,false,null]],true,0]),
Insert(_pico_index, [{_pico_privilege},1,"_pico_privilege_object","tree",[{{"unique":false}}],[["object_type","string",null,false,null],["object_id","integer",null,false,null]],true,0]),
@@ -449,6 +451,7 @@ Insert(_pico_index, [{_pico_tier},0,"_pico_tier_name","tree",[{{"unique":true}}]
Insert(_pico_table, [{_pico_routine},"_pico_routine",{{"Global":null}},[{{"field_type":"unsigned","is_nullable":false,"name":"id"}},{{"field_type":"string","is_nullable":false,"name":"name"}},{{"field_type":"string","is_nullable":false,"name":"kind"}},{{"field_type":"array","is_nullable":false,"name":"params"}},{{"field_type":"array","is_nullable":false,"name":"returns"}},{{"field_type":"string","is_nullable":false,"name":"language"}},{{"field_type":"string","is_nullable":false,"name":"body"}},{{"field_type":"string","is_nullable":false,"name":"security"}},{{"field_type":"boolean","is_nullable":false,"name":"operable"}},{{"field_type":"unsigned","is_nullable":false,"name":"schema_version"}},{{"field_type":"unsigned","is_nullable":false,"name":"owner"}}],0,true,"memtx",1,""]),
Insert(_pico_index, [{_pico_routine},0,"_pico_routine_id","tree",[{{"unique":true}}],[["id","unsigned",null,false,null]],true,0]),
Insert(_pico_index, [{_pico_routine},1,"_pico_routine_name","tree",[{{"unique":true}}],[["name","string",null,false,null]],true,0]),
+Insert(_pico_index, [{_pico_routine},2,"_pico_routine_owner_id","tree",[{{"unique":false}}],[["owner","unsigned",null,false,null]],true,0]),
Insert(_pico_table, [{_pico_plugin},"_pico_plugin",{{"Global":null}},[{{"field_type":"string","is_nullable":false,"name":"name"}},{{"field_type":"boolean","is_nullable":false,"name":"enabled"}},{{"field_type":"array","is_nullable":false,"name":"services"}},{{"field_type":"string","is_nullable":false,"name":"version"}},{{"field_type":"string","is_nullable":false,"name":"description"}},{{"field_type":"array","is_nullable":false,"name":"migration_list"}}],0,true,"memtx",1,""]),
Insert(_pico_index, [{_pico_plugin},0,"_pico_plugin_name","tree",[{{"unique":true}}],[["name","string",null,false,null],["version","string",null,false,null]],true,0]),
Insert(_pico_table, [{_pico_service},"_pico_service",{{"Global":null}},[{{"field_type":"string","is_nullable":false,"name":"plugin_name"}},{{"field_type":"string","is_nullable":false,"name":"name"}},{{"field_type":"string","is_nullable":false,"name":"version"}},{{"field_type":"array","is_nullable":false,"name":"tiers"}},{{"field_type":"string","is_nullable":false,"name":"description"}}],0,true,"memtx",1,""]),
diff --git a/test/int/test_sql.py b/test/int/test_sql.py
index 0a6488006b5c1ef3c0194359baf1d4f1e2b95ce8..fb9403cc286c95fd34624bfb78a138c3805ada31 100644
--- a/test/int/test_sql.py
+++ b/test/int/test_sql.py
@@ -3950,6 +3950,113 @@ Alter access to user 'boba' is denied for user 'biba'\
assert boba in names_from_pico_user_table()
+def test_drop_user(cluster: Cluster):
+ i1, _ = cluster.deploy(instance_count=2)
+ user = "user"
+ password = "Passw0rd"
+
+ # Create and drop the created user
+ i1.sql(f""" create user "{user}" with password '{password}' """)
+ i1.sql(f""" drop user "{user}" """)
+
+ # Create user with privileges
+ i1.sql(f""" create user "{user}" with password '{password}' """)
+ i1.sql(f""" grant create table to "{user}" """, sudo=True)
+ i1.sql(f""" grant create procedure to "{user}" """, sudo=True)
+
+ # Drop user shouldn't fail despite the fact that the user has privileges
+ i1.sql(f""" drop user "{user}" """)
+
+ # Create user with privileges
+ i1.sql(f""" create user "{user}" with password '{password}' """)
+ i1.sql(f""" grant create table to "{user}" """, sudo=True)
+ i1.sql(f""" grant create procedure to "{user}" """, sudo=True)
+
+ # User creates a table
+ ddl = i1.sql(
+ """
+ create table t (a text not null, b text not null, c text, primary key (a))
+ using memtx
+ distributed by (a)
+ option (timeout = 3)
+ """,
+ user=user,
+ password=password,
+ )
+ assert ddl["row_count"] == 1
+
+ # User creates a procedure
+ data = i1.sql(
+ """
+ create procedure proc1(int)
+ language SQL
+ as $$insert into t values(?, ?, ?)$$
+ """,
+ user=user,
+ password=password,
+ )
+ assert data["row_count"] == 1
+
+ # Drop user should fail as the user owns tables and routines
+ with pytest.raises(
+ TarantoolError,
+ match=r"user cannot be dropped because some objects depend on it.*"
+ r"owner of tables t.*"
+ r"owner of procedures proc1.*",
+ ):
+ i1.sql(f""" drop user "{user}" """)
+
+ # Drop user objects
+ data = i1.sql("drop table t")
+ assert data["row_count"] == 1
+
+ data = i1.sql("drop procedure proc1")
+ assert data["row_count"] == 1
+
+ # Grant create user and create role
+ data = i1.sql(f'grant create user to "{user}"', sudo=True)
+ assert data["row_count"] == 1
+
+ data = i1.sql(f'grant create role to "{user}"', sudo=True)
+ assert data["row_count"] == 1
+
+ # User creates user
+ data = i1.sql(
+ "create user lol with password 'Passw0rd'",
+ user=user,
+ password=password,
+ )
+ assert data["row_count"] == 1
+
+ # User creates role
+ data = i1.sql(
+ "create role kek",
+ user=user,
+ password=password,
+ )
+ assert data["row_count"] == 1
+
+ # Drop user should fail as the user owns another user and role
+ with pytest.raises(
+ TarantoolError,
+ match=r"user cannot be dropped because some objects depend on it.*"
+ r"owner of users lol.*"
+ r"owner of roles kek.*",
+ ):
+ i1.sql(f""" drop user "{user}" """)
+
+ # Drop user objects
+ data = i1.sql("drop user lol")
+ assert data["row_count"] == 1
+
+ data = i1.sql("drop role kek")
+ assert data["row_count"] == 1
+
+ # Drop user with no objects
+ data = i1.sql(f'drop user "{user}"')
+ assert data["row_count"] == 1
+
+
def test_index(cluster: Cluster):
cluster.deploy(instance_count=1)
i1 = cluster.instances[0]