From 53113c7bdd70b7cec646cc74de8a5f32984db8b2 Mon Sep 17 00:00:00 2001
From: Egor Ivkov <e.o.ivkov@gmail.com>
Date: Fri, 8 Dec 2023 16:53:43 +0300
Subject: [PATCH] fix: log object_name in audit instead of object_id
---
src/schema.rs | 25 +++++++++++++++++++++++++
src/storage.rs | 31 +++++++++++++++++++------------
test/int/test_audit.py | 4 ++--
3 files changed, 46 insertions(+), 14 deletions(-)
diff --git a/src/schema.rs b/src/schema.rs
index f2955ec545..d8bea39066 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 5930d03f20..08b26b5cdf 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 140ce02d8b..b799bc1deb 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 (
--
GitLab