This is an automated email from the ASF dual-hosted git repository. JackieTien97 pushed a commit to branch rc/2.0.10 in repository https://gitbox.apache.org/repos/asf/iotdb.git
commit 540c00891345834bae1f07e7c945412e21700bea Author: Caideyipi <[email protected]> AuthorDate: Thu Jun 4 18:23:07 2026 +0800 Fix account unlock propagation for pipe password checks (#17814) * Fix account unlock propagation for pipe password checks * Fix account unlock user validation --- .../treemodel/manual/IoTDBPipePermissionIT.java | 1 + .../consensus/request/ConfigPhysicalPlan.java | 2 + .../consensus/request/ConfigPhysicalPlanType.java | 1 + .../request/ConfigPhysicalPlanVisitor.java | 12 ++++ .../confignode/persistence/auth/AuthorInfo.java | 6 +- .../persistence/auth/AuthorPlanExecutor.java | 12 ++++ .../persistence/executor/ConfigPlanExecutor.java | 1 + .../impl/sync/AuthOperationProcedure.java | 6 ++ .../persistence/auth/AuthorPlanExecutorTest.java | 80 ++++++++++++++++++++++ .../impl/sync/AuthOperationProcedureTest.java | 56 +++++++++++++++ .../iotdb/db/auth/ClusterAuthorityFetcher.java | 35 +++------- .../impl/DataNodeInternalRPCServiceImpl.java | 14 ++++ 12 files changed, 201 insertions(+), 25 deletions(-) diff --git a/integration-test/src/test/java/org/apache/iotdb/pipe/it/dual/treemodel/manual/IoTDBPipePermissionIT.java b/integration-test/src/test/java/org/apache/iotdb/pipe/it/dual/treemodel/manual/IoTDBPipePermissionIT.java index d43bb6564b1..23d0b48d440 100644 --- a/integration-test/src/test/java/org/apache/iotdb/pipe/it/dual/treemodel/manual/IoTDBPipePermissionIT.java +++ b/integration-test/src/test/java/org/apache/iotdb/pipe/it/dual/treemodel/manual/IoTDBPipePermissionIT.java @@ -595,6 +595,7 @@ public class IoTDBPipePermissionIT extends AbstractPipeDualTreeModelManualIT { try { statement.execute("alter pipe a2b modify source ('password'='fake')"); + fail(); } catch (final SQLException e) { Assert.assertEquals("801: Failed to check password for pipe a2b.", e.getMessage()); } diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlan.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlan.java index 1ad547f7845..72e1fb36735 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlan.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlan.java @@ -311,6 +311,7 @@ public abstract class ConfigPhysicalPlan implements IConsensusRequest { case UpdateUserV2: case CreateUserWithRawPassword: case RenameUser: + case AccountUnlock: plan = new AuthorTreePlan(configPhysicalPlanType); break; case RCreateUser: @@ -343,6 +344,7 @@ public abstract class ConfigPhysicalPlan implements IConsensusRequest { case RRevokeUserSysPri: case RRevokeRoleSysPri: case RRenameUser: + case RAccountUnlock: plan = new AuthorRelationalPlan(configPhysicalPlanType); break; case ApplyConfigNode: diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanType.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanType.java index 83bb96bee16..d81028611b0 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanType.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanType.java @@ -333,6 +333,7 @@ public enum ConfigPhysicalPlanType { RDropUserV2((short) 2103), RenameUser((short) 2104), RRenameUser((short) 2105), + AccountUnlock((short) 2106), EnableSeparationOfAdminPowers((short) 2200), diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanVisitor.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanVisitor.java index 8363f023bc4..53e3c4cd37d 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanVisitor.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanVisitor.java @@ -104,6 +104,8 @@ public abstract class ConfigPhysicalPlanVisitor<R, C> { return visitGrantRoleToUser((AuthorTreePlan) plan, context); case RevokeRoleFromUser: return visitRevokeRoleFromUser((AuthorTreePlan) plan, context); + case AccountUnlock: + return visitAccountUnlock((AuthorTreePlan) plan, context); case RCreateUser: return visitRCreateUser((AuthorRelationalPlan) plan, context); case RCreateRole: @@ -160,6 +162,8 @@ public abstract class ConfigPhysicalPlanVisitor<R, C> { return visitRRevokeUserSysPrivilege((AuthorRelationalPlan) plan, context); case RRevokeRoleSysPri: return visitRRevokeRoleSysPrivilege((AuthorRelationalPlan) plan, context); + case RAccountUnlock: + return visitRAccountUnlock((AuthorRelationalPlan) plan, context); case SetTTL: return visitTTL((SetTTLPlan) plan, context); case PipeCreateTableOrView: @@ -310,6 +314,10 @@ public abstract class ConfigPhysicalPlanVisitor<R, C> { return visitPlan(revokeRoleFromUserPlan, context); } + public R visitAccountUnlock(final AuthorTreePlan accountUnlockPlan, final C context) { + return visitPlan(accountUnlockPlan, context); + } + public R visitRCreateUser(final AuthorRelationalPlan rCreateUserPlan, final C context) { return visitPlan(rCreateUserPlan, context); } @@ -426,6 +434,10 @@ public abstract class ConfigPhysicalPlanVisitor<R, C> { return visitPlan(rRevokeRoleSysPrivilegePlan, context); } + public R visitRAccountUnlock(final AuthorRelationalPlan rAccountUnlockPlan, final C context) { + return visitPlan(rAccountUnlockPlan, context); + } + public R visitTTL(final SetTTLPlan setTTLPlan, final C context) { return visitPlan(setTTLPlan, context); } diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/auth/AuthorInfo.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/auth/AuthorInfo.java index bb0fd51c9fb..de9441acc04 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/auth/AuthorInfo.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/auth/AuthorInfo.java @@ -74,7 +74,9 @@ public class AuthorInfo implements SnapshotProcessor { throw new IndexOutOfBoundsException(ConfigNodeMessages.INVALID_AUTHOR_TYPE_ORDINAL); } ConfigPhysicalPlanType configPhysicalPlanType; - if (authorType >= AuthorType.RENAME_USER.ordinal()) { + if (authorType == AuthorType.ACCOUNT_UNLOCK.ordinal()) { + return ConfigPhysicalPlanType.AccountUnlock; + } else if (authorType >= AuthorType.RENAME_USER.ordinal()) { AuthorType type = AuthorType.values()[authorType]; switch (type) { case RENAME_USER: @@ -105,6 +107,8 @@ public class AuthorInfo implements SnapshotProcessor { ConfigPhysicalPlanType configPhysicalPlanType; if (authorRType == AuthorRType.RENAME_USER.ordinal()) { configPhysicalPlanType = ConfigPhysicalPlanType.RRenameUser; + } else if (authorRType == AuthorRType.ACCOUNT_UNLOCK.ordinal()) { + configPhysicalPlanType = ConfigPhysicalPlanType.RAccountUnlock; } else { configPhysicalPlanType = ConfigPhysicalPlanType.values()[ diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/auth/AuthorPlanExecutor.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/auth/AuthorPlanExecutor.java index 7de717d9c3e..a65e05c8ad7 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/auth/AuthorPlanExecutor.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/auth/AuthorPlanExecutor.java @@ -124,6 +124,9 @@ public class AuthorPlanExecutor implements IAuthorPlanExecutor { case RenameUser: authorizer.renameUser(userName, newUsername); break; + case AccountUnlock: + checkUserExistsForAccountUnlock(userName); + break; case CreateUser: authorizer.createUser(userName, password); break; @@ -241,6 +244,7 @@ public class AuthorPlanExecutor implements IAuthorPlanExecutor { authorizer.renameUser(userName, newUsername); break; case RAccountUnlock: + checkUserExistsForAccountUnlock(userName); break; case RDropRole: authorizer.deleteRole(roleName); @@ -448,6 +452,14 @@ public class AuthorPlanExecutor implements IAuthorPlanExecutor { return RpcUtils.getStatus(TSStatusCode.SUCCESS_STATUS); } + private void checkUserExistsForAccountUnlock(final String userName) throws AuthException { + // Account unlock has no persistent ConfigNode auth state change, but the write path needs this + // validation before broadcasting DataNode unlocks and propagating through pipe. + if (authorizer.getUser(userName) == null) { + throw new AuthException(TSStatusCode.USER_NOT_EXIST, NO_USER_MSG + userName); + } + } + @Override public PermissionInfoResp executeListUsers(final AuthorPlan plan) throws AuthException { final PermissionInfoResp result = new PermissionInfoResp(); diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/executor/ConfigPlanExecutor.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/executor/ConfigPlanExecutor.java index eb8d5e5538b..1f2729bff39 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/executor/ConfigPlanExecutor.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/executor/ConfigPlanExecutor.java @@ -488,6 +488,7 @@ public class ConfigPlanExecutor { case RevokeRoleFromUserDep: case UpdateUserDep: case RenameUser: + case AccountUnlock: case RCreateRole: case RCreateUser: case RDropUser: diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/sync/AuthOperationProcedure.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/sync/AuthOperationProcedure.java index 90112675253..59e4f50b40b 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/sync/AuthOperationProcedure.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/sync/AuthOperationProcedure.java @@ -28,6 +28,7 @@ import org.apache.iotdb.commons.utils.ThriftCommonsSerDeUtils; import org.apache.iotdb.confignode.client.sync.CnToDnSyncRequestType; import org.apache.iotdb.confignode.client.sync.SyncDataNodeClientPool; import org.apache.iotdb.confignode.consensus.request.ConfigPhysicalPlan; +import org.apache.iotdb.confignode.consensus.request.ConfigPhysicalPlanType; import org.apache.iotdb.confignode.consensus.request.write.auth.AuthorPlan; import org.apache.iotdb.confignode.consensus.request.write.pipe.payload.PipeEnrichedPlan; import org.apache.iotdb.confignode.i18n.ProcedureMessages; @@ -100,6 +101,11 @@ public class AuthOperationProcedure extends AbstractNodeProcedure<AuthOperationP TSStatus status; req.setUsername(user); req.setRoleName(role); + if (plan.getAuthorType() == ConfigPhysicalPlanType.AccountUnlock + || plan.getAuthorType() == ConfigPhysicalPlanType.RAccountUnlock) { + // For account unlock, role carries the optional login address. + req.setNeedDisconnect(true); + } Iterator<Pair<TDataNodeConfiguration, Long>> it = dataNodesToInvalid.iterator(); while (it.hasNext()) { Pair<TDataNodeConfiguration, Long> pair = it.next(); diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/persistence/auth/AuthorPlanExecutorTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/persistence/auth/AuthorPlanExecutorTest.java new file mode 100644 index 00000000000..4daa94dd53f --- /dev/null +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/persistence/auth/AuthorPlanExecutorTest.java @@ -0,0 +1,80 @@ +/* + * 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.confignode.persistence.auth; + +import org.apache.iotdb.common.rpc.thrift.TSStatus; +import org.apache.iotdb.commons.auth.authorizer.IAuthorizer; +import org.apache.iotdb.confignode.consensus.request.ConfigPhysicalPlanType; +import org.apache.iotdb.confignode.consensus.request.write.auth.AuthorRelationalPlan; +import org.apache.iotdb.confignode.consensus.request.write.auth.AuthorTreePlan; +import org.apache.iotdb.rpc.TSStatusCode; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.Collections; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class AuthorPlanExecutorTest { + + @Test + public void testAccountUnlockRequiresExistingUser() throws Exception { + final IAuthorizer authorizer = mock(IAuthorizer.class); + when(authorizer.getUser("missing")).thenReturn(null); + + final AuthorPlanExecutor executor = new AuthorPlanExecutor(authorizer); + final TSStatus status = + executor.executeAuthorNonQuery( + new AuthorTreePlan( + ConfigPhysicalPlanType.AccountUnlock, + "missing", + "", + "", + "", + Collections.emptySet(), + false, + Collections.emptyList())); + + Assert.assertEquals(TSStatusCode.USER_NOT_EXIST.getStatusCode(), status.getCode()); + } + + @Test + public void testRAccountUnlockRequiresExistingUser() throws Exception { + final IAuthorizer authorizer = mock(IAuthorizer.class); + when(authorizer.getUser("missing")).thenReturn(null); + + final AuthorPlanExecutor executor = new AuthorPlanExecutor(authorizer); + final TSStatus status = + executor.executeRelationalAuthorNonQuery( + new AuthorRelationalPlan( + ConfigPhysicalPlanType.RAccountUnlock, + "missing", + "", + "", + "", + Collections.emptySet(), + false, + "")); + + Assert.assertEquals(TSStatusCode.USER_NOT_EXIST.getStatusCode(), status.getCode()); + } +} diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/impl/sync/AuthOperationProcedureTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/impl/sync/AuthOperationProcedureTest.java index 60fdb3bd981..56479669e5b 100644 --- a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/impl/sync/AuthOperationProcedureTest.java +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/impl/sync/AuthOperationProcedureTest.java @@ -98,6 +98,34 @@ public class AuthOperationProcedureTest { fail(); } + try { + final AuthOperationProcedure proc = + new AuthOperationProcedure( + new AuthorTreePlan( + ConfigPhysicalPlanType.AccountUnlock, + "user1", + "", + "", + "", + Collections.emptySet(), + false, + Collections.emptyList()), + datanodes, + false); + proc.serialize(outputStream); + final ByteBuffer buffer = + ByteBuffer.wrap(byteArrayOutputStream.getBuf(), 0, byteArrayOutputStream.size()); + + final AuthOperationProcedure proc2 = + (AuthOperationProcedure) ProcedureFactory.getInstance().create(buffer); + Assert.assertEquals(proc, proc2); + buffer.clear(); + byteArrayOutputStream.reset(); + } catch (final Exception e) { + e.printStackTrace(); + fail(); + } + try { final int begin = ConfigPhysicalPlanType.RCreateUser.ordinal(); final int end = ConfigPhysicalPlanType.RRevokeRoleSysPri.ordinal(); @@ -129,5 +157,33 @@ public class AuthOperationProcedureTest { e.printStackTrace(); fail(); } + + try { + final AuthOperationProcedure proc = + new AuthOperationProcedure( + new AuthorRelationalPlan( + ConfigPhysicalPlanType.RAccountUnlock, + "user1", + "127.0.0.1", + "", + "", + Collections.emptySet(), + false, + ""), + datanodes, + false); + proc.serialize(outputStream); + final ByteBuffer buffer = + ByteBuffer.wrap(byteArrayOutputStream.getBuf(), 0, byteArrayOutputStream.size()); + + final AuthOperationProcedure proc2 = + (AuthOperationProcedure) ProcedureFactory.getInstance().create(buffer); + Assert.assertEquals(proc, proc2); + buffer.clear(); + byteArrayOutputStream.reset(); + } catch (final Exception e) { + e.printStackTrace(); + fail(); + } } } diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/ClusterAuthorityFetcher.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/ClusterAuthorityFetcher.java index 9ee232c921b..641ead173d2 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/ClusterAuthorityFetcher.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/ClusterAuthorityFetcher.java @@ -424,44 +424,31 @@ public class ClusterAuthorityFetcher implements IAuthorityFetcher { @Override public SettableFuture<ConfigTaskResult> operatePermission(AuthorStatement authorStatement) { - return handleAccountUnlock( - authorStatement, - authorStatement.getUserName(), - false, - () -> onOperatePermissionSuccess(authorStatement)); + return handleAccountUnlock(authorStatement, false); } @Override public SettableFuture<ConfigTaskResult> operatePermission( RelationalAuthorStatement authorStatement) { - return handleAccountUnlock( - authorStatement, - authorStatement.getUserName(), - true, - () -> onOperatePermissionSuccess(authorStatement)); + return handleAccountUnlock(authorStatement, true); } private SettableFuture<ConfigTaskResult> handleAccountUnlock( - Object authorStatement, String username, boolean isRelational, Runnable successCallback) { + Object authorStatement, boolean isRelational) { if (isUnlockStatement(authorStatement, isRelational)) { - final SettableFuture<ConfigTaskResult> future = SettableFuture.create(); - final User user; - try { - user = getUser(username, false); - } catch (final IoTDBRuntimeException e) { - future.setException(e); - return future; - } String loginAddr = isRelational ? ((RelationalAuthorStatement) authorStatement).getLoginAddr() : ((AuthorStatement) authorStatement).getLoginAddr(); - LoginLockManager.getInstance().unlock(user.getUserId(), loginAddr); - successCallback.run(); - future.set(new ConfigTaskResult(TSStatusCode.SUCCESS_STATUS)); - return future; + // Reuse roleName to carry the optional login address for the internal unlock broadcast. + if (isRelational) { + ((RelationalAuthorStatement) authorStatement).setRoleName(loginAddr); + } else { + ((AuthorStatement) authorStatement).setRoleName(loginAddr); + } + return operatePermissionInternal(authorStatement, isRelational); } return operatePermissionInternal(authorStatement, isRelational); } @@ -748,7 +735,7 @@ public class ClusterAuthorityFetcher implements IAuthorityFetcher { private TAuthorizerReq statementToAuthorizerReq(AuthorStatement authorStatement) throws AuthException { - if (authorStatement.getAuthorType() == null) { + if (authorStatement.getNodeNameList() == null) { authorStatement.setNodeNameList(new ArrayList<>()); } return new TAuthorizerReq( diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImpl.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImpl.java index ebef68b0486..f7f2f13b14c 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImpl.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImpl.java @@ -109,6 +109,7 @@ import org.apache.iotdb.consensus.exception.ConsensusGroupAlreadyExistException; import org.apache.iotdb.consensus.exception.ConsensusGroupNotExistException; import org.apache.iotdb.db.audit.DNAuditLogger; import org.apache.iotdb.db.auth.AuthorityChecker; +import org.apache.iotdb.db.auth.LoginLockManager; import org.apache.iotdb.db.conf.IoTDBDescriptor; import org.apache.iotdb.db.consensus.DataRegionConsensusImpl; import org.apache.iotdb.db.consensus.SchemaRegionConsensusImpl; @@ -2453,12 +2454,25 @@ public class DataNodeInternalRPCServiceImpl implements IDataNodeRPCService.Iface @Override public TSStatus invalidatePermissionCache(TInvalidatePermissionCacheReq req) { + if (req.isSetNeedDisconnect() && req.isNeedDisconnect()) { + return unlockAccountAndInvalidateCache(req); + } if (AuthorityChecker.invalidateCache(req.getUsername(), req.getRoleName())) { return RpcUtils.getStatus(TSStatusCode.SUCCESS_STATUS); } return RpcUtils.getStatus(TSStatusCode.CLEAR_PERMISSION_CACHE_ERROR); } + private TSStatus unlockAccountAndInvalidateCache(TInvalidatePermissionCacheReq req) { + // For account-unlock broadcasts, roleName carries the optional login address. + AuthorityChecker.getUserId(req.getUsername()) + .ifPresent(userId -> LoginLockManager.getInstance().unlock(userId, req.getRoleName())); + if (AuthorityChecker.invalidateCache(req.getUsername(), null)) { + return RpcUtils.getStatus(TSStatusCode.SUCCESS_STATUS); + } + return RpcUtils.getStatus(TSStatusCode.CLEAR_PERMISSION_CACHE_ERROR); + } + @Override public TSStatus enableSeparationOfAdminPower() throws TException { return null;
