From a98dc69194707ef54d462307c90989d82535f7c6 Mon Sep 17 00:00:00 2001
From: Egor Ivkov <e.ivkov@picodata.io>
Date: Mon, 4 Dec 2023 17:00:22 +0000
Subject: [PATCH] feat: track owner of tables, roles and users
This change allows for creators of corresponding objects to consequently have all privileges on them.
---
src/access_control.rs | 348 ++++++++++++++++++++++++++++++++-------
src/bootstrap_entries.rs | 4 +
src/cas.rs | 10 +-
src/luamod.lua | 13 +-
src/luamod.rs | 2 +-
src/schema.rs | 64 +++++--
src/sql.rs | 10 +-
src/storage.rs | 15 +-
src/traft/node.rs | 2 +
src/traft/op.rs | 1 +
test/int/test_acl.py | 28 +++-
test/int/test_basics.py | 8 +-
test/int/test_ddl.py | 17 +-
test/int/test_sql.py | 6 +-
14 files changed, 427 insertions(+), 101 deletions(-)
diff --git a/src/access_control.rs b/src/access_control.rs
index d101878bc4..0600f49fbc 100644
--- a/src/access_control.rs
+++ b/src/access_control.rs
@@ -120,6 +120,12 @@ fn user_by_id(id: UserId) -> tarantool::Result<Tuple> {
/// This wrapper is needed because usually before checking permissions we need to
/// retrieve some metadata about the object being tested for access and this requires
/// access to system spaces which original user is not required to have.
+///
+/// # Panicking
+///
+/// Note that not all combinations of parameters are valid.
+/// For in depth description of cases when this function may panic see
+/// [box_access_check_ddl](::tarantool::access_control::box_access_check_ddl) in tarantool module.
fn box_access_check_ddl_as_user(
object_name: &str,
object_id: u32,
@@ -142,14 +148,23 @@ fn access_check_dml(dml: &op::Dml, as_user: UserId) -> tarantool::Result<()> {
/// vanilla tarantool in on_replace_dd_space and on_replace_dd_index respectively
fn access_check_ddl(ddl: &op::Ddl, as_user: UserId) -> tarantool::Result<()> {
match ddl {
- op::Ddl::CreateTable { id, name, .. } => box_access_check_ddl_as_user(
- name,
- *id,
- as_user,
- TntSchemaObjectType::Space,
- PrivType::Create,
- as_user,
- ),
+ op::Ddl::CreateTable {
+ id, name, owner, ..
+ } => {
+ assert_eq!(
+ *owner, as_user,
+ "when creating objects creator is the owner"
+ );
+
+ box_access_check_ddl_as_user(
+ name,
+ *id,
+ *owner,
+ TntSchemaObjectType::Space,
+ PrivType::Create,
+ as_user,
+ )
+ }
op::Ddl::DropTable { id } => {
let space = space_by_id(*id)?;
let meta = space.meta()?;
@@ -223,7 +238,7 @@ fn access_check_grant_revoke(
match priv_def.object_type {
PicoSchemaObjectType::Universe => {
- // only admin can grant on universe
+ // Only admin can grant on universe
if grantor_id != ADMIN_USER_ID {
return Err(make_access_denied(
access_name,
@@ -234,12 +249,12 @@ fn access_check_grant_revoke(
}
}
PicoSchemaObjectType::Table => {
- // only owner or admin can grant on space
let space = space_by_id(object_id)?;
let meta = space.meta()?;
assert_eq!(object_id, meta.id, "user metadata id mismatch");
+ // Only owner or admin can grant on space
if meta.user_id != grantor_id && grantor_id != ADMIN_USER_ID {
return Err(make_access_denied(
access_name,
@@ -252,7 +267,7 @@ fn access_check_grant_revoke(
return box_access_check_ddl_as_user(
&meta.name,
meta.id,
- meta.user_id,
+ grantor_id,
TntSchemaObjectType::Space,
access,
grantor_id,
@@ -264,7 +279,7 @@ fn access_check_grant_revoke(
assert_eq!(object_id, sys_role_meta.id, "user metadata id mismatch");
- // Only the creator of the role can grant or revoke it.
+ // Only the creator of the role or admin can grant or revoke it.
// Everyone can grant 'PUBLIC' role.
// Note that having a role means having execute privilege on it.
if sys_role_meta.owner_id != grantor_id
@@ -285,7 +300,7 @@ fn access_check_grant_revoke(
return box_access_check_ddl_as_user(
&sys_role_meta.name,
sys_role_meta.id,
- sys_role_meta.owner_id,
+ grantor_id,
TntSchemaObjectType::Role,
access,
grantor_id,
@@ -298,6 +313,7 @@ fn access_check_grant_revoke(
return Err(make_no_such_user(&target_sys_user_meta.name));
}
+ // Only owner or admin can grant on user
if target_sys_user_meta.owner_id != grantor_id && grantor_id != ADMIN_USER_ID {
return Err(make_access_denied(
access_name,
@@ -310,7 +326,7 @@ fn access_check_grant_revoke(
return box_access_check_ddl_as_user(
&target_sys_user_meta.name,
target_sys_user_meta.id,
- target_sys_user_meta.owner_id,
+ grantor_id,
TntSchemaObjectType::User,
access,
grantor_id,
@@ -323,14 +339,20 @@ fn access_check_grant_revoke(
fn access_check_acl(acl: &op::Acl, as_user: UserId) -> tarantool::Result<()> {
match acl {
- op::Acl::CreateUser { user_def } => box_access_check_ddl_as_user(
- &user_def.name,
- user_def.id,
- as_user,
- TntSchemaObjectType::User,
- PrivType::Create,
- as_user,
- ),
+ op::Acl::CreateUser { user_def } => {
+ assert_eq!(
+ user_def.owner, as_user,
+ "when creating objects creator is the owner"
+ );
+ box_access_check_ddl_as_user(
+ &user_def.name,
+ user_def.id,
+ user_def.owner,
+ TntSchemaObjectType::User,
+ PrivType::Create,
+ as_user,
+ )
+ }
op::Acl::ChangeAuth { user_id, .. } => {
let sys_user = user_by_id(*user_id)?;
let sys_user_meta: UserMetadata = sys_user.decode()?;
@@ -361,14 +383,20 @@ fn access_check_acl(acl: &op::Acl, as_user: UserId) -> tarantool::Result<()> {
as_user,
)
}
- op::Acl::CreateRole { role_def } => box_access_check_ddl_as_user(
- &role_def.name,
- role_def.id,
- as_user,
- TntSchemaObjectType::Role,
- PrivType::Create,
- as_user,
- ),
+ op::Acl::CreateRole { role_def } => {
+ assert_eq!(
+ role_def.owner, as_user,
+ "when creating objects creator is the owner"
+ );
+ box_access_check_ddl_as_user(
+ &role_def.name,
+ role_def.id,
+ role_def.owner,
+ TntSchemaObjectType::Role,
+ PrivType::Create,
+ as_user,
+ )
+ }
op::Acl::DropRole { role_id, .. } => {
// In vanilla roles and users are stored in the same space
// so we can reuse the definition
@@ -422,19 +450,22 @@ mod tests {
use super::{access_check_acl, access_check_ddl, user_by_id, UserMetadata};
use crate::{
- access_control::UserMetadataKind,
- schema::{Distribution, PrivilegeDef, PrivilegeType, RoleDef, SchemaObjectType, UserDef},
+ access_control::{access_check_op, UserMetadataKind},
+ schema::{
+ Distribution, PrivilegeDef, PrivilegeType, RoleDef, SchemaObjectType, UserDef, ADMIN_ID,
+ },
storage::acl::{
on_master_create_role, on_master_create_user, on_master_grant_privilege,
on_master_revoke_privilege,
},
- traft::op::{Acl, Ddl},
+ traft::op::{Acl, Ddl, Dml, Op},
ADMIN_USER_ID,
};
use tarantool::{
auth::{AuthData, AuthDef, AuthMethod},
session::{self, UserId},
space::{Space, SpaceCreateOptions, SpaceEngineType},
+ tuple::{Tuple, TupleBuffer},
};
static mut NEXT_USER_ID: u32 = 42;
@@ -467,38 +498,49 @@ mod tests {
)
}
- fn dummy_user_def(id: UserId, name: String) -> UserDef {
+ fn dummy_user_def(id: UserId, name: String, owner: Option<UserId>) -> UserDef {
UserDef {
id,
name,
schema_version: 0,
auth: dummy_auth_def(),
+ owner: owner.unwrap_or_else(|| session::uid().unwrap()),
}
}
#[track_caller]
- fn make_user(name: &str) -> u32 {
+ fn make_user(name: &str, owner: Option<UserId>) -> u32 {
let id = next_user_id();
- let user_def = dummy_user_def(id, name.to_owned());
+ let user_def = dummy_user_def(id, name.to_owned(), owner);
on_master_create_user(&user_def).unwrap();
id
}
#[track_caller]
fn grant(
- grantee_id: UserId,
privilege: PrivilegeType,
object_type: SchemaObjectType,
object_id: i64,
+ grantee_id: UserId,
+ grantor_id: Option<UserId>,
) {
let priv_def = PrivilegeDef {
- grantor_id: session::uid().unwrap(),
+ grantor_id: grantor_id.unwrap_or(session::uid().unwrap()),
grantee_id,
object_type,
object_id,
privilege,
schema_version: 0,
};
+
+ access_check_op(
+ &Op::Acl(Acl::GrantPrivilege {
+ priv_def: priv_def.clone(),
+ }),
+ priv_def.grantor_id,
+ )
+ .unwrap();
+
on_master_grant_privilege(&priv_def).unwrap()
}
@@ -517,6 +559,14 @@ mod tests {
privilege,
schema_version: 0,
};
+
+ access_check_op(
+ &Op::Acl(Acl::RevokePrivilege {
+ priv_def: priv_def.clone(),
+ }),
+ priv_def.grantor_id,
+ )
+ .unwrap();
on_master_revoke_privilege(&priv_def).unwrap()
}
@@ -524,7 +574,7 @@ mod tests {
fn validate_access_check_ddl() {
let user_name = "box_access_check_space_test_user";
- let user_id = make_user(user_name);
+ let user_id = make_user(user_name, None);
// space
let space_name = "test_box_access_check_ddl";
@@ -539,6 +589,7 @@ mod tests {
primary_key: vec![],
distribution: Distribution::Global,
engine: SpaceEngineType::Blackhole,
+ owner: user_id,
};
let e = access_check_ddl(&space_to_be_created, user_id).unwrap_err();
@@ -548,7 +599,13 @@ mod tests {
format!("tarantool error: AccessDenied: Create access to space 'space_to_be_created' is denied for user '{user_name}'"),
);
- grant(user_id, PrivilegeType::Create, SchemaObjectType::Table, -1);
+ grant(
+ PrivilegeType::Create,
+ SchemaObjectType::Table,
+ -1,
+ user_id,
+ None,
+ );
access_check_ddl(&space_to_be_created, user_id).unwrap();
}
@@ -562,7 +619,13 @@ mod tests {
format!("tarantool error: AccessDenied: Drop access to space '{space_name}' is denied for user '{user_name}'"),
);
- grant(user_id, PrivilegeType::Drop, SchemaObjectType::Table, -1);
+ grant(
+ PrivilegeType::Drop,
+ SchemaObjectType::Table,
+ -1,
+ user_id,
+ None,
+ );
access_check_ddl(&Ddl::DropTable { id: space.id() }, user_id).unwrap();
}
@@ -579,21 +642,67 @@ mod tests {
);
grant(
- user_id,
PrivilegeType::Drop,
SchemaObjectType::Table,
space.id() as i64,
+ user_id,
+ None,
);
access_check_ddl(&Ddl::DropTable { id: space.id() }, user_id).unwrap();
}
- // TODO: logic with ownership doesnt work yet, because owner is not set correctly
+ // owner has privileges on the object
// owner can grant permissions on the object to other users
- grant(user_id, PrivilegeType::Create, SchemaObjectType::User, -1);
+ {
+ let grantee_user_name = format!("{user_name}_grantee");
+ let grantee_user_id = make_user(&grantee_user_name, None);
+
+ let space_name_grant = format!("{space_name}_grant");
+ let space_opts = SpaceCreateOptions {
+ user: Some(user_name.into()),
+ ..Default::default()
+ };
+ let space_grant = Space::create(&space_name_grant, &space_opts).unwrap();
- let grantee_user_name = format!("{user_name}_grantee");
- let _grantee_user_id = make_user(&grantee_user_name);
+ let drop_op = Op::DdlPrepare {
+ schema_version: 0,
+ ddl: Ddl::DropTable {
+ id: space_grant.id(),
+ },
+ };
+ let write_op = Op::Dml(Dml::Insert {
+ table: space_grant.id(),
+ tuple: TupleBuffer::from(Tuple::new(&(1,)).unwrap()),
+ });
+ for (privilege, privilege_name, op) in [
+ (PrivilegeType::Drop, "Drop", drop_op),
+ (PrivilegeType::Write, "Write", write_op),
+ ] {
+ // owner himself has permission on an object
+ access_check_op(&op, user_id).unwrap();
+
+ // run access check for another user, it fails without grant
+ let e = access_check_op(&op, grantee_user_id).unwrap_err();
+
+ assert_eq!(
+ e.to_string(),
+ format!("tarantool error: AccessDenied: {privilege_name} access to space '{space_name_grant}' is denied for user '{grantee_user_name}'"),
+ );
+
+ // grant permission on behalf of the user owning the space
+ grant(
+ privilege,
+ SchemaObjectType::Table,
+ space_grant.id() as _,
+ grantee_user_id,
+ Some(user_id),
+ );
+
+ // access check should succeed
+ access_check_op(&op, grantee_user_id).unwrap();
+ }
+ }
}
#[tarantool::test]
@@ -602,14 +711,18 @@ mod tests {
let actor_user_name = "box_access_check_ddl_test_user_actor";
let user_under_test_name = "box_access_check_ddl_test_user";
- let actor_user_id = make_user(actor_user_name);
- let user_under_test_id = make_user(user_under_test_name);
+ let actor_user_id = make_user(actor_user_name, None);
+ let user_under_test_id = make_user(user_under_test_name, None);
// create works with passed id
{
let e = access_check_acl(
&Acl::CreateUser {
- user_def: dummy_user_def(123, String::from("user_to_be_created")),
+ user_def: dummy_user_def(
+ 123,
+ String::from("user_to_be_created"),
+ Some(actor_user_id),
+ ),
},
actor_user_id,
)
@@ -621,15 +734,20 @@ mod tests {
);
grant(
- actor_user_id,
PrivilegeType::Create,
SchemaObjectType::User,
-1,
+ actor_user_id,
+ None,
);
access_check_acl(
&Acl::CreateUser {
- user_def: dummy_user_def(123, String::from("user_to_be_created")),
+ user_def: dummy_user_def(
+ 123,
+ String::from("user_to_be_created"),
+ Some(actor_user_id),
+ ),
},
actor_user_id,
)
@@ -653,10 +771,11 @@ mod tests {
);
grant(
- actor_user_id,
PrivilegeType::Drop,
SchemaObjectType::User,
-1,
+ actor_user_id,
+ None,
);
access_check_acl(
@@ -687,10 +806,11 @@ mod tests {
);
grant(
- actor_user_id,
PrivilegeType::Alter,
SchemaObjectType::User,
-1,
+ actor_user_id,
+ None,
);
access_check_acl(
@@ -734,10 +854,11 @@ mod tests {
);
grant(
- actor_user_id,
PrivilegeType::Drop,
SchemaObjectType::User,
user_under_test_id as i64,
+ actor_user_id,
+ None,
);
access_check_acl(
@@ -768,10 +889,11 @@ mod tests {
);
grant(
- actor_user_id,
PrivilegeType::Alter,
SchemaObjectType::User,
user_under_test_id as i64,
+ actor_user_id,
+ None,
);
access_check_acl(
@@ -784,19 +906,68 @@ mod tests {
)
.unwrap();
}
+
+ // owner has privileges on the object
+ // owner can grant permissions on the object to other users
+ {
+ let grantee_user_name = format!("{actor_user_name}_grantee");
+ let grantee_user_id = make_user(&grantee_user_name, None);
+
+ let user_name_grant = format!("{actor_user_name}_grant");
+ let user_id_grant = make_user(&user_name_grant, Some(actor_user_id));
+
+ let drop_op = Op::Acl(Acl::DropUser {
+ user_id: user_id_grant,
+ schema_version: 0,
+ });
+
+ let alter_op = Op::Acl(Acl::ChangeAuth {
+ user_id: user_id_grant,
+ auth: dummy_auth_def(),
+ schema_version: 0,
+ });
+ for (privilege, privilege_name, op) in [
+ (PrivilegeType::Drop, "Drop", drop_op),
+ (PrivilegeType::Alter, "Alter", alter_op),
+ ] {
+ // owner himself has permission on an object
+ access_check_op(&op, actor_user_id).unwrap();
+
+ // run access check for another user, it fails without grant
+ let e = access_check_op(&op, grantee_user_id).unwrap_err();
+
+ assert_eq!(
+ e.to_string(),
+ format!("tarantool error: AccessDenied: {privilege_name} access to user '{user_name_grant}' is denied for user '{grantee_user_name}'"),
+ );
+
+ // grant permission on behalf of the user owning the user
+ grant(
+ privilege,
+ SchemaObjectType::User,
+ user_id_grant as _,
+ grantee_user_id,
+ Some(actor_user_id),
+ );
+
+ // access check should succeed
+ access_check_op(&op, grantee_user_id).unwrap();
+ }
+ }
}
#[tarantool::test]
fn validate_access_check_acl_role() {
let user_name = "box_access_check_ddl_test_role";
- let user_id = make_user(user_name);
+ let user_id = make_user(user_name, None);
let role_name = "box_access_check_ddl_test_role_some_role";
let role_def = RoleDef {
id: next_user_id(),
name: String::from(role_name),
schema_version: 0,
+ owner: ADMIN_ID,
};
on_master_create_role(&role_def).expect("create role shouldnt fail");
@@ -806,6 +977,7 @@ mod tests {
id: 123,
name: String::from("role_to_be_created"),
schema_version: 0,
+ owner: user_id,
};
let e = access_check_acl(
@@ -821,7 +993,13 @@ mod tests {
format!("tarantool error: AccessDenied: Create access to role 'role_to_be_created' is denied for user '{user_name}'"),
);
- grant(user_id, PrivilegeType::Create, SchemaObjectType::Role, -1);
+ grant(
+ PrivilegeType::Create,
+ SchemaObjectType::Role,
+ -1,
+ user_id,
+ None,
+ );
access_check_acl(
&Acl::CreateRole {
@@ -848,7 +1026,13 @@ mod tests {
format!("tarantool error: AccessDenied: Drop access to role '{role_name}' is denied for user '{user_name}'"),
);
- grant(user_id, PrivilegeType::Drop, SchemaObjectType::Role, -1);
+ grant(
+ PrivilegeType::Drop,
+ SchemaObjectType::Role,
+ -1,
+ user_id,
+ None,
+ );
access_check_acl(
&Acl::DropRole {
@@ -879,10 +1063,11 @@ mod tests {
);
grant(
- user_id,
PrivilegeType::Drop,
SchemaObjectType::Role,
role_def.id as i64,
+ user_id,
+ None,
);
access_check_acl(
@@ -894,5 +1079,50 @@ mod tests {
)
.unwrap();
}
+
+ // owner has privileges on the object
+ // owner can grant permissions on the object to other users
+ {
+ let grantee_user_name = format!("{user_name}_grantee");
+ let grantee_user_id = make_user(&grantee_user_name, None);
+
+ let role_name_grant = format!("{role_name}_grant");
+ let role_id_grant = next_user_id();
+ let role_def = RoleDef {
+ id: role_id_grant,
+ name: role_name_grant.clone(),
+ schema_version: 0,
+ owner: user_id,
+ };
+ on_master_create_role(&role_def).expect("create role shouldn't fail");
+
+ let op = Op::Acl(Acl::DropRole {
+ role_id: role_id_grant,
+ schema_version: 0,
+ });
+
+ // owner himself has permission on an object
+ access_check_op(&op, user_id).unwrap();
+
+ // run access check for another user, it fails without grant
+ let e = access_check_op(&op, grantee_user_id).unwrap_err();
+
+ assert_eq!(
+ e.to_string(),
+ format!("tarantool error: AccessDenied: Drop access to role '{role_name_grant}' is denied for user '{grantee_user_name}'"),
+ );
+
+ // grant permission on behalf of the user owning the role
+ grant(
+ PrivilegeType::Drop,
+ SchemaObjectType::Role,
+ role_id_grant as _,
+ grantee_user_id,
+ Some(user_id),
+ );
+
+ // access check should succeed
+ access_check_op(&op, grantee_user_id).unwrap();
+ }
}
}
diff --git a/src/bootstrap_entries.rs b/src/bootstrap_entries.rs
index e858b4aa10..2b5602a70f 100644
--- a/src/bootstrap_entries.rs
+++ b/src/bootstrap_entries.rs
@@ -130,6 +130,7 @@ pub(super) fn prepare(args: &args::Run, instance: &Instance, tiers: &[Tier]) ->
AuthMethod::ChapSha1,
AuthData::new(&AuthMethod::ChapSha1, "guest", "").into_string(),
),
+ owner: ADMIN_ID,
},
));
@@ -145,6 +146,7 @@ pub(super) fn prepare(args: &args::Run, instance: &Instance, tiers: &[Tier]) ->
// we use ChapSha with invalid password
// (its impossible to get empty string as output of sha1)
auth: AuthDef::new(AuthMethod::ChapSha1, String::from("")),
+ owner: ADMIN_ID,
},
));
@@ -155,6 +157,7 @@ pub(super) fn prepare(args: &args::Run, instance: &Instance, tiers: &[Tier]) ->
id: PUBLIC_ID,
name: String::from("public"),
schema_version: 0,
+ owner: ADMIN_ID,
},
));
@@ -165,6 +168,7 @@ pub(super) fn prepare(args: &args::Run, instance: &Instance, tiers: &[Tier]) ->
id: SUPER_ID,
name: String::from("super"),
schema_version: 0,
+ owner: ADMIN_ID,
},
));
diff --git a/src/cas.rs b/src/cas.rs
index 0ceddf346b..2fa33bf723 100644
--- a/src/cas.rs
+++ b/src/cas.rs
@@ -140,7 +140,7 @@ fn proc_cas_local(req: Request) -> Result<Response> {
req.op,
Op::Acl(..) | Op::Dml(..) | Op::DdlPrepare { .. } | Op::DdlAbort
) {
- return Err(TraftError::Cas(Error::InvalidOpKind(req.op)));
+ return Err(TraftError::Cas(Error::InvalidOpKind(Box::new(req.op))));
}
let Predicate {
@@ -382,8 +382,10 @@ pub enum Error {
#[error("KeyTypeMismatch: failed comparing predicate ranges: {0}")]
KeyTypeMismatch(#[from] TntError),
+ /// NOTE: Boxing is caused by big difference in size compared to other variants
+ /// See <https://rust-lang.github.io/rust-clippy/master/index.html#/result_large_err> for more context
#[error("InvalidOpKind: Expected one of Acl, Dml, DdlPrepare or DdlAbort, got {0}")]
- InvalidOpKind(Op),
+ InvalidOpKind(Box<Op>),
}
/// Represents a lua table describing a [`Predicate`].
@@ -743,7 +745,7 @@ mod tests {
use tarantool::space::SpaceEngineType;
use tarantool::tuple::ToTupleBuffer;
- use crate::schema::{Distribution, TableDef};
+ use crate::schema::{Distribution, TableDef, ADMIN_ID};
use crate::storage::TClusterwideTable as _;
use crate::storage::{Clusterwide, Properties, PropertyName};
use crate::traft::op::DdlBuilder;
@@ -776,6 +778,7 @@ mod tests {
primary_key: vec![],
distribution: Distribution::Global,
engine: SpaceEngineType::Memtx,
+ owner: ADMIN_ID,
});
let drop_space = builder.with_op(Ddl::DropTable { id: space_id });
let create_index = builder.with_op(Ddl::CreateIndex {
@@ -813,6 +816,7 @@ mod tests {
format: vec![],
schema_version: 1,
engine: SpaceEngineType::Memtx,
+ owner: ADMIN_ID,
})
.unwrap();
assert!(t(&drop_space, Range::new(props).eq(&pending_schema_change)).is_err());
diff --git a/src/luamod.lua b/src/luamod.lua
index acdafc8a70..11bff3d272 100644
--- a/src/luamod.lua
+++ b/src/luamod.lua
@@ -232,6 +232,8 @@ Returns:
]]
function pico.create_user(user, password, opts)
local auth_type = box.cfg.auth_type
+ -- Set the creator/owner of the user being created to current user
+ local owner = box.session.euid()
local ok, err = pcall(function()
box.internal.check_param_table(opts, {
@@ -283,7 +285,8 @@ function pico.create_user(user, password, opts)
auth = {
method = auth_type,
data = auth_data,
- }
+ },
+ owner = owner
}
}
end
@@ -478,6 +481,9 @@ Returns:
(nil, error) in case of an error
]]
function pico.create_role(role, opts)
+ -- Set the creator/owner user of the role being created to current user
+ local owner = box.session.euid()
+
local ok, err = pcall(function()
box.internal.check_param(role, 'role', 'string')
box.internal.check_param_table(opts, { timeout = 'number' })
@@ -512,6 +518,7 @@ function pico.create_role(role, opts)
id = get_next_grantee_id(),
name = role,
schema_version = next_schema_version(),
+ owner = owner
}
}
end
@@ -1075,6 +1082,9 @@ See also:
[1]: https://www.tarantool.io/en/doc/latest/reference/reference_rock/vshard/vshard_router/
]]
function pico.create_table(opts)
+ -- Set the creator/owner user of the table being created to current user
+ opts.owner = box.session.euid()
+
local ok, err = pcall(function()
box.internal.check_param_table(opts, {
name = 'string',
@@ -1087,6 +1097,7 @@ function pico.create_table(opts)
sharding_fn = 'string',
engine = 'string',
timeout = 'number',
+ owner = 'number,'
})
mandatory_param(opts, 'opts')
mandatory_param(opts.name, 'opts.name')
diff --git a/src/luamod.rs b/src/luamod.rs
index af93531365..feaa7acf2e 100644
--- a/src/luamod.rs
+++ b/src/luamod.rs
@@ -1271,7 +1271,7 @@ pub(crate) fn setup(args: &args::Run) {
return Ok(None);
}
params.choose_id_if_not_specified()?;
- params.test_create_space()?;
+ params.test_create_space(storage)?;
let ddl = params.into_ddl()?;
let schema_version = storage.properties.next_schema_version()?;
let op = Op::DdlPrepare {
diff --git a/src/schema.rs b/src/schema.rs
index 600e2007aa..390f7a5e2c 100644
--- a/src/schema.rs
+++ b/src/schema.rs
@@ -25,7 +25,7 @@ use tarantool::{
use serde::{Deserialize, Serialize};
use crate::cas::{self, compare_and_swap};
-use crate::storage::SPACE_ID_INTERNAL_MAX;
+use crate::storage::{Clusterwide, SPACE_ID_INTERNAL_MAX};
use crate::storage::{ClusterwideTable, PropertyName};
use crate::traft::error::Error;
use crate::traft::op::{Ddl, Op};
@@ -48,6 +48,7 @@ pub struct TableDef {
pub schema_version: u64,
pub operable: bool,
pub engine: SpaceEngineType,
+ pub owner: UserId,
}
impl Encode for TableDef {}
@@ -68,6 +69,7 @@ impl TableDef {
Field::from(("schema_version", FieldType::Unsigned)),
Field::from(("operable", FieldType::Boolean)),
Field::from(("engine", FieldType::String)),
+ Field::from(("owner", FieldType::Unsigned)),
]
}
@@ -82,12 +84,11 @@ impl TableDef {
schema_version: 420,
operable: true,
engine: SpaceEngineType::Blackhole,
+ owner: 42,
}
}
pub fn to_space_metadata(&self) -> traft::Result<SpaceMetadata> {
- use tarantool::session::uid;
-
// FIXME: this is copy pasted from tarantool::schema::space::create_space
let format = self
.format
@@ -109,8 +110,7 @@ impl TableDef {
let space_def = SpaceMetadata {
id: self.id,
- // Do we want to be more explicit about user_id?
- user_id: uid()? as _,
+ user_id: self.owner,
name: self.name.as_str().into(),
engine: self.engine,
field_count: 0,
@@ -251,6 +251,7 @@ pub struct UserDef {
pub name: String,
pub schema_version: u64,
pub auth: AuthDef,
+ pub owner: UserId,
}
impl Encode for UserDef {}
@@ -270,6 +271,7 @@ impl UserDef {
Field::from(("name", FieldType::String)),
Field::from(("schema_version", FieldType::Unsigned)),
Field::from(("auth", FieldType::Array)),
+ Field::from(("owner", FieldType::Unsigned)),
]
}
@@ -281,6 +283,7 @@ impl UserDef {
name: "david".into(),
schema_version: 421,
auth: AuthDef::new(tarantool::auth::AuthMethod::ChapSha1, "".into()),
+ owner: 42,
}
}
}
@@ -295,6 +298,7 @@ pub struct RoleDef {
pub id: UserId,
pub name: String,
pub schema_version: u64,
+ pub owner: UserId,
}
impl Encode for RoleDef {}
@@ -308,6 +312,7 @@ impl RoleDef {
Field::from(("id", FieldType::Unsigned)),
Field::from(("name", FieldType::String)),
Field::from(("schema_version", FieldType::Unsigned)),
+ Field::from(("owner", FieldType::Unsigned)),
]
}
@@ -318,6 +323,7 @@ impl RoleDef {
id: 13,
name: "devops".into(),
schema_version: 419,
+ owner: 42,
}
}
}
@@ -619,6 +625,7 @@ pub struct CreateTableParams {
pub(crate) sharding_key: Option<Vec<String>>,
pub(crate) sharding_fn: Option<ShardingFn>,
pub(crate) engine: Option<SpaceEngineType>,
+ pub(crate) owner: UserId,
/// Timeout in seconds.
///
/// Specifying the timeout identifies how long user is ready to wait for ddl to be applied.
@@ -748,9 +755,15 @@ impl CreateTableParams {
/// Create space and then rollback.
///
/// Should be used for checking if a space with these params can be created.
- pub fn test_create_space(&self) -> traft::Result<()> {
+ pub fn test_create_space(&self, storage: &Clusterwide) -> traft::Result<()> {
let id = self.id.expect("space id should've been chosen by now");
+ let user = storage
+ .users
+ .by_id(self.owner)?
+ .ok_or_else(|| Error::Other(format!("user with id {} not found", self.owner).into()))?
+ .name;
let err = transaction(|| -> Result<(), Option<tarantool::error::Error>> {
+ // TODO: allow create_space to accept user by id
::tarantool::schema::space::create_space(
&self.name,
&SpaceCreateOptions {
@@ -758,7 +771,7 @@ impl CreateTableParams {
engine: self.engine.unwrap_or_default(),
id: Some(id),
field_count: self.format.len() as u32,
- user: None,
+ user: Some(user),
space_type: SpaceType::Normal,
format: Some(
self.format
@@ -866,6 +879,7 @@ impl CreateTableParams {
primary_key,
distribution,
engine: self.engine.unwrap_or_default(),
+ owner: self.owner,
};
Ok(res)
}
@@ -947,12 +961,28 @@ pub fn abort_ddl(timeout: Duration) -> traft::Result<RaftIndex> {
}
mod tests {
- use tarantool::space::FieldType;
+ use tarantool::{auth::AuthMethod, space::FieldType};
use super::*;
+ fn storage() -> Clusterwide {
+ let storage = Clusterwide::for_tests();
+ storage
+ .users
+ .insert(&UserDef {
+ id: ADMIN_ID,
+ name: String::from("admin"),
+ schema_version: 0,
+ auth: AuthDef::new(AuthMethod::ChapSha1, String::from("")),
+ owner: ADMIN_ID,
+ })
+ .unwrap();
+ storage
+ }
+
#[::tarantool::test]
fn test_create_space() {
+ let storage = storage();
CreateTableParams {
id: Some(1337),
name: "friends_of_peppa".into(),
@@ -975,8 +1005,9 @@ mod tests {
sharding_fn: None,
engine: None,
timeout: None,
+ owner: ADMIN_ID,
}
- .test_create_space()
+ .test_create_space(&storage)
.unwrap();
assert!(tarantool::space::Space::find("friends_of_peppa").is_none());
@@ -991,8 +1022,9 @@ mod tests {
sharding_fn: None,
engine: Some(SpaceEngineType::Vinyl),
timeout: None,
+ owner: ADMIN_ID,
}
- .test_create_space()
+ .test_create_space(&storage)
.unwrap();
assert!(tarantool::space::Space::find("friends_of_peppa").is_none());
@@ -1007,8 +1039,9 @@ mod tests {
sharding_fn: None,
engine: None,
timeout: None,
+ owner: ADMIN_ID,
}
- .test_create_space()
+ .test_create_space(&storage)
.unwrap_err();
assert_eq!(
err.to_string(),
@@ -1042,6 +1075,7 @@ mod tests {
sharding_fn: None,
engine: None,
timeout: None,
+ owner: ADMIN_ID,
}
.validate()
.unwrap_err();
@@ -1058,6 +1092,7 @@ mod tests {
sharding_fn: None,
engine: None,
timeout: None,
+ owner: ADMIN_ID,
}
.validate()
.unwrap_err();
@@ -1074,6 +1109,7 @@ mod tests {
sharding_fn: None,
engine: None,
timeout: None,
+ owner: ADMIN_ID,
}
.validate()
.unwrap_err();
@@ -1090,6 +1126,7 @@ mod tests {
sharding_fn: None,
engine: None,
timeout: None,
+ owner: ADMIN_ID,
}
.validate()
.unwrap_err();
@@ -1106,6 +1143,7 @@ mod tests {
sharding_fn: None,
engine: None,
timeout: None,
+ owner: ADMIN_ID,
}
.validate()
.unwrap_err();
@@ -1122,6 +1160,7 @@ mod tests {
sharding_fn: None,
engine: None,
timeout: None,
+ owner: ADMIN_ID,
}
.validate()
.unwrap_err();
@@ -1141,6 +1180,7 @@ mod tests {
sharding_fn: None,
engine: None,
timeout: None,
+ owner: ADMIN_ID,
}
.validate()
.unwrap_err();
@@ -1160,6 +1200,7 @@ mod tests {
sharding_fn: None,
engine: None,
timeout: None,
+ owner: ADMIN_ID,
}
.validate()
.unwrap();
@@ -1175,6 +1216,7 @@ mod tests {
sharding_fn: None,
engine: Some(SpaceEngineType::Vinyl),
timeout: None,
+ owner: ADMIN_ID,
}
.validate()
.unwrap_err();
diff --git a/src/sql.rs b/src/sql.rs
index 3a3321c269..173cffeca0 100644
--- a/src/sql.rs
+++ b/src/sql.rs
@@ -14,7 +14,7 @@ use crate::traft::error::Error;
use crate::traft::node::Node as TraftNode;
use crate::traft::op::{Acl as OpAcl, Ddl as OpDdl, Op};
use crate::traft::{self, node};
-use crate::util::duration_from_secs_f64_clamped;
+use crate::util::{duration_from_secs_f64_clamped, effective_user_id};
use crate::{cas, unwrap_ok_or, ADMIN_USER_ID};
use sbroad::backend::sql::ir::{EncodedPatternWithParams, PatternWithParams};
@@ -581,6 +581,8 @@ fn reenterable_schema_change_request(
ir_node: IrNode,
) -> traft::Result<ConsumerResult> {
let storage = &node.storage;
+ // Save current user as later user is switched to admin
+ let current_user = effective_user_id();
let timeout = match &ir_node {
IrNode::Ddl(ddl) => ddl.timeout()?,
@@ -626,6 +628,7 @@ fn reenterable_schema_change_request(
sharding_fn: Some(ShardingFn::Murmur3),
engine: Some(engine_type),
timeout: None,
+ owner: current_user,
};
params.validate()?;
Params::CreateTable(params)
@@ -730,7 +733,7 @@ fn reenterable_schema_change_request(
// painfull in rust.
let mut params = params.clone();
params.choose_id_if_not_specified()?;
- params.test_create_space()?;
+ params.test_create_space(storage)?;
let ddl = params.into_ddl()?;
Op::DdlPrepare {
// This field will be updated later.
@@ -770,6 +773,7 @@ fn reenterable_schema_change_request(
// This field will be updated later.
schema_version: 0,
auth: auth.clone(),
+ owner: current_user,
};
Op::Acl(OpAcl::CreateUser { user_def })
}
@@ -875,6 +879,7 @@ fn reenterable_schema_change_request(
name: name.clone(),
// This field will be updated later.
schema_version: 0,
+ owner: current_user,
};
Op::Acl(OpAcl::CreateRole { role_def })
}
@@ -991,6 +996,7 @@ fn reenterable_schema_change_request(
NoLogin,
}
+ // THOUGHT: should `owner_id` be part of `CreateUser`, `CreateRole` params?
enum Params {
CreateTable(CreateTableParams),
DropTable(String),
diff --git a/src/storage.rs b/src/storage.rs
index dfc299ae6f..55917c1f42 100644
--- a/src/storage.rs
+++ b/src/storage.rs
@@ -3002,16 +3002,23 @@ pub mod acl {
/// Create a tarantool user. Grant it default privileges.
pub fn on_master_create_user(user_def: &UserDef) -> tarantool::Result<()> {
let sys_user = Space::from(SystemSpace::User);
+ let user_id = user_def.id;
// This implementation was copied from box.schema.user.create excluding the
// password hashing.
- let user_id = user_def.id;
- let euid = ::tarantool::session::euid()?;
// Tarantool expects auth info to be a map of form `{ method: data }`,
// and currently the simplest way to achieve this is to use a HashMap.
let auth_map = HashMap::from([(user_def.auth.method, &user_def.auth.data)]);
- sys_user.insert(&(user_id, euid, &user_def.name, "user", auth_map, &[(); 0], 0))?;
+ sys_user.insert(&(
+ user_id,
+ user_def.owner,
+ &user_def.name,
+ "user",
+ auth_map,
+ &[(); 0],
+ 0,
+ ))?;
let lua = ::tarantool::lua_state();
lua.exec_with("box.schema.user.grant(...)", (user_id, "public"))
@@ -3071,7 +3078,7 @@ pub mod acl {
// way to achieve this is to use a HashMap.
sys_user.insert(&(
role_def.id,
- ::tarantool::session::euid()?,
+ role_def.owner,
&role_def.name,
"role",
HashMap::<(), ()>::new(),
diff --git a/src/traft/node.rs b/src/traft/node.rs
index 4f59fda226..a845747d1c 100644
--- a/src/traft/node.rs
+++ b/src/traft/node.rs
@@ -1039,6 +1039,7 @@ impl NodeImpl {
mut primary_key,
distribution,
engine,
+ owner,
} => {
use ::tarantool::util::NumOrStr::*;
@@ -1166,6 +1167,7 @@ impl NodeImpl {
format,
operable: false,
engine,
+ owner,
};
let res = self.storage.tables.insert(&space_def);
if let Err(e) = res {
diff --git a/src/traft/op.rs b/src/traft/op.rs
index 6472996f29..2f03e89ae0 100644
--- a/src/traft/op.rs
+++ b/src/traft/op.rs
@@ -477,6 +477,7 @@ pub enum Ddl {
primary_key: Vec<Part>,
distribution: Distribution,
engine: SpaceEngineType,
+ owner: UserId,
},
DropTable {
id: SpaceId,
diff --git a/test/int/test_acl.py b/test/int/test_acl.py
index 89515fe320..95fddd8c25 100644
--- a/test/int/test_acl.py
+++ b/test/int/test_acl.py
@@ -889,7 +889,7 @@ def test_builtin_users_and_roles(cluster: Cluster):
)
-def test_create_space_smoke(cluster: Cluster):
+def test_create_table_smoke(cluster: Cluster):
i1, *_ = cluster.deploy(instance_count=1)
index = i1.call("pico.create_user", "Dave", VALID_PASSWORD)
@@ -908,14 +908,24 @@ def test_create_space_smoke(cluster: Cluster):
i1.grant_privilege("Dave", "create", "table")
- ddl = i1.sql(
- """
- create table t (a int not null, primary key (a)) distributed by (a)
- """,
- user="Dave",
- password=VALID_PASSWORD,
- )
- assert ddl["row_count"] == 1
+ with i1.connect(timeout=1, user="Dave", password=VALID_PASSWORD) as conn:
+ ddl = conn.sql(
+ """
+ create table t (a int not null, primary key (a)) distributed by (a)
+ """,
+ )
+ assert ddl["row_count"] == 1
+
+ # Dave is the owner thus has all privileges on t
+ ret = conn.sql("insert into t values(1);")
+ assert ret["row_count"] == 1
+
+ ret = conn.sql("select * from t")
+
+ assert ret == {"metadata": [{"name": "A", "type": "integer"}], "rows": [[1]]}
+
+ ddl = conn.sql("drop table t")
+ assert ddl["row_count"] == 1
def test_grant_and_revoke_default_users_privileges(cluster: Cluster):
diff --git a/test/int/test_basics.py b/test/int/test_basics.py
index d84c7bb7f9..a95c3d7f96 100644
--- a/test/int/test_basics.py
+++ b/test/int/test_basics.py
@@ -251,10 +251,10 @@ def test_raft_log(instance: Instance):
| 9 | 1 |1.0.9|Insert({_pico_property}, ["max_pg_portals",50])|
| 10 | 1 |1.0.10|Insert({_pico_property}, ["snapshot_chunk_max_size",16777216])|
| 11 | 1 |1.0.11|Insert({_pico_property}, ["snapshot_read_view_close_timeout",86400.0])|
-| 12 | 1 |1.0.12|Insert({_pico_user}, [0,"guest",0,["chap-sha1","vhvewKp0tNyweZQ+cFKAlsyphfg="]])|
-| 13 | 1 |1.0.13|Insert({_pico_user}, [1,"admin",0,["chap-sha1",""]])|
-| 14 | 1 |1.0.14|Insert({_pico_role}, [2,"public",0])|
-| 15 | 1 |1.0.15|Insert({_pico_role}, [31,"super",0])|
+| 12 | 1 |1.0.12|Insert({_pico_user}, [0,"guest",0,["chap-sha1","vhvewKp0tNyweZQ+cFKAlsyphfg="],1])|
+| 13 | 1 |1.0.13|Insert({_pico_user}, [1,"admin",0,["chap-sha1",""],1])|
+| 14 | 1 |1.0.14|Insert({_pico_role}, [2,"public",0,1])|
+| 15 | 1 |1.0.15|Insert({_pico_role}, [31,"super",0,1])|
| 16 | 1 |1.0.16|Insert({_pico_privilege}, ["usage","universe",0,0,1,0])|
| 17 | 1 |1.0.17|Insert({_pico_privilege}, ["session","universe",0,0,1,0])|
| 18 | 1 |1.0.18|Insert({_pico_privilege}, ["execute","role",2,0,1,0])|
diff --git a/test/int/test_ddl.py b/test/int/test_ddl.py
index 1667fd1f3b..04610b9037 100644
--- a/test/int/test_ddl.py
+++ b/test/int/test_ddl.py
@@ -88,6 +88,7 @@ def test_ddl_lua_api(cluster: Cluster):
2,
True,
"memtx",
+ 0,
]
assert i1.call("box.space._pico_table:get", space_id) == pico_space_def
assert i2.call("box.space._pico_table:get", space_id) == pico_space_def
@@ -110,6 +111,7 @@ def test_ddl_lua_api(cluster: Cluster):
3,
True,
"memtx",
+ 0,
]
assert i1.call("box.space._pico_table:get", space_id) == pico_space_def
assert i2.call("box.space._pico_table:get", space_id) == pico_space_def
@@ -129,6 +131,7 @@ def test_ddl_lua_api(cluster: Cluster):
4,
True,
"vinyl",
+ 0,
]
cluster.create_table(
dict(
@@ -216,6 +219,7 @@ def test_ddl_create_table_bulky(cluster: Cluster):
primary_key=[dict(field="(this will cause an error)")],
distribution=dict(kind="global"),
engine="memtx",
+ owner=0,
),
)
@@ -280,6 +284,7 @@ def test_ddl_create_table_bulky(cluster: Cluster):
2,
True,
"memtx",
+ 0,
]
assert i1.call("box.space._pico_table:get", space_id) == pico_space_def
assert i2.call("box.space._pico_table:get", space_id) == pico_space_def
@@ -288,7 +293,7 @@ def test_ddl_create_table_bulky(cluster: Cluster):
tt_space_def = [
space_id,
- 1,
+ 0,
"stuff",
"memtx",
0,
@@ -395,13 +400,14 @@ def test_ddl_create_sharded_space(cluster: Cluster):
schema_version,
True,
"memtx",
+ 0,
]
assert i1.call("box.space._pico_table:get", space_id) == pico_space_def
assert i2.call("box.space._pico_table:get", space_id) == pico_space_def
tt_space_def = [
space_id,
- 1,
+ 0,
"stuff",
"memtx",
0,
@@ -488,6 +494,7 @@ def test_ddl_create_table_unfinished_from_snapshot(cluster: Cluster):
kind="sharded_implicitly", sharding_key=["id"], sharding_fn="murmur3"
),
engine="memtx",
+ owner=0,
),
wait_index=False,
)
@@ -551,6 +558,7 @@ def test_ddl_create_table_abort(cluster: Cluster):
sharding_fn="murmur3",
),
engine="memtx",
+ owner=0,
),
wait_index=False,
)
@@ -610,6 +618,7 @@ def test_ddl_create_table_partial_failure(cluster: Cluster):
primary_key=[dict(field="id")],
distribution=dict(kind="global"),
engine="memtx",
+ owner=0, # guest
)
index = i1.propose_create_space(space_def)
@@ -722,7 +731,7 @@ def test_ddl_create_table_from_snapshot_at_boot(cluster: Cluster):
tt_space_def = [
space_id,
- 1,
+ 0,
"stuff",
"memtx",
0,
@@ -805,7 +814,7 @@ def test_ddl_create_table_from_snapshot_at_catchup(cluster: Cluster):
tt_space_def = [
space_id,
- 1,
+ 0,
"stuff",
"memtx",
0,
diff --git a/test/int/test_sql.py b/test/int/test_sql.py
index 142c802618..960982c2b6 100644
--- a/test/int/test_sql.py
+++ b/test/int/test_sql.py
@@ -713,10 +713,10 @@ def test_sql_acl_users_roles(cluster: Cluster):
rolename = "Role"
upper_rolename = "ROLE"
default_users = [
- [0, "guest", 0, ["chap-sha1", "vhvewKp0tNyweZQ+cFKAlsyphfg="]],
- [1, "admin", 0, ["chap-sha1", ""]],
+ [0, "guest", 0, ["chap-sha1", "vhvewKp0tNyweZQ+cFKAlsyphfg="], 1],
+ [1, "admin", 0, ["chap-sha1", ""], 1],
]
- default_roles = [[2, "public", 0], [31, "super", 0]]
+ default_roles = [[2, "public", 0, 1], [31, "super", 0, 1]]
acl = i1.sql(
f"""
create user "{username}" with password '{password}'
--
GitLab