Skip to content

Commit 8cd1418

Browse files
authored
Fix the check of grant option for tree model (#16845)
1 parent cb705d1 commit 8cd1418

File tree

3 files changed

+111
-2
lines changed

3 files changed

+111
-2
lines changed

integration-test/src/test/java/org/apache/iotdb/db/it/auth/IoTDBAuthIT.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,8 @@ public void testGrantAndGrantOpt() throws SQLException {
981981
adminStmt.execute("CREATE USER user1 'password123456'");
982982
adminStmt.execute("CREATE USER user2 'password123456'");
983983
adminStmt.execute("CREATE USER user3 'password123456'");
984+
adminStmt.execute("CREATE USER user4 'password123456'");
985+
adminStmt.execute("CREATE USER user5 'password123456'");
984986
adminStmt.execute("CREATE ROLE testRole");
985987
adminStmt.execute("GRANT system ON root.** TO ROLE testRole WITH GRANT OPTION");
986988
adminStmt.execute("GRANT READ_DATA ON root.t1.** TO ROLE testRole");
@@ -1095,6 +1097,18 @@ public void testGrantAndGrantOpt() throws SQLException {
10951097
}
10961098
}
10971099

1100+
try (Connection userCon = EnvFactory.getEnv().getConnection("user4", "password123456");
1101+
Statement userStmt = userCon.createStatement()) {
1102+
adminStmt.execute("GRANT SYSTEM ON root.** TO USER user4");
1103+
try {
1104+
Assert.assertThrows(
1105+
SQLException.class, () -> userStmt.execute("GRANT SYSTEM ON root.** TO USER user5"));
1106+
adminStmt.execute("GRANT SYSTEM ON root.** TO USER user5");
1107+
} finally {
1108+
userStmt.close();
1109+
}
1110+
}
1111+
10981112
adminStmt.close();
10991113
}
11001114

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TreeAccessCheckVisitor.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,7 @@ public TSStatus visitAuthor(AuthorStatement statement, TreeAccessCheckContext co
643643
for (String s : statement.getPrivilegeList()) {
644644
PrivilegeType privilegeType = PrivilegeType.valueOf(s.toUpperCase());
645645
if (privilegeType.isSystemPrivilege()) {
646-
if (!checkHasGlobalAuth(context, privilegeType, auditObject)) {
646+
if (!checkHasGlobalAuth(context, privilegeType, auditObject, true)) {
647647
return AuthorityChecker.getTSStatus(
648648
false,
649649
"Has no permission to execute "
@@ -1932,13 +1932,24 @@ protected TSStatus checkGlobalAuth(
19321932

19331933
protected boolean checkHasGlobalAuth(
19341934
IAuditEntity context, PrivilegeType requiredPrivilege, Supplier<String> auditObject) {
1935+
return checkHasGlobalAuth(context, requiredPrivilege, auditObject, false);
1936+
}
1937+
1938+
protected boolean checkHasGlobalAuth(
1939+
IAuditEntity context,
1940+
PrivilegeType requiredPrivilege,
1941+
Supplier<String> auditObject,
1942+
boolean checkGrantOption) {
19351943
if (AuthorityChecker.SUPER_USER.equals(context.getUsername())) {
19361944
recordObjectAuthenticationAuditLog(
19371945
context.setPrivilegeType(requiredPrivilege).setResult(true), auditObject);
19381946
return true;
19391947
}
19401948
boolean result =
1941-
AuthorityChecker.checkSystemPermission(context.getUsername(), requiredPrivilege);
1949+
checkGrantOption
1950+
? AuthorityChecker.checkSystemPermissionGrantOption(
1951+
context.getUsername(), requiredPrivilege)
1952+
: AuthorityChecker.checkSystemPermission(context.getUsername(), requiredPrivilege);
19421953
recordObjectAuthenticationAuditLog(
19431954
context.setPrivilegeType(requiredPrivilege).setResult(result), auditObject);
19441955
return result;
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.iotdb.db.auth;
21+
22+
import org.apache.iotdb.commons.auth.entity.PrivilegeType;
23+
import org.apache.iotdb.commons.auth.entity.User;
24+
import org.apache.iotdb.db.queryengine.plan.relational.security.TreeAccessCheckContext;
25+
import org.apache.iotdb.db.queryengine.plan.relational.security.TreeAccessCheckVisitor;
26+
import org.apache.iotdb.db.queryengine.plan.statement.AuthorType;
27+
import org.apache.iotdb.db.queryengine.plan.statement.sys.AuthorStatement;
28+
import org.apache.iotdb.rpc.TSStatusCode;
29+
30+
import org.junit.After;
31+
import org.junit.Assert;
32+
import org.junit.Before;
33+
import org.junit.Test;
34+
import org.mockito.Mockito;
35+
36+
public class TreeAccessTest {
37+
38+
@Before
39+
public void setup() {
40+
AuthorityChecker.getAuthorityFetcher().getAuthorCache().invalidAllCache();
41+
}
42+
43+
@After
44+
public void teardown() {
45+
AuthorityChecker.getAuthorityFetcher().getAuthorCache().invalidAllCache();
46+
}
47+
48+
@Test
49+
public void test1() {
50+
User mockUser = Mockito.mock(User.class);
51+
Mockito.when(mockUser.getName()).thenReturn("user1");
52+
Mockito.when(mockUser.getUserId()).thenReturn(10000L);
53+
Mockito.when(mockUser.checkSysPriGrantOpt(PrivilegeType.SYSTEM)).thenReturn(false);
54+
AuthorityChecker.getAuthorityFetcher()
55+
.getAuthorCache()
56+
.putUserCache(mockUser.getName(), mockUser);
57+
User mockUser2 = Mockito.mock(User.class);
58+
Mockito.when(mockUser2.getName()).thenReturn("user2");
59+
Mockito.when(mockUser2.getUserId()).thenReturn(10001L);
60+
AuthorityChecker.getAuthorityFetcher()
61+
.getAuthorCache()
62+
.putUserCache(mockUser.getName(), mockUser);
63+
AuthorityChecker.getAuthorityFetcher()
64+
.getAuthorCache()
65+
.putUserCache(mockUser2.getName(), mockUser2);
66+
TreeAccessCheckVisitor treeAccessCheckVisitor = new TreeAccessCheckVisitor();
67+
68+
AuthorStatement authorStatement = new AuthorStatement(AuthorType.GRANT_USER);
69+
authorStatement.setPrivilegeList(new String[] {"SYSTEM"});
70+
authorStatement.setUserName("user2");
71+
authorStatement.setGrantOpt(true);
72+
Assert.assertEquals(
73+
TSStatusCode.NO_PERMISSION.getStatusCode(),
74+
treeAccessCheckVisitor
75+
.visitAuthor(authorStatement, new TreeAccessCheckContext(10000L, "user1", ""))
76+
.getCode());
77+
Mockito.when(mockUser.checkSysPriGrantOpt(PrivilegeType.SYSTEM)).thenReturn(true);
78+
Assert.assertEquals(
79+
TSStatusCode.SUCCESS_STATUS.getStatusCode(),
80+
treeAccessCheckVisitor
81+
.visitAuthor(authorStatement, new TreeAccessCheckContext(10000L, "user1", ""))
82+
.getCode());
83+
}
84+
}

0 commit comments

Comments
 (0)