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 ea4322722f3 Fix some issue in auth model.
ea4322722f3 is described below
commit ea4322722f3cb39e43ff9c5ace7fe558a2cc9ce3
Author: Colin Lee <[email protected]>
AuthorDate: Fri Mar 7 21:59:27 2025 +0800
Fix some issue in auth model.
---
.../iotdb/db/it/auth/IoTDBRelationalAuthIT.java | 55 ++++++++++++++++++++--
.../relational/security/AccessControlImpl.java | 5 ++
.../plan/relational/sql/parser/AstBuilder.java | 46 ++++++++----------
.../commons/auth/authorizer/BasicAuthorizer.java | 20 ++------
.../iotdb/commons/auth/user/BasicUserManager.java | 12 +----
.../db/relational/grammar/sql/RelationalSql.g4 | 2 +-
6 files changed, 83 insertions(+), 57 deletions(-)
diff --git
a/integration-test/src/test/java/org/apache/iotdb/db/it/auth/IoTDBRelationalAuthIT.java
b/integration-test/src/test/java/org/apache/iotdb/db/it/auth/IoTDBRelationalAuthIT.java
index 3ce14d7325c..017b3d57ceb 100644
---
a/integration-test/src/test/java/org/apache/iotdb/db/it/auth/IoTDBRelationalAuthIT.java
+++
b/integration-test/src/test/java/org/apache/iotdb/db/it/auth/IoTDBRelationalAuthIT.java
@@ -26,6 +26,7 @@ import org.apache.iotdb.it.framework.IoTDBTestRunner;
import org.apache.iotdb.itbase.category.TableClusterIT;
import org.apache.iotdb.itbase.category.TableLocalStandaloneIT;
import org.apache.iotdb.itbase.env.BaseEnv;
+import org.apache.iotdb.jdbc.IoTDBSQLException;
import org.junit.After;
import org.junit.Assert;
@@ -74,6 +75,12 @@ public class IoTDBRelationalAuthIT {
() -> {
adminStmt.execute("GRANT read_data on database db1 to user
testuser");
});
+ // No Maintain privilege in table model.
+ Assert.assertThrows(
+ SQLException.class,
+ () -> {
+ adminStmt.execute("GRANT MAINTAIN to user testuser");
+ });
adminStmt.execute("GRANT MANAGE_ROLE TO USER testuser");
adminStmt.execute("GRANT SELECT ON ANY TO USER testuser");
@@ -221,11 +228,11 @@ public class IoTDBRelationalAuthIT {
",testdb.tb,INSERT,false,"));
TestUtils.assertResultSetEqual(rs, "Role,Scope,Privileges,GrantOption,",
ans);
rs = userStmt.executeQuery("List privileges of role testrole");
- ans = new HashSet<>();
- TestUtils.assertResultSetEqual(rs, "Role,Scope,Privileges,GrantOption,",
ans);
+ TestUtils.assertResultSetEqual(
+ rs, "Role,Scope,Privileges,GrantOption,", Collections.emptySet());
rs = userStmt.executeQuery("List privileges of role testrole2");
- ans = new HashSet<>();
- TestUtils.assertResultSetEqual(rs, "Role,Scope,Privileges,GrantOption,",
ans);
+ TestUtils.assertResultSetEqual(
+ rs, "Role,Scope,Privileges,GrantOption,", Collections.emptySet());
// testdb.TB's privilege is not grant option.
Assert.assertThrows(
SQLException.class,
@@ -525,4 +532,44 @@ public class IoTDBRelationalAuthIT {
resultSet, "Role,Scope,Privileges,GrantOption,",
Collections.emptySet());
}
}
+
+ @Test
+ public void testCreateUserAndRole() throws SQLException {
+ try (Connection adminCon =
EnvFactory.getEnv().getConnection(BaseEnv.TABLE_SQL_DIALECT);
+ Statement adminStmt = adminCon.createStatement()) {
+ // normal case
+ adminStmt.execute("create user testuser 'password'");
+ // username abnormal
+ adminStmt.execute("create user \"!@#$%^*()_+-=1\" 'password'");
+
+ // username and password abnormal
+ adminStmt.execute("create user \"!@#$%^*()_+-=2\" '!@#$%^*()_+-='");
+
+ // rolename abnormal
+ adminStmt.execute("create role \"!@#$%^*()_+-=3\" ");
+
+ ResultSet resultSet = adminStmt.executeQuery("List user");
+ Set<String> resultSetList = new HashSet<>();
+ resultSetList.add("root,");
+ resultSetList.add("testuser,");
+ resultSetList.add("!@#$%^*()_+-=1,");
+ resultSetList.add("!@#$%^*()_+-=2,");
+ TestUtils.assertResultSetEqual(resultSet, "User,", resultSetList);
+ resultSet = adminStmt.executeQuery("List role");
+ TestUtils.assertResultSetEqual(resultSet, "Role,",
Collections.singleton("!@#$%^*()_+-=3,"));
+ adminStmt.execute("GRANT role \"!@#$%^*()_+-=3\" to
\"!@#$%^*()_+-=1\"");
+ adminStmt.execute("ALTER user \"!@#$%^*()_+-=1\" set password
'!@#$%^*()_+-=\'");
+ }
+
+ try (Connection userCon =
+ EnvFactory.getEnv()
+ .getConnection("!@#$%^*()_+-=1", "!@#$%^*()_+-=",
BaseEnv.TABLE_SQL_DIALECT);
+ Statement userConStatement = userCon.createStatement()) {
+ // List his role.
+ ResultSet set = userConStatement.executeQuery("List role of user
\"!@#$%^*()_+-=1\"");
+ TestUtils.assertResultSetEqual(set, "Role,",
Collections.singleton("!@#$%^*()_+-=3,"));
+ } catch (IoTDBSQLException e) {
+ Assert.fail();
+ }
+ }
}
diff --git
a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControlImpl.java
b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControlImpl.java
index 6517347da36..33543f60040 100644
---
a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControlImpl.java
+++
b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControlImpl.java
@@ -175,6 +175,11 @@ public class AccessControlImpl implements AccessControl {
if (AuthorityChecker.SUPER_USER.equals(userName)) {
return;
}
+
+ // user can list his roles.
+ if (statement.getUserName() != null &&
statement.getUserName().equals(userName)) {
+ return;
+ }
authChecker.checkGlobalPrivilege(userName,
TableModelPrivilege.MANAGE_ROLE);
return;
case LIST_ROLE_PRIV:
diff --git
a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/parser/AstBuilder.java
b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/parser/AstBuilder.java
index 317480219e0..77f2d3318d8 100644
---
a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/parser/AstBuilder.java
+++
b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/parser/AstBuilder.java
@@ -1435,64 +1435,58 @@ public class AstBuilder extends
RelationalSqlBaseVisitor<Node> {
}
// ********************** author expressions ********************
-
- private String stripQuotes(String text) {
- if (text != null && text.length() >= 2 && text.startsWith("'") &&
text.endsWith("'")) {
- return text.substring(1, text.length() - 1).replace("''", "'");
- }
- return text;
- }
-
@Override
public Node
visitCreateUserStatement(RelationalSqlParser.CreateUserStatementContext ctx) {
RelationalAuthorStatement stmt = new
RelationalAuthorStatement(AuthorRType.CREATE_USER);
- stmt.setUserName(ctx.userName.getText());
- stmt.setPassword(stripQuotes(ctx.password.getText()));
+ stmt.setUserName(((Identifier) visit(ctx.userName)).getValue());
+ String password = ((StringLiteral) visit(ctx.password)).getValue();
+ stmt.setPassword(password);
return stmt;
}
@Override
public Node
visitCreateRoleStatement(RelationalSqlParser.CreateRoleStatementContext ctx) {
RelationalAuthorStatement stmt = new
RelationalAuthorStatement(AuthorRType.CREATE_ROLE);
- stmt.setRoleName(ctx.roleName.getText());
+ stmt.setRoleName(((Identifier) visit(ctx.roleName)).getValue());
return stmt;
}
@Override
public Node
visitDropUserStatement(RelationalSqlParser.DropUserStatementContext ctx) {
RelationalAuthorStatement stmt = new
RelationalAuthorStatement(AuthorRType.DROP_USER);
- stmt.setUserName(ctx.userName.getText());
+ stmt.setUserName(((Identifier) visit(ctx.userName)).getValue());
return stmt;
}
@Override
public Node
visitDropRoleStatement(RelationalSqlParser.DropRoleStatementContext ctx) {
RelationalAuthorStatement stmt = new
RelationalAuthorStatement(AuthorRType.DROP_ROLE);
- stmt.setRoleName(ctx.roleName.getText());
+ stmt.setRoleName(((Identifier) visit(ctx.roleName)).getValue());
return stmt;
}
@Override
public Node
visitAlterUserStatement(RelationalSqlParser.AlterUserStatementContext ctx) {
RelationalAuthorStatement stmt = new
RelationalAuthorStatement(AuthorRType.UPDATE_USER);
- stmt.setUserName(ctx.userName.getText());
- stmt.setPassword(stripQuotes(ctx.password.getText()));
+ stmt.setUserName(((Identifier) visit(ctx.userName)).getValue());
+ String password = ((StringLiteral) visit(ctx.password)).getValue();
+ stmt.setPassword(password);
return stmt;
}
@Override
public Node
visitGrantUserRoleStatement(RelationalSqlParser.GrantUserRoleStatementContext
ctx) {
RelationalAuthorStatement stmt = new
RelationalAuthorStatement(AuthorRType.GRANT_USER_ROLE);
- stmt.setUserName(ctx.userName.getText());
- stmt.setRoleName(ctx.roleName.getText());
+ stmt.setUserName(((Identifier) visit(ctx.userName)).getValue());
+ stmt.setRoleName(((Identifier) visit(ctx.roleName)).getValue());
return stmt;
}
@Override
public Node
visitRevokeUserRoleStatement(RelationalSqlParser.RevokeUserRoleStatementContext
ctx) {
RelationalAuthorStatement stmt = new
RelationalAuthorStatement(AuthorRType.REVOKE_USER_ROLE);
- stmt.setUserName(ctx.userName.getText());
- stmt.setRoleName(ctx.roleName.getText());
+ stmt.setUserName(((Identifier) visit(ctx.userName)).getValue());
+ stmt.setRoleName(((Identifier) visit(ctx.roleName)).getValue());
return stmt;
}
@@ -1500,7 +1494,7 @@ public class AstBuilder extends
RelationalSqlBaseVisitor<Node> {
public Node visitListUserPrivilegeStatement(
RelationalSqlParser.ListUserPrivilegeStatementContext ctx) {
RelationalAuthorStatement stmt = new
RelationalAuthorStatement(AuthorRType.LIST_USER_PRIV);
- stmt.setUserName(ctx.userName.getText());
+ stmt.setUserName(((Identifier) visit(ctx.userName)).getValue());
return stmt;
}
@@ -1508,7 +1502,7 @@ public class AstBuilder extends
RelationalSqlBaseVisitor<Node> {
public Node visitListRolePrivilegeStatement(
RelationalSqlParser.ListRolePrivilegeStatementContext ctx) {
RelationalAuthorStatement stmt = new
RelationalAuthorStatement(AuthorRType.LIST_ROLE_PRIV);
- stmt.setRoleName(ctx.roleName.getText());
+ stmt.setRoleName(((Identifier) visit(ctx.roleName)).getValue());
return stmt;
}
@@ -1516,8 +1510,7 @@ public class AstBuilder extends
RelationalSqlBaseVisitor<Node> {
public Node
visitListUserStatement(RelationalSqlParser.ListUserStatementContext ctx) {
RelationalAuthorStatement stmt = new
RelationalAuthorStatement(AuthorRType.LIST_USER);
if (ctx.OF() != null) {
- String roleName = ctx.roleName.getText();
- stmt.setRoleName(roleName);
+ stmt.setRoleName(((Identifier) visit(ctx.roleName)).getValue());
}
return stmt;
}
@@ -1526,8 +1519,7 @@ public class AstBuilder extends
RelationalSqlBaseVisitor<Node> {
public Node
visitListRoleStatement(RelationalSqlParser.ListRoleStatementContext ctx) {
RelationalAuthorStatement stmt = new
RelationalAuthorStatement(AuthorRType.LIST_ROLE);
if (ctx.OF() != null) {
- String userName = ctx.userName.getText();
- stmt.setUserName(userName);
+ stmt.setUserName(((Identifier) visit(ctx.userName)).getValue());
}
return stmt;
}
@@ -1564,7 +1556,7 @@ public class AstBuilder extends
RelationalSqlBaseVisitor<Node> {
boolean toUser;
String name;
toUser = ctx.holderType().getText().equalsIgnoreCase("user");
- name = ctx.holderName.getText();
+ name = (((Identifier) visit(ctx.holderName)).getValue());
boolean grantOption = ctx.grantOpt() != null;
boolean toTable;
Set<PrivilegeType> privileges;
@@ -1638,7 +1630,7 @@ public class AstBuilder extends
RelationalSqlBaseVisitor<Node> {
boolean fromUser;
String name;
fromUser = ctx.holderType().getText().equalsIgnoreCase("user");
- name = ctx.holderName.getText();
+ name = (((Identifier) visit(ctx.holderName)).getValue());
boolean grantOption = ctx.revokeGrantOpt() != null;
boolean fromTable = false;
Set<PrivilegeType> privileges = new HashSet<>();
diff --git
a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/authorizer/BasicAuthorizer.java
b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/authorizer/BasicAuthorizer.java
index d4fe42d8055..ef33a795276 100644
---
a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/authorizer/BasicAuthorizer.java
+++
b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/authorizer/BasicAuthorizer.java
@@ -231,17 +231,11 @@ public abstract class BasicAuthorizer implements
IAuthorizer, IService {
TSStatusCode.ROLE_NOT_EXIST, String.format(NO_SUCH_ROLE_EXCEPTION,
roleName));
}
// the role may be deleted before it ts granted to the user, so a double
check is necessary.
- boolean success = userManager.grantRoleToUser(roleName, userName);
- if (success) {
- role = roleManager.getEntity(roleName);
- if (role == null) {
- throw new AuthException(
- TSStatusCode.ROLE_NOT_EXIST, String.format(NO_SUCH_ROLE_EXCEPTION,
roleName));
- }
- } else {
+ userManager.grantRoleToUser(roleName, userName);
+ role = roleManager.getEntity(roleName);
+ if (role == null) {
throw new AuthException(
- TSStatusCode.USER_ALREADY_HAS_ROLE,
- String.format("User %s already has role %s", userName, roleName));
+ TSStatusCode.ROLE_NOT_EXIST, String.format(NO_SUCH_ROLE_EXCEPTION,
roleName));
}
}
@@ -257,11 +251,7 @@ public abstract class BasicAuthorizer implements
IAuthorizer, IService {
throw new AuthException(
TSStatusCode.ROLE_NOT_EXIST, String.format(NO_SUCH_ROLE_EXCEPTION,
roleName));
}
- if (!userManager.revokeRoleFromUser(roleName, userName)) {
- throw new AuthException(
- TSStatusCode.USER_NOT_HAS_ROLE,
- String.format("User %s does not have role %s", userName, roleName));
- }
+ userManager.revokeRoleFromUser(roleName, userName);
}
@Override
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 f47e0c453a4..bc2c283a363 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
@@ -150,7 +150,7 @@ public abstract class BasicUserManager extends
BasicRoleManager {
}
}
- public boolean grantRoleToUser(String roleName, String username) throws
AuthException {
+ public void grantRoleToUser(String roleName, String username) throws
AuthException {
lock.writeLock(username);
try {
User user = this.getEntity(username);
@@ -158,17 +158,13 @@ public abstract class BasicUserManager extends
BasicRoleManager {
throw new AuthException(
getEntityNotExistErrorCode(),
String.format(getNoSuchEntityError(), username));
}
- if (user.hasRole(roleName)) {
- return false;
- }
user.getRoleSet().add(roleName);
- return true;
} finally {
lock.writeUnlock(username);
}
}
- public boolean revokeRoleFromUser(String roleName, String username) throws
AuthException {
+ public void revokeRoleFromUser(String roleName, String username) throws
AuthException {
lock.writeLock(username);
try {
User user = this.getEntity(username);
@@ -176,11 +172,7 @@ public abstract class BasicUserManager extends
BasicRoleManager {
throw new AuthException(
getEntityNotExistErrorCode(),
String.format(getNoSuchEntityError(), username));
}
- if (!user.hasRole(roleName)) {
- return false;
- }
user.getRoleSet().remove(roleName);
- return true;
} finally {
lock.writeUnlock(username);
}
diff --git
a/iotdb-core/relational-grammar/src/main/antlr4/org/apache/iotdb/db/relational/grammar/sql/RelationalSql.g4
b/iotdb-core/relational-grammar/src/main/antlr4/org/apache/iotdb/db/relational/grammar/sql/RelationalSql.g4
index 8e1ded08a06..b5e9341bb6e 100644
---
a/iotdb-core/relational-grammar/src/main/antlr4/org/apache/iotdb/db/relational/grammar/sql/RelationalSql.g4
+++
b/iotdb-core/relational-grammar/src/main/antlr4/org/apache/iotdb/db/relational/grammar/sql/RelationalSql.g4
@@ -613,7 +613,7 @@ dropRoleStatement
;
alterUserStatement
- : ALTER USER userName=identifier SET PASSWORD password=identifier
+ : ALTER USER userName=identifier SET PASSWORD password=string
;
grantUserRoleStatement
