Patch for AUDIT privilege authentication on table model (apache#16563)

CRZbulabula · web-flow · commit 18964f41cdad · 2025-10-12T21:05:49.000+08:00
diff --git a/integration-test/src/test/java/org/apache/iotdb/db/it/audit/IoTDBAuditLogBasicIT.java b/integration-test/src/test/java/org/apache/iotdb/db/it/audit/IoTDBAuditLogBasicIT.java
@@ -56,9 +56,9 @@
 @Category({LocalStandaloneIT.class})
 public class IoTDBAuditLogBasicIT {
 
-  private static final long ENSURE_AUDIT_LOG_SLEEP_IN_MS = 250;
+  public static final long ENSURE_AUDIT_LOG_SLEEP_IN_MS = 250;
 
-  private static final List<String> AUDIT_TABLE_COLUMNS =
+  public static final List<String> AUDIT_TABLE_COLUMNS =
       Arrays.asList(
           AbstractAuditLogger.AUDIT_LOG_NODE_ID,
           AbstractAuditLogger.AUDIT_LOG_USER_ID,
@@ -73,28 +73,28 @@ public class IoTDBAuditLogBasicIT {
           AbstractAuditLogger.AUDIT_LOG_SQL_STRING,
           AbstractAuditLogger.AUDIT_LOG_LOG);
 
-  private static final List<String> AUDIT_TABLE_DATA_TYPES =
+  public static final List<String> AUDIT_TABLE_DATA_TYPES =
       Arrays.asList(
           "STRING", "STRING", "STRING", "STRING", "STRING", "STRING", "STRING", "STRING", "BOOLEAN",
           "STRING", "STRING", "STRING");
 
-  private static final List<String> AUDIT_TABLE_CATEGORIES =
+  public static final List<String> AUDIT_TABLE_CATEGORIES =
       Arrays.asList(
           "TAG", "TAG", "FIELD", "FIELD", "FIELD", "FIELD", "FIELD", "FIELD", "FIELD", "FIELD",
           "FIELD", "FIELD");
 
-  private static final boolean ENABLE_AUDIT_LOG = true;
-  private static final String AUDITABLE_OPERATION_TYPE =
+  public static final boolean ENABLE_AUDIT_LOG = true;
+  public static final String AUDITABLE_OPERATION_TYPE =
       new StringJoiner(",")
           .add(AuditLogOperation.DDL.toString())
           .add(AuditLogOperation.DML.toString())
           .add(AuditLogOperation.QUERY.toString())
           .add(AuditLogOperation.CONTROL.toString())
           .toString();
 
-  private static final String AUDITABLE_OPERATION_LEVEL = PrivilegeLevel.GLOBAL.toString();
+  public static final String AUDITABLE_OPERATION_LEVEL = PrivilegeLevel.GLOBAL.toString();
 
-  private static final String AUDITABLE_OPERATION_RESULT = "SUCCESS,FAIL";
+  public static final String AUDITABLE_OPERATION_RESULT = "SUCCESS,FAIL";
 
   @Before
   public void setUp() throws SQLException, InterruptedException {
@@ -135,7 +135,7 @@ public void tearDown() {
     EnvFactory.getEnv().cleanClusterEnvironment();
   }
 
-  private static void closeConnectionCompletely(Connection connection) throws InterruptedException {
+  public static void closeConnectionCompletely(Connection connection) throws InterruptedException {
     // Ensure the session conns in test env are closed completely,
     // in order to generate logout audit log
     // TODO: Optimize this func after the close func of connection is optimized
@@ -2378,15 +2378,13 @@ public void basicAuditLogTestForTreeModel() throws SQLException, InterruptedExce
     Statement statement = connection.createStatement();
     for (String sql : TREE_MODEL_AUDIT_SQLS_USER_ROOT) {
       statement.execute(sql);
-      TimeUnit.MILLISECONDS.sleep(ENSURE_AUDIT_LOG_SLEEP_IN_MS);
     }
     closeConnectionCompletely(connection);
     connection =
         EnvFactory.getEnv().getConnection("user1", "IoTDB@2025abc", BaseEnv.TREE_SQL_DIALECT);
     statement = connection.createStatement();
     for (String sql : TREE_MODEL_AUDIT_SQLS_USER_USER1) {
       statement.execute(sql);
-      TimeUnit.MILLISECONDS.sleep(ENSURE_AUDIT_LOG_SLEEP_IN_MS);
     }
     closeConnectionCompletely(connection);
     connection =
@@ -2395,7 +2393,6 @@ public void basicAuditLogTestForTreeModel() throws SQLException, InterruptedExce
     for (String sql : TREE_MODEL_AUDIT_SQLS_USER_USER2) {
       try {
         statement.execute(sql);
-        TimeUnit.MILLISECONDS.sleep(ENSURE_AUDIT_LOG_SLEEP_IN_MS);
       } catch (SQLException e) {
         // Ignore, only record audit log
       }
@@ -2405,7 +2402,6 @@ public void basicAuditLogTestForTreeModel() throws SQLException, InterruptedExce
     statement = connection.createStatement();
     for (String sql : TREE_MODEL_AUDIT_SQLS_USER_ROOT_FINAL) {
       statement.execute(sql);
-      TimeUnit.MILLISECONDS.sleep(ENSURE_AUDIT_LOG_SLEEP_IN_MS);
     }
     ResultSet resultSet =
         statement.executeQuery("SELECT * FROM root.__audit.log.** ORDER BY TIME ALIGN BY DEVICE");
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/DNAuditLogger.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/DNAuditLogger.java
@@ -86,9 +86,10 @@ public class DNAuditLogger extends AbstractAuditLogger {
   public static final String PREFIX_PASSWORD_HISTORY = "root.__audit.password_history";
   private static final Logger logger = LoggerFactory.getLogger(DNAuditLogger.class);
 
-  // TODO: @zhujt20 Optimize the following stupid retry
+  // TODO: @zhujt20 Optimize the following stupid intervals
   private static final int INSERT_RETRY_COUNT = 5;
   private static final int INSERT_RETRY_INTERVAL_MS = 2000;
+  private static final int INSERT_INTERVAL_MS = 50;
 
   private static final IoTDBConfig config = IoTDBDescriptor.getInstance().getConfig();
 
@@ -354,49 +355,46 @@ public void createViewIfNecessary() {
   }
 
   @Override
-  public void log(IAuditEntity auditLogFields, Supplier<String> log) {
+  public synchronized void log(IAuditEntity auditLogFields, Supplier<String> log) {
     if (!IS_AUDIT_LOG_ENABLED) {
       return;
     }
-    createViewIfNecessary();
-    if (noNeedInsertAuditLog(auditLogFields)) {
-      return;
-    }
-    long userId = auditLogFields.getUserId();
-    String user = String.valueOf(userId);
-    if (userId == -1) {
-      user = "none";
-    }
-    String dataNodeId = String.valueOf(config.getDataNodeId());
-    InsertRowStatement statement;
     try {
-      statement =
+      createViewIfNecessary();
+      if (noNeedInsertAuditLog(auditLogFields)) {
+        return;
+      }
+      long userId = auditLogFields.getUserId();
+      String user = String.valueOf(userId);
+      if (userId == -1) {
+        user = "none";
+      }
+      String dataNodeId = String.valueOf(config.getDataNodeId());
+      InsertRowStatement statement =
           generateInsertStatement(
               auditLogFields,
               log.get(),
               DEVICE_PATH_CACHE.getPartialPath(String.format(AUDIT_LOG_DEVICE, dataNodeId, user)));
-    } catch (IllegalPathException e) {
-      logger.error("Failed to log audit events because ", e);
-      return;
-    }
-    for (int retry = 0; retry < INSERT_RETRY_COUNT; retry++) {
-      ExecutionResult insertResult =
-          coordinator.executeForTreeModel(
-              statement,
-              SESSION_MANAGER.requestQueryId(),
-              sessionInfo,
-              "",
-              ClusterPartitionFetcher.getInstance(),
-              SCHEMA_FETCHER);
-      if (insertResult.status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
-        return;
-      }
-      try {
+      for (int retry = 0; retry < INSERT_RETRY_COUNT; retry++) {
+        ExecutionResult insertResult =
+            coordinator.executeForTreeModel(
+                statement,
+                SESSION_MANAGER.requestQueryId(),
+                sessionInfo,
+                "",
+                ClusterPartitionFetcher.getInstance(),
+                SCHEMA_FETCHER);
+        if (insertResult.status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+          TimeUnit.MILLISECONDS.sleep(INSERT_INTERVAL_MS);
+          return;
+        }
         TimeUnit.MILLISECONDS.sleep(INSERT_RETRY_INTERVAL_MS);
-      } catch (InterruptedException e) {
-        Thread.currentThread().interrupt();
-        logger.error("Audit log insertion retry sleep was interrupted", e);
       }
+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+      logger.warn("[AUDIT] Audit log insertion retry sleep was interrupted because", e);
+    } catch (Exception e) {
+      logger.warn("[AUDIT] Failed to log audit events because", e);
     }
     AuditEventType type = auditLogFields.getAuditEventType();
     if (isLoginEvent(type)) {
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
@@ -66,23 +66,19 @@ public void checkDatabaseVisibility(
     }
 
     if (TABLE_MODEL_AUDIT_DATABASE.equalsIgnoreCase(databaseName)) {
-      if (AuthorityChecker.checkSystemPermission(userName, PrivilegeType.AUDIT)) {
-        recordAuditLog(
-            auditEntity
-                .setAuditLogOperation(AuditLogOperation.QUERY)
-                .setPrivilegeType(PrivilegeType.READ_SCHEMA)
-                .setResult(true),
-            () -> databaseName);
+      // The audit database only requires audit privilege
+      boolean hasAuditPrivilege =
+          AuthorityChecker.checkSystemPermission(userName, PrivilegeType.AUDIT);
+      recordAuditLog(
+          auditEntity
+              .setAuditLogOperation(AuditLogOperation.QUERY)
+              .setPrivilegeType(PrivilegeType.AUDIT)
+              .setResult(hasAuditPrivilege),
+          () -> databaseName);
+      if (hasAuditPrivilege) {
         return;
-      } else {
-        recordAuditLog(
-            auditEntity
-                .setAuditLogOperation(AuditLogOperation.QUERY)
-                .setPrivilegeType(PrivilegeType.READ_SCHEMA)
-                .setResult(false),
-            () -> databaseName);
-        throw new AccessDeniedException("DATABASE " + databaseName);
       }
+      throw new AccessDeniedException("DATABASE " + databaseName);
     }
 
     if (AuthorityChecker.checkSystemPermission(userName, PrivilegeType.SYSTEM)) {
@@ -341,14 +337,19 @@ public void checkTableVisibility(
     }
 
     String databaseName = tableName.getDatabaseName();
-    if (TABLE_MODEL_AUDIT_DATABASE.equalsIgnoreCase(databaseName)
-        && !AuthorityChecker.checkSystemPermission(userName, PrivilegeType.AUDIT)) {
+    if (TABLE_MODEL_AUDIT_DATABASE.equalsIgnoreCase(databaseName)) {
+      // The audit table only requires audit privilege
+      boolean hasAuditPrivilege =
+          AuthorityChecker.checkSystemPermission(userName, PrivilegeType.AUDIT);
       recordAuditLog(
           auditEntity
               .setAuditLogOperation(AuditLogOperation.QUERY)
-              .setPrivilegeType(PrivilegeType.READ_SCHEMA)
-              .setResult(false),
+              .setPrivilegeType(PrivilegeType.AUDIT)
+              .setResult(hasAuditPrivilege),
           tableName::getObjectName);
+      if (hasAuditPrivilege) {
+        return;
+      }
       throw new AccessDeniedException("TABLE " + tableName);
     }
 
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TreeAccessCheckVisitor.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TreeAccessCheckVisitor.java
@@ -1900,7 +1900,7 @@ private TSStatus checkOnlySuperUser(
     return AuthorityChecker.getTSStatus(false, "Only the admin user can perform this operation");
   }
 
-  private static void recordObjectAuthenticationAuditLog(
+  protected static void recordObjectAuthenticationAuditLog(
       IAuditEntity auditEntity, Supplier<String> auditObject) {
     AUDIT_LOGGER.log(
         auditEntity.setAuditEventType(AuditEventType.OBJECT_AUTHENTICATION),

Original file line number	Diff line number	Diff line change
`@@ -1900,7 +1900,7 @@ private TSStatus checkOnlySuperUser(`
`1900`	`1900`	`return AuthorityChecker.getTSStatus(false, "Only the admin user can perform this operation");`
`1901`	`1901`	`}`
`1902`	`1902`
`1903`		`- private static void recordObjectAuthenticationAuditLog(`
	`1903`	`+ protected static void recordObjectAuthenticationAuditLog(`
`1904`	`1904`	`IAuditEntity auditEntity, Supplier<String> auditObject) {`
`1905`	`1905`	`AUDIT_LOGGER.log(`
`1906`	`1906`	`auditEntity.setAuditEventType(AuditEventType.OBJECT_AUTHENTICATION),`