Skip to content

Commit 2a7e7f7

Browse files
seawindeCopilot
andcommitted
[fix](auth) Fix test failures in CTE privilege check tests
### What problem does this PR solve? Issue Number: N/A Related PR: apache#62339 Problem Summary: Fix two test failures introduced in the CTE privilege bypass fix: 1. **test_cte_privilege_check.groovy**: The regression test user lacked database-level access to `regression_test`, causing `connect()` to fail before any CTE query was executed. Added `grant select_priv on regression_test` to the test setup. 2. **TestCheckPrivileges.testCtePrivilegeCheck**: The new test method called `useUser("test_cte_privilege_user")` on the shared `connectContext` but never restored it. When `testPrivilegesAndPolicies` ran afterward, the unprivileged user caused `createCatalog` to throw `AnalysisException` instead of the expected `DdlException`. Wrapped the test body in try-finally to restore root user. ### Release note None ### Check List (For Author) - Test: Unit Test / Regression test fix - Behavior changed: No - Does this need documentation: No Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 455ff06 commit 2a7e7f7

2 files changed

Lines changed: 40 additions & 35 deletions

File tree

fe/fe-core/src/test/java/org/apache/doris/nereids/privileges/TestCheckPrivileges.java

Lines changed: 39 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -254,43 +254,47 @@ public void testCtePrivilegeCheck() throws Exception {
254254
addUser(user, true);
255255
useUser(user);
256256

257-
List<MakeTablePrivileges> privileges = ImmutableList.of(
258-
MakePrivileges.table("internal", cteDb, "allowed_tbl").allowSelectTable(user)
259-
);
260-
261-
AccessControllerManager accessManager = Env.getCurrentEnv().getAccessManager();
262-
CatalogAccessController catalogAccessController = accessManager.getAccessControllerOrDefault(catalog);
263-
new Expectations(accessManager) {
264-
{
265-
accessManager.getAccessControllerOrDefault("internal");
266-
minTimes = 0;
267-
result = catalogAccessController;
268-
}
269-
};
270-
271-
withPrivileges(privileges, () -> {
272-
// CTE with authorized table should succeed
273-
query("with cte as (select * from allowed_tbl) select * from cte");
274-
275-
// CTE with unauthorized table should fail (this was the bug:
276-
// consumer was checked first, setting privChecked=true on shared StatementContext,
277-
// so producer's CheckPrivileges was skipped)
278-
Assertions.assertThrows(AnalysisException.class, () ->
279-
query("with cte as (select * from denied_tbl) select * from cte")
280-
);
281-
282-
// CTE referenced twice forces no-inline path through RewriteCteChildren
283-
Assertions.assertThrows(AnalysisException.class, () ->
284-
query("with cte as (select * from denied_tbl) "
285-
+ "select * from cte union all select * from cte")
257+
try {
258+
List<MakeTablePrivileges> privileges = ImmutableList.of(
259+
MakePrivileges.table("internal", cteDb, "allowed_tbl").allowSelectTable(user)
286260
);
287261

288-
// CTE authorized + direct unauthorized table should fail
289-
Assertions.assertThrows(AnalysisException.class, () ->
290-
query("with cte as (select * from allowed_tbl) "
291-
+ "select * from cte union all select * from denied_tbl")
292-
);
293-
});
262+
AccessControllerManager accessManager = Env.getCurrentEnv().getAccessManager();
263+
CatalogAccessController catalogAccessController = accessManager.getAccessControllerOrDefault(catalog);
264+
new Expectations(accessManager) {
265+
{
266+
accessManager.getAccessControllerOrDefault("internal");
267+
minTimes = 0;
268+
result = catalogAccessController;
269+
}
270+
};
271+
272+
withPrivileges(privileges, () -> {
273+
// CTE with authorized table should succeed
274+
query("with cte as (select * from allowed_tbl) select * from cte");
275+
276+
// CTE with unauthorized table should fail (this was the bug:
277+
// consumer was checked first, setting privChecked=true on shared StatementContext,
278+
// so producer's CheckPrivileges was skipped)
279+
Assertions.assertThrows(AnalysisException.class, () ->
280+
query("with cte as (select * from denied_tbl) select * from cte")
281+
);
282+
283+
// CTE referenced twice forces no-inline path through RewriteCteChildren
284+
Assertions.assertThrows(AnalysisException.class, () ->
285+
query("with cte as (select * from denied_tbl) "
286+
+ "select * from cte union all select * from cte")
287+
);
288+
289+
// CTE authorized + direct unauthorized table should fail
290+
Assertions.assertThrows(AnalysisException.class, () ->
291+
query("with cte as (select * from allowed_tbl) "
292+
+ "select * from cte union all select * from denied_tbl")
293+
);
294+
});
295+
} finally {
296+
useUser("root");
297+
}
294298
}
295299

296300
private void query(String sql) {

regression-test/suites/auth_p0/test_cte_privilege_check.groovy

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ suite("test_cte_privilege_check","p0,auth") {
6969
sql """insert into ${dbName}.${tableName2} values (3, 'charlie'), (4, 'dave')"""
7070

7171
// Grant select only on tableName1, NOT on tableName2
72+
sql """grant select_priv on regression_test to ${user}"""
7273
sql """grant select_priv on ${dbName}.${tableName1} to ${user}"""
7374

7475
// Case 1: CTE references unauthorized table -> should fail

0 commit comments

Comments
 (0)