From 9a0fd4e491155ad1171f8b08120d016fe1d0dd6d Mon Sep 17 00:00:00 2001 From: HTHou Date: Wed, 29 Jul 2026 16:47:28 +0800 Subject: [PATCH] Fix table function audit operation type --- .../config/TableConfigTaskVisitor.java | 4 +- .../relational/security/AccessControl.java | 13 +++++ .../security/AccessControlImpl.java | 11 ++++ .../security/AllowAllAccessControl.java | 7 +++ .../security/ITableAuthChecker.java | 13 +++++ .../security/ITableAuthCheckerImpl.java | 38 +++++++++++++- .../event/PipeTsFileInsertionEventTest.java | 7 +++ .../plan/relational/analyzer/AuthTest.java | 51 +++++++++++++++++++ 8 files changed, 140 insertions(+), 4 deletions(-) diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/TableConfigTaskVisitor.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/TableConfigTaskVisitor.java index 820e4aa4075db..3e2eb129dc8d3 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/TableConfigTaskVisitor.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/TableConfigTaskVisitor.java @@ -1659,7 +1659,7 @@ public IConfigTask visitKillQuery(KillQuery node, MPPQueryContext context) { @Override public IConfigTask visitCreateFunction(CreateFunction node, MPPQueryContext context) { context.setQueryType(QueryType.OTHER); - accessControl.checkUserGlobalSysPrivilege(context); + accessControl.checkUserGlobalSysPrivilege(context, AuditLogOperation.DDL, node::getUdfName); if (node.getUriString().map(ExecutableManager::isUriTrusted).orElse(true)) { // 1. user specified uri and that uri is trusted // 2. user doesn't specify uri @@ -1679,7 +1679,7 @@ public IConfigTask visitShowFunctions(ShowFunctions node, MPPQueryContext contex @Override public IConfigTask visitDropFunction(DropFunction node, MPPQueryContext context) { context.setQueryType(QueryType.OTHER); - accessControl.checkUserGlobalSysPrivilege(context); + accessControl.checkUserGlobalSysPrivilege(context, AuditLogOperation.DDL, node::getUdfName); return new DropFunctionTask(Model.TABLE, node.getUdfName()); } diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControl.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControl.java index 95b1786c7348c..4ee69aa986ce7 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControl.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControl.java @@ -20,6 +20,7 @@ package org.apache.iotdb.db.queryengine.plan.relational.security; import org.apache.iotdb.common.rpc.thrift.TSStatus; +import org.apache.iotdb.commons.audit.AuditLogOperation; import org.apache.iotdb.commons.audit.IAuditEntity; import org.apache.iotdb.commons.auth.entity.PrivilegeType; import org.apache.iotdb.commons.exception.auth.AccessDeniedException; @@ -204,6 +205,18 @@ void checkUserCanRunRelationalAuthorStatement( */ void checkUserGlobalSysPrivilege(IAuditEntity auditEntity); + /** + * Check if user has global SYSTEM privilege and record the authentication audit log with the + * specified operation and object. + * + * @param auditEntity records necessary info for audit log + * @param auditLogOperation operation type of the statement being authorized + * @param auditObject object affected by the statement + * @throws AccessDeniedException if not allowed + */ + void checkUserGlobalSysPrivilege( + IAuditEntity auditEntity, AuditLogOperation auditLogOperation, Supplier auditObject); + /** * Check if user has sepecified global privilege * diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControlImpl.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControlImpl.java index 1a50f15e9cd49..9e7e987180797 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControlImpl.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControlImpl.java @@ -546,6 +546,17 @@ public void checkUserGlobalSysPrivilege(IAuditEntity auditEntity) { } } + @Override + public void checkUserGlobalSysPrivilege( + IAuditEntity auditEntity, AuditLogOperation auditLogOperation, Supplier auditObject) { + authChecker.checkGlobalPrivilege( + auditEntity.getUsername(), + TableModelPrivilege.SYSTEM, + auditLogOperation, + auditEntity, + auditObject); + } + @Override public boolean hasGlobalPrivilege(IAuditEntity entity, PrivilegeType privilegeType) { return AuthorityChecker.SUPER_USER_ID == entity.getUserId() diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AllowAllAccessControl.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AllowAllAccessControl.java index 4777d01fcc85c..890e54db85ccf 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AllowAllAccessControl.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AllowAllAccessControl.java @@ -20,6 +20,7 @@ package org.apache.iotdb.db.queryengine.plan.relational.security; import org.apache.iotdb.common.rpc.thrift.TSStatus; +import org.apache.iotdb.commons.audit.AuditLogOperation; import org.apache.iotdb.commons.audit.IAuditEntity; import org.apache.iotdb.commons.auth.entity.PrivilegeType; import org.apache.iotdb.commons.path.PartialPath; @@ -106,6 +107,12 @@ public void checkUserIsAdmin(IAuditEntity entity) { @Override public void checkUserGlobalSysPrivilege(IAuditEntity auditEntity) {} + @Override + public void checkUserGlobalSysPrivilege( + IAuditEntity auditEntity, + AuditLogOperation auditLogOperation, + Supplier auditObject) {} + @Override public boolean hasGlobalPrivilege(IAuditEntity entity, PrivilegeType privilegeType) { return true; diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/ITableAuthChecker.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/ITableAuthChecker.java index c3dfa7420bf1e..c20a2b4228aa0 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/ITableAuthChecker.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/ITableAuthChecker.java @@ -19,12 +19,14 @@ package org.apache.iotdb.db.queryengine.plan.relational.security; +import org.apache.iotdb.commons.audit.AuditLogOperation; import org.apache.iotdb.commons.audit.IAuditEntity; import org.apache.iotdb.commons.auth.entity.PrivilegeType; import org.apache.iotdb.commons.exception.auth.AccessDeniedException; import org.apache.iotdb.commons.queryengine.plan.relational.metadata.QualifiedObjectName; import java.util.Collection; +import java.util.function.Supplier; public interface ITableAuthChecker { @@ -108,6 +110,17 @@ void checkTableVisibility( void checkGlobalPrivilege( String userName, TableModelPrivilege privilege, IAuditEntity auditEntity); + /** + * Check if user has the specified global privilege and record the authentication audit log with + * the operation and object of the statement being authorized. + */ + void checkGlobalPrivilege( + String userName, + TableModelPrivilege privilege, + AuditLogOperation auditLogOperation, + IAuditEntity auditEntity, + Supplier auditObject); + void checkGlobalPrivileges( String username, Collection privileges, IAuditEntity auditEntity); diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/ITableAuthCheckerImpl.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/ITableAuthCheckerImpl.java index 7d10285b2a6bf..42579f1830dfd 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/ITableAuthCheckerImpl.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/ITableAuthCheckerImpl.java @@ -399,6 +399,30 @@ public void checkGlobalPrivilege( recordAuditLogViaAuthenticationResult(() -> userName, privilege, auditEntity, result); } + @Override + public void checkGlobalPrivilege( + String userName, + TableModelPrivilege privilege, + AuditLogOperation auditLogOperation, + IAuditEntity auditEntity, + Supplier auditObject) { + if (AuthorityChecker.SUPER_USER_ID == auditEntity.getUserId()) { + AUDIT_LOGGER.recordObjectAuthenticationAuditLog( + auditEntity + .setAuditLogOperation(auditLogOperation) + .setPrivilegeType(privilege.getPrivilegeType()) + .setResult(true), + auditObject); + return; + } + TSStatus result = + AuthorityChecker.getTSStatus( + AuthorityChecker.checkSystemPermission(userName, privilege.getPrivilegeType()), + privilege.getPrivilegeType()); + recordAuditLogViaAuthenticationResult( + auditObject, privilege, auditLogOperation, auditEntity, result); + } + @Override public void checkGlobalPrivileges( String username, Collection privileges, IAuditEntity auditEntity) { @@ -468,10 +492,20 @@ private void recordAuditLogViaAuthenticationResult( TableModelPrivilege privilege, IAuditEntity auditEntity, TSStatus result) { + recordAuditLogViaAuthenticationResult( + auditObject, privilege, privilege.getAuditLogOperation(), auditEntity, result); + } + + private void recordAuditLogViaAuthenticationResult( + Supplier auditObject, + TableModelPrivilege privilege, + AuditLogOperation auditLogOperation, + IAuditEntity auditEntity, + TSStatus result) { if (result.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { AUDIT_LOGGER.recordObjectAuthenticationAuditLog( auditEntity - .setAuditLogOperation(privilege.getAuditLogOperation()) + .setAuditLogOperation(auditLogOperation) .setPrivilegeType(privilege.getPrivilegeType()) .setResult(false), auditObject); @@ -479,7 +513,7 @@ private void recordAuditLogViaAuthenticationResult( } AUDIT_LOGGER.recordObjectAuthenticationAuditLog( auditEntity - .setAuditLogOperation(privilege.getAuditLogOperation()) + .setAuditLogOperation(auditLogOperation) .setPrivilegeType(privilege.getPrivilegeType()) .setResult(true), auditObject); diff --git a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/pipe/event/PipeTsFileInsertionEventTest.java b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/pipe/event/PipeTsFileInsertionEventTest.java index 41cbf4358e539..5d5be1dbadee9 100644 --- a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/pipe/event/PipeTsFileInsertionEventTest.java +++ b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/pipe/event/PipeTsFileInsertionEventTest.java @@ -20,6 +20,7 @@ package org.apache.iotdb.db.pipe.event; import org.apache.iotdb.common.rpc.thrift.TSStatus; +import org.apache.iotdb.commons.audit.AuditLogOperation; import org.apache.iotdb.commons.audit.IAuditEntity; import org.apache.iotdb.commons.auth.entity.PrivilegeType; import org.apache.iotdb.commons.conf.IoTDBConstant; @@ -374,6 +375,12 @@ public void checkUserIsAdmin(IAuditEntity auditEntity) {} @Override public void checkUserGlobalSysPrivilege(IAuditEntity auditEntity) {} + @Override + public void checkUserGlobalSysPrivilege( + IAuditEntity auditEntity, + AuditLogOperation auditLogOperation, + Supplier auditObject) {} + @Override public boolean hasGlobalPrivilege(IAuditEntity auditEntity, PrivilegeType privilegeType) { return false; diff --git a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/AuthTest.java b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/AuthTest.java index e758e49962827..9aaada25c834e 100644 --- a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/AuthTest.java +++ b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/AuthTest.java @@ -19,6 +19,9 @@ package org.apache.iotdb.db.queryengine.plan.relational.analyzer; +import org.apache.iotdb.commons.audit.AuditLogOperation; +import org.apache.iotdb.commons.audit.UserEntity; +import org.apache.iotdb.commons.auth.entity.PrivilegeType; import org.apache.iotdb.commons.exception.auth.AccessDeniedException; import org.apache.iotdb.commons.queryengine.common.SessionInfo; import org.apache.iotdb.commons.queryengine.common.SqlDialect; @@ -30,6 +33,7 @@ import org.apache.iotdb.db.queryengine.plan.execution.config.TableConfigTaskVisitor; import org.apache.iotdb.db.queryengine.plan.relational.security.AccessControlImpl; import org.apache.iotdb.db.queryengine.plan.relational.security.ITableAuthChecker; +import org.apache.iotdb.db.queryengine.plan.relational.security.ITableAuthCheckerImpl; import org.apache.iotdb.db.queryengine.plan.relational.security.TableModelPrivilege; import org.apache.iotdb.db.queryengine.plan.relational.security.TreeAccessCheckVisitor; import org.apache.iotdb.db.queryengine.plan.relational.sql.parser.SqlParser; @@ -47,9 +51,13 @@ import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.TestUtils.QUERY_ID; import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.TestUtils.TEST_MATADATA; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; public class AuthTest { @@ -206,6 +214,49 @@ public void testDatabaseManagementRelatedAuth() { } + @Test + public void testFunctionManagementAuditOperation() { + ITableAuthChecker authChecker = Mockito.mock(ITableAuthChecker.class); + String functionName = "test_function"; + + analyzeConfigTask( + String.format( + "CREATE FUNCTION %s AS 'org.apache.iotdb.db.query.udf.example.relational.AllSum'", + functionName), + user1, + authChecker); + analyzeConfigTask(String.format("DROP FUNCTION %s", functionName), user1, authChecker); + + verify(authChecker, times(2)) + .checkGlobalPrivilege( + eq(user1), + eq(TableModelPrivilege.SYSTEM), + eq(AuditLogOperation.DDL), + any(), + argThat(auditObject -> functionName.equals(auditObject.get()))); + } + + @Test + public void testExplicitGlobalPrivilegeAuditOperation() { + ITableAuthCheckerImpl authChecker = new ITableAuthCheckerImpl(); + UserEntity auditEntity = new UserEntity(0, userRoot, "127.0.0.1"); + + authChecker.checkGlobalPrivilege( + userRoot, + TableModelPrivilege.SYSTEM, + AuditLogOperation.DDL, + auditEntity, + () -> "test_function"); + + assertEquals(AuditLogOperation.DDL, auditEntity.getAuditLogOperation()); + assertEquals(Collections.singletonList(PrivilegeType.SYSTEM), auditEntity.getPrivilegeTypes()); + assertTrue(auditEntity.getResult()); + + UserEntity defaultAuditEntity = new UserEntity(0, userRoot, "127.0.0.1"); + authChecker.checkGlobalPrivilege(userRoot, TableModelPrivilege.SYSTEM, defaultAuditEntity); + assertEquals(AuditLogOperation.CONTROL, defaultAuditEntity.getAuditLogOperation()); + } + private void analyzeSQL(String sql, String userName, ITableAuthChecker authChecker) { analyzeSQL(sql, userName, authChecker, null); }