This is an automated email from the ASF dual-hosted git repository.
jackietien pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iotdb.git
The following commit(s) were added to refs/heads/master by this push:
new 8a845dd05b4 Sl fix auth bugs
8a845dd05b4 is described below
commit 8a845dd05b43a085a5b415f5357b575faec9332f
Author: Colin Li <[email protected]>
AuthorDate: Mon Sep 25 08:55:38 2023 +0800
Sl fix auth bugs
---
.../org/apache/iotdb/db/it/auth/IoTDBAuthIT.java | 4 ++--
.../java/org/apache/iotdb/rpc/TSStatusCode.java | 1 -
.../iotdb/confignode/manager/ProcedureManager.java | 3 +--
.../org/apache/iotdb/db/auth/AuthorityChecker.java | 8 ++++++-
.../metadata/view/CreateLogicalViewStatement.java | 27 +++++++++++++++++-----
.../plan/statement/sys/AuthorStatement.java | 13 +++++++----
.../auth/authorizer/LocalFileAuthorizerTest.java | 5 ++++
.../db/auth/user/LocalFileUserManagerTest.java | 4 ++--
.../plan/parser/StatementGeneratorTest.java | 26 +++++++++++++++++++++
.../iotdb/commons/auth/user/BasicUserManager.java | 8 ++++---
.../org/apache/iotdb/commons/utils/AuthUtils.java | 4 ++--
.../apache/iotdb/commons/utils/AuthUtilsTest.java | 2 ++
12 files changed, 82 insertions(+), 23 deletions(-)
diff --git
a/integration-test/src/test/java/org/apache/iotdb/db/it/auth/IoTDBAuthIT.java
b/integration-test/src/test/java/org/apache/iotdb/db/it/auth/IoTDBAuthIT.java
index f4dee7f5f55..6abb63b615a 100644
---
a/integration-test/src/test/java/org/apache/iotdb/db/it/auth/IoTDBAuthIT.java
+++
b/integration-test/src/test/java/org/apache/iotdb/db/it/auth/IoTDBAuthIT.java
@@ -919,7 +919,7 @@ public class IoTDBAuthIT {
adminStmt.execute("CREATE USER user2 'password'");
adminStmt.execute("CREATE USER user3 'password'");
adminStmt.execute("CREATE ROLE testRole");
- adminStmt.execute("GRANT MANAGE_DATABASE ON root.** TO ROLE testRole WITH
GRANT OPTION");
+ adminStmt.execute("GRANT manage_database ON root.** TO ROLE testRole WITH
GRANT OPTION");
adminStmt.execute("GRANT READ_DATA ON root.t1.** TO ROLE testRole");
adminStmt.execute("GRANT READ_SCHEMA ON root.t3.t2.** TO ROLE testRole
WITH GRANT OPTION");
@@ -1048,7 +1048,7 @@ public class IoTDBAuthIT {
Statement userStmt = userCon.createStatement()) {
try {
// because role's privilege
- userStmt.execute("GRANT MANAGE_DATABASE ON root.** TO USER user1");
+ userStmt.execute("GRANT manage_database ON root.** TO USER user1");
Assert.assertThrows(
SQLException.class,
() -> userStmt.execute("GRANT READ_DATA ON root.t1.** TO USER
user1"));
diff --git
a/iotdb-client/service-rpc/src/main/java/org/apache/iotdb/rpc/TSStatusCode.java
b/iotdb-client/service-rpc/src/main/java/org/apache/iotdb/rpc/TSStatusCode.java
index 2893da6339c..475a4ec0c1e 100644
---
a/iotdb-client/service-rpc/src/main/java/org/apache/iotdb/rpc/TSStatusCode.java
+++
b/iotdb-client/service-rpc/src/main/java/org/apache/iotdb/rpc/TSStatusCode.java
@@ -123,7 +123,6 @@ public enum TSStatusCode {
USER_NOT_HAS_ROLE(807),
ROLE_NOT_EXIST(808),
ROLE_ALREADY_EXIST(809),
- ALREADY_HAS_PRIVILEGE(810),
NOT_HAS_PRIVILEGE(811),
CLEAR_PERMISSION_CACHE_ERROR(812),
UNKNOWN_AUTH_PRIVILEGE(813),
diff --git
a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java
b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java
index 9f3d0ebefca..da5696811c4 100644
---
a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java
+++
b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java
@@ -897,8 +897,7 @@ public class ProcedureManager {
if (isSucceed) {
return RpcUtils.SUCCESS_STATUS;
} else {
- return new
TSStatus(TSStatusCode.AUTH_OPERATE_EXCEPTION.getStatusCode())
- .setMessage(statusList.get(0).getMessage());
+ return new
TSStatus(statusList.get(0).getCode()).setMessage(statusList.get(0).getMessage());
}
} catch (Exception e) {
return new TSStatus(TSStatusCode.AUTH_OPERATE_EXCEPTION.getStatusCode())
diff --git
a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/AuthorityChecker.java
b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/AuthorityChecker.java
index ea155343580..d2504863776 100644
---
a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/AuthorityChecker.java
+++
b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/AuthorityChecker.java
@@ -110,6 +110,12 @@ public class AuthorityChecker {
}
}
+ public static TSStatus getOptTSStatus(boolean hasGrantOpt, String errMsg) {
+ return hasGrantOpt
+ ? new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode())
+ : new
TSStatus(TSStatusCode.NOT_HAS_PRIVILEGE_GRANTOPT.getStatusCode()).setMessage(errMsg);
+ }
+
public static TSStatus getTSStatus(boolean hasPermission, String errMsg) {
return hasPermission
? new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode())
@@ -182,7 +188,7 @@ public class AuthorityChecker {
String userName, String[] privilegeList, List<PartialPath> nodeNameList)
{
for (String s : privilegeList) {
if (!authorityFetcher.checkUserPrivilegeGrantOpt(
- userName, nodeNameList, PrivilegeType.valueOf(s).ordinal())) {
+ userName, nodeNameList,
PrivilegeType.valueOf(s.toUpperCase()).ordinal())) {
return false;
}
}
diff --git
a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/metadata/view/CreateLogicalViewStatement.java
b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/metadata/view/CreateLogicalViewStatement.java
index c7d3a2ef52a..f4b8846bddc 100644
---
a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/metadata/view/CreateLogicalViewStatement.java
+++
b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/metadata/view/CreateLogicalViewStatement.java
@@ -78,12 +78,27 @@ public class CreateLogicalViewStatement extends Statement {
if (AuthorityChecker.SUPER_USER.equals(userName)) {
return new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
}
- TSStatus status =
- AuthorityChecker.getTSStatus(
- AuthorityChecker.checkFullPathListPermission(
- userName, getSourcePaths().fullPathList,
PrivilegeType.READ_SCHEMA.ordinal()),
- getSourcePaths().fullPathList,
- PrivilegeType.READ_SCHEMA);
+ TSStatus status = new
TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
+ if (getSourcePaths().fullPathList != null) {
+ status =
+ AuthorityChecker.getTSStatus(
+ AuthorityChecker.checkFullPathListPermission(
+ userName, getSourcePaths().fullPathList,
PrivilegeType.READ_SCHEMA.ordinal()),
+ getSourcePaths().fullPathList,
+ PrivilegeType.READ_SCHEMA);
+ }
+ if (getQueryStatement() != null
+ && status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ status =
+ AuthorityChecker.getTSStatus(
+ AuthorityChecker.checkFullPathListPermission(
+ userName,
+ getQueryStatement().getFromComponent().getPrefixPaths(),
+ PrivilegeType.READ_SCHEMA.ordinal()),
+ getQueryStatement().getFromComponent().getPrefixPaths(),
+ PrivilegeType.READ_SCHEMA);
+ }
+
if (status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
return AuthorityChecker.getTSStatus(
AuthorityChecker.checkFullPathListPermission(
diff --git
a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/sys/AuthorStatement.java
b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/sys/AuthorStatement.java
index 2bf2113a8cf..dd56fadaf04 100644
---
a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/sys/AuthorStatement.java
+++
b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/sys/AuthorStatement.java
@@ -290,6 +290,10 @@ public class AuthorStatement extends Statement implements
IConfigStatement {
}
case CREATE_ROLE:
+ if (AuthorityChecker.SUPER_USER.equals(this.roleName)) {
+ return AuthorityChecker.getTSStatus(
+ false, "Cannot create role has same name with admin user");
+ }
case DROP_ROLE:
case GRANT_USER_ROLE:
case REVOKE_USER_ROLE:
@@ -301,25 +305,26 @@ public class AuthorStatement extends Statement implements
IConfigStatement {
PrivilegeType.MANAGE_ROLE);
case REVOKE_USER:
+ case GRANT_USER:
if (AuthorityChecker.SUPER_USER.equals(this.userName)) {
- return AuthorityChecker.getTSStatus(false, "Cannot revoke privileges
of admin user");
+ return AuthorityChecker.getTSStatus(
+ false, "Cannot grant/revoke privileges of admin user");
}
if (AuthorityChecker.SUPER_USER.equals(userName)) {
return new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
}
- return AuthorityChecker.getTSStatus(
+ return AuthorityChecker.getOptTSStatus(
AuthorityChecker.checkGrantOption(userName, privilegeList,
nodeNameList),
"Has no permission to "
+ authorType
+ ", please ensure you have these privileges and the grant
option is TRUE when granted");
- case GRANT_USER:
case GRANT_ROLE:
case REVOKE_ROLE:
if (AuthorityChecker.SUPER_USER.equals(userName)) {
return new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
}
- return AuthorityChecker.getTSStatus(
+ return AuthorityChecker.getOptTSStatus(
AuthorityChecker.checkGrantOption(userName, privilegeList,
nodeNameList),
"Has no permission to "
+ authorType
diff --git
a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/authorizer/LocalFileAuthorizerTest.java
b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/authorizer/LocalFileAuthorizerTest.java
index 0d3f0a5181f..ee555bf258b 100644
---
a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/authorizer/LocalFileAuthorizerTest.java
+++
b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/authorizer/LocalFileAuthorizerTest.java
@@ -92,6 +92,11 @@ public class LocalFileAuthorizerTest {
} catch (AuthException e) {
assertEquals("Default administrator cannot be deleted", e.getMessage());
}
+ try {
+ authorizer.deleteUser("nouser");
+ } catch (AuthException e) {
+ assertEquals("User nouser does not exist", e.getMessage());
+ }
}
@Test
diff --git
a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/user/LocalFileUserManagerTest.java
b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/user/LocalFileUserManagerTest.java
index 70c4f61cd20..cfa017aa772 100644
---
a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/user/LocalFileUserManagerTest.java
+++
b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/user/LocalFileUserManagerTest.java
@@ -109,10 +109,10 @@ public class LocalFileUserManagerTest {
Assert.assertThrows(AuthException.class, () -> manager.createUser("short",
"too", false));
// delete
- assertTrue(manager.deleteUser("not a user"));
+ assertFalse(manager.deleteUser("not a user"));
assertTrue(manager.deleteUser(users[users.length - 1].getName()));
assertNull(manager.getUser(users[users.length - 1].getName()));
- assertTrue(manager.deleteUser(users[users.length - 1].getName()));
+ assertFalse(manager.deleteUser(users[users.length - 1].getName()));
// grant privilege
user = manager.getUser(users[0].getName());
diff --git
a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/parser/StatementGeneratorTest.java
b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/parser/StatementGeneratorTest.java
index bf0620fc264..268ddae322f 100644
---
a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/parser/StatementGeneratorTest.java
+++
b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/parser/StatementGeneratorTest.java
@@ -54,6 +54,7 @@ import
org.apache.iotdb.db.queryengine.plan.statement.metadata.template.CreateSc
import
org.apache.iotdb.db.queryengine.plan.statement.metadata.template.DropSchemaTemplateStatement;
import
org.apache.iotdb.db.queryengine.plan.statement.metadata.template.ShowNodesInSchemaTemplateStatement;
import
org.apache.iotdb.db.queryengine.plan.statement.metadata.template.UnsetSchemaTemplateStatement;
+import
org.apache.iotdb.db.queryengine.plan.statement.metadata.view.CreateLogicalViewStatement;
import org.apache.iotdb.db.queryengine.plan.statement.sys.AuthorStatement;
import org.apache.iotdb.isession.template.TemplateNode;
import org.apache.iotdb.mpp.rpc.thrift.TDeleteModelMetricsReq;
@@ -90,6 +91,7 @@ import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.time.ZonedDateTime;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
@@ -780,6 +782,30 @@ public class StatementGeneratorTest {
assertEquals("role1", stmt.getRoleName());
}
+ private CreateLogicalViewStatement createViewStmt(String sql) {
+ Statement stmt = StatementGenerator.createStatement(sql,
ZonedDateTime.now().getOffset());
+ CreateLogicalViewStatement viewStmt = (CreateLogicalViewStatement) stmt;
+ return viewStmt;
+ }
+
+ @Test
+ public void testCreateView() throws IllegalPathException {
+ // 1. create with select
+ CreateLogicalViewStatement stmt =
+ createViewStmt("create view root.sg.view_dd as select s1 from
root.sg.d1;");
+ List<PartialPath> path = new ArrayList<>();
+ path.add(new PartialPath("root.sg.d1"));
+ assertEquals(null, stmt.getSourcePaths().fullPathList);
+ assertEquals(path,
stmt.getQueryStatement().getFromComponent().getPrefixPaths());
+
+ // 2. create with path
+ stmt = createViewStmt("create view root.sg as root.sg.d2;");
+ List<PartialPath> path2 = new ArrayList<>();
+ path2.add(new PartialPath("root.sg.d2"));
+ assertEquals(path2, stmt.getSourcePaths().fullPathList);
+ assertEquals(null, stmt.getQueryStatement());
+ }
+
// TODO: add more tests
private void checkQueryStatement(
diff --git
a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/user/BasicUserManager.java
b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/user/BasicUserManager.java
index 2ea7a08ae85..6c16a8dc12e 100644
---
a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/user/BasicUserManager.java
+++
b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/user/BasicUserManager.java
@@ -139,9 +139,11 @@ public abstract class BasicUserManager implements
IUserManager {
@Override
public boolean deleteUser(String username) {
lock.writeLock(username);
- userMap.remove(username);
- lock.writeUnlock(username);
- return true;
+ try {
+ return userMap.remove(username) != null;
+ } finally {
+ lock.writeUnlock(username);
+ }
}
@Override
diff --git
a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/utils/AuthUtils.java
b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/utils/AuthUtils.java
index e49eff9cf0d..7e379b09570 100644
---
a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/utils/AuthUtils.java
+++
b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/utils/AuthUtils.java
@@ -55,7 +55,7 @@ public class AuthUtils {
// match number, character, and !@#$%^*()_+-=
// pattern: ^[-\w!@#\$%\^\(\)\+=]*$
- private static final String REX_PATTERN = "^[-\\w!@#\\$%\\^\\*()\\+=]*$";
+ private static final String REX_PATTERN = "^[-\\w!@#$%^&*()+=]*$";
private AuthUtils() {
// Empty constructor
@@ -120,7 +120,7 @@ public class AuthUtils {
} else if (!str.matches(REX_PATTERN)) {
throw new AuthException(
TSStatusCode.ILLEGAL_PARAMETER,
- "The name or password can only contain letters, numbers, underscores
or !@#$%^*()_+-=");
+ "The name or password can only contain letters, numbers or
!@#$%^*()_+-=");
}
}
diff --git
a/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/utils/AuthUtilsTest.java
b/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/utils/AuthUtilsTest.java
index a15ed0589f9..4ef713caa78 100644
---
a/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/utils/AuthUtilsTest.java
+++
b/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/utils/AuthUtilsTest.java
@@ -46,6 +46,8 @@ public class AuthUtilsTest {
AuthUtils.validatePassword("he!l*^^+=");
AuthUtils.validatePassword("he!!l*^^+=");
AuthUtils.validatePassword("he!!l*()^^+=");
+ AuthUtils.validateUsername("!@#$%&^&*()_+-=");
+ AuthUtils.validateUsername("!@!%^&!@#%$#@#$%&^&*()_+-=");
Assert.assertThrows(AuthException.class, () ->
AuthUtils.validatePassword("he!!l\\*()^^+="));
Assert.assertThrows(AuthException.class, () ->
AuthUtils.validatePassword("he!l^^ +="));
Assert.assertThrows(AuthException.class, () ->
AuthUtils.validatePassword("he"));
