Skip to content

Commit

Permalink
[#5106] improve(auth-ranger): Filter Catalog securiable object in the…
Browse files Browse the repository at this point in the history 
… onOwnerSet
  • Loading branch information
@xunliu
xunliu committed Oct 12, 2024
1 parent fdb07ab commit 0c32387
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 20 deletions.
55 changes: 55 additions & 0 deletions ...er/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.authorization.Role;
import org.apache.gravitino.authorization.RoleChange;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.authorization.User;
import org.apache.gravitino.authorization.ranger.reference.VXGroup;
import org.apache.gravitino.authorization.ranger.reference.VXGroupList;
Expand Down Expand Up @@ -112,6 +113,11 @@ public Set<String> translatePrivilege(Privilege.Name name) {
*/
@Override
public Boolean onRoleCreated(Role role) throws RuntimeException {
if (containCatalogSecurableObject(role.securableObjects())) {
// Filter the catalog object, because we don't need to add the catalog privilege to the Ranger
return false;
}

rangerHelper.createRangerRoleIfNotExists(role.name());
return onRoleUpdated(
role,
Expand All @@ -122,12 +128,20 @@ public Boolean onRoleCreated(Role role) throws RuntimeException {

@Override
public Boolean onRoleAcquired(Role role) throws RuntimeException {
if (containCatalogSecurableObject(role.securableObjects())) {
// Filter the catalog object, because we don't need to add the catalog privilege to the Ranger
return false;
}
return rangerHelper.checkRangerRole(role.name());
}

/** Remove the role name from the Ranger policy item, and delete this Role in the Ranger. <br> */
@Override
public Boolean onRoleDeleted(Role role) throws RuntimeException {
if (containCatalogSecurableObject(role.securableObjects())) {
// Filter the catalog object, because we don't need to add the catalog privilege to the Ranger
return false;
}
// First, remove the role in the Ranger policy
onRoleUpdated(
role,
Expand Down Expand Up @@ -181,6 +195,10 @@ public Boolean onRoleUpdated(Role role, RoleChange... changes) throws RuntimeExc
@Override
public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner newOwner)
throws RuntimeException {
if (metadataObject.type() == MetadataObject.Type.CATALOG) {
// Filter the catalog object, because we don't need to add the catalog privilege to the Ranger
return Boolean.FALSE;
}
RangerHelper.check(newOwner != null, "The newOwner must be not null");

// Add the user or group to the Ranger
Expand Down Expand Up @@ -238,6 +256,11 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n
*/
@Override
public Boolean onGrantedRolesToUser(List<Role> roles, User user) throws RuntimeException {
if (roles.stream().anyMatch(role -> containCatalogSecurableObject(role.securableObjects()))) {
// Filter the catalog object, because we don't need to add the catalog privilege to the Ranger
return false;
}

// If the user does not exist, then create it.
onUserAdded(user);

Expand Down Expand Up @@ -269,6 +292,10 @@ public Boolean onGrantedRolesToUser(List<Role> roles, User user) throws RuntimeE
*/
@Override
public Boolean onRevokedRolesFromUser(List<Role> roles, User user) throws RuntimeException {
if (roles.stream().anyMatch(role -> containCatalogSecurableObject(role.securableObjects()))) {
// Filter the catalog object, because we don't need to add the catalog privilege to the Ranger
return false;
}
// If the user does not exist, then create it.
onUserAdded(user);

Expand Down Expand Up @@ -300,6 +327,10 @@ public Boolean onRevokedRolesFromUser(List<Role> roles, User user) throws Runtim
*/
@Override
public Boolean onGrantedRolesToGroup(List<Role> roles, Group group) throws RuntimeException {
if (roles.stream().anyMatch(role -> containCatalogSecurableObject(role.securableObjects()))) {
// Filter the catalog object, because we don't need to add the catalog privilege to the Ranger
return false;
}
// If the group does not exist, then create it.
onGroupAdded(group);

Expand Down Expand Up @@ -330,6 +361,10 @@ public Boolean onGrantedRolesToGroup(List<Role> roles, Group group) throws Runti
*/
@Override
public Boolean onRevokedRolesFromGroup(List<Role> roles, Group group) throws RuntimeException {
if (roles.stream().anyMatch(role -> containCatalogSecurableObject(role.securableObjects()))) {
// Filter the catalog object, because we don't need to add the catalog privilege to the Ranger
return false;
}
onGroupAdded(group);
roles.stream()
.forEach(
Expand Down Expand Up @@ -415,6 +450,11 @@ public Boolean onGroupAcquired(Group group) {
* 3. If the policy does not exist, then create a new policy. <br>
*/
private boolean doAddSecurableObject(RoleChange.AddSecurableObject change) {
if (containCatalogSecurableObject(Collections.singletonList(change.getSecurableObject()))) {
// Filter the catalog object, because we don't need to add the catalog privilege to the Ranger
return false;
}

RangerPolicy policy = rangerHelper.findManagedPolicy(change.getSecurableObject());

if (policy != null) {
Expand Down Expand Up @@ -466,6 +506,10 @@ private boolean doAddSecurableObject(RoleChange.AddSecurableObject change) {
* 3. If policy does not contain any policy item, then delete this policy. <br>
*/
private boolean doRemoveSecurableObject(RoleChange.RemoveSecurableObject change) {
if (containCatalogSecurableObject(Collections.singletonList(change.getSecurableObject()))) {
// Filter the catalog object, because we don't need to add the catalog privilege to the Ranger
return false;
}
RangerPolicy policy = rangerHelper.findManagedPolicy(change.getSecurableObject());
if (policy == null) {
LOG.warn(
Expand Down Expand Up @@ -528,6 +572,12 @@ private boolean doRemoveSecurableObject(RoleChange.RemoveSecurableObject change)
* 3. If the policy does not exist, return false. <br>
*/
private boolean doUpdateSecurableObject(RoleChange.UpdateSecurableObject change) {
if (containCatalogSecurableObject(Collections.singletonList(change.getSecurableObject()))
|| containCatalogSecurableObject(
Collections.singletonList(change.getNewSecurableObject()))) {
// Filter the catalog object, because we don't need to add the catalog privilege to the Ranger
return false;
}
RangerPolicy policy = rangerHelper.findManagedPolicy(change.getSecurableObject());
if (policy == null) {
LOG.warn(
Expand All @@ -554,4 +604,9 @@ private boolean doUpdateSecurableObject(RoleChange.UpdateSecurableObject change)

@Override
public void close() throws IOException {}

private boolean containCatalogSecurableObject(List<SecurableObject> securableObjects) {
return securableObjects.stream()
.anyMatch(securableObject -> securableObject.type() == MetadataObject.Type.CATALOG);
}
}
3 changes: 2 additions & 1 deletion ...test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveE2EIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ private static void generateRangerSparkSecurityXML() throws IOException {
}

@AfterAll
public static void stop() throws IOException {
public static void stop() {
RangerITEnv.cleanup();
if (client != null) {
Arrays.stream(catalog.asSchemas().listSchemas())
.filter(schema -> !schema.equals("default"))
Expand Down
120 changes: 101 additions & 19 deletions ...rc/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import org.apache.gravitino.authorization.ranger.reference.RangerDefines;
import org.apache.gravitino.connector.AuthorizationPropertiesMeta;
import org.apache.gravitino.integration.test.container.ContainerSuite;
import org.apache.gravitino.integration.test.container.HiveContainer;
import org.apache.gravitino.integration.test.container.RangerContainer;
import org.apache.gravitino.integration.test.util.GravitinoITUtils;
import org.apache.gravitino.meta.AuditInfo;
Expand All @@ -57,6 +56,7 @@
import org.apache.gravitino.meta.UserEntity;
import org.apache.ranger.RangerServiceException;
import org.apache.ranger.plugin.model.RangerPolicy;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Tag;
Expand All @@ -69,7 +69,6 @@ public class RangerHiveIT {
private static final Logger LOG = LoggerFactory.getLogger(RangerHiveIT.class);

private static final ContainerSuite containerSuite = ContainerSuite.getInstance();
private static final String adminUser = "gravitino";
private static RangerAuthorizationPlugin rangerAuthPlugin;
private static RangerHelper rangerHelper;
private final AuditInfo auditInfo =
Expand All @@ -79,23 +78,6 @@ public class RangerHiveIT {
public static void setup() {
RangerITEnv.setup();

containerSuite.startHiveRangerContainer(
new HashMap<>(
ImmutableMap.of(
HiveContainer.HIVE_RUNTIME_VERSION,
HiveContainer.HIVE3,
RangerContainer.DOCKER_ENV_RANGER_SERVER_URL,
String.format(
"http://%s:%d",
containerSuite.getRangerContainer().getContainerIpAddress(),
RangerContainer.RANGER_SERVER_PORT),
RangerContainer.DOCKER_ENV_RANGER_HIVE_REPOSITORY_NAME,
RangerITEnv.RANGER_HIVE_REPO_NAME,
RangerContainer.DOCKER_ENV_RANGER_HDFS_REPOSITORY_NAME,
RangerITEnv.RANGER_HDFS_REPO_NAME,
HiveContainer.HADOOP_USER_NAME,
adminUser)));

rangerAuthPlugin =
RangerAuthorizationHivePlugin.getInstance(
ImmutableMap.of(
Expand All @@ -122,6 +104,11 @@ public static void setup() {
rangerAuthPlugin.policyResourceDefinesRule());
}

@AfterAll
public static void stop() {
RangerITEnv.cleanup();
}

/**
* Create a mock role with 3 securable objects <br>
* 1. catalog.db1.tab1 with CREATE_TABLE privilege. <br>
Expand Down Expand Up @@ -166,6 +153,17 @@ public void testOnRoleCreated() {
verifyRoleInRanger(rangerAuthPlugin, role);
}

@Test
public void testOnRoleCreatedCatalog() {
Role mockCatalogRole = mockCatalogRole(currentFunName());
Assertions.assertFalse(rangerAuthPlugin.onRoleCreated(mockCatalogRole));
// Check if exist this policy
mockCatalogRole.securableObjects().stream()
.forEach(
securableObject ->
Assertions.assertNull(rangerHelper.findManagedPolicy(securableObject)));
}

@Test
public void testOnRoleDeleted() {
// prepare to create a role
Expand All @@ -181,6 +179,20 @@ public void testOnRoleDeleted() {
Assertions.assertNull(rangerHelper.findManagedPolicy(securableObject)));
}

@Test
public void testOnRoleDeletedCatalog() {
// prepare to create a role
Role mockCatalogRole = mockCatalogRole(currentFunName());

// delete this role
Assertions.assertFalse(rangerAuthPlugin.onRoleDeleted(mockCatalogRole));
// Check if exist this policy
mockCatalogRole.securableObjects().stream()
.forEach(
securableObject ->
Assertions.assertNull(rangerHelper.findManagedPolicy(securableObject)));
}

@Test
public void testOnRoleDeleted2() {
// prepare to create a role
Expand Down Expand Up @@ -332,6 +344,21 @@ public void testRoleChangeAddSecurableObject() {
RoleChange.addSecurableObject(mockCatalogRole.name(), securableObject3)));
}

@Test
public void testRoleChangeAddCatalogSecurableObject() {
Role mockCatalogRole = mockCatalogRole(currentFunName());
Assertions.assertFalse(
rangerAuthPlugin.onRoleUpdated(
mockCatalogRole,
RoleChange.addSecurableObject(
mockCatalogRole.name(), mockCatalogRole.securableObjects().get(0))));
// Check if exist this policy
mockCatalogRole.securableObjects().stream()
.forEach(
securableObject ->
Assertions.assertNull(rangerHelper.findManagedPolicy(securableObject)));
}

@Test
public void testRoleChangeRemoveSecurableObject() {
// Prepare a role contain 3 securable objects
Expand Down Expand Up @@ -363,6 +390,23 @@ public void testRoleChangeRemoveSecurableObject() {
}
}

@Test
public void testRoleChangeRemoveCatalogSecurableObject() {
String currentFunName = currentFunName();
Role mockCatalogRole = mockCatalogRole(currentFunName);

Assertions.assertFalse(
rangerAuthPlugin.onRoleUpdated(
mockCatalogRole,
RoleChange.removeSecurableObject(
mockCatalogRole.name(), mockCatalogRole.securableObjects().get(0))));
// Check if exist this policy
mockCatalogRole.securableObjects().stream()
.forEach(
securableObject ->
Assertions.assertNull(rangerHelper.findManagedPolicy(securableObject)));
}

@Test
public void testRoleChangeUpdateSecurableObject() {
SecurableObject oldSecurableObject =
Expand Down Expand Up @@ -403,6 +447,32 @@ public void testRoleChangeUpdateSecurableObject() {
verifyRoleInRanger(rangerAuthPlugin, verifyRole);
}

@Test
public void testRoleChangeUpdateCatalogSecurableObject() {
String currentFunName = currentFunName();
Role mockCatalogRole = mockCatalogRole(currentFunName);

// Keep the same matedata namespace and type, but change privileges
SecurableObject newSecurableObject =
SecurableObjects.parse(
mockCatalogRole.securableObjects().get(0).fullName(),
mockCatalogRole.securableObjects().get(0).type(),
Lists.newArrayList(Privileges.SelectTable.allow()));

Assertions.assertFalse(
rangerAuthPlugin.onRoleUpdated(
mockCatalogRole,
RoleChange.updateSecurableObject(
mockCatalogRole.name(),
mockCatalogRole.securableObjects().get(0),
newSecurableObject)));
// Check if exist this policy
mockCatalogRole.securableObjects().stream()
.forEach(
securableObject ->
Assertions.assertNull(rangerHelper.findManagedPolicy(securableObject)));
}

@Test
public void testRoleChangeCombinedOperation() {
MetadataObject oldMetadataObject =
Expand Down Expand Up @@ -662,6 +732,18 @@ public void testOnOwnerSet() {
Lists.newArrayList(groupName1));
}

@Test
public void testOnOwnerSetCatalog() {
MetadataObject metadataObject =
MetadataObjects.parse(
String.format("catalog-%s", currentFunName()), MetadataObject.Type.CATALOG);
String userName1 = "user1";
Owner owner1 = new MockOwner(userName1, Owner.Type.USER);
Assertions.assertFalse(rangerAuthPlugin.onOwnerSet(metadataObject, null, owner1));
// Check if exist this policy
Assertions.assertNull(rangerHelper.findManagedPolicy(metadataObject));
}

@Test
public void testCreateUser() {
UserEntity user =
Expand Down
1 change: 1 addition & 0 deletions build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ tasks.rat {
"DISCLAIMER.txt",
"ROADMAP.md",
"clients/client-python/.pytest_cache/*",
"clients/client-python/**/__pycache__",
"clients/client-python/.venv/*",
"clients/client-python/venv/*",
"clients/client-python/apache_gravitino.egg-info/*",
Expand Down

0 comments on commit 0c32387

Please sign in to comment.