Skip to content

Commit a205c6c

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: #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 f0c436f commit a205c6c

File tree

2 files changed

+40
-35
lines changed

2 files changed

+40
-35
lines changed

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
@@ -256,43 +256,47 @@ public void testCtePrivilegeCheck() throws Exception {
256256
addUser(user, true);
257257
useUser(user);
258258

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

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

298302
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)