Caideyipi commented on code in PR #13158:
URL: https://github.com/apache/iotdb/pull/13158#discussion_r1901430688
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/pipe/extractor/PipeConfigPhysicalPlanPatternParseVisitor.java:
##########
@@ -189,27 +190,28 @@ public Optional<ConfigPhysicalPlan> visitRevokeRole(
private Optional<ConfigPhysicalPlan> visitPathRelatedAuthorPlan(
final AuthorPlan pathRelatedAuthorPlan, final IoTDBTreePattern pattern) {
+ AuthorTreePlan plan = (AuthorTreePlan) pathRelatedAuthorPlan;
final List<PartialPath> intersectedPaths =
- pathRelatedAuthorPlan.getNodeNameList().stream()
+ plan.getNodeNameList().stream()
.map(pattern::getIntersection)
.flatMap(Collection::stream)
.collect(Collectors.toList());
final Set<Integer> permissions =
!intersectedPaths.isEmpty()
- ? pathRelatedAuthorPlan.getPermissions()
- : pathRelatedAuthorPlan.getPermissions().stream()
- .filter(permission ->
!PrivilegeType.values()[permission].isPathRelevant())
+ ? plan.getPermissions()
+ : plan.getPermissions().stream()
+ .filter(permission ->
!PrivilegeType.values()[permission].isPathPrivilege())
.collect(Collectors.toSet());
return !permissions.isEmpty()
? Optional.of(
- new AuthorPlan(
- pathRelatedAuthorPlan.getAuthorType(),
- pathRelatedAuthorPlan.getUserName(),
- pathRelatedAuthorPlan.getRoleName(),
- pathRelatedAuthorPlan.getPassword(),
- pathRelatedAuthorPlan.getNewPassword(),
- permissions,
- pathRelatedAuthorPlan.getGrantOpt(),
+ new AuthorTreePlan(
+ plan.getAuthorType(),
+ plan.getUserName(),
+ plan.getRoleName(),
+ plan.getPassword(),
+ plan.getNewPassword(),
+ plan.getPermissions(),
Review Comment:
Better change this to back to "permissions"... This has been trimmed by
user's pattern....
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/write/auth/AuthorRelationalPlan.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.
+ */
+package org.apache.iotdb.confignode.consensus.request.write.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 {
+ protected int permission;
+ protected String databaseName;
+ protected 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 int permission,
+ final boolean grantOpt,
+ final String password) {
+ super(authorType);
+ this.userName = userName;
+ this.roleName = roleName;
+ this.grantOpt = grantOpt;
+ this.databaseName = databaseName;
+ this.tableName = tableName;
+ this.permission = permission;
+ this.password = password;
+ }
+
+ public String getDatabaseName() {
+ return databaseName;
+ }
+
+ public String getTableName() {
+ return tableName;
+ }
+
+ public Set<Integer> getPermissions() {
+ return Collections.singleton(permission);
+ }
+
+ public int getPermission() {
+ return permission;
+ }
+
+ public void setPermission(int permission) {
+ this.permission = permission;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o instanceof AuthorRelationalPlan) {
+ AuthorRelationalPlan that = (AuthorRelationalPlan) o;
+ return super.equals(o)
+ && Objects.equals(this.databaseName, that.databaseName)
+ && Objects.equals(this.tableName, that.tableName)
+ && Objects.equals(this.permission, that.permission);
+ } else {
+ return false;
+ }
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(super.hashCode(), databaseName, tableName, permission);
+ }
+
+ @Override
+ public String toString() {
+ return "[type:"
+ + super.getType()
+ + ", name:"
+ + userName
+ + ", role:"
+ + roleName
+ + ", permissions"
+ + (permission == -1 ? "-1" : PrivilegeType.values()[permission])
+ + ", grant option:"
+ + grantOpt
+ + ", DB:"
+ + databaseName
+ + ", TABLE:"
+ + tableName
+ + "]";
+ }
+
+ @Override
+ protected void serializeImpl(DataOutputStream stream) throws IOException {
+ ReadWriteIOUtils.write(getType().getPlanType(), stream);
+ BasicStructureSerDeUtil.write(userName, stream);
+ BasicStructureSerDeUtil.write(roleName, stream);
+ BasicStructureSerDeUtil.write(password, stream);
Review Comment:
Do you need to write "newPassword"?
##########
iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/utils/AuthUtilsTest.java:
##########
@@ -63,82 +63,78 @@ public void authUtilsTest_ParameterCheck() throws
AuthException, IllegalPathExce
public void authUtilsTest_PrivilegeGrantRevokeCheck() throws
IllegalPathException {
PartialPath path = new PartialPath(new String("root.t1"));
PathPrivilege pathWithPri = new PathPrivilege(path);
- pathWithPri.grantPrivilege(PrivilegeType.READ_SCHEMA.ordinal(), false);
- pathWithPri.grantPrivilege(PrivilegeType.READ_DATA.ordinal(), false);
+ pathWithPri.grantPrivilege(PrivilegeType.READ_SCHEMA, false);
+ pathWithPri.grantPrivilege(PrivilegeType.READ_DATA, false);
PartialPath path2 = new PartialPath(new String("root.t2"));
PathPrivilege pathWithPri2 = new PathPrivilege(path2);
- pathWithPri2.grantPrivilege(PrivilegeType.WRITE_SCHEMA.ordinal(), false);
+ pathWithPri2.grantPrivilege(PrivilegeType.WRITE_SCHEMA, false);
PartialPath path3 = new PartialPath(new String("root.**"));
PathPrivilege pathWithPri3 = new PathPrivilege(path3);
- pathWithPri3.grantPrivilege(PrivilegeType.READ_DATA.ordinal(), false);
+ pathWithPri3.grantPrivilege(PrivilegeType.READ_DATA, false);
/** root.t1 : read schema, read data; root.t2 : write schema; root.** :
read data */
// Privilege list is empty.
- Assert.assertFalse(
- AuthUtils.checkPathPrivilege(path2,
PrivilegeType.READ_SCHEMA.ordinal(), null));
+ Assert.assertFalse(AuthUtils.checkPathPrivilege(path2,
PrivilegeType.READ_SCHEMA, null));
List<PathPrivilege> privilegeList = new ArrayList<>();
privilegeList.add(pathWithPri);
privilegeList.add(pathWithPri2);
privilegeList.add(pathWithPri3);
Assert.assertTrue(
- AuthUtils.checkPathPrivilege(path2,
PrivilegeType.READ_SCHEMA.ordinal(), privilegeList));
- Assert.assertTrue(
- AuthUtils.checkPathPrivilege(path,
PrivilegeType.READ_SCHEMA.ordinal(), privilegeList));
+ AuthUtils.checkPathPrivilege(path2, PrivilegeType.READ_SCHEMA,
privilegeList));
+ Assert.assertTrue(AuthUtils.checkPathPrivilege(path,
PrivilegeType.READ_SCHEMA, privilegeList));
- pathWithPri.revokePrivilege(PrivilegeType.READ_SCHEMA.ordinal());
+ pathWithPri.revokePrivilege(PrivilegeType.READ_SCHEMA);
/** root.t1 : read data; root.t2 : write schema ; root.** : read data */
Assert.assertFalse(
- AuthUtils.checkPathPrivilege(path,
PrivilegeType.READ_SCHEMA.ordinal(), privilegeList));
- Assert.assertTrue(
- AuthUtils.checkPathPrivilege(path, PrivilegeType.READ_DATA.ordinal(),
privilegeList));
+ AuthUtils.checkPathPrivilege(path, PrivilegeType.READ_SCHEMA,
privilegeList));
+ Assert.assertTrue(AuthUtils.checkPathPrivilege(path,
PrivilegeType.READ_DATA, privilegeList));
// root.t2 have read data privilege because root.**
- Assert.assertTrue(
- AuthUtils.checkPathPrivilege(path2, PrivilegeType.READ_DATA.ordinal(),
privilegeList));
+ Assert.assertTrue(AuthUtils.checkPathPrivilege(path2,
PrivilegeType.READ_DATA, privilegeList));
Assert.assertFalse(
- AuthUtils.hasPrivilegeToReovke(path2,
PrivilegeType.READ_DATA.ordinal(), privilegeList));
+ AuthUtils.hasPrivilegeToRevoke(path2, PrivilegeType.READ_DATA,
privilegeList));
Assert.assertEquals(AuthUtils.getPrivileges(path, privilegeList).size(),
1);
Assert.assertEquals(AuthUtils.getPrivileges(path, null).size(), 0);
- pathWithPri.grantPrivilege(PrivilegeType.WRITE_DATA.ordinal(), false);
+ pathWithPri.grantPrivilege(PrivilegeType.WRITE_DATA, false);
Assert.assertTrue(
- AuthUtils.getPrivileges(path,
privilegeList).contains(PrivilegeType.WRITE_DATA.ordinal()));
+ AuthUtils.getPrivileges(path,
privilegeList).contains(PrivilegeType.WRITE_DATA));
}
@Test
public void authUtilsTest_PathPrivilegeAddRemove() throws
IllegalPathException, AuthException {
List<PathPrivilege> privs = new ArrayList<>();
PartialPath path1 = new PartialPath("root.t1");
- AuthUtils.addPrivilege(path1, PrivilegeType.READ_SCHEMA.ordinal(), privs,
false);
- AuthUtils.addPrivilege(path1, PrivilegeType.READ_DATA.ordinal(), privs,
false);
- AuthUtils.addPrivilege(path1, PrivilegeType.WRITE_SCHEMA.ordinal(), privs,
true);
+ AuthUtils.addPrivilege(path1, PrivilegeType.READ_SCHEMA, privs, false);
+ AuthUtils.addPrivilege(path1, PrivilegeType.READ_DATA, privs, false);
+ AuthUtils.addPrivilege(path1, PrivilegeType.WRITE_SCHEMA, privs, true);
Assert.assertEquals(privs.size(), 1);
- Assert.assertEquals(privs.get(0).getPrivileges().size(), 3);
+ Assert.assertEquals(privs.get(0).getPrivilegeIntSet().size(), 3);
Assert.assertEquals(privs.get(0).getGrantOpt().size(), 1);
PartialPath path2 = new PartialPath("root.t2");
- AuthUtils.addPrivilege(path2, PrivilegeType.READ_SCHEMA.ordinal(), privs,
false);
+ AuthUtils.addPrivilege(path2, PrivilegeType.READ_SCHEMA, privs, false);
Assert.assertEquals(privs.size(), 2);
- AuthUtils.removePrivilege(path2, PrivilegeType.READ_SCHEMA.ordinal(),
privs);
+ AuthUtils.removePrivilege(path2, PrivilegeType.READ_SCHEMA, privs);
Assert.assertEquals(privs.size(), 1);
// if we revoke privileges on root.**, privileges on root.t1 and root.t2
will also be removed.
PartialPath rootPath = new PartialPath("root.**");
- AuthUtils.removePrivilege(rootPath, PrivilegeType.READ_DATA.ordinal(),
privs);
- Assert.assertEquals(privs.get(0).getPrivileges().size(), 2);
-
Assert.assertFalse(privs.get(0).getPrivileges().contains(PrivilegeType.READ_DATA.ordinal()));
+ AuthUtils.removePrivilege(rootPath, PrivilegeType.READ_DATA, privs);
+ Assert.assertEquals(privs.get(0).getPrivilegeIntSet().size(), 2);
+
Assert.assertFalse(privs.get(0).getPrivilegeIntSet().contains(PrivilegeType.READ_DATA));
Review Comment:
Always false?
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/column/ColumnHeaderConstant.java:
##########
@@ -604,7 +605,7 @@ private ColumnHeaderConstant() {
public static final List<ColumnHeader> LIST_USER_PRIVILEGES_Column_HEADERS =
Review Comment:
Use upper case here, and better name it as "USER_OR_ROLE_PRIVILEGES"
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/user/BasicUserManager.java:
##########
@@ -132,15 +100,11 @@ private void initAdmin() throws AuthException {
}
@Override
- public User getUser(String username) throws AuthException {
- lock.readLock(username);
- User user = userMap.get(username);
- lock.readUnlock(username);
- return user;
+ public User getEntry(String username) {
+ return (User) super.getEntry(username);
}
- @Override
- public boolean createUser(
+ public boolean createEntry(
Review Comment:
Name it "createUser" if this is not overrided from its super class.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TableModelPrivilege.java:
##########
@@ -19,6 +19,8 @@
package org.apache.iotdb.db.queryengine.plan.relational.security;
+import org.apache.iotdb.commons.auth.entity.PrivilegeType;
+
public enum TableModelPrivilege {
Review Comment:
Why do we need this class? What if we just reuse the "PrivilegeType"?
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControl.java:
##########
@@ -130,4 +131,12 @@ public interface AccessControl {
* @throws AccessDeniedException if not allowed
*/
void checkUserHasMaintainPrivilege(String userName);
+
+ /**
+ * Check if user can run relational author statement.
+ *
+ * @param userName name of user
+ * @throws AccessDeniedException if not allowed
+ */
+ void checkUserCanRunAuthorStatement(String userName,
RelationalAuthorStatement statement);
Review Comment:
checkUserCanRunRelationalAuthorStatement?
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/CNPhysicalPlanGenerator.java:
##########
@@ -181,23 +181,19 @@ public void checkException() throws Exception {
private void generateUserRolePhysicalPlan(boolean isUser) {
try (DataInputStream dataInputStream =
new DataInputStream(new BufferedInputStream(inputStream))) {
- final Pair<String, Boolean> versionAndName =
- readAuthString(dataInputStream, STRING_ENCODING, strBufferLocal);
- if (versionAndName == null) {
- return;
- }
- String user = versionAndName.left;
+ int tag = dataInputStream.readInt();
+ String user = readString(dataInputStream, STRING_ENCODING,
strBufferLocal);
Review Comment:
The older version's metadata shall be able to reach the newer version by
pipe....
##########
iotdb-core/relational-grammar/src/main/antlr4/org/apache/iotdb/db/relational/grammar/sql/RelationalSql.g4:
##########
@@ -1191,6 +1301,7 @@ PASSING: 'PASSING';
PAST: 'PAST';
PATH: 'PATH';
PATTERN: 'PATTERN';
+PASSWORD: 'PASSWORD';
Review Comment:
Be sure to check the dictionary order...
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/write/auth/AuthorRelationalPlan.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.
+ */
+package org.apache.iotdb.confignode.consensus.request.write.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 {
+ protected int permission;
+ protected String databaseName;
+ protected 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 int permission,
+ final boolean grantOpt,
+ final String password) {
+ super(authorType);
+ this.userName = userName;
+ this.roleName = roleName;
+ this.grantOpt = grantOpt;
+ this.databaseName = databaseName;
+ this.tableName = tableName;
+ this.permission = permission;
+ this.password = password;
+ }
+
+ public String getDatabaseName() {
+ return databaseName;
+ }
+
+ public String getTableName() {
+ return tableName;
+ }
+
+ public Set<Integer> getPermissions() {
+ return Collections.singleton(permission);
+ }
+
+ public int getPermission() {
+ return permission;
+ }
+
+ public void setPermission(int permission) {
+ this.permission = permission;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o instanceof AuthorRelationalPlan) {
+ AuthorRelationalPlan that = (AuthorRelationalPlan) o;
+ return super.equals(o)
+ && Objects.equals(this.databaseName, that.databaseName)
+ && Objects.equals(this.tableName, that.tableName)
+ && Objects.equals(this.permission, that.permission);
+ } else {
+ return false;
+ }
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(super.hashCode(), databaseName, tableName, permission);
+ }
+
+ @Override
+ public String toString() {
+ return "[type:"
+ + super.getType()
+ + ", name:"
+ + userName
+ + ", role:"
+ + roleName
+ + ", permissions"
+ + (permission == -1 ? "-1" : PrivilegeType.values()[permission])
+ + ", grant option:"
+ + grantOpt
+ + ", DB:"
+ + databaseName
+ + ", TABLE:"
+ + tableName
+ + "]";
+ }
+
+ @Override
+ protected void serializeImpl(DataOutputStream stream) throws IOException {
+ ReadWriteIOUtils.write(getType().getPlanType(), stream);
+ BasicStructureSerDeUtil.write(userName, stream);
+ BasicStructureSerDeUtil.write(roleName, stream);
+ BasicStructureSerDeUtil.write(password, stream);
+ if (databaseName == null) {
Review Comment:
Can we write this string directly?
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/pipe/extractor/PipeConfigPhysicalPlanPatternParseVisitor.java:
##########
@@ -189,27 +190,28 @@ public Optional<ConfigPhysicalPlan> visitRevokeRole(
private Optional<ConfigPhysicalPlan> visitPathRelatedAuthorPlan(
final AuthorPlan pathRelatedAuthorPlan, final IoTDBTreePattern pattern) {
Review Comment:
Better change this to AuthorTreePlan BTW.... Though of course you can leave
it to me...
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/write/auth/AuthorRelationalPlan.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.
+ */
+package org.apache.iotdb.confignode.consensus.request.write.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 {
+ protected int permission;
+ protected String databaseName;
+ protected 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 int permission,
+ final boolean grantOpt,
+ final String password) {
+ super(authorType);
+ this.userName = userName;
+ this.roleName = roleName;
+ this.grantOpt = grantOpt;
+ this.databaseName = databaseName;
+ this.tableName = tableName;
+ this.permission = permission;
+ this.password = password;
+ }
+
+ public String getDatabaseName() {
+ return databaseName;
+ }
+
+ public String getTableName() {
+ return tableName;
+ }
+
+ public Set<Integer> getPermissions() {
Review Comment:
Seemingly not useful?
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/user/BasicUserManager.java:
##########
@@ -92,38 +68,30 @@ protected BasicUserManager(IUserAccessor accessor) throws
AuthException {
* @throws AuthException if an exception is raised when interacting with the
lower storage.
*/
private void initAdmin() throws AuthException {
- User admin;
- try {
- admin =
getUser(CommonDescriptor.getInstance().getConfig().getAdminName());
- } catch (AuthException e) {
- LOGGER.warn("Cannot load admin, Creating a new one", e);
- admin = null;
- }
+ User admin =
getEntry(CommonDescriptor.getInstance().getConfig().getAdminName());
Review Comment:
”Entry“ may be too unclear... Maybe "identity" is better?
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/util/SqlFormatter.java:
##########
@@ -1169,6 +1170,67 @@ protected Void visitShowTopics(ShowTopics node, Integer
context) {
return null;
}
+ @Override
+ protected Void visitRelationalAuthorPlan(RelationalAuthorStatement node,
Integer context) {
Review Comment:
If you implement this you may write the full sql identical to the input one,
not just a header like "GRANT USER PRIVILEGES"
##########
iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/utils/AuthUtilsTest.java:
##########
@@ -63,82 +63,78 @@ public void authUtilsTest_ParameterCheck() throws
AuthException, IllegalPathExce
public void authUtilsTest_PrivilegeGrantRevokeCheck() throws
IllegalPathException {
PartialPath path = new PartialPath(new String("root.t1"));
PathPrivilege pathWithPri = new PathPrivilege(path);
- pathWithPri.grantPrivilege(PrivilegeType.READ_SCHEMA.ordinal(), false);
- pathWithPri.grantPrivilege(PrivilegeType.READ_DATA.ordinal(), false);
+ pathWithPri.grantPrivilege(PrivilegeType.READ_SCHEMA, false);
+ pathWithPri.grantPrivilege(PrivilegeType.READ_DATA, false);
PartialPath path2 = new PartialPath(new String("root.t2"));
PathPrivilege pathWithPri2 = new PathPrivilege(path2);
- pathWithPri2.grantPrivilege(PrivilegeType.WRITE_SCHEMA.ordinal(), false);
+ pathWithPri2.grantPrivilege(PrivilegeType.WRITE_SCHEMA, false);
PartialPath path3 = new PartialPath(new String("root.**"));
PathPrivilege pathWithPri3 = new PathPrivilege(path3);
- pathWithPri3.grantPrivilege(PrivilegeType.READ_DATA.ordinal(), false);
+ pathWithPri3.grantPrivilege(PrivilegeType.READ_DATA, false);
/** root.t1 : read schema, read data; root.t2 : write schema; root.** :
read data */
// Privilege list is empty.
- Assert.assertFalse(
- AuthUtils.checkPathPrivilege(path2,
PrivilegeType.READ_SCHEMA.ordinal(), null));
+ Assert.assertFalse(AuthUtils.checkPathPrivilege(path2,
PrivilegeType.READ_SCHEMA, null));
List<PathPrivilege> privilegeList = new ArrayList<>();
privilegeList.add(pathWithPri);
privilegeList.add(pathWithPri2);
privilegeList.add(pathWithPri3);
Assert.assertTrue(
- AuthUtils.checkPathPrivilege(path2,
PrivilegeType.READ_SCHEMA.ordinal(), privilegeList));
- Assert.assertTrue(
- AuthUtils.checkPathPrivilege(path,
PrivilegeType.READ_SCHEMA.ordinal(), privilegeList));
+ AuthUtils.checkPathPrivilege(path2, PrivilegeType.READ_SCHEMA,
privilegeList));
+ Assert.assertTrue(AuthUtils.checkPathPrivilege(path,
PrivilegeType.READ_SCHEMA, privilegeList));
- pathWithPri.revokePrivilege(PrivilegeType.READ_SCHEMA.ordinal());
+ pathWithPri.revokePrivilege(PrivilegeType.READ_SCHEMA);
/** root.t1 : read data; root.t2 : write schema ; root.** : read data */
Assert.assertFalse(
- AuthUtils.checkPathPrivilege(path,
PrivilegeType.READ_SCHEMA.ordinal(), privilegeList));
- Assert.assertTrue(
- AuthUtils.checkPathPrivilege(path, PrivilegeType.READ_DATA.ordinal(),
privilegeList));
+ AuthUtils.checkPathPrivilege(path, PrivilegeType.READ_SCHEMA,
privilegeList));
+ Assert.assertTrue(AuthUtils.checkPathPrivilege(path,
PrivilegeType.READ_DATA, privilegeList));
// root.t2 have read data privilege because root.**
- Assert.assertTrue(
- AuthUtils.checkPathPrivilege(path2, PrivilegeType.READ_DATA.ordinal(),
privilegeList));
+ Assert.assertTrue(AuthUtils.checkPathPrivilege(path2,
PrivilegeType.READ_DATA, privilegeList));
Assert.assertFalse(
- AuthUtils.hasPrivilegeToReovke(path2,
PrivilegeType.READ_DATA.ordinal(), privilegeList));
+ AuthUtils.hasPrivilegeToRevoke(path2, PrivilegeType.READ_DATA,
privilegeList));
Assert.assertEquals(AuthUtils.getPrivileges(path, privilegeList).size(),
1);
Assert.assertEquals(AuthUtils.getPrivileges(path, null).size(), 0);
- pathWithPri.grantPrivilege(PrivilegeType.WRITE_DATA.ordinal(), false);
+ pathWithPri.grantPrivilege(PrivilegeType.WRITE_DATA, false);
Assert.assertTrue(
- AuthUtils.getPrivileges(path,
privilegeList).contains(PrivilegeType.WRITE_DATA.ordinal()));
+ AuthUtils.getPrivileges(path,
privilegeList).contains(PrivilegeType.WRITE_DATA));
}
@Test
public void authUtilsTest_PathPrivilegeAddRemove() throws
IllegalPathException, AuthException {
List<PathPrivilege> privs = new ArrayList<>();
PartialPath path1 = new PartialPath("root.t1");
- AuthUtils.addPrivilege(path1, PrivilegeType.READ_SCHEMA.ordinal(), privs,
false);
- AuthUtils.addPrivilege(path1, PrivilegeType.READ_DATA.ordinal(), privs,
false);
- AuthUtils.addPrivilege(path1, PrivilegeType.WRITE_SCHEMA.ordinal(), privs,
true);
+ AuthUtils.addPrivilege(path1, PrivilegeType.READ_SCHEMA, privs, false);
+ AuthUtils.addPrivilege(path1, PrivilegeType.READ_DATA, privs, false);
+ AuthUtils.addPrivilege(path1, PrivilegeType.WRITE_SCHEMA, privs, true);
Assert.assertEquals(privs.size(), 1);
- Assert.assertEquals(privs.get(0).getPrivileges().size(), 3);
+ Assert.assertEquals(privs.get(0).getPrivilegeIntSet().size(), 3);
Assert.assertEquals(privs.get(0).getGrantOpt().size(), 1);
PartialPath path2 = new PartialPath("root.t2");
- AuthUtils.addPrivilege(path2, PrivilegeType.READ_SCHEMA.ordinal(), privs,
false);
+ AuthUtils.addPrivilege(path2, PrivilegeType.READ_SCHEMA, privs, false);
Assert.assertEquals(privs.size(), 2);
- AuthUtils.removePrivilege(path2, PrivilegeType.READ_SCHEMA.ordinal(),
privs);
+ AuthUtils.removePrivilege(path2, PrivilegeType.READ_SCHEMA, privs);
Assert.assertEquals(privs.size(), 1);
// if we revoke privileges on root.**, privileges on root.t1 and root.t2
will also be removed.
PartialPath rootPath = new PartialPath("root.**");
- AuthUtils.removePrivilege(rootPath, PrivilegeType.READ_DATA.ordinal(),
privs);
- Assert.assertEquals(privs.get(0).getPrivileges().size(), 2);
-
Assert.assertFalse(privs.get(0).getPrivileges().contains(PrivilegeType.READ_DATA.ordinal()));
+ AuthUtils.removePrivilege(rootPath, PrivilegeType.READ_DATA, privs);
+ Assert.assertEquals(privs.get(0).getPrivilegeIntSet().size(), 2);
Review Comment:
May put the "expected value" first...
##########
integration-test/src/test/java/org/apache/iotdb/db/it/auth/IoTDBAuthIT.java:
##########
@@ -1115,9 +1118,10 @@ public void testRevokeAndGrantOpt() throws SQLException {
// user2 has all privileges without grant option on root.**
// user2 has all privileges without grant option on root.t1.**
for (PrivilegeType item : PrivilegeType.values()) {
+ if (item.isRelationalPrivilege()) continue;
Review Comment:
Better not use this code style..
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/user/BasicUserManager.java:
##########
@@ -132,15 +100,11 @@ private void initAdmin() throws AuthException {
}
@Override
- public User getUser(String username) throws AuthException {
- lock.readLock(username);
- User user = userMap.get(username);
- lock.readUnlock(username);
- return user;
+ public User getEntry(String username) {
+ return (User) super.getEntry(username);
Review Comment:
Do we need to override it here?
--
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]
