From a91be8deac20de89895890a6f3a8b4806c18db58 Mon Sep 17 00:00:00 2001 From: heguanhui Date: Mon, 25 May 2026 22:12:09 +0800 Subject: [PATCH] [fix](fe) Add user existence check in DropRowPolicyCommand ### What problem does this PR solve? Issue Number: close #63322 Problem Summary: DropRowPolicyCommand does not validate whether the user specified in the DROP ROW POLICY statement actually exists. This is inconsistent with CreatePolicyCommand, which checks user existence via doesUserExist(). Without this validation, a typo in the user name could silently match no policy, or the user might mistakenly believe the drop succeeded. Also, the auth check should be performed before user existence check to prevent information leakage. ### Release note DROP ROW POLICY now validates that the specified user exists, returning an error if it does not. This makes the behavior consistent with CREATE ROW POLICY. ### Check List (For Author) - Test: Unit Test, Regression test - Added testValidateUserNotExist in DropRowPolicyCommandTest - Added regression test in test_row_policy.groovy - Behavior changed: Yes. DROP ROW POLICY now throws AnalysisException when the specified user does not exist, whereas previously it would proceed without validation. - Does this need documentation: No --- .../plans/commands/DropRowPolicyCommand.java | 11 +++++---- .../commands/DropRowPolicyCommandTest.java | 23 +++++++++++++++++++ .../suites/query_p0/test_row_policy.groovy | 7 ++++++ 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropRowPolicyCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropRowPolicyCommand.java index 4ca1a65b4deeda..ebbdcd5f6884d5 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropRowPolicyCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropRowPolicyCommand.java @@ -70,16 +70,19 @@ public void doRun(ConnectContext ctx, StmtExecutor executor) throws Exception { * validate */ public void validate(ConnectContext ctx) throws AnalysisException { - tableNameInfo.analyze(ctx.getNameSpaceContext()); - if (user != null) { - user.analyze(); - } // check auth if (!Env.getCurrentEnv().getAccessManager() .checkGlobalPriv(ConnectContext.get(), PrivPredicate.GRANT)) { ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, PrivPredicate.GRANT.getPrivs().toString()); } + tableNameInfo.analyze(ctx.getNameSpaceContext()); + if (user != null) { + user.analyze(); + if (!Env.getCurrentEnv().getAuth().doesUserExist(user)) { + throw new AnalysisException("user not exist: " + user); + } + } } public boolean isIfExists() { diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/DropRowPolicyCommandTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/DropRowPolicyCommandTest.java index bde4a2fff0695e..fb8ce52bed03ec 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/DropRowPolicyCommandTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/DropRowPolicyCommandTest.java @@ -20,8 +20,10 @@ import org.apache.doris.analysis.UserIdentity; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.info.TableNameInfo; +import org.apache.doris.common.AnalysisException; import org.apache.doris.common.jmockit.Deencapsulation; import org.apache.doris.mysql.privilege.AccessControllerManager; +import org.apache.doris.mysql.privilege.Auth; import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.qe.ConnectContext; import org.apache.doris.utframe.TestWithFeService; @@ -36,6 +38,7 @@ public class DropRowPolicyCommandTest extends TestWithFeService { private ConnectContext connectContext; private Env env; private AccessControllerManager accessControllerManager; + private Auth auth; private UserIdentity user; private TableNameInfo tableNameInfo; @@ -43,6 +46,7 @@ private void runBefore() throws IOException { connectContext = createDefaultCtx(); env = Env.getCurrentEnv(); accessControllerManager = env.getAccessManager(); + auth = env.getAuth(); user = new UserIdentity("jack", "127.0.0.1", true); tableNameInfo = new TableNameInfo("test_db", "test_tbl"); } @@ -54,7 +58,26 @@ public void testValidateNormal() throws Exception { Mockito.doReturn(true).when(spyAcm).checkGlobalPriv( Mockito.nullable(ConnectContext.class), Mockito.eq(PrivPredicate.GRANT)); Deencapsulation.setField(env, "accessManager", spyAcm); + Auth spyAuth = Mockito.spy(auth); + Mockito.doReturn(true).when(spyAuth).doesUserExist(Mockito.any(UserIdentity.class)); + Deencapsulation.setField(env, "auth", spyAuth); DropRowPolicyCommand command = new DropRowPolicyCommand(false, "test_policy", tableNameInfo, user, "role1"); Assertions.assertDoesNotThrow(() -> command.validate(connectContext)); } + + @Test + public void testValidateUserNotExist() throws Exception { + runBefore(); + AccessControllerManager spyAcm = Mockito.spy(accessControllerManager); + Mockito.doReturn(true).when(spyAcm).checkGlobalPriv( + Mockito.nullable(ConnectContext.class), Mockito.eq(PrivPredicate.GRANT)); + Deencapsulation.setField(env, "accessManager", spyAcm); + Auth spyAuth = Mockito.spy(auth); + Mockito.doReturn(false).when(spyAuth).doesUserExist(Mockito.any(UserIdentity.class)); + Deencapsulation.setField(env, "auth", spyAuth); + DropRowPolicyCommand command = new DropRowPolicyCommand(false, "test_policy", tableNameInfo, user, null); + AnalysisException ex = Assertions.assertThrows(AnalysisException.class, + () -> command.validate(connectContext)); + Assertions.assertTrue(ex.getMessage().contains("user not exist")); + } } diff --git a/regression-test/suites/query_p0/test_row_policy.groovy b/regression-test/suites/query_p0/test_row_policy.groovy index 6bd7b8a4632a1a..cac25b23d304b8 100644 --- a/regression-test/suites/query_p0/test_row_policy.groovy +++ b/regression-test/suites/query_p0/test_row_policy.groovy @@ -90,4 +90,11 @@ suite("test_row_policy") { """ exception "system user" } + + test { + sql """ + DROP ROW POLICY policy_01 ON ${tableName} FOR non_exist_user + """ + exception "non_exist_user" + } } \ No newline at end of file