Caideyipi commented on code in PR #13158:
URL: https://github.com/apache/iotdb/pull/13158#discussion_r1753025065
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/auth/AuthorRelationalPlan.java:
##########
@@ -0,0 +1,139 @@
+package org.apache.iotdb.confignode.consensus.request.auth;
Review Comment:
May add license?
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/ast/AuthorRStatement.java:
##########
@@ -0,0 +1,189 @@
+package org.apache.iotdb.db.queryengine.plan.relational.sql.ast;
+
+import org.apache.iotdb.commons.auth.entity.PrivilegeType;
+import org.apache.iotdb.db.queryengine.plan.analyze.QueryType;
+import org.apache.iotdb.db.queryengine.plan.relational.type.AuthorRType;
+
+import com.google.common.collect.ImmutableList;
+
+import java.util.List;
+import java.util.Objects;
+
+public class AuthorRStatement extends Statement {
+
+ private final AuthorRType authorType;
+
+ private String tableName;
+ private String database;
+ private String userName;
+ private String roleName;
+
+ private String password;
+
+ private PrivilegeType privilegeType;
+
+ private boolean grantOption;
+
+ public AuthorRStatement(
+ AuthorRType authorType,
+ String database,
+ String table,
+ PrivilegeType type,
+ String username,
+ String rolename,
+ boolean grantOption) {
+ super(null);
+ this.authorType = authorType;
+ this.database = database;
+ this.tableName = table;
+ this.privilegeType = type;
+ this.roleName = rolename;
+ this.userName = username;
+ this.grantOption = grantOption;
+ }
+
+ public AuthorRStatement(AuthorRType statementType) {
+ super(null);
+ this.authorType = statementType;
+ }
+
+ public AuthorRStatement(
+ AuthorRType statementType,
+ PrivilegeType type,
+ String username,
+ String roleName,
+ boolean grantOption) {
+ super(null);
+ this.authorType = statementType;
+ this.privilegeType = type;
+ this.userName = username;
+ this.roleName = roleName;
+ this.grantOption = grantOption;
+ this.tableName = null;
+ this.database = null;
+ }
+
+ public AuthorRType getAuthorType() {
+ return authorType;
+ }
+
+ public String getTableName() {
+ return tableName;
+ }
+
+ public String getDatabase() {
+ return this.database;
+ }
+
+ public String getUserName() {
+ return userName;
+ }
+
+ public String getRoleName() {
+ return roleName;
+ }
+
+ public String getPassword() {
+ return this.password;
+ }
+
+ public boolean hasGrantOption() {
+ return grantOption;
+ }
+
+ public PrivilegeType getPrivilegeType() {
+ return privilegeType;
+ }
+
+ public void setDatabase(String database) {
+ this.database = database;
+ }
+
+ public void setUserName(String userName) {
+ this.userName = userName;
+ }
+
+ public void setRoleName(String roleName) {
+ this.roleName = roleName;
+ }
+
+ public void setPassword(String password) {
+ this.password = password;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ AuthorRStatement that = (AuthorRStatement) o;
+ return grantOption == that.grantOption
+ && authorType == that.authorType
+ && Objects.equals(database, that.database)
+ && Objects.equals(tableName, that.tableName)
+ && Objects.equals(userName, that.userName)
+ && Objects.equals(roleName, that.roleName);
+ }
+
+ @Override
+ public <R, C> R accept(AstVisitor<R, C> visitor, C context) {
+ return visitor.visitAuthorRPlan(this, context);
+ }
+
+ @Override
+ public List<Node> getChildren() {
+ return ImmutableList.of();
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(
+ authorType, database, tableName, privilegeType, userName, roleName,
grantOption);
+ }
+
+ public QueryType getQueryType() {
+ switch (this.authorType) {
+ case GRANT_ROLE_DB:
+ case GRANT_USER_DB:
+ case REVOKE_ROLE_DB:
+ case REVOKE_USER_DB:
+ case GRANT_ROLE_TB:
+ case GRANT_USER_TB:
+ case REVOKE_ROLE_TB:
+ case REVOKE_USER_TB:
+ case DROP_ROLE:
+ case DROP_USER:
+ case GRANT_USER_ROLE:
+ case GRANT_USER_SYS:
+ case GRANT_ROLE_SYS:
+ case CREATE_ROLE:
+ case CREATE_USER:
+ case REVOKE_ROLE_SYS:
+ case REVOKE_USER_SYS:
+ case REVOKE_USER_ROLE:
+ return QueryType.WRITE;
+ case LIST_ROLE:
+ case LIST_USER:
+ return QueryType.READ;
+ default:
+ throw new IllegalArgumentException("Unknow authorType:" +
this.authorType);
+ }
+ }
+
+ @Override
+ public String toString() {
+ return "auth statement: "
+ + authorType
+ + "to Database"
+ + database
+ + " "
+ + tableName
+ + "user name:"
+ + userName
+ + "role name"
Review Comment:
The format can be more normalized..
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/IAuthorityFetcher.java:
##########
@@ -37,18 +39,23 @@ public interface IAuthorityFetcher {
boolean checkRole(String username, String rolename);
List<Integer> checkUserPathPrivileges(
- String username, List<? extends PartialPath> allPath, int permission);
+ String username, List<? extends PartialPath> allPath, PrivilegeType
permission);
- TSStatus checkUserSysPrivileges(String username, int permisssion);
+ TSStatus checkUserSysPrivileges(String username, PrivilegeType permisssion);
Review Comment:
Better change it to "permission", not "permisssion"..
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/parser/AstBuilder.java:
##########
@@ -702,6 +704,113 @@ public Node
visitExplain(RelationalSqlParser.ExplainContext ctx) {
public Node visitExplainAnalyze(RelationalSqlParser.ExplainAnalyzeContext
ctx) {
return super.visitExplainAnalyze(ctx);
}
+ // ********************** 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 visitCreateUser(RelationalSqlParser.CreateUserContext ctx) {
+ AuthorRStatement stmt = new AuthorRStatement(AuthorRType.CREATE_USER);
+ stmt.setUserName(ctx.userName.getText());
+ stmt.setPassword(stripQuotes(ctx.password.getText()));
+ return stmt;
+ }
+
+ @Override
+ public Node visitCreateRole(RelationalSqlParser.CreateRoleContext ctx) {
+ AuthorRStatement stmt = new AuthorRStatement(AuthorRType.CREATE_ROLE);
+ stmt.setRoleName(ctx.roleName.getText());
+ return stmt;
+ }
+
+ @Override
+ public Node visitDropUser(RelationalSqlParser.DropUserContext ctx) {
+ AuthorRStatement stmt = new AuthorRStatement(AuthorRType.DROP_USER);
+ stmt.setUserName(ctx.userName.getText());
+ return stmt;
+ }
+
+ @Override
+ public Node visitDropRole(RelationalSqlParser.DropRoleContext ctx) {
+ AuthorRStatement stmt = new AuthorRStatement(AuthorRType.DROP_ROLE);
+ stmt.setRoleName(ctx.roleName.getText());
+ return stmt;
+ }
+
+ @Override
+ public Node visitGrantUserRole(RelationalSqlParser.GrantUserRoleContext ctx)
{
+ AuthorRStatement stmt = new AuthorRStatement(AuthorRType.GRANT_USER_ROLE);
+ stmt.setUserName(ctx.userName.getText());
+ stmt.setRoleName(ctx.roleName.getText());
+ return stmt;
+ }
+
+ @Override
+ public Node visitRevokeUserRole(RelationalSqlParser.RevokeUserRoleContext
ctx) {
+ AuthorRStatement stmt = new AuthorRStatement(AuthorRType.REVOKE_USER_ROLE);
+ stmt.setUserName(ctx.userName.getText());
+ stmt.setRoleName(ctx.roleName.getText());
+ return stmt;
+ }
+
+ @Override
+ public Node
visitListUserPrivileges(RelationalSqlParser.ListUserPrivilegesContext ctx) {
+ AuthorRStatement stmt = new AuthorRStatement(AuthorRType.LIST_USER);
+ stmt.setUserName(ctx.userName.getText());
+ return stmt;
+ }
+
+ @Override
+ public Node
visitListRolePrivileges(RelationalSqlParser.ListRolePrivilegesContext ctx) {
+ AuthorRStatement stmt = new AuthorRStatement(AuthorRType.LIST_ROLE);
+ stmt.setRoleName(ctx.roleName.getText());
+ return stmt;
+ }
+
+ @Override
+ public Node visitGrantStatement(RelationalSqlParser.GrantStatementContext
ctx) {
+ boolean toUser;
+ String username;
+ String objectName;
+ toUser = ctx.roleType().USER() != null;
+ username = ctx.role_name.getText();
+ boolean grantOption = ctx.grantOpt() != null;
+ boolean toTable = false;
+
+ if (ctx.grant_privilege_object().ON() != null) {
+ String privilegeText =
ctx.grant_privilege_object().object_privilege().getText();
+ PrivilegeType priv = PrivilegeType.valueOf(privilegeText.toUpperCase());
+ if
(ctx.grant_privilege_object().object_type().getText().equalsIgnoreCase("TABLE"))
{
Review Comment:
Are the "db.table" and "ANY" taken into account here?
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlan.java:
##########
@@ -317,7 +318,28 @@ public static ConfigPhysicalPlan create(ByteBuffer buffer)
throws IOException {
case RevokeRoleFromUser:
case UpdateUser:
case CreateUserWithRawPassword:
- plan = new AuthorPlan(configPhysicalPlanType);
+ plan = new AuthorTreePlan(configPhysicalPlanType);
+ break;
+ case RCreateRole:
Review Comment:
Better keep the order consistent to the definitions.
##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/user/LocalFileUserManagerTest.java:
##########
@@ -1,30 +1,8 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
Review Comment:
....
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/AuthorInfo.java:
##########
@@ -311,9 +273,84 @@ public TSStatus authorNonQuery(AuthorPlan authorPlan) {
}
} catch (AuthException e) {
return RpcUtils.getStatus(e.getCode(), e.getMessage());
- } finally {
- authorizer.setUserForPreVersion(false);
- authorizer.setRoleForPreVersion(false);
+ }
+ return RpcUtils.getStatus(TSStatusCode.SUCCESS_STATUS);
+ }
+
+ public TSStatus authorNonQuery(AuthorRelationalPlan authorPlan) {
Review Comment:
Don't we not support update password here?
##########
iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanSerDeTest.java:
##########
@@ -576,7 +577,7 @@ public void AuthorPlanTest() throws IOException,
IllegalPathException {
// create user
req0 =
- new AuthorPlan(
+ new AuthorTreePlan(
Review Comment:
Remember to add the ser/de tests for new plans.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/auth/AuthorRelationalPlan.java:
##########
@@ -0,0 +1,139 @@
+package org.apache.iotdb.confignode.consensus.request.auth;
+
+import org.apache.iotdb.commons.auth.entity.PrivilegeType;
+import org.apache.iotdb.commons.utils.BasicStructureSerDeUtil;
+import org.apache.iotdb.confignode.consensus.request.ConfigPhysicalPlanType;
+
+import org.apache.tsfile.utils.ReadWriteIOUtils;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.Objects;
+import java.util.Set;
+
+public class AuthorRelationalPlan extends AuthorPlan {
+ private int permission;
+ private String databaseName;
+ private String tableName;
+
+ public AuthorRelationalPlan(final ConfigPhysicalPlanType authorType) {
+ super(authorType);
+ }
+
+ public AuthorRelationalPlan(
+ final ConfigPhysicalPlanType authorType,
+ final String userName,
+ final String roleName,
+ final String databaseName,
+ final String tableName,
+ final boolean grantOpt,
+ final int permission,
+ final String password) {
+ super(authorType);
Review Comment:
May directly call the super() with more parameters.
##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/user/LocalFileUserAccessorTest.java:
##########
@@ -1,25 +1,7 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
Review Comment:
.....
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/ast/AuthorRStatement.java:
##########
@@ -0,0 +1,189 @@
+package org.apache.iotdb.db.queryengine.plan.relational.sql.ast;
+
+import org.apache.iotdb.commons.auth.entity.PrivilegeType;
+import org.apache.iotdb.db.queryengine.plan.analyze.QueryType;
+import org.apache.iotdb.db.queryengine.plan.relational.type.AuthorRType;
+
+import com.google.common.collect.ImmutableList;
+
+import java.util.List;
+import java.util.Objects;
+
+public class AuthorRStatement extends Statement {
+
+ private final AuthorRType authorType;
+
+ private String tableName;
+ private String database;
+ private String userName;
+ private String roleName;
+
+ private String password;
+
+ private PrivilegeType privilegeType;
+
+ private boolean grantOption;
+
+ public AuthorRStatement(
+ AuthorRType authorType,
+ String database,
+ String table,
+ PrivilegeType type,
+ String username,
+ String rolename,
+ boolean grantOption) {
+ super(null);
+ this.authorType = authorType;
+ this.database = database;
+ this.tableName = table;
+ this.privilegeType = type;
+ this.roleName = rolename;
+ this.userName = username;
+ this.grantOption = grantOption;
+ }
+
+ public AuthorRStatement(AuthorRType statementType) {
+ super(null);
+ this.authorType = statementType;
+ }
+
+ public AuthorRStatement(
+ AuthorRType statementType,
+ PrivilegeType type,
+ String username,
+ String roleName,
+ boolean grantOption) {
+ super(null);
+ this.authorType = statementType;
+ this.privilegeType = type;
+ this.userName = username;
+ this.roleName = roleName;
+ this.grantOption = grantOption;
+ this.tableName = null;
+ this.database = null;
+ }
+
+ public AuthorRType getAuthorType() {
+ return authorType;
+ }
+
+ public String getTableName() {
+ return tableName;
+ }
+
+ public String getDatabase() {
+ return this.database;
+ }
+
+ public String getUserName() {
+ return userName;
+ }
+
+ public String getRoleName() {
+ return roleName;
+ }
+
+ public String getPassword() {
+ return this.password;
+ }
+
+ public boolean hasGrantOption() {
+ return grantOption;
+ }
+
+ public PrivilegeType getPrivilegeType() {
+ return privilegeType;
+ }
+
+ public void setDatabase(String database) {
+ this.database = database;
+ }
+
+ public void setUserName(String userName) {
+ this.userName = userName;
+ }
+
+ public void setRoleName(String roleName) {
+ this.roleName = roleName;
+ }
+
+ public void setPassword(String password) {
+ this.password = password;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ AuthorRStatement that = (AuthorRStatement) o;
+ return grantOption == that.grantOption
+ && authorType == that.authorType
+ && Objects.equals(database, that.database)
+ && Objects.equals(tableName, that.tableName)
+ && Objects.equals(userName, that.userName)
+ && Objects.equals(roleName, that.roleName);
+ }
+
+ @Override
+ public <R, C> R accept(AstVisitor<R, C> visitor, C context) {
+ return visitor.visitAuthorRPlan(this, context);
+ }
+
+ @Override
+ public List<Node> getChildren() {
+ return ImmutableList.of();
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(
+ authorType, database, tableName, privilegeType, userName, roleName,
grantOption);
+ }
+
+ public QueryType getQueryType() {
+ switch (this.authorType) {
+ case GRANT_ROLE_DB:
+ case GRANT_USER_DB:
+ case REVOKE_ROLE_DB:
+ case REVOKE_USER_DB:
+ case GRANT_ROLE_TB:
+ case GRANT_USER_TB:
+ case REVOKE_ROLE_TB:
+ case REVOKE_USER_TB:
+ case DROP_ROLE:
+ case DROP_USER:
+ case GRANT_USER_ROLE:
+ case GRANT_USER_SYS:
+ case GRANT_ROLE_SYS:
+ case CREATE_ROLE:
+ case CREATE_USER:
+ case REVOKE_ROLE_SYS:
+ case REVOKE_USER_SYS:
+ case REVOKE_USER_ROLE:
+ return QueryType.WRITE;
+ case LIST_ROLE:
+ case LIST_USER:
Review Comment:
Has the "List privilege" been designed, or are they shown in the "list
role/user"?
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/parser/AstBuilder.java:
##########
@@ -702,6 +704,113 @@ public Node
visitExplain(RelationalSqlParser.ExplainContext ctx) {
public Node visitExplainAnalyze(RelationalSqlParser.ExplainAnalyzeContext
ctx) {
return super.visitExplainAnalyze(ctx);
}
+ // ********************** 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
Review Comment:
Bettter add a blank line, and why do we use "''" in the password?
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/auth/AuthorRelationalPlan.java:
##########
@@ -0,0 +1,139 @@
+package org.apache.iotdb.confignode.consensus.request.auth;
Review Comment:
May add license?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
