From efd5d5a4b7112a440760a1083aa8da7902cb4400 Mon Sep 17 00:00:00 2001 From: libo Date: Wed, 29 Jul 2026 18:24:04 +0800 Subject: [PATCH 1/3] Record audit logs for failed role grants --- .../audit/GrantRoleFailureAuditContext.java | 91 +++++++++++ .../thrift/impl/ClientRPCServiceImpl.java | 45 +++++- .../GrantRoleFailureAuditContextTest.java | 149 ++++++++++++++++++ 3 files changed, 283 insertions(+), 2 deletions(-) create mode 100644 iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContext.java create mode 100644 iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContextTest.java diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContext.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContext.java new file mode 100644 index 0000000000000..70b6dc299a174 --- /dev/null +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContext.java @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iotdb.db.audit; + +import org.apache.iotdb.common.rpc.thrift.TSStatus; +import org.apache.iotdb.commons.audit.AuditEventType; +import org.apache.iotdb.commons.audit.AuditLogFields; +import org.apache.iotdb.commons.audit.AuditLogOperation; +import org.apache.iotdb.commons.auth.entity.PrivilegeType; +import org.apache.iotdb.db.protocol.session.IClientSession; +import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.RelationalAuthorStatement; +import org.apache.iotdb.db.queryengine.plan.relational.type.AuthorRType; +import org.apache.iotdb.db.queryengine.plan.statement.AuthorType; +import org.apache.iotdb.db.queryengine.plan.statement.sys.AuthorStatement; +import org.apache.iotdb.rpc.TSStatusCode; + +/** Tracks one role-grant statement and records an audit event only when it fails. */ +public class GrantRoleFailureAuditContext { + + private final DNAuditLogger auditLogger; + private final String sqlString; + + private IClientSession clientSession; + private String targetUsername; + + public GrantRoleFailureAuditContext(String sqlString) { + this(DNAuditLogger.getInstance(), sqlString); + } + + GrantRoleFailureAuditContext(DNAuditLogger auditLogger, String sqlString) { + this.auditLogger = auditLogger; + this.sqlString = sqlString; + } + + public void setClientSession(IClientSession clientSession) { + this.clientSession = clientSession; + } + + public void track(AuthorStatement statement) { + if (statement.getAuthorType() == AuthorType.GRANT_USER_ROLE) { + targetUsername = statement.getUserName(); + } + } + + public void track(RelationalAuthorStatement statement) { + if (statement.getAuthorType() == AuthorRType.GRANT_USER_ROLE) { + targetUsername = statement.getUserName(); + } + } + + public void log(TSStatus status) { + if (targetUsername == null || clientSession == null || isSuccessfulOrRedirected(status)) { + return; + } + auditLogger.log( + new AuditLogFields( + clientSession.getUserId(), + clientSession.getUsername(), + clientSession.getClientAddress(), + AuditEventType.GRANT_ROLE_FAILED, + AuditLogOperation.CONTROL, + PrivilegeType.SECURITY, + false, + clientSession.getDatabaseName(), + sqlString), + () -> targetUsername); + } + + private static boolean isSuccessfulOrRedirected(TSStatus status) { + return status != null + && (status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode() + || status.getCode() == TSStatusCode.REDIRECTION_RECOMMEND.getStatusCode()); + } +} diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/ClientRPCServiceImpl.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/ClientRPCServiceImpl.java index ea67ebb8523df..6bf61ee98b836 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/ClientRPCServiceImpl.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/ClientRPCServiceImpl.java @@ -59,6 +59,7 @@ import org.apache.iotdb.commons.queryengine.plan.relational.sql.parser.ParsingException; import org.apache.iotdb.commons.utils.PathUtils; import org.apache.iotdb.db.audit.DNAuditLogger; +import org.apache.iotdb.db.audit.GrantRoleFailureAuditContext; import org.apache.iotdb.db.auth.AuthorityChecker; import org.apache.iotdb.db.conf.IoTDBConfig; import org.apache.iotdb.db.conf.IoTDBDescriptor; @@ -105,6 +106,7 @@ import org.apache.iotdb.db.queryengine.plan.relational.security.TreeAccessCheckContext; import org.apache.iotdb.db.queryengine.plan.relational.sql.ParameterExtractor; import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.Execute; +import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.RelationalAuthorStatement; import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.SetSqlDialect; import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.Use; import org.apache.iotdb.db.queryengine.plan.relational.sql.parser.SqlParser; @@ -130,6 +132,7 @@ import org.apache.iotdb.db.queryengine.plan.statement.metadata.template.SetSchemaTemplateStatement; import org.apache.iotdb.db.queryengine.plan.statement.metadata.template.UnsetSchemaTemplateStatement; import org.apache.iotdb.db.queryengine.plan.statement.metadata.view.CreateTableViewStatement; +import org.apache.iotdb.db.queryengine.plan.statement.sys.AuthorStatement; import org.apache.iotdb.db.queryengine.plan.statement.sys.SetSqlDialectStatement; import org.apache.iotdb.db.schemaengine.SchemaEngine; import org.apache.iotdb.db.schemaengine.schemaregion.ISchemaRegion; @@ -329,7 +332,25 @@ public ClientRPCServiceImpl() { private TSExecuteStatementResp executeStatementInternal( NativeStatementRequest request, SelectResult setResult) { + GrantRoleFailureAuditContext grantRoleFailureAuditContext = + new GrantRoleFailureAuditContext(request.getSql()); + TSExecuteStatementResp response = null; + try { + response = + executeStatementInternalWithoutGrantRoleFailureAudit( + request, setResult, grantRoleFailureAuditContext); + return response; + } finally { + grantRoleFailureAuditContext.log(response == null ? null : response.getStatus()); + } + } + + private TSExecuteStatementResp executeStatementInternalWithoutGrantRoleFailureAudit( + NativeStatementRequest request, + SelectResult setResult, + GrantRoleFailureAuditContext grantRoleFailureAuditContext) { IClientSession clientSession = SESSION_MANAGER.getCurrSessionAndUpdateIdleTime(); + grantRoleFailureAuditContext.setClientSession(clientSession); if (!SESSION_MANAGER.checkLogin(clientSession)) { return RpcUtils.getTSExecuteStatementResp(getNotLoggedInStatus()); } @@ -353,6 +374,9 @@ private TSExecuteStatementResp executeStatementInternal( ExecutionResult result; if (clientSession.getSqlDialect() == SqlDialect.TREE) { Statement s = request.getTreeStatement(clientSession.getZoneId()); + if (s instanceof AuthorStatement) { + grantRoleFailureAuditContext.track((AuthorStatement) s); + } if (s instanceof SetSqlDialectStatement) { setSqlDialect = true; } @@ -426,6 +450,9 @@ private TSExecuteStatementResp executeStatementInternal( } else { org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Statement s = request.getTableStatement(relationSqlParser, clientSession.getZoneId(), clientSession); + if (s instanceof RelationalAuthorStatement) { + grantRoleFailureAuditContext.track((RelationalAuthorStatement) s); + } if (s instanceof Use) { useDatabase = true; @@ -2144,6 +2171,10 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) { try { for (int i = 0; i < req.getStatements().size(); i++) { String statement = req.getStatements().get(i); + GrantRoleFailureAuditContext grantRoleFailureAuditContext = + new GrantRoleFailureAuditContext(statement); + grantRoleFailureAuditContext.setClientSession(clientSession); + TSStatus auditStatus = null; long t2 = System.nanoTime(); String type = null; OperationQuota quota = null; @@ -2152,6 +2183,9 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) { ExecutionResult result; if (clientSession.getSqlDialect() == SqlDialect.TREE) { Statement s = StatementGenerator.createStatement(statement, clientSession.getZoneId()); + if (s instanceof AuthorStatement) { + grantRoleFailureAuditContext.track((AuthorStatement) s); + } if (s == null) { return RpcUtils.getStatus( TSStatusCode.EXECUTE_STATEMENT_ERROR, "This operation type is not supported"); @@ -2181,6 +2215,7 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) { clientSession.getClientAddress()) .setSqlString(statement)); if (status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + auditStatus = status; return status; } @@ -2223,6 +2258,9 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) { org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Statement s = relationSqlParser.createStatement( statement, clientSession.getZoneId(), clientSession); + if (s instanceof RelationalAuthorStatement) { + grantRoleFailureAuditContext.track((RelationalAuthorStatement) s); + } if (s instanceof Use) { useDatabase = true; @@ -2266,15 +2304,18 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) { } } - results.add(result.status); + auditStatus = result.status; + results.add(auditStatus); } catch (Exception e) { LOGGER.warn(DataNodeMiscMessages.ERROR_EXECUTING_BATCH_STATEMENT, e); TSStatus status = onQueryException( e, "\"" + statement + "\". " + OperationType.EXECUTE_BATCH_STATEMENT); isAllSuccessful = false; - results.add(status); + auditStatus = status; + results.add(auditStatus); } finally { + grantRoleFailureAuditContext.log(auditStatus); CommonUtils.addStatementExecutionLatency( OperationType.EXECUTE_STATEMENT, type, System.nanoTime() - t2); if (quota != null) { diff --git a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContextTest.java b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContextTest.java new file mode 100644 index 0000000000000..cb41aac589f3a --- /dev/null +++ b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContextTest.java @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iotdb.db.audit; + +import org.apache.iotdb.commons.audit.AuditEventType; +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.db.protocol.session.IClientSession; +import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.RelationalAuthorStatement; +import org.apache.iotdb.db.queryengine.plan.relational.type.AuthorRType; +import org.apache.iotdb.db.queryengine.plan.statement.AuthorType; +import org.apache.iotdb.db.queryengine.plan.statement.sys.AuthorStatement; +import org.apache.iotdb.rpc.RpcUtils; +import org.apache.iotdb.rpc.TSStatusCode; + +import org.junit.Test; +import org.mockito.ArgumentCaptor; + +import java.util.function.Supplier; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class GrantRoleFailureAuditContextTest { + + @Test + public void testTreeGrantRoleFailure() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class); + GrantRoleFailureAuditContext context = newContext(auditLogger); + AuthorStatement statement = new AuthorStatement(AuthorType.GRANT_USER_ROLE); + statement.setUserName("user1"); + + context.track(statement); + context.log(RpcUtils.getStatus(TSStatusCode.NO_PERMISSION)); + + assertAuditLog(auditLogger); + } + + @Test + public void testTableGrantRoleFailure() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class); + GrantRoleFailureAuditContext context = newContext(auditLogger); + RelationalAuthorStatement statement = + new RelationalAuthorStatement(AuthorRType.GRANT_USER_ROLE); + statement.setUserName("user1"); + + context.track(statement); + context.log(RpcUtils.getStatus(TSStatusCode.ROLE_NOT_EXIST)); + + assertAuditLog(auditLogger); + } + + @Test + public void testSuccessfulGrantIsIgnored() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class); + GrantRoleFailureAuditContext context = newContext(auditLogger); + context.track(treeGrantRoleStatement()); + + context.log(RpcUtils.SUCCESS_STATUS); + + verify(auditLogger, never()).log(any(), any()); + } + + @Test + public void testRedirectedGrantIsIgnored() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class); + GrantRoleFailureAuditContext context = newContext(auditLogger); + context.track(treeGrantRoleStatement()); + + context.log(RpcUtils.getStatus(TSStatusCode.REDIRECTION_RECOMMEND)); + + verify(auditLogger, never()).log(any(), any()); + } + + @Test + public void testPrivilegeGrantIsIgnored() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class); + GrantRoleFailureAuditContext context = newContext(auditLogger); + context.track(new AuthorStatement(AuthorType.GRANT_USER)); + + context.log(RpcUtils.getStatus(TSStatusCode.NO_PERMISSION)); + + verify(auditLogger, never()).log(any(), any()); + } + + private static GrantRoleFailureAuditContext newContext(DNAuditLogger auditLogger) { + GrantRoleFailureAuditContext context = + new GrantRoleFailureAuditContext(auditLogger, "GRANT ROLE role1 TO user1"); + context.setClientSession(mockSession()); + return context; + } + + private static AuthorStatement treeGrantRoleStatement() { + AuthorStatement statement = new AuthorStatement(AuthorType.GRANT_USER_ROLE); + statement.setUserName("user1"); + return statement; + } + + private static IClientSession mockSession() { + IClientSession session = mock(IClientSession.class); + when(session.getUserId()).thenReturn(7L); + when(session.getUsername()).thenReturn("operator"); + when(session.getClientAddress()).thenReturn("127.0.0.1"); + when(session.getDatabaseName()).thenReturn("database"); + return session; + } + + @SuppressWarnings("unchecked") + private static void assertAuditLog(DNAuditLogger auditLogger) { + ArgumentCaptor entityCaptor = ArgumentCaptor.forClass(IAuditEntity.class); + ArgumentCaptor> logCaptor = ArgumentCaptor.forClass(Supplier.class); + verify(auditLogger).log(entityCaptor.capture(), logCaptor.capture()); + + IAuditEntity entity = entityCaptor.getValue(); + assertEquals(7L, entity.getUserId()); + assertEquals("operator", entity.getUsername()); + assertEquals("127.0.0.1", entity.getCliHostname()); + assertEquals(AuditEventType.GRANT_ROLE_FAILED, entity.getAuditEventType()); + assertEquals(AuditLogOperation.CONTROL, entity.getAuditLogOperation()); + assertEquals(PrivilegeType.SECURITY, entity.getPrivilegeTypes().get(0)); + assertFalse(entity.getResult()); + assertEquals("database", entity.getDatabase()); + assertEquals("GRANT ROLE role1 TO user1", entity.getSqlString()); + assertEquals("user1", logCaptor.getValue().get()); + } +} From 6f2027777e13f6cf518ac76bd1b4b5d26519e8fc Mon Sep 17 00:00:00 2001 From: libo Date: Sat, 1 Aug 2026 07:41:18 +0800 Subject: [PATCH 2/3] Fix FMT_SMR.2 user role audit logging --- .../iotdb/db/i18n/DataNodeMiscMessages.java | 1 + .../iotdb/db/i18n/DataNodeMiscMessages.java | 1 + .../apache/iotdb/db/audit/DNAuditLogger.java | 143 ++++++++++++ .../audit/GrantRoleFailureAuditContext.java | 91 -------- .../iotdb/db/auth/AuthorityChecker.java | 28 ++- .../thrift/impl/ClientRPCServiceImpl.java | 45 +--- .../db/queryengine/plan/Coordinator.java | 78 ++++--- ...DNAuditLoggerUserRoleModificationTest.java | 214 ++++++++++++++++++ .../GrantRoleFailureAuditContextTest.java | 149 ------------ .../iotdb/commons/audit/AuditEventType.java | 1 + 10 files changed, 428 insertions(+), 323 deletions(-) delete mode 100644 iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContext.java create mode 100644 iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/DNAuditLoggerUserRoleModificationTest.java delete mode 100644 iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContextTest.java diff --git a/iotdb-core/datanode/src/main/i18n/en/org/apache/iotdb/db/i18n/DataNodeMiscMessages.java b/iotdb-core/datanode/src/main/i18n/en/org/apache/iotdb/db/i18n/DataNodeMiscMessages.java index b43037206ccdc..f3e8154dcde69 100644 --- a/iotdb-core/datanode/src/main/i18n/en/org/apache/iotdb/db/i18n/DataNodeMiscMessages.java +++ b/iotdb-core/datanode/src/main/i18n/en/org/apache/iotdb/db/i18n/DataNodeMiscMessages.java @@ -121,6 +121,7 @@ public final class DataNodeMiscMessages { public static final String CREATE_NEW_REGION_ERROR_FMT = "create new region %s error, exception:%s"; public static final String CREATE_NEW_REGION_SUCCEED_FMT = "create new region %s succeed"; + public static final String LOG_USER_ARG_ROLE_ARG_422D48D3 = "user: %s, role: %s"; private DataNodeMiscMessages() {} // --------------------------------------------------------------------------- diff --git a/iotdb-core/datanode/src/main/i18n/zh/org/apache/iotdb/db/i18n/DataNodeMiscMessages.java b/iotdb-core/datanode/src/main/i18n/zh/org/apache/iotdb/db/i18n/DataNodeMiscMessages.java index a319bce5000e6..5dd49c8804e7a 100644 --- a/iotdb-core/datanode/src/main/i18n/zh/org/apache/iotdb/db/i18n/DataNodeMiscMessages.java +++ b/iotdb-core/datanode/src/main/i18n/zh/org/apache/iotdb/db/i18n/DataNodeMiscMessages.java @@ -121,6 +121,7 @@ public final class DataNodeMiscMessages { public static final String CREATE_NEW_REGION_ERROR_FMT = "创建新 region %s 错误，异常：%s"; public static final String CREATE_NEW_REGION_SUCCEED_FMT = "创建新 region %s 成功"; + public static final String LOG_USER_ARG_ROLE_ARG_422D48D3 = "用户：%s，角色：%s"; private DataNodeMiscMessages() {} // --------------------------------------------------------------------------- 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 index 90026e37ba0fe..1a24f6ebaa339 100644 --- 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 @@ -19,16 +19,30 @@ package org.apache.iotdb.db.audit; +import org.apache.iotdb.common.rpc.thrift.TSStatus; import org.apache.iotdb.commons.audit.AbstractAuditLogger; +import org.apache.iotdb.commons.audit.AuditEventType; import org.apache.iotdb.commons.audit.AuditLogFields; +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.IllegalPathException; import org.apache.iotdb.commons.path.PartialPath; +import org.apache.iotdb.commons.queryengine.common.SessionInfo; +import org.apache.iotdb.db.i18n.DataNodeMiscMessages; import org.apache.iotdb.db.queryengine.plan.Coordinator; +import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.RelationalAuthorStatement; +import org.apache.iotdb.db.queryengine.plan.relational.type.AuthorRType; +import org.apache.iotdb.db.queryengine.plan.statement.AuthorType; +import org.apache.iotdb.db.queryengine.plan.statement.Statement; import org.apache.iotdb.db.queryengine.plan.statement.crud.InsertRowStatement; +import org.apache.iotdb.db.queryengine.plan.statement.sys.AuthorStatement; +import org.apache.iotdb.rpc.TSStatusCode; import jakarta.validation.constraints.NotNull; +import javax.annotation.Nullable; + import java.util.function.Supplier; public class DNAuditLogger extends AbstractAuditLogger { @@ -61,6 +75,135 @@ public synchronized void log(IAuditEntity auditLogFields, Supplier log) public void logFromCN(AuditLogFields auditLogFields, String log, int nodeId) throws IllegalPathException {} + public void logUserRoleModificationAuthorizationFailure( + Statement statement, IAuditEntity auditEntity, @Nullable TSStatus status) { + if (isSuccessful(status) || isRedirected(status)) { + return; + } + logUserRoleModification(getUserRoleTarget(statement), auditEntity, status); + } + + public void logUserRoleModification( + Statement statement, + SessionInfo sessionInfo, + @Nullable String sql, + @Nullable TSStatus status) { + logUserRoleModification(getUserRoleTarget(statement), sessionInfo, sql, status); + } + + public void logUserRoleModification( + org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Statement statement, + SessionInfo sessionInfo, + @Nullable String sql, + @Nullable TSStatus status) { + logUserRoleModification(getUserRoleTarget(statement), sessionInfo, sql, status); + } + + private void logUserRoleModification( + @Nullable UserRoleTarget target, + @Nullable SessionInfo sessionInfo, + @Nullable String sql, + @Nullable TSStatus status) { + if (target == null || sessionInfo == null || isRedirected(status)) { + return; + } + logUserRoleModification( + target, + sessionInfo.getUserId(), + sessionInfo.getUserName(), + sessionInfo.getCliHostname(), + sessionInfo.getDatabaseName().orElse(null), + sql, + status); + } + + private void logUserRoleModification( + @Nullable UserRoleTarget target, IAuditEntity auditEntity, @Nullable TSStatus status) { + if (target == null || isRedirected(status)) { + return; + } + logUserRoleModification( + target, + auditEntity.getUserId(), + auditEntity.getUsername(), + auditEntity.getCliHostname(), + auditEntity.getDatabase(), + auditEntity.getSqlString(), + status); + } + + private void logUserRoleModification( + UserRoleTarget target, + long userId, + String username, + String clientAddress, + @Nullable String database, + @Nullable String sql, + @Nullable TSStatus status) { + log( + new AuditLogFields( + userId, + username, + clientAddress, + AuditEventType.MODIFY_USER_ROLE, + AuditLogOperation.CONTROL, + PrivilegeType.SECURITY, + isSuccessful(status), + database, + sql), + () -> + String.format( + DataNodeMiscMessages.LOG_USER_ARG_ROLE_ARG_422D48D3, + target.username, + target.roleName)); + } + + @Nullable + private static UserRoleTarget getUserRoleTarget(Statement statement) { + if (!(statement instanceof AuthorStatement)) { + return null; + } + AuthorStatement authorStatement = (AuthorStatement) statement; + if (authorStatement.getAuthorType() != AuthorType.GRANT_USER_ROLE + && authorStatement.getAuthorType() != AuthorType.REVOKE_USER_ROLE) { + return null; + } + return new UserRoleTarget(authorStatement.getUserName(), authorStatement.getRoleName()); + } + + @Nullable + private static UserRoleTarget getUserRoleTarget( + org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Statement statement) { + if (!(statement instanceof RelationalAuthorStatement)) { + return null; + } + RelationalAuthorStatement authorStatement = (RelationalAuthorStatement) statement; + if (authorStatement.getAuthorType() != AuthorRType.GRANT_USER_ROLE + && authorStatement.getAuthorType() != AuthorRType.REVOKE_USER_ROLE) { + return null; + } + return new UserRoleTarget(authorStatement.getUserName(), authorStatement.getRoleName()); + } + + private static boolean isSuccessful(@Nullable TSStatus status) { + return status != null && status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode(); + } + + private static boolean isRedirected(@Nullable TSStatus status) { + return status != null && status.getCode() == TSStatusCode.REDIRECTION_RECOMMEND.getStatusCode(); + } + + private static class UserRoleTarget { + + private final String username; + private final String roleName; + + private UserRoleTarget(String username, String roleName) { + this.username = username; + this.roleName = roleName; + } + } + private static class DNAuditLoggerHolder { private static final DNAuditLogger INSTANCE = new DNAuditLogger(); diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContext.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContext.java deleted file mode 100644 index 70b6dc299a174..0000000000000 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContext.java +++ /dev/null @@ -1,91 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.iotdb.db.audit; - -import org.apache.iotdb.common.rpc.thrift.TSStatus; -import org.apache.iotdb.commons.audit.AuditEventType; -import org.apache.iotdb.commons.audit.AuditLogFields; -import org.apache.iotdb.commons.audit.AuditLogOperation; -import org.apache.iotdb.commons.auth.entity.PrivilegeType; -import org.apache.iotdb.db.protocol.session.IClientSession; -import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.RelationalAuthorStatement; -import org.apache.iotdb.db.queryengine.plan.relational.type.AuthorRType; -import org.apache.iotdb.db.queryengine.plan.statement.AuthorType; -import org.apache.iotdb.db.queryengine.plan.statement.sys.AuthorStatement; -import org.apache.iotdb.rpc.TSStatusCode; - -/** Tracks one role-grant statement and records an audit event only when it fails. */ -public class GrantRoleFailureAuditContext { - - private final DNAuditLogger auditLogger; - private final String sqlString; - - private IClientSession clientSession; - private String targetUsername; - - public GrantRoleFailureAuditContext(String sqlString) { - this(DNAuditLogger.getInstance(), sqlString); - } - - GrantRoleFailureAuditContext(DNAuditLogger auditLogger, String sqlString) { - this.auditLogger = auditLogger; - this.sqlString = sqlString; - } - - public void setClientSession(IClientSession clientSession) { - this.clientSession = clientSession; - } - - public void track(AuthorStatement statement) { - if (statement.getAuthorType() == AuthorType.GRANT_USER_ROLE) { - targetUsername = statement.getUserName(); - } - } - - public void track(RelationalAuthorStatement statement) { - if (statement.getAuthorType() == AuthorRType.GRANT_USER_ROLE) { - targetUsername = statement.getUserName(); - } - } - - public void log(TSStatus status) { - if (targetUsername == null || clientSession == null || isSuccessfulOrRedirected(status)) { - return; - } - auditLogger.log( - new AuditLogFields( - clientSession.getUserId(), - clientSession.getUsername(), - clientSession.getClientAddress(), - AuditEventType.GRANT_ROLE_FAILED, - AuditLogOperation.CONTROL, - PrivilegeType.SECURITY, - false, - clientSession.getDatabaseName(), - sqlString), - () -> targetUsername); - } - - private static boolean isSuccessfulOrRedirected(TSStatus status) { - return status != null - && (status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode() - || status.getCode() == TSStatusCode.REDIRECTION_RECOMMEND.getStatusCode()); - } -} diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/AuthorityChecker.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/AuthorityChecker.java index 646f093ba5746..00b7918aae25b 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/AuthorityChecker.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/AuthorityChecker.java @@ -39,6 +39,7 @@ import org.apache.iotdb.confignode.rpc.thrift.TRoleResp; import org.apache.iotdb.confignode.rpc.thrift.TTablePrivilege; import org.apache.iotdb.confignode.rpc.thrift.TUserResp; +import org.apache.iotdb.db.audit.DNAuditLogger; import org.apache.iotdb.db.i18n.DataNodeMiscMessages; import org.apache.iotdb.db.pipe.source.dataregion.realtime.listener.PipeInsertionDataNodeListener; import org.apache.iotdb.db.protocol.session.IClientSession; @@ -193,21 +194,28 @@ public static IAuditEntity createIAuditEntity(String userName, IClientSession cl public static TSStatus checkAuthority(Statement statement, IAuditEntity auditEntity) { long startTime = System.nanoTime(); + TSStatus status = null; try { if (auditEntity instanceof TreeAccessCheckContext) { - return accessControl.checkPermissionBeforeProcess( - statement, (TreeAccessCheckContext) auditEntity); + status = + accessControl.checkPermissionBeforeProcess( + statement, (TreeAccessCheckContext) auditEntity); + } else { + status = + accessControl.checkPermissionBeforeProcess( + statement, + (TreeAccessCheckContext) + new TreeAccessCheckContext( + auditEntity.getUserId(), + auditEntity.getUsername(), + auditEntity.getCliHostname()) + .setSqlString(auditEntity.getSqlString())); } - return accessControl.checkPermissionBeforeProcess( - statement, - (TreeAccessCheckContext) - new TreeAccessCheckContext( - auditEntity.getUserId(), - auditEntity.getUsername(), - auditEntity.getCliHostname()) - .setSqlString(auditEntity.getSqlString())); + return status; } finally { PERFORMANCE_OVERVIEW_METRICS.recordAuthCost(System.nanoTime() - startTime); + DNAuditLogger.getInstance() + .logUserRoleModificationAuthorizationFailure(statement, auditEntity, status); } } diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/ClientRPCServiceImpl.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/ClientRPCServiceImpl.java index 6bf61ee98b836..ea67ebb8523df 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/ClientRPCServiceImpl.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/ClientRPCServiceImpl.java @@ -59,7 +59,6 @@ import org.apache.iotdb.commons.queryengine.plan.relational.sql.parser.ParsingException; import org.apache.iotdb.commons.utils.PathUtils; import org.apache.iotdb.db.audit.DNAuditLogger; -import org.apache.iotdb.db.audit.GrantRoleFailureAuditContext; import org.apache.iotdb.db.auth.AuthorityChecker; import org.apache.iotdb.db.conf.IoTDBConfig; import org.apache.iotdb.db.conf.IoTDBDescriptor; @@ -106,7 +105,6 @@ import org.apache.iotdb.db.queryengine.plan.relational.security.TreeAccessCheckContext; import org.apache.iotdb.db.queryengine.plan.relational.sql.ParameterExtractor; import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.Execute; -import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.RelationalAuthorStatement; import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.SetSqlDialect; import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.Use; import org.apache.iotdb.db.queryengine.plan.relational.sql.parser.SqlParser; @@ -132,7 +130,6 @@ import org.apache.iotdb.db.queryengine.plan.statement.metadata.template.SetSchemaTemplateStatement; import org.apache.iotdb.db.queryengine.plan.statement.metadata.template.UnsetSchemaTemplateStatement; import org.apache.iotdb.db.queryengine.plan.statement.metadata.view.CreateTableViewStatement; -import org.apache.iotdb.db.queryengine.plan.statement.sys.AuthorStatement; import org.apache.iotdb.db.queryengine.plan.statement.sys.SetSqlDialectStatement; import org.apache.iotdb.db.schemaengine.SchemaEngine; import org.apache.iotdb.db.schemaengine.schemaregion.ISchemaRegion; @@ -332,25 +329,7 @@ public ClientRPCServiceImpl() { private TSExecuteStatementResp executeStatementInternal( NativeStatementRequest request, SelectResult setResult) { - GrantRoleFailureAuditContext grantRoleFailureAuditContext = - new GrantRoleFailureAuditContext(request.getSql()); - TSExecuteStatementResp response = null; - try { - response = - executeStatementInternalWithoutGrantRoleFailureAudit( - request, setResult, grantRoleFailureAuditContext); - return response; - } finally { - grantRoleFailureAuditContext.log(response == null ? null : response.getStatus()); - } - } - - private TSExecuteStatementResp executeStatementInternalWithoutGrantRoleFailureAudit( - NativeStatementRequest request, - SelectResult setResult, - GrantRoleFailureAuditContext grantRoleFailureAuditContext) { IClientSession clientSession = SESSION_MANAGER.getCurrSessionAndUpdateIdleTime(); - grantRoleFailureAuditContext.setClientSession(clientSession); if (!SESSION_MANAGER.checkLogin(clientSession)) { return RpcUtils.getTSExecuteStatementResp(getNotLoggedInStatus()); } @@ -374,9 +353,6 @@ private TSExecuteStatementResp executeStatementInternalWithoutGrantRoleFailureAu ExecutionResult result; if (clientSession.getSqlDialect() == SqlDialect.TREE) { Statement s = request.getTreeStatement(clientSession.getZoneId()); - if (s instanceof AuthorStatement) { - grantRoleFailureAuditContext.track((AuthorStatement) s); - } if (s instanceof SetSqlDialectStatement) { setSqlDialect = true; } @@ -450,9 +426,6 @@ private TSExecuteStatementResp executeStatementInternalWithoutGrantRoleFailureAu } else { org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Statement s = request.getTableStatement(relationSqlParser, clientSession.getZoneId(), clientSession); - if (s instanceof RelationalAuthorStatement) { - grantRoleFailureAuditContext.track((RelationalAuthorStatement) s); - } if (s instanceof Use) { useDatabase = true; @@ -2171,10 +2144,6 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) { try { for (int i = 0; i < req.getStatements().size(); i++) { String statement = req.getStatements().get(i); - GrantRoleFailureAuditContext grantRoleFailureAuditContext = - new GrantRoleFailureAuditContext(statement); - grantRoleFailureAuditContext.setClientSession(clientSession); - TSStatus auditStatus = null; long t2 = System.nanoTime(); String type = null; OperationQuota quota = null; @@ -2183,9 +2152,6 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) { ExecutionResult result; if (clientSession.getSqlDialect() == SqlDialect.TREE) { Statement s = StatementGenerator.createStatement(statement, clientSession.getZoneId()); - if (s instanceof AuthorStatement) { - grantRoleFailureAuditContext.track((AuthorStatement) s); - } if (s == null) { return RpcUtils.getStatus( TSStatusCode.EXECUTE_STATEMENT_ERROR, "This operation type is not supported"); @@ -2215,7 +2181,6 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) { clientSession.getClientAddress()) .setSqlString(statement)); if (status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { - auditStatus = status; return status; } @@ -2258,9 +2223,6 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) { org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Statement s = relationSqlParser.createStatement( statement, clientSession.getZoneId(), clientSession); - if (s instanceof RelationalAuthorStatement) { - grantRoleFailureAuditContext.track((RelationalAuthorStatement) s); - } if (s instanceof Use) { useDatabase = true; @@ -2304,18 +2266,15 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) { } } - auditStatus = result.status; - results.add(auditStatus); + results.add(result.status); } catch (Exception e) { LOGGER.warn(DataNodeMiscMessages.ERROR_EXECUTING_BATCH_STATEMENT, e); TSStatus status = onQueryException( e, "\"" + statement + "\". " + OperationType.EXECUTE_BATCH_STATEMENT); isAllSuccessful = false; - auditStatus = status; - results.add(auditStatus); + results.add(status); } finally { - grantRoleFailureAuditContext.log(auditStatus); CommonUtils.addStatementExecutionLatency( OperationType.EXECUTE_STATEMENT, type, System.nanoTime() - t2); if (quota != null) { diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/Coordinator.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/Coordinator.java index a7edb91c5cfbe..fdb874a2389ae 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/Coordinator.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/Coordinator.java @@ -20,6 +20,7 @@ package org.apache.iotdb.db.queryengine.plan; import org.apache.iotdb.common.rpc.thrift.TEndPoint; +import org.apache.iotdb.common.rpc.thrift.TSStatus; import org.apache.iotdb.commons.client.ClientPoolFactory; import org.apache.iotdb.commons.client.IClientManager; import org.apache.iotdb.commons.client.async.AsyncDataNodeInternalServiceClient; @@ -42,6 +43,7 @@ import org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Table; import org.apache.iotdb.commons.queryengine.plan.relational.type.InternalTypeManager; import org.apache.iotdb.commons.queryengine.plan.relational.type.TypeManager; +import org.apache.iotdb.db.audit.DNAuditLogger; import org.apache.iotdb.db.auth.AuthorityChecker; import org.apache.iotdb.db.conf.IoTDBConfig; import org.apache.iotdb.db.conf.IoTDBDescriptor; @@ -393,20 +395,28 @@ public ExecutionResult executeForTreeModel( long timeOut, boolean userQuery, boolean debug) { - return execution( - queryId, - session, - sql, - userQuery, - debug, - ((queryContext, startTime) -> - createQueryExecutionForTreeModel( - statement, - queryContext, - partitionFetcher, - schemaFetcher, - timeOut > 0 ? timeOut : CONFIG.getQueryTimeoutThreshold(), - startTime))); + ExecutionResult result = null; + try { + result = + execution( + queryId, + session, + sql, + userQuery, + debug, + ((queryContext, startTime) -> + createQueryExecutionForTreeModel( + statement, + queryContext, + partitionFetcher, + schemaFetcher, + timeOut > 0 ? timeOut : CONFIG.getQueryTimeoutThreshold(), + startTime))); + return result; + } finally { + TSStatus status = result == null ? null : result.status; + DNAuditLogger.getInstance().logUserRoleModification(statement, session, sql, status); + } } private IQueryExecution createQueryExecutionForTreeModel( @@ -531,22 +541,30 @@ public ExecutionResult executeForTableModel( boolean userQuery, boolean debug, boolean readOnlyInternalQuery) { - return execution( - queryId, - session, - sql, - userQuery, - debug, - readOnlyInternalQuery, - ((queryContext, startTime) -> - createQueryExecutionForTableModel( - statement, - sqlParser, - clientSession, - queryContext, - metadata, - timeOut > 0 ? timeOut : CONFIG.getQueryTimeoutThreshold(), - startTime))); + ExecutionResult result = null; + try { + result = + execution( + queryId, + session, + sql, + userQuery, + debug, + readOnlyInternalQuery, + ((queryContext, startTime) -> + createQueryExecutionForTableModel( + statement, + sqlParser, + clientSession, + queryContext, + metadata, + timeOut > 0 ? timeOut : CONFIG.getQueryTimeoutThreshold(), + startTime))); + return result; + } finally { + TSStatus status = result == null ? null : result.status; + DNAuditLogger.getInstance().logUserRoleModification(statement, session, sql, status); + } } /** For compatibility of MQTT and REST, this method should never be called. */ diff --git a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/DNAuditLoggerUserRoleModificationTest.java b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/DNAuditLoggerUserRoleModificationTest.java new file mode 100644 index 0000000000000..4242635b26cea --- /dev/null +++ b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/DNAuditLoggerUserRoleModificationTest.java @@ -0,0 +1,214 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iotdb.db.audit; + +import org.apache.iotdb.commons.audit.AuditEventType; +import org.apache.iotdb.commons.audit.AuditLogOperation; +import org.apache.iotdb.commons.audit.IAuditEntity; +import org.apache.iotdb.commons.audit.UserEntity; +import org.apache.iotdb.commons.auth.entity.PrivilegeType; +import org.apache.iotdb.commons.queryengine.common.SessionInfo; +import org.apache.iotdb.commons.queryengine.common.SqlDialect; +import org.apache.iotdb.db.queryengine.plan.relational.security.TreeAccessCheckContext; +import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.RelationalAuthorStatement; +import org.apache.iotdb.db.queryengine.plan.relational.type.AuthorRType; +import org.apache.iotdb.db.queryengine.plan.statement.AuthorType; +import org.apache.iotdb.db.queryengine.plan.statement.sys.AuthorStatement; +import org.apache.iotdb.rpc.RpcUtils; +import org.apache.iotdb.rpc.TSStatusCode; + +import org.junit.Test; +import org.mockito.ArgumentCaptor; + +import java.time.ZoneId; +import java.util.function.Supplier; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +public class DNAuditLoggerUserRoleModificationTest { + + @Test + public void testTreeGrantRoleSuccess() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class, CALLS_REAL_METHODS); + String sql = "GRANT ROLE role1 TO user1"; + + auditLogger.logUserRoleModification( + treeStatement(AuthorType.GRANT_USER_ROLE), sessionInfo(), sql, RpcUtils.SUCCESS_STATUS); + + assertAuditLog(auditLogger, true, sql); + } + + @Test + public void testTreeRevokeRoleFailure() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class, CALLS_REAL_METHODS); + String sql = "REVOKE ROLE role1 FROM user1"; + + auditLogger.logUserRoleModification( + treeStatement(AuthorType.REVOKE_USER_ROLE), + sessionInfo(), + sql, + RpcUtils.getStatus(TSStatusCode.USER_NOT_HAS_ROLE)); + + assertAuditLog(auditLogger, false, sql); + } + + @Test + public void testTableGrantRoleFailure() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class, CALLS_REAL_METHODS); + String sql = "GRANT ROLE role1 TO user1"; + + auditLogger.logUserRoleModification( + tableStatement(AuthorRType.GRANT_USER_ROLE), + sessionInfo(), + sql, + RpcUtils.getStatus(TSStatusCode.ROLE_NOT_EXIST)); + + assertAuditLog(auditLogger, false, sql); + } + + @Test + public void testTableRevokeRoleSuccess() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class, CALLS_REAL_METHODS); + String sql = "REVOKE ROLE role1 FROM user1"; + + auditLogger.logUserRoleModification( + tableStatement(AuthorRType.REVOKE_USER_ROLE), sessionInfo(), sql, RpcUtils.SUCCESS_STATUS); + + assertAuditLog(auditLogger, true, sql); + } + + @Test + public void testTreeAuthorizationFailure() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class, CALLS_REAL_METHODS); + String sql = "GRANT ROLE role1 TO user1"; + + auditLogger.logUserRoleModificationAuthorizationFailure( + treeStatement(AuthorType.GRANT_USER_ROLE), + treeAuditEntity(sql), + RpcUtils.getStatus(TSStatusCode.NO_PERMISSION)); + + assertAuditLog(auditLogger, false, sql); + } + + @Test + public void testSuccessfulAuthorizationIsLoggedAfterExecutionOnly() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class, CALLS_REAL_METHODS); + + auditLogger.logUserRoleModificationAuthorizationFailure( + treeStatement(AuthorType.GRANT_USER_ROLE), + treeAuditEntity("grant role"), + RpcUtils.SUCCESS_STATUS); + + verify(auditLogger, never()).log(any(), any()); + } + + @Test + public void testRedirectIsIgnored() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class, CALLS_REAL_METHODS); + + auditLogger.logUserRoleModification( + treeStatement(AuthorType.GRANT_USER_ROLE), + sessionInfo(), + "grant role", + RpcUtils.getStatus(TSStatusCode.REDIRECTION_RECOMMEND)); + + verify(auditLogger, never()).log(any(), any()); + } + + @Test + public void testPrivilegeGrantIsIgnored() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class, CALLS_REAL_METHODS); + AuthorStatement statement = new AuthorStatement(AuthorType.GRANT_USER); + statement.setUserName("user1"); + + auditLogger.logUserRoleModification(statement, sessionInfo(), "grant privilege", null); + + verify(auditLogger, never()).log(any(), any()); + } + + @Test + public void testMissingSessionIsIgnored() { + DNAuditLogger auditLogger = mock(DNAuditLogger.class, CALLS_REAL_METHODS); + + auditLogger.logUserRoleModification( + treeStatement(AuthorType.GRANT_USER_ROLE), null, "grant role", null); + + verify(auditLogger, never()).log(any(), any()); + } + + private static AuthorStatement treeStatement(AuthorType type) { + AuthorStatement statement = new AuthorStatement(type); + statement.setUserName("user1"); + statement.setRoleName("role1"); + return statement; + } + + private static RelationalAuthorStatement tableStatement(AuthorRType type) { + RelationalAuthorStatement statement = new RelationalAuthorStatement(type); + statement.setUserName("user1"); + statement.setRoleName("role1"); + return statement; + } + + private static IAuditEntity treeAuditEntity(String sql) { + return new TreeAccessCheckContext(7L, "operator", "127.0.0.1") + .setDatabase("database") + .setSqlString(sql); + } + + private static SessionInfo sessionInfo() { + return new SessionInfo( + 1L, + new UserEntity(7L, "operator", "127.0.0.1"), + ZoneId.systemDefault(), + "database", + SqlDialect.TABLE); + } + + @SuppressWarnings("unchecked") + private static void assertAuditLog(DNAuditLogger auditLogger, boolean result, String sql) { + ArgumentCaptor entityCaptor = ArgumentCaptor.forClass(IAuditEntity.class); + ArgumentCaptor> logCaptor = ArgumentCaptor.forClass(Supplier.class); + verify(auditLogger).log(entityCaptor.capture(), logCaptor.capture()); + + IAuditEntity entity = entityCaptor.getValue(); + assertEquals(7L, entity.getUserId()); + assertEquals("operator", entity.getUsername()); + assertEquals("127.0.0.1", entity.getCliHostname()); + assertEquals(AuditEventType.MODIFY_USER_ROLE, entity.getAuditEventType()); + assertEquals(AuditLogOperation.CONTROL, entity.getAuditLogOperation()); + assertEquals(PrivilegeType.SECURITY, entity.getPrivilegeTypes().get(0)); + if (result) { + assertTrue(entity.getResult()); + } else { + assertFalse(entity.getResult()); + } + assertEquals("database", entity.getDatabase()); + assertEquals(sql, entity.getSqlString()); + assertEquals("user: user1, role: role1", logCaptor.getValue().get()); + } +} diff --git a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContextTest.java b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContextTest.java deleted file mode 100644 index cb41aac589f3a..0000000000000 --- a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/GrantRoleFailureAuditContextTest.java +++ /dev/null @@ -1,149 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.iotdb.db.audit; - -import org.apache.iotdb.commons.audit.AuditEventType; -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.db.protocol.session.IClientSession; -import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.RelationalAuthorStatement; -import org.apache.iotdb.db.queryengine.plan.relational.type.AuthorRType; -import org.apache.iotdb.db.queryengine.plan.statement.AuthorType; -import org.apache.iotdb.db.queryengine.plan.statement.sys.AuthorStatement; -import org.apache.iotdb.rpc.RpcUtils; -import org.apache.iotdb.rpc.TSStatusCode; - -import org.junit.Test; -import org.mockito.ArgumentCaptor; - -import java.util.function.Supplier; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -public class GrantRoleFailureAuditContextTest { - - @Test - public void testTreeGrantRoleFailure() { - DNAuditLogger auditLogger = mock(DNAuditLogger.class); - GrantRoleFailureAuditContext context = newContext(auditLogger); - AuthorStatement statement = new AuthorStatement(AuthorType.GRANT_USER_ROLE); - statement.setUserName("user1"); - - context.track(statement); - context.log(RpcUtils.getStatus(TSStatusCode.NO_PERMISSION)); - - assertAuditLog(auditLogger); - } - - @Test - public void testTableGrantRoleFailure() { - DNAuditLogger auditLogger = mock(DNAuditLogger.class); - GrantRoleFailureAuditContext context = newContext(auditLogger); - RelationalAuthorStatement statement = - new RelationalAuthorStatement(AuthorRType.GRANT_USER_ROLE); - statement.setUserName("user1"); - - context.track(statement); - context.log(RpcUtils.getStatus(TSStatusCode.ROLE_NOT_EXIST)); - - assertAuditLog(auditLogger); - } - - @Test - public void testSuccessfulGrantIsIgnored() { - DNAuditLogger auditLogger = mock(DNAuditLogger.class); - GrantRoleFailureAuditContext context = newContext(auditLogger); - context.track(treeGrantRoleStatement()); - - context.log(RpcUtils.SUCCESS_STATUS); - - verify(auditLogger, never()).log(any(), any()); - } - - @Test - public void testRedirectedGrantIsIgnored() { - DNAuditLogger auditLogger = mock(DNAuditLogger.class); - GrantRoleFailureAuditContext context = newContext(auditLogger); - context.track(treeGrantRoleStatement()); - - context.log(RpcUtils.getStatus(TSStatusCode.REDIRECTION_RECOMMEND)); - - verify(auditLogger, never()).log(any(), any()); - } - - @Test - public void testPrivilegeGrantIsIgnored() { - DNAuditLogger auditLogger = mock(DNAuditLogger.class); - GrantRoleFailureAuditContext context = newContext(auditLogger); - context.track(new AuthorStatement(AuthorType.GRANT_USER)); - - context.log(RpcUtils.getStatus(TSStatusCode.NO_PERMISSION)); - - verify(auditLogger, never()).log(any(), any()); - } - - private static GrantRoleFailureAuditContext newContext(DNAuditLogger auditLogger) { - GrantRoleFailureAuditContext context = - new GrantRoleFailureAuditContext(auditLogger, "GRANT ROLE role1 TO user1"); - context.setClientSession(mockSession()); - return context; - } - - private static AuthorStatement treeGrantRoleStatement() { - AuthorStatement statement = new AuthorStatement(AuthorType.GRANT_USER_ROLE); - statement.setUserName("user1"); - return statement; - } - - private static IClientSession mockSession() { - IClientSession session = mock(IClientSession.class); - when(session.getUserId()).thenReturn(7L); - when(session.getUsername()).thenReturn("operator"); - when(session.getClientAddress()).thenReturn("127.0.0.1"); - when(session.getDatabaseName()).thenReturn("database"); - return session; - } - - @SuppressWarnings("unchecked") - private static void assertAuditLog(DNAuditLogger auditLogger) { - ArgumentCaptor entityCaptor = ArgumentCaptor.forClass(IAuditEntity.class); - ArgumentCaptor> logCaptor = ArgumentCaptor.forClass(Supplier.class); - verify(auditLogger).log(entityCaptor.capture(), logCaptor.capture()); - - IAuditEntity entity = entityCaptor.getValue(); - assertEquals(7L, entity.getUserId()); - assertEquals("operator", entity.getUsername()); - assertEquals("127.0.0.1", entity.getCliHostname()); - assertEquals(AuditEventType.GRANT_ROLE_FAILED, entity.getAuditEventType()); - assertEquals(AuditLogOperation.CONTROL, entity.getAuditLogOperation()); - assertEquals(PrivilegeType.SECURITY, entity.getPrivilegeTypes().get(0)); - assertFalse(entity.getResult()); - assertEquals("database", entity.getDatabase()); - assertEquals("GRANT ROLE role1 TO user1", entity.getSqlString()); - assertEquals("user1", logCaptor.getValue().get()); - } -} diff --git a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/audit/AuditEventType.java b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/audit/AuditEventType.java index 510912a302fdc..4d8eb9c550e22 100644 --- a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/audit/AuditEventType.java +++ b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/audit/AuditEventType.java @@ -37,6 +37,7 @@ public enum AuditEventType { LOGIN_FINAL, MODIFY_SECURITY_OPTIONS, MODIFY_DEFAULT_SECURITY_VALUES, + MODIFY_USER_ROLE, REVOKE_FAILED, GRANT_ROLE_FAILED, LOGIN_RESOURCE_RESTRICT, From 594a5383f35889c99b1d169e4add02a6cda09a99 Mon Sep 17 00:00:00 2001 From: libo Date: Mon, 3 Aug 2026 17:05:00 +0800 Subject: [PATCH 3/3] Refine FMT_SMR.2 role membership audit logging --- .../apache/iotdb/db/audit/DNAuditLogger.java | 2 +- .../UserRoleModificationAuditContext.java | 123 +++++++++++++++++ .../audit/UserRoleModificationAuditTask.java | 101 ++++++++++++++ .../db/queryengine/plan/Coordinator.java | 13 +- .../config/TableConfigTaskVisitor.java | 15 ++- .../config/TreeConfigTaskVisitor.java | 39 ++++-- ...DNAuditLoggerUserRoleModificationTest.java | 125 +++++++++++++++++- .../iotdb/commons/audit/AuditEventType.java | 2 +- 8 files changed, 394 insertions(+), 26 deletions(-) create mode 100644 iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/UserRoleModificationAuditContext.java create mode 100644 iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/UserRoleModificationAuditTask.java 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 index 26bd141f8b05c..c7243b178ac84 100644 --- 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 @@ -343,7 +343,7 @@ private void logUserRoleModification( userId, username, clientAddress, - AuditEventType.MODIFY_USER_ROLE, + AuditEventType.MODIFY_ROLE_MEMBERSHIP, AuditLogOperation.CONTROL, PrivilegeType.SECURITY, isSuccessful(status), diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/UserRoleModificationAuditContext.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/UserRoleModificationAuditContext.java new file mode 100644 index 0000000000000..fc85c214a7633 --- /dev/null +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/UserRoleModificationAuditContext.java @@ -0,0 +1,123 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iotdb.db.audit; + +import org.apache.iotdb.common.rpc.thrift.TSStatus; +import org.apache.iotdb.commons.queryengine.common.SessionInfo; +import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.RelationalAuthorStatement; +import org.apache.iotdb.db.queryengine.plan.relational.type.AuthorRType; +import org.apache.iotdb.db.queryengine.plan.statement.AuthorType; +import org.apache.iotdb.db.queryengine.plan.statement.Statement; +import org.apache.iotdb.db.queryengine.plan.statement.sys.AuthorStatement; +import org.apache.iotdb.rpc.TSStatusCode; + +import javax.annotation.Nullable; + +import java.util.concurrent.atomic.AtomicBoolean; + +/** Ensures one user-role modification attempt produces at most one execution audit record. */ +public final class UserRoleModificationAuditContext { + + private static final UserRoleModificationAuditContext EMPTY = + new UserRoleModificationAuditContext(null); + + private final AuditLogWriter auditLogWriter; + private final AtomicBoolean logged = new AtomicBoolean(false); + + private UserRoleModificationAuditContext(@Nullable AuditLogWriter auditLogWriter) { + this.auditLogWriter = auditLogWriter; + } + + public static UserRoleModificationAuditContext forTreeStatement( + Statement statement, @Nullable SessionInfo sessionInfo, @Nullable String sql) { + return forTreeStatement( + statement, + sessionInfo, + status -> + DNAuditLogger.getInstance() + .logUserRoleModification(statement, sessionInfo, sql, status)); + } + + static UserRoleModificationAuditContext forTreeStatement( + Statement statement, @Nullable SessionInfo sessionInfo, AuditLogWriter auditLogWriter) { + return isUserRoleModification(statement) && sessionInfo != null + ? new UserRoleModificationAuditContext(auditLogWriter) + : EMPTY; + } + + public static UserRoleModificationAuditContext forTableStatement( + org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Statement statement, + @Nullable SessionInfo sessionInfo, + @Nullable String sql) { + return forTableStatement( + statement, + sessionInfo, + status -> + DNAuditLogger.getInstance() + .logUserRoleModification(statement, sessionInfo, sql, status)); + } + + static UserRoleModificationAuditContext forTableStatement( + org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Statement statement, + @Nullable SessionInfo sessionInfo, + AuditLogWriter auditLogWriter) { + return isUserRoleModification(statement) && sessionInfo != null + ? new UserRoleModificationAuditContext(auditLogWriter) + : EMPTY; + } + + public void log(@Nullable TSStatus status) { + if (auditLogWriter == null || isRedirected(status) || !logged.compareAndSet(false, true)) { + return; + } + auditLogWriter.log(status); + } + + boolean isEnabled() { + return auditLogWriter != null; + } + + private static boolean isUserRoleModification(Statement statement) { + if (!(statement instanceof AuthorStatement)) { + return false; + } + AuthorType type = ((AuthorStatement) statement).getAuthorType(); + return type == AuthorType.GRANT_USER_ROLE || type == AuthorType.REVOKE_USER_ROLE; + } + + private static boolean isUserRoleModification( + org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Statement statement) { + if (!(statement instanceof RelationalAuthorStatement)) { + return false; + } + AuthorRType type = ((RelationalAuthorStatement) statement).getAuthorType(); + return type == AuthorRType.GRANT_USER_ROLE || type == AuthorRType.REVOKE_USER_ROLE; + } + + private static boolean isRedirected(@Nullable TSStatus status) { + return status != null && status.getCode() == TSStatusCode.REDIRECTION_RECOMMEND.getStatusCode(); + } + + @FunctionalInterface + interface AuditLogWriter { + + void log(@Nullable TSStatus status); + } +} diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/UserRoleModificationAuditTask.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/UserRoleModificationAuditTask.java new file mode 100644 index 0000000000000..48bc11ffc3acc --- /dev/null +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/UserRoleModificationAuditTask.java @@ -0,0 +1,101 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iotdb.db.audit; + +import org.apache.iotdb.common.rpc.thrift.TSStatus; +import org.apache.iotdb.commons.exception.IoTDBException; +import org.apache.iotdb.commons.exception.IoTDBRuntimeException; +import org.apache.iotdb.db.queryengine.plan.execution.config.ConfigTaskResult; +import org.apache.iotdb.db.queryengine.plan.execution.config.IConfigTask; +import org.apache.iotdb.db.queryengine.plan.execution.config.executor.IConfigTaskExecutor; +import org.apache.iotdb.rpc.RpcUtils; + +import com.google.common.util.concurrent.FutureCallback; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; + +import jakarta.validation.constraints.NotNull; + +/** Records the final result of a user-role modification at the concrete config task. */ +public final class UserRoleModificationAuditTask implements IConfigTask { + + private final IConfigTask delegate; + private final UserRoleModificationAuditContext auditContext; + + private UserRoleModificationAuditTask( + IConfigTask delegate, UserRoleModificationAuditContext auditContext) { + this.delegate = delegate; + this.auditContext = auditContext; + } + + public static IConfigTask wrap( + IConfigTask delegate, UserRoleModificationAuditContext auditContext) { + return auditContext.isEnabled() + ? new UserRoleModificationAuditTask(delegate, auditContext) + : delegate; + } + + @Override + public ListenableFuture execute(IConfigTaskExecutor configTaskExecutor) + throws InterruptedException { + try { + ListenableFuture future = delegate.execute(configTaskExecutor); + Futures.addCallback( + future, + new FutureCallback() { + @Override + public void onSuccess(ConfigTaskResult result) { + auditContext.log(toStatus(result)); + } + + @Override + public void onFailure(@NotNull Throwable throwable) { + auditContext.log(toStatus(throwable)); + } + }, + MoreExecutors.directExecutor()); + return future; + } catch (InterruptedException | RuntimeException | Error e) { + auditContext.log(toStatus(e)); + throw e; + } + } + + private static TSStatus toStatus(ConfigTaskResult result) { + if (result == null) { + return null; + } + if (result.getStatus() != null) { + return result.getStatus(); + } + return result.getStatusCode() == null ? null : RpcUtils.getStatus(result.getStatusCode()); + } + + private static TSStatus toStatus(Throwable throwable) { + if (throwable instanceof IoTDBException) { + return ((IoTDBException) throwable).getStatus(); + } + if (throwable instanceof IoTDBRuntimeException) { + return ((IoTDBRuntimeException) throwable).getStatus(); + } + return null; + } +} diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/Coordinator.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/Coordinator.java index ed89a6dc163ca..8919182c7ce93 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/Coordinator.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/Coordinator.java @@ -20,7 +20,6 @@ package org.apache.iotdb.db.queryengine.plan; import org.apache.iotdb.common.rpc.thrift.TEndPoint; -import org.apache.iotdb.common.rpc.thrift.TSStatus; import org.apache.iotdb.commons.client.ClientPoolFactory; import org.apache.iotdb.commons.client.IClientManager; import org.apache.iotdb.commons.client.async.AsyncDataNodeInternalServiceClient; @@ -414,10 +413,8 @@ public ExecutionResult executeForTreeModel( startTime))); return result; } finally { - DNAuditLogger auditLogger = DNAuditLogger.getInstance(); - TSStatus status = result == null ? null : result.status; - auditLogger.logRevokeFailure(statement, session, sql, status); - auditLogger.logUserRoleModification(statement, session, sql, status); + DNAuditLogger.getInstance() + .logRevokeFailure(statement, session, sql, result == null ? null : result.status); } } @@ -564,10 +561,8 @@ public ExecutionResult executeForTableModel( startTime))); return result; } finally { - DNAuditLogger auditLogger = DNAuditLogger.getInstance(); - TSStatus status = result == null ? null : result.status; - auditLogger.logRevokeFailure(statement, session, sql, status); - auditLogger.logUserRoleModification(statement, session, sql, status); + DNAuditLogger.getInstance() + .logRevokeFailure(statement, session, sql, result == null ? null : result.status); } } 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 57a2bbd447922..b431d18cb0dc2 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 @@ -54,6 +54,8 @@ import org.apache.iotdb.db.audit.DNAuditLogger; import org.apache.iotdb.db.audit.PasswordChangeAuditContext; import org.apache.iotdb.db.audit.PasswordChangeAuditTask; +import org.apache.iotdb.db.audit.UserRoleModificationAuditContext; +import org.apache.iotdb.db.audit.UserRoleModificationAuditTask; import org.apache.iotdb.db.auth.AuthorityChecker; import org.apache.iotdb.db.conf.IoTDBConfig; import org.apache.iotdb.db.i18n.DataNodeQueryMessages; @@ -1628,8 +1630,11 @@ public IConfigTask visitShowCurrentTimestamp(ShowCurrentTimestamp node, MPPQuery @Override public IConfigTask visitRelationalAuthorPlan( RelationalAuthorStatement node, MPPQueryContext context) { - PasswordChangeAuditContext auditContext = + PasswordChangeAuditContext passwordAuditContext = PasswordChangeAuditContext.forTableStatement(node, context.getSession()); + UserRoleModificationAuditContext userRoleAuditContext = + UserRoleModificationAuditContext.forTableStatement( + node, context.getSession(), context.getSql()); boolean executionDelegated = false; try { context.setQueryType(node.getQueryType()); @@ -1644,12 +1649,16 @@ public IConfigTask visitRelationalAuthorPlan( visitUpdateUser(node); } IConfigTask task = - PasswordChangeAuditTask.wrap(new RelationalAuthorizerTask(node), auditContext); + PasswordChangeAuditTask.wrap( + UserRoleModificationAuditTask.wrap( + new RelationalAuthorizerTask(node), userRoleAuditContext), + passwordAuditContext); executionDelegated = true; return task; } finally { if (!executionDelegated) { - auditContext.log(null); + passwordAuditContext.log(null); + userRoleAuditContext.log(null); } } } diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/TreeConfigTaskVisitor.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/TreeConfigTaskVisitor.java index 573cf86b2cf99..a37a8df7dfb7e 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/TreeConfigTaskVisitor.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/TreeConfigTaskVisitor.java @@ -29,6 +29,8 @@ import org.apache.iotdb.commons.pipe.config.constant.SystemConstant; import org.apache.iotdb.db.audit.PasswordChangeAuditContext; import org.apache.iotdb.db.audit.PasswordChangeAuditTask; +import org.apache.iotdb.db.audit.UserRoleModificationAuditContext; +import org.apache.iotdb.db.audit.UserRoleModificationAuditTask; import org.apache.iotdb.db.auth.AuthorityChecker; import org.apache.iotdb.db.i18n.DataNodeQueryMessages; import org.apache.iotdb.db.queryengine.common.MPPQueryContext; @@ -339,18 +341,33 @@ public IConfigTask visitTestConnection( @Override public IConfigTask visitAuthor(AuthorStatement statement, MPPQueryContext context) { - statement.setExecutedByUserId(context.getUserId()); - if (statement.getAuthorType() == AuthorType.UPDATE_USER) { - return visitUpdateUser(statement, context); - } - if (statement.getAuthorType() == AuthorType.RENAME_USER) { - visitRenameUser(statement); - } - TSStatus status = statement.checkStatementIsValid(context.getSession().getUserName()); - if (status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { - throw new AccessDeniedException(status.getMessage()); + UserRoleModificationAuditContext auditContext = + UserRoleModificationAuditContext.forTreeStatement( + statement, context.getSession(), context.getSql()); + boolean executionDelegated = false; + try { + statement.setExecutedByUserId(context.getUserId()); + if (statement.getAuthorType() == AuthorType.UPDATE_USER) { + IConfigTask task = visitUpdateUser(statement, context); + executionDelegated = true; + return task; + } + if (statement.getAuthorType() == AuthorType.RENAME_USER) { + visitRenameUser(statement); + } + TSStatus status = statement.checkStatementIsValid(context.getSession().getUserName()); + if (status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + throw new AccessDeniedException(status.getMessage()); + } + IConfigTask task = + UserRoleModificationAuditTask.wrap(new AuthorizerTask(statement), auditContext); + executionDelegated = true; + return task; + } finally { + if (!executionDelegated) { + auditContext.log(null); + } } - return new AuthorizerTask(statement); } private IConfigTask visitUpdateUser(AuthorStatement statement, MPPQueryContext context) { diff --git a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/DNAuditLoggerUserRoleModificationTest.java b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/DNAuditLoggerUserRoleModificationTest.java index 4242635b26cea..62f222de9fe62 100644 --- a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/DNAuditLoggerUserRoleModificationTest.java +++ b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/audit/DNAuditLoggerUserRoleModificationTest.java @@ -19,13 +19,17 @@ package org.apache.iotdb.db.audit; +import org.apache.iotdb.common.rpc.thrift.TSStatus; import org.apache.iotdb.commons.audit.AuditEventType; import org.apache.iotdb.commons.audit.AuditLogOperation; import org.apache.iotdb.commons.audit.IAuditEntity; import org.apache.iotdb.commons.audit.UserEntity; import org.apache.iotdb.commons.auth.entity.PrivilegeType; +import org.apache.iotdb.commons.exception.IoTDBException; import org.apache.iotdb.commons.queryengine.common.SessionInfo; import org.apache.iotdb.commons.queryengine.common.SqlDialect; +import org.apache.iotdb.db.queryengine.plan.execution.config.ConfigTaskResult; +import org.apache.iotdb.db.queryengine.plan.execution.config.IConfigTask; import org.apache.iotdb.db.queryengine.plan.relational.security.TreeAccessCheckContext; import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.RelationalAuthorStatement; import org.apache.iotdb.db.queryengine.plan.relational.type.AuthorRType; @@ -34,6 +38,7 @@ import org.apache.iotdb.rpc.RpcUtils; import org.apache.iotdb.rpc.TSStatusCode; +import com.google.common.util.concurrent.SettableFuture; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -42,6 +47,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.CALLS_REAL_METHODS; @@ -160,6 +166,115 @@ public void testMissingSessionIsIgnored() { verify(auditLogger, never()).log(any(), any()); } + @Test + public void testConcreteTreeConfigTaskRecordsSuccess() throws Exception { + UserRoleModificationAuditContext.AuditLogWriter auditLogWriter = + mock(UserRoleModificationAuditContext.AuditLogWriter.class); + UserRoleModificationAuditContext context = + UserRoleModificationAuditContext.forTreeStatement( + treeStatement(AuthorType.GRANT_USER_ROLE), sessionInfo(), auditLogWriter); + SettableFuture future = SettableFuture.create(); + IConfigTask task = UserRoleModificationAuditTask.wrap(ignored -> future, context); + + assertSame(future, task.execute(null)); + future.set(new ConfigTaskResult(TSStatusCode.SUCCESS_STATUS)); + + assertAuditStatus(auditLogWriter, TSStatusCode.SUCCESS_STATUS); + } + + @Test + public void testConcreteTableConfigTaskRecordsFailure() throws Exception { + UserRoleModificationAuditContext.AuditLogWriter auditLogWriter = + mock(UserRoleModificationAuditContext.AuditLogWriter.class); + UserRoleModificationAuditContext context = + UserRoleModificationAuditContext.forTableStatement( + tableStatement(AuthorRType.REVOKE_USER_ROLE), sessionInfo(), auditLogWriter); + SettableFuture future = SettableFuture.create(); + IConfigTask task = UserRoleModificationAuditTask.wrap(ignored -> future, context); + + task.execute(null); + future.set(new ConfigTaskResult(RpcUtils.getStatus(TSStatusCode.USER_NOT_HAS_ROLE))); + + assertAuditStatus(auditLogWriter, TSStatusCode.USER_NOT_HAS_ROLE); + } + + @Test + public void testConcreteConfigTaskRecordsUnexpectedFailure() throws Exception { + UserRoleModificationAuditContext.AuditLogWriter auditLogWriter = + mock(UserRoleModificationAuditContext.AuditLogWriter.class); + UserRoleModificationAuditContext context = + UserRoleModificationAuditContext.forTreeStatement( + treeStatement(AuthorType.GRANT_USER_ROLE), sessionInfo(), auditLogWriter); + SettableFuture future = SettableFuture.create(); + IConfigTask task = UserRoleModificationAuditTask.wrap(ignored -> future, context); + + task.execute(null); + future.setException(new RuntimeException()); + + verify(auditLogWriter).log(null); + } + + @Test + public void testConcreteConfigTaskIgnoresRedirectResult() throws Exception { + UserRoleModificationAuditContext.AuditLogWriter auditLogWriter = + mock(UserRoleModificationAuditContext.AuditLogWriter.class); + UserRoleModificationAuditContext context = + UserRoleModificationAuditContext.forTreeStatement( + treeStatement(AuthorType.GRANT_USER_ROLE), sessionInfo(), auditLogWriter); + SettableFuture future = SettableFuture.create(); + IConfigTask task = UserRoleModificationAuditTask.wrap(ignored -> future, context); + + task.execute(null); + future.set(new ConfigTaskResult(TSStatusCode.REDIRECTION_RECOMMEND)); + + verify(auditLogWriter, never()).log(any()); + } + + @Test + public void testConcreteConfigTaskIgnoresRedirectException() throws Exception { + UserRoleModificationAuditContext.AuditLogWriter auditLogWriter = + mock(UserRoleModificationAuditContext.AuditLogWriter.class); + UserRoleModificationAuditContext context = + UserRoleModificationAuditContext.forTreeStatement( + treeStatement(AuthorType.GRANT_USER_ROLE), sessionInfo(), auditLogWriter); + SettableFuture future = SettableFuture.create(); + IConfigTask task = UserRoleModificationAuditTask.wrap(ignored -> future, context); + + task.execute(null); + future.setException(new IoTDBException(RpcUtils.getStatus(TSStatusCode.REDIRECTION_RECOMMEND))); + + verify(auditLogWriter, never()).log(any()); + } + + @Test + public void testUserRoleAuditContextRecordsOnlyOnce() { + UserRoleModificationAuditContext.AuditLogWriter auditLogWriter = + mock(UserRoleModificationAuditContext.AuditLogWriter.class); + UserRoleModificationAuditContext context = + UserRoleModificationAuditContext.forTreeStatement( + treeStatement(AuthorType.GRANT_USER_ROLE), sessionInfo(), auditLogWriter); + + context.log(RpcUtils.SUCCESS_STATUS); + context.log(RpcUtils.getStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR)); + + assertAuditStatus(auditLogWriter, TSStatusCode.SUCCESS_STATUS); + } + + @Test + public void testPrivilegeGrantDoesNotWrapConcreteTask() { + UserRoleModificationAuditContext.AuditLogWriter auditLogWriter = + mock(UserRoleModificationAuditContext.AuditLogWriter.class); + UserRoleModificationAuditContext context = + UserRoleModificationAuditContext.forTreeStatement( + treeStatement(AuthorType.GRANT_USER), sessionInfo(), auditLogWriter); + IConfigTask delegate = ignored -> SettableFuture.create(); + + assertSame(delegate, UserRoleModificationAuditTask.wrap(delegate, context)); + context.log(RpcUtils.SUCCESS_STATUS); + + verify(auditLogWriter, never()).log(any()); + } + private static AuthorStatement treeStatement(AuthorType type) { AuthorStatement statement = new AuthorStatement(type); statement.setUserName("user1"); @@ -189,6 +304,14 @@ private static SessionInfo sessionInfo() { SqlDialect.TABLE); } + private static void assertAuditStatus( + UserRoleModificationAuditContext.AuditLogWriter auditLogWriter, + TSStatusCode expectedStatusCode) { + ArgumentCaptor statusCaptor = ArgumentCaptor.forClass(TSStatus.class); + verify(auditLogWriter).log(statusCaptor.capture()); + assertEquals(expectedStatusCode.getStatusCode(), statusCaptor.getValue().getCode()); + } + @SuppressWarnings("unchecked") private static void assertAuditLog(DNAuditLogger auditLogger, boolean result, String sql) { ArgumentCaptor entityCaptor = ArgumentCaptor.forClass(IAuditEntity.class); @@ -199,7 +322,7 @@ private static void assertAuditLog(DNAuditLogger auditLogger, boolean result, St assertEquals(7L, entity.getUserId()); assertEquals("operator", entity.getUsername()); assertEquals("127.0.0.1", entity.getCliHostname()); - assertEquals(AuditEventType.MODIFY_USER_ROLE, entity.getAuditEventType()); + assertEquals(AuditEventType.MODIFY_ROLE_MEMBERSHIP, entity.getAuditEventType()); assertEquals(AuditLogOperation.CONTROL, entity.getAuditLogOperation()); assertEquals(PrivilegeType.SECURITY, entity.getPrivilegeTypes().get(0)); if (result) { diff --git a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/audit/AuditEventType.java b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/audit/AuditEventType.java index 4d8eb9c550e22..58e6f752b6096 100644 --- a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/audit/AuditEventType.java +++ b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/audit/AuditEventType.java @@ -37,7 +37,7 @@ public enum AuditEventType { LOGIN_FINAL, MODIFY_SECURITY_OPTIONS, MODIFY_DEFAULT_SECURITY_VALUES, - MODIFY_USER_ROLE, + MODIFY_ROLE_MEMBERSHIP, REVOKE_FAILED, GRANT_ROLE_FAILED, LOGIN_RESOURCE_RESTRICT,