Caideyipi commented on code in PR #13158:
URL: https://github.com/apache/iotdb/pull/13158#discussion_r1905069061
##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/authorizer/LocalFileAuthorizerTest.java:
##########
@@ -101,174 +98,143 @@ public void createAndDeleteUser() throws AuthException {
@Test
public void createAndDeleteRole() throws AuthException {
- authorizer.createRole(user.getName());
+ authorizer.createRole(userName);
try {
- authorizer.createRole(user.getName());
+ authorizer.createRole(userName);
} catch (AuthException e) {
assertEquals("Role user already exists", e.getMessage());
}
- authorizer.deleteRole(user.getName());
+ authorizer.createUser("test", "password");
Review Comment:
Better use "userName"?
##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/user/LocalFileUserAccessorTest.java:
##########
@@ -65,100 +62,54 @@ public void tearDown() throws Exception {
@Test
public void test() throws IOException, IllegalPathException {
- User[] users = new User[4];
- for (int i = 0; i < users.length; i++) {
- users[i] = new User("user" + i, "password" + i);
- for (int j = 0; j <= i; j++) {
- PathPrivilege pathPrivilege = new PathPrivilege(new
PartialPath("root.a.b.c" + j));
- pathPrivilege.getPrivileges().add(j);
- users[i].getPathPrivilegeList().add(pathPrivilege);
- users[i].getSysPrivilege().add(j + 5);
- users[i].getRoleList().add("role" + j);
- }
- }
-
- // save
- for (User user : users) {
- try {
- accessor.saveUser(user);
- } catch (IOException e) {
- fail(e.getMessage());
- }
- }
-
- // load
- for (User user : users) {
- try {
- User loadedUser = accessor.loadUser(user.getName());
- assertEquals(user, loadedUser);
- } catch (IOException e) {
- fail(e.getMessage());
- }
- }
- assertNull(accessor.loadUser("not a user"));
-
- // list
- List<String> usernames = accessor.listAllUsers();
- usernames.sort(null);
- for (int i = 0; i < users.length; i++) {
- assertEquals(users[i].getName(), usernames.get(i));
- }
-
- // delete
- assertTrue(accessor.deleteUser("not a user"));
- assertTrue(accessor.deleteUser(users[users.length - 1].getName()));
- usernames = accessor.listAllUsers();
- assertEquals(users.length - 1, usernames.size());
- usernames.sort(null);
- for (int i = 0; i < users.length - 1; i++) {
- assertEquals(users[i].getName(), usernames.get(i));
- }
- User nullUser = accessor.loadUser(users[users.length - 1].getName());
- assertNull(nullUser);
+ User user = new User("test", "password");
+ user.grantSysPrivilege(PrivilegeType.EXTEND_TEMPLATE, false);
+ user.grantSysPrivilege(PrivilegeType.MANAGE_USER, false);
+ PathPrivilege pathPrivilege = new PathPrivilege(new
PartialPath("root.test"));
+ pathPrivilege.grantPrivilege(PrivilegeType.READ_DATA, true);
+ pathPrivilege.grantPrivilege(PrivilegeType.WRITE_DATA, false);
+ user.getPathPrivilegeList().add(pathPrivilege);
+ user.grantAnyScopePrivilege(PrivilegeType.SELECT, false);
+ user.grantAnyScopePrivilege(PrivilegeType.ALTER, true);
+ user.grantDBPrivilege("testdb", PrivilegeType.SELECT, false);
+ user.grantTBPrivilege("testdb", "testtb", PrivilegeType.ALTER, true);
+ user.addRole("testRole1");
+ user.addRole("testRole2");
+ accessor.saveEntry(user);
+ accessor.reset();
+ User loadUser = accessor.loadEntry("test");
+ assertEquals(user, loadUser);
Review Comment:
Better reserve the "List" and "Delete" tests.
##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/authorizer/LocalFileAuthorizerTest.java:
##########
@@ -101,174 +98,143 @@ public void createAndDeleteUser() throws AuthException {
@Test
public void createAndDeleteRole() throws AuthException {
- authorizer.createRole(user.getName());
+ authorizer.createRole(userName);
try {
- authorizer.createRole(user.getName());
+ authorizer.createRole(userName);
} catch (AuthException e) {
assertEquals("Role user already exists", e.getMessage());
}
- authorizer.deleteRole(user.getName());
+ authorizer.createUser("test", "password");
+ authorizer.grantRoleToUser(userName, "test");
+ authorizer.deleteRole(userName);
try {
- authorizer.deleteRole(user.getName());
+ authorizer.deleteRole(userName);
} catch (AuthException e) {
assertEquals("Role user does not exist", e.getMessage());
}
+ assertEquals(0, authorizer.getUser("test").getRoleSet().size());
}
@Test
- public void testUserPermission() throws AuthException {
- authorizer.createUser(user.getName(), user.getPassword());
- String admin = "root";
+ public void testTreePermission() throws AuthException {
+ authorizer.createUser(userName, password);
authorizer.grantPrivilegeToUser(
- user.getName(), nodeName, PrivilegeType.READ_DATA.ordinal(), false);
+ userName, new PrivilegeUnion(nodeName, PrivilegeType.READ_DATA,
false));
try {
authorizer.grantPrivilegeToUser(
- user.getName(), nodeName, PrivilegeType.READ_DATA.ordinal(), false);
+ userName, new PrivilegeUnion(nodeName, PrivilegeType.READ_DATA,
false));
} catch (AuthException e) {
assertEquals("User user already has READ_DATA on root.laptop.d1",
e.getMessage());
}
try {
- authorizer.grantPrivilegeToUser("error", nodeName,
PrivilegeType.READ_DATA.ordinal(), false);
+ authorizer.grantPrivilegeToUser(
+ "error", new PrivilegeUnion(nodeName, PrivilegeType.READ_DATA,
false));
} catch (AuthException e) {
assertEquals("No such user error", e.getMessage());
}
try {
- authorizer.grantPrivilegeToUser("root", nodeName,
PrivilegeType.READ_DATA.ordinal(), false);
+ authorizer.grantPrivilegeToUser(
+ "root", new PrivilegeUnion(nodeName, PrivilegeType.READ_DATA,
false));
} catch (AuthException e) {
Assert.assertEquals(
"Invalid operation, administrator already has all privileges",
e.getMessage());
}
- try {
- authorizer.grantPrivilegeToUser(user.getName(), nodeName, 100, false);
- } catch (AuthException e) {
- assertEquals("Invalid privilegeId 100", e.getMessage());
- }
-
- authorizer.revokePrivilegeFromUser(user.getName(), nodeName,
PrivilegeType.READ_DATA.ordinal());
+ authorizer.revokePrivilegeFromUser(
+ userName, new PrivilegeUnion(nodeName, PrivilegeType.READ_DATA));
try {
authorizer.revokePrivilegeFromUser(
- user.getName(), nodeName, PrivilegeType.READ_DATA.ordinal());
+ userName, new PrivilegeUnion(nodeName, PrivilegeType.READ_DATA));
} catch (AuthException e) {
assertEquals("User user does not have READ_DATA on root.laptop.d1",
e.getMessage());
Review Comment:
This will not be thrown now, do not catch it
##########
iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/authorizer/LocalFileAuthorizerTest.java:
##########
@@ -101,174 +98,143 @@ public void createAndDeleteUser() throws AuthException {
@Test
public void createAndDeleteRole() throws AuthException {
- authorizer.createRole(user.getName());
+ authorizer.createRole(userName);
Review Comment:
Better use "roleName"?
--
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]
