diff --git a/src/schema.rs b/src/schema.rs index f2955ec545814aac807e7b0225e82480143be5d8..d8bea3906626befc83e16db4da2ad08a8758c7d6 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -610,6 +610,31 @@ impl PrivilegeDef { // alter on himself || (self.object_type == SchemaObjectType::User && self.privilege == PrivilegeType::Alter && self.grantee_id as i64 == self.object_id) } + + /// Retrieves object_name from system spaces based on `object_id` and `object_type`. + /// Returns `Ok(None)` in the case when the privilege has no target object (e.g. `object_id == -1`) + /// or when target object is universe. + /// Returns `Err` in case when target object was not found in system tables. + /// + /// # Panics + /// 1. On storage failure + pub fn resolve_object_name(&self, storage: &Clusterwide) -> Result<Option<String>, Error> { + let Some(id) = self.object_id() else { + return Ok(None); + }; + let name = match self.object_type { + SchemaObjectType::Table => storage.tables.get(id).map(|t| t.map(|t| t.name)), + SchemaObjectType::Role => storage.roles.by_id(id).map(|t| t.map(|t| t.name)), + SchemaObjectType::User => storage.users.by_id(id).map(|t| t.map(|t| t.name)), + SchemaObjectType::Universe => { + debug_assert_eq!(self.object_id, 0); + return Ok(None); + } + } + .expect("storage should not fail") + .ok_or_else(|| Error::other(format!("object with id {id} should exist")))?; + Ok(Some(name)) + } } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/storage.rs b/src/storage.rs index 5930d03f2059a89d201849b4375c56d1e4f45256..08b26b5cdf63b6e1248ad71b2613658d9c0f0c1c 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -3024,8 +3024,12 @@ pub mod acl { ) -> tarantool::Result<()> { storage.privileges.insert(priv_def)?; - let privilege = priv_def.privilege(); - let (object, object_type) = (priv_def.object_id(), priv_def.object_type()); + let privilege = &priv_def.privilege(); + let object = priv_def + .resolve_object_name(storage) + .expect("target object should exist"); + let object_type = &priv_def.object_type(); + let (grantee_type, grantee) = priv_def.grantee_type_and_name(storage)?; let initiator_def = user_by_id(priv_def.grantor_id())?; @@ -3037,24 +3041,24 @@ pub mod acl { message: "granted role `{object}` to {grantee_type} `{grantee}`", title: "grant_role", severity: High, - role: object, + role: &object, grantee: &grantee, grantee_type: grantee_type, initiator: initiator_def.name, ); } _ => { - let object = match object { + let object_fmt = match &object { Some(object) => format!("`{object}` "), None => "".into(), }; crate::audit!( - message: "granted privilege {privilege} on {object_type} {object}\ + message: "granted privilege {privilege} on {object_type} {object_fmt}\ to {grantee_type} `{grantee}`", title: "grant_privilege", severity: High, privilege: privilege.as_str(), - object: priv_def.object_id(), + object: object, object_type: object_type.as_str(), grantee: &grantee, grantee_type: grantee_type, @@ -3080,8 +3084,11 @@ pub mod acl { &priv_def.privilege(), )?; - let privilege = priv_def.privilege(); - let (object, object_type) = (priv_def.object_id(), &priv_def.object_type()); + let privilege = &priv_def.privilege(); + let object = priv_def + .resolve_object_name(storage) + .expect("target object should exist"); + let object_type = &priv_def.object_type(); let (grantee_type, grantee) = priv_def.grantee_type_and_name(storage)?; let initiator_def = user_by_id(initiator)?; @@ -3093,24 +3100,24 @@ pub mod acl { message: "revoked role `{object}` from {grantee_type} `{grantee}`", title: "revoke_role", severity: High, - role: object, + role: &object, grantee: &grantee, grantee_type: grantee_type, initiator: initiator_def.name, ); } _ => { - let object = match object { + let object_fmt = match &object { Some(object) => format!("`{object}` "), None => "".into(), }; crate::audit!( - message: "revoked privilege {privilege} on {object_type} {object}\ + message: "revoked privilege {privilege} on {object_type} {object_fmt}\ from {grantee_type} `{grantee}`", title: "revoke_privilege", severity: High, privilege: privilege.as_str(), - object: priv_def.object_id(), + object: object, object_type: object_type.as_str(), grantee: &grantee, grantee_type: grantee_type, diff --git a/test/int/test_audit.py b/test/int/test_audit.py index 140ce02d8b9cd85fa96673a4623ca67091bb3cff..b799bc1deb7d00188c414d57cfa3f4d215d3208d 100644 --- a/test/int/test_audit.py +++ b/test/int/test_audit.py @@ -447,7 +447,7 @@ def test_role(instance: Instance): grant_role = take_until_type(events, EventGrantRole) assert grant_role is not None - # assert grant_role.role == "dummy" # FIXME: currently it's u32 + assert grant_role.role == "dummy" assert grant_role.grantee == "skibidi" assert grant_role.grantee_type == "role" assert ( @@ -459,7 +459,7 @@ def test_role(instance: Instance): revoke_role = take_until_type(events, EventRevokeRole) assert revoke_role is not None - # assert revoke_role.role == "dummy" # FIXME: currently it's u32 + assert revoke_role.role == "dummy" assert revoke_role.grantee == "skibidi" assert revoke_role.grantee_type == "role" assert (