This is an automated email from the ASF dual-hosted git repository.
liuxun pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 688c1c9421 [#6116] fix(authz): Fix the check of duplicated privileges
(#6117)
688c1c9421 is described below
commit 688c1c94210016248cb3766e9e015f8fc4f7d377
Author: roryqi <[email protected]>
AuthorDate: Tue Jan 7 16:46:32 2025 +0800
[#6116] fix(authz): Fix the check of duplicated privileges (#6117)
### What changes were proposed in this pull request?
Fix the check of duplicated privileges
### Why are the changes needed?
Fix: #6116
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added new UT.
---
.../ranger/integration/test/RangerBaseE2EIT.java | 7 ++--
.../ranger/integration/test/RangerFilesetIT.java | 9 +++--
.../cli/commands/GrantPrivilegesToRole.java | 10 ++---
.../cli/commands/RevokePrivilegesFromRole.java | 10 ++---
.../org/apache/gravitino/client/DTOConverters.java | 3 +-
.../apache/gravitino/client/GravitinoClient.java | 45 +++++++++++++++++++++
.../apache/gravitino/client/GravitinoMetalake.java | 46 ++++++++++++++++++++++
.../apache/gravitino/client/TestPermission.java | 9 +++--
.../test/authorization/AccessControlIT.java | 34 ++++++++++------
.../authorization/AccessControlDispatcher.java | 5 ++-
.../authorization/AccessControlManager.java | 5 ++-
.../gravitino/authorization/PermissionManager.java | 14 ++++---
.../hook/AccessControlHookDispatcher.java | 5 ++-
.../TestAccessControlManagerForPermissions.java | 13 +++---
.../server/web/rest/PermissionOperations.java | 4 +-
15 files changed, 166 insertions(+), 53 deletions(-)
diff --git
a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerBaseE2EIT.java
b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerBaseE2EIT.java
index 919551bd92..d3b158fb1f 100644
---
a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerBaseE2EIT.java
+++
b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerBaseE2EIT.java
@@ -21,6 +21,7 @@ package
org.apache.gravitino.authorization.ranger.integration.test;
import static
org.apache.gravitino.authorization.ranger.integration.test.RangerITEnv.currentFunName;
import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
@@ -905,7 +906,7 @@ public abstract class RangerBaseE2EIT extends BaseIT {
MetadataObject catalogObject =
MetadataObjects.of(null, catalogName, MetadataObject.Type.CATALOG);
metalake.revokePrivilegesFromRole(
- roleName, catalogObject,
Lists.newArrayList(Privileges.CreateSchema.allow()));
+ roleName, catalogObject,
Sets.newHashSet(Privileges.CreateSchema.allow()));
waitForUpdatingPolicies();
// Use Spark to show this databases(schema)
@@ -920,7 +921,7 @@ public abstract class RangerBaseE2EIT extends BaseIT {
MetadataObject schemaObject =
MetadataObjects.of(catalogName, schemaName,
MetadataObject.Type.SCHEMA);
metalake.grantPrivilegesToRole(
- roleName, schemaObject,
Lists.newArrayList(Privileges.UseSchema.allow()));
+ roleName, schemaObject, Sets.newHashSet(Privileges.UseSchema.allow()));
waitForUpdatingPolicies();
// Use Spark to show this databases(schema) again
@@ -1020,7 +1021,7 @@ public abstract class RangerBaseE2EIT extends BaseIT {
metalake.grantPrivilegesToRole(
roleName,
MetadataObjects.of(null, metalakeName, MetadataObject.Type.METALAKE),
- Lists.newArrayList(Privileges.CreateSchema.allow()));
+ Sets.newHashSet(Privileges.CreateSchema.allow()));
// Fail to create a schema
Assertions.assertThrows(
diff --git
a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerFilesetIT.java
b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerFilesetIT.java
index ab74b0449a..0b1112278c 100644
---
a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerFilesetIT.java
+++
b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerFilesetIT.java
@@ -28,6 +28,7 @@ import static
org.apache.gravitino.integration.test.container.RangerContainer.RA
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
import java.io.IOException;
import java.security.PrivilegedExceptionAction;
import java.util.Arrays;
@@ -262,7 +263,7 @@ public class RangerFilesetIT extends BaseIT {
String.format("%s.%s", catalogName, schemaName),
fileset.name(),
MetadataObject.Type.FILESET),
- Lists.newArrayList(Privileges.WriteFileset.allow()));
+ Sets.newHashSet(Privileges.WriteFileset.allow()));
policies =
rangerClient.getPoliciesInService(RangerITEnv.RANGER_HDFS_REPO_NAME);
Assertions.assertEquals(1, policies.size());
@@ -320,7 +321,7 @@ public class RangerFilesetIT extends BaseIT {
String.format("%s.%s", catalogName, schemaName),
fileset.name(),
MetadataObject.Type.FILESET),
- Lists.newArrayList(Privileges.ReadFileset.allow(),
Privileges.WriteFileset.allow()));
+ Sets.newHashSet(Privileges.ReadFileset.allow(),
Privileges.WriteFileset.allow()));
policies =
rangerClient.getPoliciesInService(RangerITEnv.RANGER_HDFS_REPO_NAME);
Assertions.assertEquals(1, policies.size());
Assertions.assertEquals(3, policies.get(0).getPolicyItems().size());
@@ -460,7 +461,7 @@ public class RangerFilesetIT extends BaseIT {
fileset.name(),
MetadataObject.Type.FILESET);
metalake.grantPrivilegesToRole(
- filesetRole, filesetObject,
Lists.newArrayList(Privileges.WriteFileset.allow()));
+ filesetRole, filesetObject,
Sets.newHashSet(Privileges.WriteFileset.allow()));
RangerBaseE2EIT.waitForUpdatingPolicies();
UserGroupInformation.createProxyUser(userName,
UserGroupInformation.getCurrentUser())
.doAs(
@@ -488,7 +489,7 @@ public class RangerFilesetIT extends BaseIT {
metalake.revokePrivilegesFromRole(
filesetRole,
filesetObject,
- Lists.newArrayList(Privileges.ReadFileset.allow(),
Privileges.WriteFileset.allow()));
+ Sets.newHashSet(Privileges.ReadFileset.allow(),
Privileges.WriteFileset.allow()));
RangerBaseE2EIT.waitForUpdatingPolicies();
UserGroupInformation.createProxyUser(userName,
UserGroupInformation.getCurrentUser())
.doAs(
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/GrantPrivilegesToRole.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/GrantPrivilegesToRole.java
index 8630282ea6..b21a74ff48 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/GrantPrivilegesToRole.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/GrantPrivilegesToRole.java
@@ -19,8 +19,8 @@
package org.apache.gravitino.cli.commands;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.HashSet;
+import java.util.Set;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.cli.ErrorMessages;
@@ -69,7 +69,7 @@ public class GrantPrivilegesToRole extends MetadataCommand {
public void handle() {
try {
GravitinoClient client = buildClient(metalake);
- List<Privilege> privilegesList = new ArrayList<>();
+ Set<Privilege> privilegesSet = new HashSet<>();
for (String privilege : privileges) {
if (!Privileges.isValid(privilege)) {
@@ -81,11 +81,11 @@ public class GrantPrivilegesToRole extends MetadataCommand {
.withName(Privileges.toName(privilege))
.withCondition(Privilege.Condition.ALLOW)
.build();
- privilegesList.add(privilegeDTO);
+ privilegesSet.add(privilegeDTO);
}
MetadataObject metadataObject = constructMetadataObject(entity, client);
- client.grantPrivilegesToRole(role, metadataObject, privilegesList);
+ client.grantPrivilegesToRole(role, metadataObject, privilegesSet);
} catch (NoSuchMetalakeException err) {
System.err.println(ErrorMessages.UNKNOWN_METALAKE);
return;
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java
index 3bfa7cd452..fbf273ce0d 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java
@@ -19,8 +19,8 @@
package org.apache.gravitino.cli.commands;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.HashSet;
+import java.util.Set;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.cli.ErrorMessages;
@@ -69,7 +69,7 @@ public class RevokePrivilegesFromRole extends MetadataCommand
{
public void handle() {
try {
GravitinoClient client = buildClient(metalake);
- List<Privilege> privilegesList = new ArrayList<>();
+ Set<Privilege> privilegesSet = new HashSet<>();
for (String privilege : privileges) {
if (!Privileges.isValid(privilege)) {
@@ -81,11 +81,11 @@ public class RevokePrivilegesFromRole extends
MetadataCommand {
.withName(Privileges.toName(privilege))
.withCondition(Privilege.Condition.DENY)
.build();
- privilegesList.add(privilegeDTO);
+ privilegesSet.add(privilegeDTO);
}
MetadataObject metadataObject = constructMetadataObject(entity, client);
- client.revokePrivilegesFromRole(role, metadataObject, privilegesList);
+ client.revokePrivilegesFromRole(role, metadataObject, privilegesSet);
} catch (NoSuchMetalakeException err) {
System.err.println(ErrorMessages.UNKNOWN_METALAKE);
return;
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
index 560dae06d1..c9a88239f5 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.client;
import static org.apache.gravitino.dto.util.DTOConverters.toFunctionArg;
+import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;
import org.apache.gravitino.Catalog;
@@ -321,7 +322,7 @@ class DTOConverters {
.build();
}
- static List<PrivilegeDTO> toPrivileges(List<Privilege> privileges) {
+ static List<PrivilegeDTO> toPrivileges(Collection<Privilege> privileges) {
return privileges.stream()
.map(
privilege ->
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
index c0310f2387..fb2a990891 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
@@ -20,9 +20,11 @@
package org.apache.gravitino.client;
import com.google.common.base.Preconditions;
+import com.google.common.collect.Sets;
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.CatalogChange;
import org.apache.gravitino.MetadataObject;
@@ -420,12 +422,33 @@ public class GravitinoClient extends GravitinoClientBase
* @throws IllegalPrivilegeException If any privilege can't be bind to the
metadata object.
* @throws RuntimeException If granting roles to a role encounters storage
issues.
*/
+ @Deprecated
public Role grantPrivilegesToRole(String role, MetadataObject object,
List<Privilege> privileges)
throws NoSuchRoleException, NoSuchMetadataObjectException,
NoSuchMetalakeException,
IllegalPrivilegeException {
return getMetalake().grantPrivilegesToRole(role, object, privileges);
}
+ /**
+ * Grant privileges to a role.
+ *
+ * @param role The name of the role.
+ * @param privileges The privileges to grant.
+ * @param object The object is associated with privileges to grant.
+ * @return The role after granted.
+ * @throws NoSuchRoleException If the role with the given name does not
exist.
+ * @throws NoSuchMetadataObjectException If the metadata object with the
given name does not
+ * exist.
+ * @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
+ * @throws IllegalPrivilegeException If any privilege can't be bind to the
metadata object.
+ * @throws RuntimeException If granting roles to a role encounters storage
issues.
+ */
+ public Role grantPrivilegesToRole(String role, MetadataObject object,
Set<Privilege> privileges)
+ throws NoSuchRoleException, NoSuchMetadataObjectException,
NoSuchMetalakeException,
+ IllegalPrivilegeException {
+ return getMetalake().grantPrivilegesToRole(role, object, privileges);
+ }
+
/**
* Revoke privileges from a role.
*
@@ -440,10 +463,32 @@ public class GravitinoClient extends GravitinoClientBase
* @throws IllegalPrivilegeException If any privilege can't be bind to the
metadata object.
* @throws RuntimeException If revoking privileges from a role encounters
storage issues.
*/
+ @Deprecated
public Role revokePrivilegesFromRole(
String role, MetadataObject object, List<Privilege> privileges)
throws NoSuchRoleException, NoSuchMetadataObjectException,
NoSuchMetalakeException,
IllegalPrivilegeException {
+ return getMetalake().revokePrivilegesFromRole(role, object,
Sets.newHashSet(privileges));
+ }
+
+ /**
+ * Revoke privileges from a role.
+ *
+ * @param role The name of the role.
+ * @param privileges The privileges to revoke.
+ * @param object The object is associated with privileges to revoke.
+ * @return The role after revoked.
+ * @throws NoSuchRoleException If the role with the given name does not
exist.
+ * @throws NoSuchMetadataObjectException If the metadata object with the
given name does not
+ * exist.
+ * @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
+ * @throws IllegalPrivilegeException If any privilege can't be bind to the
metadata object.
+ * @throws RuntimeException If revoking privileges from a role encounters
storage issues.
+ */
+ public Role revokePrivilegesFromRole(
+ String role, MetadataObject object, Set<Privilege> privileges)
+ throws NoSuchRoleException, NoSuchMetadataObjectException,
NoSuchMetalakeException,
+ IllegalPrivilegeException {
return getMetalake().revokePrivilegesFromRole(role, object, privileges);
}
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
index 14e17a851e..60bb47adac 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.client;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Sets;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
@@ -27,6 +28,7 @@ import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.Catalog;
@@ -1018,6 +1020,27 @@ public class GravitinoMetalake extends MetalakeDTO
public Role grantPrivilegesToRole(String role, MetadataObject object,
List<Privilege> privileges)
throws NoSuchRoleException, NoSuchMetadataObjectException,
NoSuchMetalakeException,
IllegalPrivilegeException {
+ Set<Privilege> privilegeSet = Sets.newHashSet(privileges);
+ return grantPrivilegesToRole(role, object, privilegeSet);
+ }
+
+ /**
+ * Grant privileges to a role.
+ *
+ * @param role The name of the role.
+ * @param privileges The privileges to grant.
+ * @param object The object is associated with privileges to grant.
+ * @return The role after granted.
+ * @throws NoSuchRoleException If the role with the given name does not
exist.
+ * @throws NoSuchMetadataObjectException If the metadata object with the
given name does not
+ * exist.
+ * @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
+ * @throws IllegalPrivilegeException If any privilege can't be bind to the
metadata object.
+ * @throws RuntimeException If granting privileges to a role encounters
storage issues.
+ */
+ public Role grantPrivilegesToRole(String role, MetadataObject object,
Set<Privilege> privileges)
+ throws NoSuchRoleException, NoSuchMetadataObjectException,
NoSuchMetalakeException,
+ IllegalPrivilegeException {
PrivilegeGrantRequest request =
new PrivilegeGrantRequest(DTOConverters.toPrivileges(privileges));
request.validate();
@@ -1056,10 +1079,33 @@ public class GravitinoMetalake extends MetalakeDTO
* @throws IllegalPrivilegeException If any privilege can't be bind to the
metadata object.
* @throws RuntimeException If revoking privileges from a role encounters
storage issues.
*/
+ @Deprecated
public Role revokePrivilegesFromRole(
String role, MetadataObject object, List<Privilege> privileges)
throws NoSuchRoleException, NoSuchMetadataObjectException,
NoSuchMetalakeException,
IllegalPrivilegeException {
+ Set<Privilege> privilegeSet = Sets.newHashSet(privileges);
+ return revokePrivilegesFromRole(role, object, privilegeSet);
+ }
+
+ /**
+ * Revoke privileges from a role.
+ *
+ * @param role The name of the role.
+ * @param privileges The privileges to revoke.
+ * @param object The object is associated with privileges to revoke.
+ * @return The role after revoked.
+ * @throws NoSuchRoleException If the role with the given name does not
exist.
+ * @throws NoSuchMetadataObjectException If the metadata object with the
given name does not
+ * exist.
+ * @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
+ * @throws IllegalPrivilegeException If any privilege can't be bind to the
metadata object.
+ * @throws RuntimeException If revoking privileges from a role encounters
storage issues.
+ */
+ public Role revokePrivilegesFromRole(
+ String role, MetadataObject object, Set<Privilege> privileges)
+ throws NoSuchRoleException, NoSuchMetadataObjectException,
NoSuchMetalakeException,
+ IllegalPrivilegeException {
PrivilegeRevokeRequest request =
new PrivilegeRevokeRequest(DTOConverters.toPrivileges(privileges));
request.validate();
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/TestPermission.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/TestPermission.java
index 006a47b43c..d1731607a8 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/TestPermission.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/TestPermission.java
@@ -22,6 +22,7 @@ import static javax.servlet.http.HttpServletResponse.SC_OK;
import static org.apache.hc.core5.http.HttpStatus.SC_SERVER_ERROR;
import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
import java.time.Instant;
import java.util.List;
import org.apache.gravitino.MetadataObject;
@@ -231,7 +232,7 @@ public class TestPermission extends TestBase {
MetadataObject object = MetadataObjects.of(null, metalakeName,
MetadataObject.Type.METALAKE);
Role grantedRole =
gravitinoClient.grantPrivilegesToRole(
- role, object, Lists.newArrayList(Privileges.CreateTable.allow()));
+ role, object, Sets.newHashSet(Privileges.CreateTable.allow()));
Assertions.assertEquals(grantedRole.name(), role);
Assertions.assertEquals(1, grantedRole.securableObjects().size());
SecurableObject securableObject = grantedRole.securableObjects().get(0);
@@ -249,7 +250,7 @@ public class TestPermission extends TestBase {
RuntimeException.class,
() ->
gravitinoClient.grantPrivilegesToRole(
- role, object,
Lists.newArrayList(Privileges.CreateTable.allow())));
+ role, object,
Sets.newHashSet(Privileges.CreateTable.allow())));
}
@Test
@@ -280,7 +281,7 @@ public class TestPermission extends TestBase {
MetadataObject object = MetadataObjects.of(null, metalakeName,
MetadataObject.Type.METALAKE);
Role revokedRole =
gravitinoClient.revokePrivilegesFromRole(
- role, object, Lists.newArrayList(Privileges.CreateTable.allow()));
+ role, object, Sets.newHashSet(Privileges.CreateTable.allow()));
Assertions.assertEquals(revokedRole.name(), role);
Assertions.assertTrue(revokedRole.securableObjects().isEmpty());
@@ -291,6 +292,6 @@ public class TestPermission extends TestBase {
RuntimeException.class,
() ->
gravitinoClient.revokePrivilegesFromRole(
- role, object,
Lists.newArrayList(Privileges.CreateTable.allow())));
+ role, object,
Sets.newHashSet(Privileges.CreateTable.allow())));
}
}
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java
index 268ed20f3c..07232e8a8d 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java
@@ -20,6 +20,7 @@ package
org.apache.gravitino.client.integration.test.authorization;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
@@ -468,7 +469,7 @@ public class AccessControlIT extends BaseIT {
// grant a privilege
Role role =
metalake.grantPrivilegesToRole(
- roleName, metadataObject,
Lists.newArrayList(Privileges.CreateCatalog.allow()));
+ roleName, metadataObject,
Sets.newHashSet(Privileges.CreateCatalog.allow()));
Assertions.assertEquals(1, role.securableObjects().size());
// grant a wrong privilege
@@ -477,7 +478,7 @@ public class AccessControlIT extends BaseIT {
IllegalPrivilegeException.class,
() ->
metalake.grantPrivilegesToRole(
- roleName, catalog,
Lists.newArrayList(Privileges.CreateCatalog.allow())));
+ roleName, catalog,
Sets.newHashSet(Privileges.CreateCatalog.allow())));
// grant a wrong catalog type privilege
MetadataObject wrongCatalog =
@@ -486,7 +487,7 @@ public class AccessControlIT extends BaseIT {
IllegalPrivilegeException.class,
() ->
metalake.grantPrivilegesToRole(
- roleName, wrongCatalog,
Lists.newArrayList(Privileges.SelectTable.allow())));
+ roleName, wrongCatalog,
Sets.newHashSet(Privileges.SelectTable.allow())));
// grant a duplicated privilege
MetadataObject duplicatedCatalog =
@@ -497,12 +498,23 @@ public class AccessControlIT extends BaseIT {
metalake.grantPrivilegesToRole(
roleName,
duplicatedCatalog,
- Lists.newArrayList(Privileges.SelectTable.allow(),
Privileges.SelectTable.deny())));
+ Sets.newHashSet(Privileges.ReadFileset.allow(),
Privileges.ReadFileset.deny())));
+
+ // repeat to grant a privilege
+ metalake.grantPrivilegesToRole(
+ roleName, duplicatedCatalog,
Sets.newHashSet(Privileges.ReadFileset.allow()));
+ Assertions.assertThrows(
+ IllegalPrivilegeException.class,
+ () ->
+ metalake.grantPrivilegesToRole(
+ roleName, duplicatedCatalog,
Sets.newHashSet(Privileges.ReadFileset.deny())));
+ metalake.revokePrivilegesFromRole(
+ roleName, duplicatedCatalog,
Sets.newHashSet(Privileges.ReadFileset.allow()));
// repeat to grant a privilege
role =
metalake.grantPrivilegesToRole(
- roleName, metadataObject,
Lists.newArrayList(Privileges.CreateCatalog.allow()));
+ roleName, metadataObject,
Sets.newHashSet(Privileges.CreateCatalog.allow()));
Assertions.assertEquals(1, role.securableObjects().size());
// grant a not-existing role
@@ -510,12 +522,12 @@ public class AccessControlIT extends BaseIT {
NoSuchRoleException.class,
() ->
metalake.grantPrivilegesToRole(
- "not-exist", metadataObject,
Lists.newArrayList(Privileges.CreateCatalog.allow())));
+ "not-exist", metadataObject,
Sets.newHashSet(Privileges.CreateCatalog.allow())));
// revoke a privilege
role =
metalake.revokePrivilegesFromRole(
- roleName, metadataObject,
Lists.newArrayList(Privileges.CreateCatalog.allow()));
+ roleName, metadataObject,
Sets.newHashSet(Privileges.CreateCatalog.allow()));
Assertions.assertTrue(role.securableObjects().isEmpty());
// revoke a wrong privilege
@@ -523,19 +535,19 @@ public class AccessControlIT extends BaseIT {
IllegalPrivilegeException.class,
() ->
metalake.revokePrivilegesFromRole(
- roleName, catalog,
Lists.newArrayList(Privileges.CreateCatalog.allow())));
+ roleName, catalog,
Sets.newHashSet(Privileges.CreateCatalog.allow())));
// revoke a wrong catalog type privilege
Assertions.assertThrows(
IllegalPrivilegeException.class,
() ->
metalake.revokePrivilegesFromRole(
- roleName, wrongCatalog,
Lists.newArrayList(Privileges.SelectTable.allow())));
+ roleName, wrongCatalog,
Sets.newHashSet(Privileges.SelectTable.allow())));
// repeat to revoke a privilege
role =
metalake.revokePrivilegesFromRole(
- roleName, metadataObject,
Lists.newArrayList(Privileges.CreateCatalog.allow()));
+ roleName, metadataObject,
Sets.newHashSet(Privileges.CreateCatalog.allow()));
Assertions.assertTrue(role.securableObjects().isEmpty());
// revoke a not-existing role
@@ -543,7 +555,7 @@ public class AccessControlIT extends BaseIT {
NoSuchRoleException.class,
() ->
metalake.revokePrivilegesFromRole(
- "not-exist", metadataObject,
Lists.newArrayList(Privileges.CreateCatalog.allow())));
+ "not-exist", metadataObject,
Sets.newHashSet(Privileges.CreateCatalog.allow())));
// Cleanup
metalake.deleteRole(roleName);
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java
b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java
index b75a67055b..9aa2b3f52b 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.authorization;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
import org.apache.gravitino.exceptions.IllegalRoleException;
@@ -293,7 +294,7 @@ public interface AccessControlDispatcher {
* @throws RuntimeException If granting roles to a role encounters storage
issues.
*/
Role grantPrivilegeToRole(
- String metalake, String role, MetadataObject object, List<Privilege>
privileges)
+ String metalake, String role, MetadataObject object, Set<Privilege>
privileges)
throws NoSuchGroupException, NoSuchRoleException;
/**
@@ -308,6 +309,6 @@ public interface AccessControlDispatcher {
* @throws RuntimeException If revoking privileges from a role encounters
storage issues.
*/
Role revokePrivilegesFromRole(
- String metalake, String role, MetadataObject object, List<Privilege>
privileges)
+ String metalake, String role, MetadataObject object, Set<Privilege>
privileges)
throws NoSuchMetalakeException, NoSuchRoleException;
}
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java
b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java
index 798285806f..2d756646b6 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.authorization;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import org.apache.gravitino.Config;
import org.apache.gravitino.Configs;
import org.apache.gravitino.EntityStore;
@@ -169,14 +170,14 @@ public class AccessControlManager implements
AccessControlDispatcher {
@Override
public Role grantPrivilegeToRole(
- String metalake, String role, MetadataObject object, List<Privilege>
privileges)
+ String metalake, String role, MetadataObject object, Set<Privilege>
privileges)
throws NoSuchRoleException, NoSuchMetalakeException {
return permissionManager.grantPrivilegesToRole(metalake, role, object,
privileges);
}
@Override
public Role revokePrivilegesFromRole(
- String metalake, String role, MetadataObject object, List<Privilege>
privileges)
+ String metalake, String role, MetadataObject object, Set<Privilege>
privileges)
throws NoSuchRoleException, NoSuchMetalakeException {
return permissionManager.revokePrivilegesFromRole(metalake, role, object,
privileges);
}
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
index bdaa8f6f74..1046723d70 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
@@ -406,7 +406,7 @@ class PermissionManager {
}
Role grantPrivilegesToRole(
- String metalake, String role, MetadataObject object, List<Privilege>
privileges) {
+ String metalake, String role, MetadataObject object, Set<Privilege>
privileges) {
try {
AuthorizationPluginCallbackWrapper authorizationPluginCallbackWrapper =
new AuthorizationPluginCallbackWrapper();
@@ -476,7 +476,7 @@ class PermissionManager {
String metalake,
String role,
MetadataObject object,
- List<Privilege> privileges,
+ Set<Privilege> privileges,
RoleEntity roleEntity,
SecurableObject targetObject,
AuthorizationPluginCallbackWrapper authorizationPluginCallbackWrapper) {
@@ -489,7 +489,7 @@ class PermissionManager {
return targetObject;
} else {
updatePrivileges.addAll(privileges);
- AuthorizationUtils.checkDuplicatedNamePrivilege(privileges);
+ AuthorizationUtils.checkDuplicatedNamePrivilege(updatePrivileges);
SecurableObject newSecurableObject =
SecurableObjects.parse(
@@ -513,7 +513,7 @@ class PermissionManager {
}
Role revokePrivilegesFromRole(
- String metalake, String role, MetadataObject object, List<Privilege>
privileges) {
+ String metalake, String role, MetadataObject object, Set<Privilege>
privileges) {
try {
AuthorizationPluginCallbackWrapper authorizationCallbackWrapper =
new AuthorizationPluginCallbackWrapper();
@@ -587,9 +587,11 @@ class PermissionManager {
String metalake,
String role,
MetadataObject object,
- List<Privilege> privileges,
+ Set<Privilege> privileges,
RoleEntity roleEntity,
AuthorizationPluginCallbackWrapper authorizationPluginCallbackWrapper) {
+ AuthorizationUtils.checkDuplicatedNamePrivilege(privileges);
+
// Add a new securable object if there doesn't exist the object in the role
SecurableObject securableObject =
SecurableObjects.parse(object.fullName(), object.type(),
Lists.newArrayList(privileges));
@@ -613,7 +615,7 @@ class PermissionManager {
String metalake,
String role,
MetadataObject object,
- List<Privilege> privileges,
+ Set<Privilege> privileges,
RoleEntity roleEntity,
SecurableObject targetObject,
AuthorizationPluginCallbackWrapper authorizationCallbackWrapper) {
diff --git
a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
index f5f5a27648..dba0177ca1 100644
---
a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.hook;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
@@ -188,14 +189,14 @@ public class AccessControlHookDispatcher implements
AccessControlDispatcher {
@Override
public Role grantPrivilegeToRole(
- String metalake, String role, MetadataObject object, List<Privilege>
privileges)
+ String metalake, String role, MetadataObject object, Set<Privilege>
privileges)
throws NoSuchMetalakeException, NoSuchRoleException {
return dispatcher.grantPrivilegeToRole(metalake, role, object, privileges);
}
@Override
public Role revokePrivilegesFromRole(
- String metalake, String role, MetadataObject object, List<Privilege>
privileges)
+ String metalake, String role, MetadataObject object, Set<Privilege>
privileges)
throws NoSuchMetalakeException, NoSuchRoleException {
return dispatcher.revokePrivilegesFromRole(metalake, role, object,
privileges);
}
diff --git
a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java
b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java
index 30084a32e2..e4b62567d4 100644
---
a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java
+++
b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java
@@ -24,6 +24,7 @@ import static org.mockito.Mockito.verify;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
import java.io.IOException;
import java.time.Instant;
import java.util.List;
@@ -355,7 +356,7 @@ public class TestAccessControlManagerForPermissions {
METALAKE,
"grantedRole",
MetadataObjects.of(null, METALAKE, MetadataObject.Type.METALAKE),
- Lists.newArrayList(Privileges.CreateTable.allow()));
+ Sets.newHashSet(Privileges.CreateTable.allow()));
List<SecurableObject> objects = role.securableObjects();
@@ -370,7 +371,7 @@ public class TestAccessControlManagerForPermissions {
METALAKE,
"grantedRole",
MetadataObjects.of(null, METALAKE, MetadataObject.Type.METALAKE),
- Lists.newArrayList(Privileges.CreateTable.allow()));
+ Sets.newHashSet(Privileges.CreateTable.allow()));
objects = role.securableObjects();
Assertions.assertEquals(2, objects.size());
@@ -383,7 +384,7 @@ public class TestAccessControlManagerForPermissions {
METALAKE,
notExist,
MetadataObjects.of(null, METALAKE,
MetadataObject.Type.METALAKE),
- Lists.newArrayList(Privileges.CreateTable.allow())));
+ Sets.newHashSet(Privileges.CreateTable.allow())));
}
@Test
@@ -396,7 +397,7 @@ public class TestAccessControlManagerForPermissions {
METALAKE,
"revokedRole",
MetadataObjects.of(null, CATALOG, MetadataObject.Type.CATALOG),
- Lists.newArrayList(Privileges.UseCatalog.allow()));
+ Sets.newHashSet(Privileges.UseCatalog.allow()));
// Test authorization plugin
verify(authorizationPlugin).onRoleUpdated(any(), any());
@@ -411,7 +412,7 @@ public class TestAccessControlManagerForPermissions {
METALAKE,
"revokedRole",
MetadataObjects.of(null, CATALOG, MetadataObject.Type.CATALOG),
- Lists.newArrayList(Privileges.UseCatalog.allow()));
+ Sets.newHashSet(Privileges.UseCatalog.allow()));
objects = role.securableObjects();
Assertions.assertTrue(objects.isEmpty());
@@ -423,6 +424,6 @@ public class TestAccessControlManagerForPermissions {
METALAKE,
notExist,
MetadataObjects.of(null, METALAKE,
MetadataObject.Type.METALAKE),
- Lists.newArrayList(Privileges.CreateTable.allow())));
+ Sets.newHashSet(Privileges.CreateTable.allow())));
}
}
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
index 3ce1517a46..3f89cc7eea 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java
@@ -242,7 +242,7 @@ public class PermissionOperations {
object,
privilegeGrantRequest.getPrivileges().stream()
.map(DTOConverters::fromPrivilegeDTO)
- .collect(Collectors.toList()))))));
+ .collect(Collectors.toSet()))))));
});
} catch (Exception e) {
return ExceptionHandlers.handleRolePermissionOperationException(
@@ -289,7 +289,7 @@ public class PermissionOperations {
object,
privilegeRevokeRequest.getPrivileges().stream()
.map(DTOConverters::fromPrivilegeDTO)
- .collect(Collectors.toList()))))));
+ .collect(Collectors.toSet()))))));
});
} catch (Exception e) {
return ExceptionHandlers.handleRolePermissionOperationException(
