From 642983dd11704085f245549082c2f16a8893365c Mon Sep 17 00:00:00 2001 From: theoryxu Date: Wed, 20 Nov 2024 16:03:50 +0800 Subject: [PATCH 01/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- .../ranger/RangerAuthorizationPlugin.java | 52 ++++++++++++----- .../authorization/ranger/RangerHelper.java | 57 +++++++++++++------ 2 files changed, 79 insertions(+), 30 deletions(-) diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java index b0e46d5c140..63c2e446836 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java @@ -607,7 +607,6 @@ public Boolean onGroupAcquired(Group group) { */ private boolean doAddSecurableObject(String roleName, RangerSecurableObject securableObject) { RangerPolicy policy = rangerHelper.findManagedPolicy(securableObject); - if (policy != null) { // Check the policy item's accesses and roles equal the Ranger securable object's privilege List allowPrivilies = @@ -621,7 +620,9 @@ private boolean doAddSecurableObject(String roleName, RangerSecurableObject secu Set policyPrivileges = policy.getPolicyItems().stream() - .filter(policyItem -> policyItem.getRoles().contains(roleName)) + .filter( + policyItem -> + policyItem.getRoles().contains(rangerHelper.rangerRoleName(roleName))) .flatMap(policyItem -> policyItem.getAccesses().stream()) .map(RangerPolicy.RangerPolicyItemAccess::getType) .map(RangerPrivileges::valueOf) @@ -629,7 +630,9 @@ private boolean doAddSecurableObject(String roleName, RangerSecurableObject secu Set policyDenyPrivileges = policy.getDenyPolicyItems().stream() - .filter(policyItem -> policyItem.getRoles().contains(roleName)) + .filter( + policyItem -> + policyItem.getRoles().contains(rangerHelper.rangerRoleName(roleName))) .flatMap(policyItem -> policyItem.getAccesses().stream()) .map(RangerPolicy.RangerPolicyItemAccess::getType) .map(RangerPrivileges::valueOf) @@ -738,6 +741,7 @@ private void removePolicyItemIfEqualRoleName( RangerPolicy.RangerPolicyItem policyItem, RangerSecurableObject rangerSecurableObject, String roleName) { + roleName = rangerHelper.rangerRoleName(roleName); boolean match = policyItem.getAccesses().stream() .allMatch( @@ -974,7 +978,23 @@ private void removePolicyByMetadataObject(List metadataNames) { .forEach( policy -> { try { - rangerClient.deletePolicy(policy.getId()); + policy.setPolicyItems( + policy.getPolicyItems().stream() + .filter(rangerHelper::isNotGravitinoManagedPolicyItemAccess) + .collect(Collectors.toList())); + policy.setDenyPolicyItems( + policy.getDenyPolicyItems().stream() + .filter(rangerHelper::isNotGravitinoManagedPolicyItemAccess) + .collect(Collectors.toList())); + policy.setRowFilterPolicyItems( + policy.getRowFilterPolicyItems().stream() + .filter(rangerHelper::isNotGravitinoManagedPolicyItemAccess) + .collect(Collectors.toList())); + policy.setDataMaskPolicyItems( + policy.getDataMaskPolicyItems().stream() + .filter(rangerHelper::isNotGravitinoManagedPolicyItemAccess) + .collect(Collectors.toList())); + rangerClient.updatePolicy(policy.getId(), policy); } catch (RangerServiceException e) { LOG.error("Failed to rename the policy {}!", policy); throw new RuntimeException(e); @@ -1003,25 +1023,29 @@ private void updatePolicyByMetadataObject( .forEach( policy -> { try { - // Update the policy name String policyName = policy.getName(); - List policyNames = Lists.newArrayList(DOT_SPLITTER.splitToList(policyName)); - Preconditions.checkArgument( - policyNames.size() >= oldMetadataNames.size(), - String.format("The policy name(%s) is invalid!", policyName)); int index = operationTypeIndex.get(operationType); - if (policyNames.get(index).equals(RangerHelper.RESOURCE_ALL)) { - // Doesn't need to rename the policy `*` - return; + + // Update the policy name is following Gravitino's spec + if (policy.getName().equals(DOT_JOINER.join(oldMetadataNames))) { + List policyNames = + Lists.newArrayList(DOT_SPLITTER.splitToList(policyName)); + Preconditions.checkArgument( + policyNames.size() >= oldMetadataNames.size(), + String.format("The policy name(%s) is invalid!", policyName)); + if (policyNames.get(index).equals(RangerHelper.RESOURCE_ALL)) { + // Doesn't need to rename the policy `*` + return; + } + policyNames.set(index, newMetadataNames.get(index)); + policy.setName(DOT_JOINER.join(policyNames)); } - policyNames.set(index, newMetadataNames.get(index)); // Update the policy resource name to new name policy .getResources() .put( rangerHelper.policyResourceDefines.get(index), new RangerPolicy.RangerPolicyResource(newMetadataNames.get(index))); - policy.setName(DOT_JOINER.join(policyNames)); boolean alreadyExist = existNewPolicies.stream() diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java index b8c9868f6d8..7b18541cfdb 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java @@ -61,6 +61,8 @@ public class RangerHelper { public static final String GRAVITINO_METALAKE_OWNER_ROLE = "GRAVITINO_METALAKE_OWNER_ROLE"; public static final String GRAVITINO_CATALOG_OWNER_ROLE = "GRAVITINO_CATALOG_OWNER_ROLE"; + public static final String GRAVITINO_ROLE_PREFIX = "GRAVITINO_"; + public RangerHelper( RangerClient rangerClient, String rangerAdminName, @@ -142,7 +144,7 @@ void addPolicyItem(RangerPolicy policy, String roleName, RangerSecurableObject s new RangerPolicy.RangerPolicyItemAccess(); access.setType(rangerPrivilege.getName()); policyItem.getAccesses().add(access); - policyItem.getRoles().add(roleName); + policyItem.getRoles().add(GRAVITINO_ROLE_PREFIX + roleName); if (Privilege.Condition.ALLOW == rangerPrivilege.condition()) { policy.getPolicyItems().add(policyItem); } else { @@ -154,8 +156,8 @@ void addPolicyItem(RangerPolicy policy, String roleName, RangerSecurableObject s .forEach( policyItem -> { // If the role is not in the policy item, then add it - if (!policyItem.getRoles().contains(roleName)) { - policyItem.getRoles().add(roleName); + if (!policyItem.getRoles().contains(GRAVITINO_ROLE_PREFIX + roleName)) { + policyItem.getRoles().add(GRAVITINO_ROLE_PREFIX + roleName); } }); } @@ -172,7 +174,6 @@ public List wildcardSearchPolies(List metadataNames) throws AuthorizationPluginException { Map searchFilters = new HashMap<>(); searchFilters.put(SearchFilter.SERVICE_NAME, rangerServiceName); - searchFilters.put(SearchFilter.POLICY_LABELS_PARTIAL, MANAGED_BY_GRAVITINO); for (int i = 0; i < metadataNames.size(); i++) { searchFilters.put( SearchFilter.RESOURCE_PREFIX + policyResourceDefines.get(i), metadataNames.get(i)); @@ -224,8 +225,7 @@ public RangerPolicy findManagedPolicy(RangerMetadataObject rangerMetadataObject) } // Only return the policies that are managed by Gravitino. if (policies.size() > 1) { - throw new AuthorizationPluginException( - "Every metadata object has only a Gravitino managed policy."); + throw new AuthorizationPluginException("Every metadata object has only a policy."); } if (policies.isEmpty()) { @@ -234,15 +234,32 @@ public RangerPolicy findManagedPolicy(RangerMetadataObject rangerMetadataObject) RangerPolicy policy = policies.get(0); // Delegating Gravitino management policies cannot contain duplicate privilege - policy.getPolicyItems().forEach(this::checkPolicyItemAccess); - policy.getDenyPolicyItems().forEach(this::checkPolicyItemAccess); - policy.getRowFilterPolicyItems().forEach(this::checkPolicyItemAccess); - policy.getDataMaskPolicyItems().forEach(this::checkPolicyItemAccess); + policy.getPolicyItems().stream() + .filter(this::isGravitinoManagedPolicyItemAccess) + .forEach(this::checkPolicyItemAccess); + policy.getDenyPolicyItems().stream() + .filter(this::isGravitinoManagedPolicyItemAccess) + .forEach(this::checkPolicyItemAccess); + policy.getRowFilterPolicyItems().stream() + .filter(this::isGravitinoManagedPolicyItemAccess) + .forEach(this::checkPolicyItemAccess); + policy.getDataMaskPolicyItems().stream() + .filter(this::isGravitinoManagedPolicyItemAccess) + .forEach(this::checkPolicyItemAccess); return policy; } + boolean isGravitinoManagedPolicyItemAccess(RangerPolicy.RangerPolicyItem policyItem) { + return policyItem.getRoles().stream().anyMatch(role -> role.startsWith(GRAVITINO_ROLE_PREFIX)); + } + + boolean isNotGravitinoManagedPolicyItemAccess(RangerPolicy.RangerPolicyItem policyItem) { + return !isGravitinoManagedPolicyItemAccess(policyItem); + } + protected boolean checkRangerRole(String roleName) throws AuthorizationPluginException { + roleName = rangerRoleName(roleName); try { rangerClient.getRole(roleName, rangerAdminName, rangerServiceName); } catch (RangerServiceException e) { @@ -252,8 +269,16 @@ protected boolean checkRangerRole(String roleName) throws AuthorizationPluginExc return true; } + String rangerRoleName(String roleName) { + if (roleName.startsWith(GRAVITINO_ROLE_PREFIX)) { + return roleName; + } + return GRAVITINO_ROLE_PREFIX + roleName; + } + protected GrantRevokeRoleRequest createGrantRevokeRoleRequest( String roleName, String userName, String groupName) { + roleName = rangerRoleName(roleName); Set users = StringUtils.isEmpty(userName) ? Sets.newHashSet() : Sets.newHashSet(userName); Set groups = @@ -278,6 +303,7 @@ protected GrantRevokeRoleRequest createGrantRevokeRoleRequest( * @param isOwnerRole The role is owner role or not */ protected RangerRole createRangerRoleIfNotExists(String roleName, boolean isOwnerRole) { + roleName = rangerRoleName(roleName); if (isOwnerRole) { Preconditions.checkArgument( roleName.equalsIgnoreCase(GRAVITINO_METALAKE_OWNER_ROLE) @@ -385,7 +411,6 @@ protected RangerPolicy createPolicyAddResources(RangerMetadataObject metadataObj RangerPolicy policy = new RangerPolicy(); policy.setService(rangerServiceName); policy.setName(metadataObject.fullName()); - policy.setPolicyLabels(Lists.newArrayList(RangerHelper.MANAGED_BY_GRAVITINO)); List nsMetadataObject = metadataObject.names(); for (int i = 0; i < nsMetadataObject.size(); i++) { RangerPolicy.RangerPolicyResource policyResource = @@ -428,7 +453,7 @@ protected RangerPolicy addOwnerRoleToNewPolicy( policyItem .getAccesses() .add(new RangerPolicy.RangerPolicyItemAccess(ownerPrivilege.getName())); - policyItem.getRoles().add(ownerRoleName); + policyItem.getRoles().add(rangerRoleName(ownerRoleName)); policy.getPolicyItems().add(policyItem); }); return policy; @@ -454,8 +479,8 @@ protected void updatePolicyOwnerRole(RangerPolicy policy, String ownerRoleName) // Add or remove the owner role in the policy item matchPolicyItems.forEach( policyItem -> { - if (!policyItem.getRoles().contains(ownerRoleName)) { - policyItem.getRoles().add(ownerRoleName); + if (!policyItem.getRoles().contains(rangerRoleName(ownerRoleName))) { + policyItem.getRoles().add(rangerRoleName(ownerRoleName)); } }); @@ -480,8 +505,8 @@ protected void updatePolicyOwnerRole(RangerPolicy policy, String ownerRoleName) policyItem .getAccesses() .add(new RangerPolicy.RangerPolicyItemAccess(ownerPrivilege.getName())); - if (!policyItem.getRoles().contains(ownerRoleName)) { - policyItem.getRoles().add(ownerRoleName); + if (!policyItem.getRoles().contains(rangerRoleName(ownerRoleName))) { + policyItem.getRoles().add(rangerRoleName(ownerRoleName)); } policy.getPolicyItems().add(policyItem); }); From 77e7d68f64cd5ff80d9f8a51fc9f0bec63631b1b Mon Sep 17 00:00:00 2001 From: theoryxu Date: Thu, 21 Nov 2024 11:23:00 +0800 Subject: [PATCH 02/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- .../ranger/RangerAuthorizationPlugin.java | 8 ++--- .../authorization/ranger/RangerHelper.java | 7 +--- .../ranger/integration/test/RangerHiveIT.java | 36 +++++++------------ .../ranger/integration/test/RangerITEnv.java | 14 ++------ 4 files changed, 20 insertions(+), 45 deletions(-) diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java index 63c2e446836..c81ec71c064 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java @@ -980,19 +980,19 @@ private void removePolicyByMetadataObject(List metadataNames) { try { policy.setPolicyItems( policy.getPolicyItems().stream() - .filter(rangerHelper::isNotGravitinoManagedPolicyItemAccess) + .filter(i -> !rangerHelper.isGravitinoManagedPolicyItemAccess(i)) .collect(Collectors.toList())); policy.setDenyPolicyItems( policy.getDenyPolicyItems().stream() - .filter(rangerHelper::isNotGravitinoManagedPolicyItemAccess) + .filter(i -> !rangerHelper.isGravitinoManagedPolicyItemAccess(i)) .collect(Collectors.toList())); policy.setRowFilterPolicyItems( policy.getRowFilterPolicyItems().stream() - .filter(rangerHelper::isNotGravitinoManagedPolicyItemAccess) + .filter(i -> !rangerHelper.isGravitinoManagedPolicyItemAccess(i)) .collect(Collectors.toList())); policy.setDataMaskPolicyItems( policy.getDataMaskPolicyItems().stream() - .filter(rangerHelper::isNotGravitinoManagedPolicyItemAccess) + .filter(i -> !rangerHelper.isGravitinoManagedPolicyItemAccess(i)) .collect(Collectors.toList())); rangerClient.updatePolicy(policy.getId(), policy); } catch (RangerServiceException e) { diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java index 7b18541cfdb..cd773de5e63 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java @@ -19,7 +19,6 @@ package org.apache.gravitino.authorization.ranger; import com.google.common.base.Preconditions; -import com.google.common.collect.Lists; import com.google.common.collect.Sets; import java.util.HashMap; import java.util.HashSet; @@ -225,7 +224,7 @@ public RangerPolicy findManagedPolicy(RangerMetadataObject rangerMetadataObject) } // Only return the policies that are managed by Gravitino. if (policies.size() > 1) { - throw new AuthorizationPluginException("Every metadata object has only a policy."); + throw new AuthorizationPluginException("Each metadata object can have at most one policy."); } if (policies.isEmpty()) { @@ -254,10 +253,6 @@ boolean isGravitinoManagedPolicyItemAccess(RangerPolicy.RangerPolicyItem policyI return policyItem.getRoles().stream().anyMatch(role -> role.startsWith(GRAVITINO_ROLE_PREFIX)); } - boolean isNotGravitinoManagedPolicyItemAccess(RangerPolicy.RangerPolicyItem policyItem) { - return !isGravitinoManagedPolicyItemAccess(policyItem); - } - protected boolean checkRangerRole(String roleName) throws AuthorizationPluginException { roleName = rangerRoleName(roleName); try { diff --git a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java index da43daca799..ae918af26a8 100644 --- a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java +++ b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java @@ -327,20 +327,16 @@ public void testFindManagedPolicy() { String dbName = currentFunName(); createHivePolicy( Lists.newArrayList(String.format("%s*", dbName), "*"), - GravitinoITUtils.genRandomName(currentFunName()), - true); + GravitinoITUtils.genRandomName(currentFunName())); createHivePolicy( Lists.newArrayList(String.format("%s*", dbName), "tab*"), - GravitinoITUtils.genRandomName(currentFunName()), - true); + GravitinoITUtils.genRandomName(currentFunName())); createHivePolicy( Lists.newArrayList(String.format("%s3", dbName), "*"), - GravitinoITUtils.genRandomName(currentFunName()), - true); + GravitinoITUtils.genRandomName(currentFunName())); createHivePolicy( Lists.newArrayList(String.format("%s3", dbName), "tab*"), - GravitinoITUtils.genRandomName(currentFunName()), - true); + GravitinoITUtils.genRandomName(currentFunName())); // findManagedPolicy function use precise search, so return null RangerSecurableObject rangerSecurableObject = rangerAuthHivePlugin.generateRangerSecurableObject( @@ -354,8 +350,7 @@ public void testFindManagedPolicy() { // Add a policy for `db3.tab1` createHivePolicy( Lists.newArrayList(String.format("%s3", dbName), "tab1"), - GravitinoITUtils.genRandomName(currentFunName()), - true); + GravitinoITUtils.genRandomName(currentFunName())); // findManagedPolicy function use precise search, so return not null Assertions.assertNotNull(rangerHelper.findManagedPolicy(rangerSecurableObject)); } @@ -372,15 +367,14 @@ public void testManagedByGravitinoLabel() { SecurableObjects.DOT_SPLITTER.splitToList(securableObject.fullName())); names.remove(0); // remove catalog node // Manual create the Ranger Policy - createHivePolicy(Lists.newArrayList(names), DOT_JOINER.join(names), false); + createHivePolicy(Lists.newArrayList(names), DOT_JOINER.join(names)); }); // Use role to create Ranger Policy Assertions.assertThrows( AuthorizationPluginException.class, () -> rangerAuthHivePlugin.onRoleCreated(role)); } - static void createHivePolicy( - List metaObjects, String roleName, boolean labelManagedByGravitino) { + static void createHivePolicy(List metaObjects, String roleName) { Assertions.assertTrue(metaObjects.size() < 4); Map policyResourceMap = new HashMap<>(); for (int i = 0; i < metaObjects.size(); i++) { @@ -404,8 +398,7 @@ static void createHivePolicy( RangerITEnv.RANGER_HIVE_REPO_NAME, roleName, policyResourceMap, - Collections.singletonList(policyItem), - labelManagedByGravitino); + Collections.singletonList(policyItem)); } static boolean deleteHivePolicy(RangerSecurableObject rangerSecurableObject) { @@ -808,20 +801,16 @@ void metadataObjectChangeRemoveMetalakeOrCatalog(String funcName, MetadataObject throws RangerServiceException { createHivePolicy( Lists.newArrayList(String.format("%s*", funcName), "*"), - GravitinoITUtils.genRandomName(currentFunName()), - true); + GravitinoITUtils.genRandomName(currentFunName())); createHivePolicy( Lists.newArrayList(String.format("%s*", funcName), "tab*"), - GravitinoITUtils.genRandomName(currentFunName()), - true); + GravitinoITUtils.genRandomName(currentFunName())); createHivePolicy( Lists.newArrayList(String.format("%s3", funcName), "*"), - GravitinoITUtils.genRandomName(currentFunName()), - true); + GravitinoITUtils.genRandomName(currentFunName())); createHivePolicy( Lists.newArrayList(String.format("%s3", funcName), "tab*"), - GravitinoITUtils.genRandomName(currentFunName()), - true); + GravitinoITUtils.genRandomName(currentFunName())); Assertions.assertEquals( 4, rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).size()); @@ -1764,7 +1753,6 @@ private void verifyRangerSecurableObjectInRanger( } Assertions.assertEquals(policy.getName(), policyName); - Assertions.assertTrue(policy.getPolicyLabels().contains(RangerHelper.MANAGED_BY_GRAVITINO)); // verify namespace List metaObjNamespaces = rangerSecurableObject.names(); diff --git a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java index 13202add720..b5945de6c1f 100644 --- a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java +++ b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java @@ -20,7 +20,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Lists; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -180,8 +179,7 @@ static void allowAnyoneAccessHDFS() { RANGER_HDFS_REPO_NAME, policyName, policyResourceMap, - Collections.singletonList(policyItem), - false); + Collections.singletonList(policyItem)); } /** @@ -218,8 +216,7 @@ static void allowAnyoneAccessInformationSchema() { RANGER_HIVE_REPO_NAME, policyName, policyResourceMap, - Collections.singletonList(policyItem), - false); + Collections.singletonList(policyItem)); } public void createRangerTrinoRepository(String trinoIp) { @@ -515,13 +512,11 @@ protected static void updateOrCreateRangerPolicy( String serviceName, String policyName, Map policyResourceMap, - List policyItems, - boolean labelManagedByGravitino) { + List policyItems) { Map resourceFilter = new HashMap<>(); // use to match the precise policy Map policyFilter = new HashMap<>(); policyFilter.put(SearchFilter.SERVICE_NAME, serviceName); - policyFilter.put(SearchFilter.POLICY_LABELS_PARTIAL, RangerHelper.MANAGED_BY_GRAVITINO); final int[] index = {0}; policyResourceMap.forEach( (k, v) -> { @@ -575,9 +570,6 @@ protected static void updateOrCreateRangerPolicy( policy.setServiceType(type); policy.setService(serviceName); policy.setName(policyName); - if (labelManagedByGravitino) { - policy.setPolicyLabels(Lists.newArrayList(RangerHelper.MANAGED_BY_GRAVITINO)); - } policy.setResources(policyResourceMap); policy.setPolicyItems(policyItems); rangerClient.createPolicy(policy); From 85a60e501d01f5231977328bd8a64b16a51a5097 Mon Sep 17 00:00:00 2001 From: theoryxu Date: Thu, 21 Nov 2024 18:59:01 +0800 Subject: [PATCH 03/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- .../ranger/RangerAuthorizationPlugin.java | 11 +- .../authorization/ranger/RangerHelper.java | 15 ++- .../ranger/integration/test/RangerHiveIT.java | 112 +++++++++++------- .../ranger/integration/test/RangerITEnv.java | 10 +- 4 files changed, 91 insertions(+), 57 deletions(-) diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java index c81ec71c064..c74329f13bd 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java @@ -148,7 +148,8 @@ public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException { .toArray(RoleChange[]::new)); // Lastly, delete the role in the Ranger try { - rangerClient.deleteRole(role.name(), rangerAdminName, rangerServiceName); + rangerClient.deleteRole( + rangerHelper.rangerRoleName(role.name()), rangerAdminName, rangerServiceName); } catch (RangerServiceException e) { // Ignore exception to support idempotent operation LOG.warn("Ranger delete role: {} failed!", role, e); @@ -333,6 +334,8 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n ownerRoleName = RangerHelper.GRAVITINO_CATALOG_OWNER_ROLE; } rangerHelper.createRangerRoleIfNotExists(ownerRoleName, true); + rangerHelper.createRangerRoleIfNotExists( + RangerHelper.GRAVITINO_PLACEHOLDER_OWNER_ROLE, true); try { if (preOwnerUserName != null || preOwnerGroupName != null) { GrantRevokeRoleRequest revokeRoleRequest = @@ -724,11 +727,7 @@ private boolean doRemoveSecurableObject( && policyItem.getGroups().isEmpty()); try { - if (policy.getPolicyItems().isEmpty() && policy.getDenyPolicyItems().isEmpty()) { - rangerClient.deletePolicy(policy.getId()); - } else { - rangerClient.updatePolicy(policy.getId(), policy); - } + rangerClient.updatePolicy(policy.getId(), policy); } catch (RangerServiceException e) { LOG.error("Failed to remove the policy item from the Ranger policy {}!", policy); throw new AuthorizationPluginException( diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java index cd773de5e63..aab3c276054 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java @@ -60,6 +60,9 @@ public class RangerHelper { public static final String GRAVITINO_METALAKE_OWNER_ROLE = "GRAVITINO_METALAKE_OWNER_ROLE"; public static final String GRAVITINO_CATALOG_OWNER_ROLE = "GRAVITINO_CATALOG_OWNER_ROLE"; + // marking owner policy items + public static final String GRAVITINO_PLACEHOLDER_OWNER_ROLE = "GRAVITINO_PLACEHOLDER_OWNER_ROLE"; + public static final String GRAVITINO_ROLE_PREFIX = "GRAVITINO_"; public RangerHelper( @@ -249,7 +252,7 @@ public RangerPolicy findManagedPolicy(RangerMetadataObject rangerMetadataObject) return policy; } - boolean isGravitinoManagedPolicyItemAccess(RangerPolicy.RangerPolicyItem policyItem) { + public boolean isGravitinoManagedPolicyItemAccess(RangerPolicy.RangerPolicyItem policyItem) { return policyItem.getRoles().stream().anyMatch(role -> role.startsWith(GRAVITINO_ROLE_PREFIX)); } @@ -302,8 +305,9 @@ protected RangerRole createRangerRoleIfNotExists(String roleName, boolean isOwne if (isOwnerRole) { Preconditions.checkArgument( roleName.equalsIgnoreCase(GRAVITINO_METALAKE_OWNER_ROLE) - || roleName.equalsIgnoreCase(GRAVITINO_CATALOG_OWNER_ROLE), - "The role name should be GRAVITINO_METALAKE_OWNER_ROLE or GRAVITINO_CATALOG_OWNER_ROLE"); + || roleName.equalsIgnoreCase(GRAVITINO_CATALOG_OWNER_ROLE) + || roleName.equalsIgnoreCase(GRAVITINO_PLACEHOLDER_OWNER_ROLE), + "The role name should be GRAVITINO_METALAKE_OWNER_ROLE or GRAVITINO_CATALOG_OWNER_ROLE or GRAVITINO_PLACEHOLDER_OWNER_ROLE"); } else { Preconditions.checkArgument( !roleName.equalsIgnoreCase(GRAVITINO_METALAKE_OWNER_ROLE) @@ -346,6 +350,7 @@ protected void updatePolicyOwner(RangerPolicy policy, Owner preOwner, Owner newO }); }); }) + .filter(this::isGravitinoManagedPolicyItemAccess) .collect(Collectors.toList()); // Add or remove the owner in the policy item matchPolicyItems.forEach( @@ -397,6 +402,8 @@ protected void updatePolicyOwner(RangerPolicy policy, Owner preOwner, Owner newO } else { policyItem.getGroups().add(newOwner.name()); } + // mark the policy item is created by Gravitino + policyItem.getRoles().add(GRAVITINO_PLACEHOLDER_OWNER_ROLE); } policy.getPolicyItems().add(policyItem); }); @@ -431,6 +438,8 @@ protected RangerPolicy addOwnerToNewPolicy(RangerMetadataObject metadataObject, } else { policyItem.getGroups().add(newOwner.name()); } + // mark the policy item is created by Gravitino + policyItem.getRoles().add(GRAVITINO_PLACEHOLDER_OWNER_ROLE); } policy.getPolicyItems().add(policyItem); }); diff --git a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java index ae918af26a8..2f22a28712e 100644 --- a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java +++ b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java @@ -50,7 +50,6 @@ import org.apache.gravitino.authorization.ranger.RangerPrivileges; import org.apache.gravitino.authorization.ranger.RangerSecurableObject; import org.apache.gravitino.authorization.ranger.reference.RangerDefines; -import org.apache.gravitino.exceptions.AuthorizationPluginException; import org.apache.gravitino.integration.test.util.GravitinoITUtils; import org.apache.gravitino.meta.AuditInfo; import org.apache.gravitino.meta.GroupEntity; @@ -209,9 +208,9 @@ public void testOnRoleCreatedCatalog() { Role mockCatalogRole = mockCatalogRole(currentFunName()); Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(mockCatalogRole)); // Check if exist this policy - assertFindManagedPolicy(mockCatalogRole, true); + assertFindManagedPolicyItems(mockCatalogRole, true); Assertions.assertTrue(rangerAuthHivePlugin.onRoleDeleted(mockCatalogRole)); - assertFindManagedPolicy(mockCatalogRole, false); + assertFindManagedPolicyItems(mockCatalogRole, false); } @Test @@ -232,7 +231,7 @@ public void testOnDenyRoleCreatedCatalog() { Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(mockCatalogRole)); // Check if exist this policy - assertFindManagedPolicy(mockCatalogRole, true); + assertFindManagedPolicyItems(mockCatalogRole, true); mockCatalogRole.securableObjects().stream() .forEach( securableObject -> { @@ -250,7 +249,7 @@ public void testOnDenyRoleCreatedCatalog() { }); Assertions.assertTrue(rangerAuthHivePlugin.onRoleDeleted(mockCatalogRole)); - assertFindManagedPolicy(mockCatalogRole, false); + assertFindManagedPolicyItems(mockCatalogRole, false); } @Test @@ -258,12 +257,12 @@ public void testOnRoleDeleted() { // prepare to create a role RoleEntity role = mock3TableRole(currentFunName()); Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(role)); - assertFindManagedPolicy(role, true); + assertFindManagedPolicyItems(role, true); // delete this role Assertions.assertTrue(rangerAuthHivePlugin.onRoleDeleted(role)); // Check if the policy is deleted - assertFindManagedPolicy(role, false); + assertFindManagedPolicyItems(role, false); } @Test @@ -274,7 +273,7 @@ public void testOnRoleDeletedCatalog() { // delete this role Assertions.assertTrue(rangerAuthHivePlugin.onRoleDeleted(mockCatalogRole)); // Check if exist this policy - assertFindManagedPolicy(mockCatalogRole, false); + assertFindManagedPolicyItems(mockCatalogRole, false); } @Test @@ -293,7 +292,7 @@ public void testOnRoleDeleted2() { // delete this role Assertions.assertTrue(rangerAuthHivePlugin.onRoleDeleted(role)); // Because this metaobject has owner, so the policy should not be deleted - assertFindManagedPolicy(role, true); + assertFindManagedPolicyItems(role, false); } @Test @@ -356,7 +355,7 @@ public void testFindManagedPolicy() { } @Test - public void testManagedByGravitinoLabel() { + public void testGravitinoAddItemsToExistingPolicy() { RoleEntity role = mock3TableRole(currentFunName()); role.securableObjects().stream() .forEach( @@ -370,8 +369,7 @@ public void testManagedByGravitinoLabel() { createHivePolicy(Lists.newArrayList(names), DOT_JOINER.join(names)); }); // Use role to create Ranger Policy - Assertions.assertThrows( - AuthorizationPluginException.class, () -> rangerAuthHivePlugin.onRoleCreated(role)); + Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(role)); } static void createHivePolicy(List metaObjects, String roleName) { @@ -466,7 +464,7 @@ public void testRoleChangeAddCatalogSecurableObject() { mockCatalogRole, RoleChange.addSecurableObject( mockCatalogRole.name(), mockCatalogRole.securableObjects().get(0)))); - assertFindManagedPolicy(mockCatalogRole, true); + assertFindManagedPolicyItems(mockCatalogRole, true); } @Test @@ -510,7 +508,7 @@ public void testRoleChangeRemoveCatalogSecurableObject() { mockCatalogRole, RoleChange.removeSecurableObject( mockCatalogRole.name(), mockCatalogRole.securableObjects().get(0)))); - assertFindManagedPolicy(mockCatalogRole, false); + assertFindManagedPolicyItems(mockCatalogRole, false); } @Test @@ -614,7 +612,7 @@ public void testRoleChangeUpdateCatalogSecurableObject() { mockCatalogRole.name(), mockCatalogRole.securableObjects().get(0), newSecurableObject))); - assertFindManagedPolicy(mockCatalogRole, false); + assertFindManagedPolicyItems(mockCatalogRole, false); } @Test @@ -636,7 +634,7 @@ public void testMetadataObjectChangeRenameMetalake() { .withSecurableObjects(Lists.newArrayList(oldSecurableObject1)) .build(); Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(role)); - assertFindManagedPolicy(role, true); + assertFindManagedPolicyItems(role, true); MetadataObject newMetadataObject = MetadataObjects.parse( @@ -656,7 +654,7 @@ public void testMetadataObjectChangeRenameMetalake() { .withAuditInfo(auditInfo) .withSecurableObjects(Lists.newArrayList(newSecurableObject1)) .build(); - assertFindManagedPolicy(newRole, true); + assertFindManagedPolicyItems(newRole, true); } @Test @@ -678,7 +676,7 @@ public void testMetadataObjectChangeRenameCatalog() { .withSecurableObjects(Lists.newArrayList(oldSecurableObject1)) .build(); Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(role)); - assertFindManagedPolicy(role, true); + assertFindManagedPolicyItems(role, true); MetadataObject newMetadataObject = MetadataObjects.parse( @@ -698,7 +696,7 @@ public void testMetadataObjectChangeRenameCatalog() { .withAuditInfo(auditInfo) .withSecurableObjects(Lists.newArrayList(newSecurableObject1)) .build(); - assertFindManagedPolicy(newRole, true); + assertFindManagedPolicyItems(newRole, true); } @Test @@ -720,7 +718,7 @@ public void testMetadataObjectChangeRenameSchema() { .withSecurableObjects(Lists.newArrayList(oldSecurableObject1)) .build(); Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(role)); - assertFindManagedPolicy(role, true); + assertFindManagedPolicyItems(role, true); MetadataObject newMetadataObject = MetadataObjects.parse( @@ -728,7 +726,7 @@ public void testMetadataObjectChangeRenameSchema() { Assertions.assertTrue( rangerAuthHivePlugin.onMetadataUpdated( MetadataObjectChange.rename(oldMetadataObject, newMetadataObject))); - assertFindManagedPolicy(role, false); + assertFindManagedPolicyItems(role, false); SecurableObject newSecurableObject1 = SecurableObjects.parse( newMetadataObject.fullName(), @@ -741,7 +739,7 @@ public void testMetadataObjectChangeRenameSchema() { .withAuditInfo(auditInfo) .withSecurableObjects(Lists.newArrayList(newSecurableObject1)) .build(); - assertFindManagedPolicy(newRole, true); + assertFindManagedPolicyItems(newRole, true); } @Test @@ -763,7 +761,7 @@ public void testMetadataObjectChangeRenameTable() { .withSecurableObjects(Lists.newArrayList(oldSecurableObject1)) .build(); Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(role)); - assertFindManagedPolicy(role, true); + assertFindManagedPolicyItems(role, true); MetadataObject newMetadataObject = MetadataObjects.parse( @@ -771,7 +769,7 @@ public void testMetadataObjectChangeRenameTable() { Assertions.assertTrue( rangerAuthHivePlugin.onMetadataUpdated( MetadataObjectChange.rename(oldMetadataObject, newMetadataObject))); - assertFindManagedPolicy(role, false); + assertFindManagedPolicyItems(role, false); SecurableObject newSecurableObject1 = SecurableObjects.parse( newMetadataObject.fullName(), @@ -784,7 +782,7 @@ public void testMetadataObjectChangeRenameTable() { .withAuditInfo(auditInfo) .withSecurableObjects(Lists.newArrayList(newSecurableObject1)) .build(); - assertFindManagedPolicy(newRole, true); + assertFindManagedPolicyItems(newRole, true); } @Test @@ -830,11 +828,11 @@ void metadataObjectChangeRemoveMetalakeOrCatalog(String funcName, MetadataObject .withSecurableObjects(Lists.newArrayList(oldSecurableObject1)) .build(); Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(role)); - assertFindManagedPolicy(role, true); + assertFindManagedPolicyItems(role, true); Assertions.assertTrue( rangerAuthHivePlugin.onMetadataUpdated(MetadataObjectChange.remove(metadataObject))); - assertFindManagedPolicy(role, false); + assertFindManagedPolicyItems(role, false); Assertions.assertEquals( 0, rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).size()); @@ -867,7 +865,7 @@ public void testMetadataObjectChangeRemoveSchema() throws RangerServiceException .withSecurableObjects(Lists.newArrayList(schemaSecurableObject, tableSecurableObject)) .build(); Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(role)); - assertFindManagedPolicy(role, true); + assertFindManagedPolicyItems(role, true); Assertions.assertEquals( 4, rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).size()); @@ -880,9 +878,9 @@ public void testMetadataObjectChangeRemoveSchema() throws RangerServiceException .withAuditInfo(auditInfo) .withSecurableObjects(Lists.newArrayList(schemaSecurableObject)) .build(); - assertFindManagedPolicy(roleVerify, false); + assertFindManagedPolicyItems(roleVerify, false); Assertions.assertEquals( - 2, rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).size()); + 4, rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).size()); } @Test @@ -912,7 +910,7 @@ public void testMetadataObjectChangeRemoveTable() throws RangerServiceException .withSecurableObjects(Lists.newArrayList(securableObjectSchema, securableObjectTab)) .build(); Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(role)); - assertFindManagedPolicy(role, true); + assertFindManagedPolicyItems(role, true); Assertions.assertTrue( rangerAuthHivePlugin.onMetadataUpdated(MetadataObjectChange.remove(tableObject))); @@ -930,10 +928,10 @@ public void testMetadataObjectChangeRemoveTable() throws RangerServiceException .withAuditInfo(auditInfo) .withSecurableObjects(Lists.newArrayList(securableObjectTab)) .build(); - assertFindManagedPolicy(verifyScheamRole, true); - assertFindManagedPolicy(verifyTableRole, false); + assertFindManagedPolicyItems(verifyScheamRole, true); + assertFindManagedPolicyItems(verifyTableRole, false); Assertions.assertEquals( - 2, rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).size()); + 4, rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).size()); } @Test @@ -995,8 +993,8 @@ public void testRoleChangeCombinedOperation() { // Delete the role Assertions.assertTrue(rangerAuthHivePlugin.onRoleDeleted(verifyRole)); - // Because these metaobjects have an owner, so the policy will not be deleted. - assertFindManagedPolicy(role, true); + // policy item will be deleted. + assertFindManagedPolicyItems(role, false); rangerAuthHivePlugin.translateOwner(oldMetadataObject).stream() .forEach( rangerSecurableObject -> { @@ -1152,7 +1150,7 @@ public void testOnRevokedRolesFromGroup() { rangerAuthHivePlugin, role, null, null, null, Lists.newArrayList(groupName1)); } - private void assertFindManagedPolicy(Role role, boolean policyExist) { + private void assertFindManagedPolicyItems(Role role, boolean gravitinoPolicyItemExist) { role.securableObjects().stream() .forEach( securableObject -> @@ -1160,16 +1158,38 @@ private void assertFindManagedPolicy(Role role, boolean policyExist) { .forEach( rangerSecurableObject -> { LOG.info("rangerSecurableObject: " + rangerSecurableObject); - if (policyExist) { - Assertions.assertNotNull( - rangerHelper.findManagedPolicy(rangerSecurableObject)); + RangerPolicy policy = + rangerHelper.findManagedPolicy(rangerSecurableObject); + List policyItems = new ArrayList<>(); + if (policy != null) { + policyItems.addAll(policy.getPolicyItems()); + policyItems.addAll(policy.getDenyPolicyItems()); + policyItems.addAll(policy.getRowFilterPolicyItems()); + policyItems.addAll(policy.getDataMaskPolicyItems()); + } + + if (gravitinoPolicyItemExist) { + Assertions.assertNotNull(policy); + Assertions.assertTrue( + hasGravitinoManagedPolicyItemAccess(policyItems, role.name())); } else { - Assertions.assertNull( - rangerHelper.findManagedPolicy(rangerSecurableObject)); + Assertions.assertTrue( + (policy == null) + || !hasGravitinoManagedPolicyItemAccess( + policyItems, role.name())); } })); } + public boolean hasGravitinoManagedPolicyItemAccess( + List items, String roleName) { + return items.stream() + .anyMatch( + i -> + i.getRoles().stream() + .anyMatch(r -> r.equals(RangerHelper.GRAVITINO_ROLE_PREFIX + roleName))); + } + private static class MockOwner implements Owner { private final String name; private final Type type; @@ -1718,14 +1738,14 @@ public void testCombinationOperation() { Lists.newArrayList(groupName1, groupName2, groupName3)); Assertions.assertTrue(rangerAuthHivePlugin.onRoleDeleted(role1)); - // Because role1's secrable object have owner, so the policy will not be deleted. - assertFindManagedPolicy(role1, true); + // policy items will be deleted. + assertFindManagedPolicyItems(role1, false); Assertions.assertTrue(rangerAuthHivePlugin.onRoleDeleted(role2)); - assertFindManagedPolicy(role2, true); + assertFindManagedPolicyItems(role2, false); Assertions.assertTrue(rangerAuthHivePlugin.onRoleDeleted(role3)); - assertFindManagedPolicy(role3, true); + assertFindManagedPolicyItems(role3, false); } /** diff --git a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java index b5945de6c1f..0f8d895f03f 100644 --- a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java +++ b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java @@ -377,7 +377,9 @@ protected static void verifyRoleInRanger( try { rangerRole = RangerITEnv.rangerClient.getRole( - role.name(), rangerAuthPlugin.rangerAdminName, RangerITEnv.RANGER_HIVE_REPO_NAME); + RangerHelper.GRAVITINO_ROLE_PREFIX + role.name(), + rangerAuthPlugin.rangerAdminName, + RangerITEnv.RANGER_HIVE_REPO_NAME); LOG.info("rangerRole: " + rangerRole.toString()); } catch (RangerServiceException e) { throw new RuntimeException(e); @@ -468,7 +470,11 @@ protected static void verifyRoleInRanger( .allMatch( policyItem -> { // Verify role name in Ranger policy item - return policyItem.getRoles().contains(role.name()); + return policyItem + .getRoles() + .contains( + RangerHelper.GRAVITINO_ROLE_PREFIX + + role.name()); }); Assertions.assertTrue(match); }); From 2277d5bb5a57b619e7eab1702bb5168919c5e47d Mon Sep 17 00:00:00 2001 From: theoryxu Date: Thu, 21 Nov 2024 19:18:02 +0800 Subject: [PATCH 04/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- .../ranger/RangerAuthorizationPlugin.java | 12 ++++++--- .../authorization/ranger/RangerHelper.java | 26 ++++++++++--------- .../ranger/integration/test/RangerHiveIT.java | 2 +- .../ranger/integration/test/RangerITEnv.java | 2 +- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java index c74329f13bd..5381ca8a6e3 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java @@ -149,7 +149,7 @@ public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException { // Lastly, delete the role in the Ranger try { rangerClient.deleteRole( - rangerHelper.rangerRoleName(role.name()), rangerAdminName, rangerServiceName); + rangerHelper.generateGravitinoRoleName(role.name()), rangerAdminName, rangerServiceName); } catch (RangerServiceException e) { // Ignore exception to support idempotent operation LOG.warn("Ranger delete role: {} failed!", role, e); @@ -625,7 +625,9 @@ private boolean doAddSecurableObject(String roleName, RangerSecurableObject secu policy.getPolicyItems().stream() .filter( policyItem -> - policyItem.getRoles().contains(rangerHelper.rangerRoleName(roleName))) + policyItem + .getRoles() + .contains(rangerHelper.generateGravitinoRoleName(roleName))) .flatMap(policyItem -> policyItem.getAccesses().stream()) .map(RangerPolicy.RangerPolicyItemAccess::getType) .map(RangerPrivileges::valueOf) @@ -635,7 +637,9 @@ private boolean doAddSecurableObject(String roleName, RangerSecurableObject secu policy.getDenyPolicyItems().stream() .filter( policyItem -> - policyItem.getRoles().contains(rangerHelper.rangerRoleName(roleName))) + policyItem + .getRoles() + .contains(rangerHelper.generateGravitinoRoleName(roleName))) .flatMap(policyItem -> policyItem.getAccesses().stream()) .map(RangerPolicy.RangerPolicyItemAccess::getType) .map(RangerPrivileges::valueOf) @@ -740,7 +744,7 @@ private void removePolicyItemIfEqualRoleName( RangerPolicy.RangerPolicyItem policyItem, RangerSecurableObject rangerSecurableObject, String roleName) { - roleName = rangerHelper.rangerRoleName(roleName); + roleName = rangerHelper.generateGravitinoRoleName(roleName); boolean match = policyItem.getAccesses().stream() .allMatch( diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java index aab3c276054..c7871780142 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java @@ -146,7 +146,7 @@ void addPolicyItem(RangerPolicy policy, String roleName, RangerSecurableObject s new RangerPolicy.RangerPolicyItemAccess(); access.setType(rangerPrivilege.getName()); policyItem.getAccesses().add(access); - policyItem.getRoles().add(GRAVITINO_ROLE_PREFIX + roleName); + policyItem.getRoles().add(generateGravitinoRoleName(roleName)); if (Privilege.Condition.ALLOW == rangerPrivilege.condition()) { policy.getPolicyItems().add(policyItem); } else { @@ -158,8 +158,10 @@ void addPolicyItem(RangerPolicy policy, String roleName, RangerSecurableObject s .forEach( policyItem -> { // If the role is not in the policy item, then add it - if (!policyItem.getRoles().contains(GRAVITINO_ROLE_PREFIX + roleName)) { - policyItem.getRoles().add(GRAVITINO_ROLE_PREFIX + roleName); + if (!policyItem + .getRoles() + .contains(generateGravitinoRoleName(roleName))) { + policyItem.getRoles().add(generateGravitinoRoleName(roleName)); } }); } @@ -257,7 +259,7 @@ public boolean isGravitinoManagedPolicyItemAccess(RangerPolicy.RangerPolicyItem } protected boolean checkRangerRole(String roleName) throws AuthorizationPluginException { - roleName = rangerRoleName(roleName); + roleName = generateGravitinoRoleName(roleName); try { rangerClient.getRole(roleName, rangerAdminName, rangerServiceName); } catch (RangerServiceException e) { @@ -267,7 +269,7 @@ protected boolean checkRangerRole(String roleName) throws AuthorizationPluginExc return true; } - String rangerRoleName(String roleName) { + public String generateGravitinoRoleName(String roleName) { if (roleName.startsWith(GRAVITINO_ROLE_PREFIX)) { return roleName; } @@ -276,7 +278,7 @@ String rangerRoleName(String roleName) { protected GrantRevokeRoleRequest createGrantRevokeRoleRequest( String roleName, String userName, String groupName) { - roleName = rangerRoleName(roleName); + roleName = generateGravitinoRoleName(roleName); Set users = StringUtils.isEmpty(userName) ? Sets.newHashSet() : Sets.newHashSet(userName); Set groups = @@ -301,7 +303,7 @@ protected GrantRevokeRoleRequest createGrantRevokeRoleRequest( * @param isOwnerRole The role is owner role or not */ protected RangerRole createRangerRoleIfNotExists(String roleName, boolean isOwnerRole) { - roleName = rangerRoleName(roleName); + roleName = generateGravitinoRoleName(roleName); if (isOwnerRole) { Preconditions.checkArgument( roleName.equalsIgnoreCase(GRAVITINO_METALAKE_OWNER_ROLE) @@ -457,7 +459,7 @@ protected RangerPolicy addOwnerRoleToNewPolicy( policyItem .getAccesses() .add(new RangerPolicy.RangerPolicyItemAccess(ownerPrivilege.getName())); - policyItem.getRoles().add(rangerRoleName(ownerRoleName)); + policyItem.getRoles().add(generateGravitinoRoleName(ownerRoleName)); policy.getPolicyItems().add(policyItem); }); return policy; @@ -483,8 +485,8 @@ protected void updatePolicyOwnerRole(RangerPolicy policy, String ownerRoleName) // Add or remove the owner role in the policy item matchPolicyItems.forEach( policyItem -> { - if (!policyItem.getRoles().contains(rangerRoleName(ownerRoleName))) { - policyItem.getRoles().add(rangerRoleName(ownerRoleName)); + if (!policyItem.getRoles().contains(generateGravitinoRoleName(ownerRoleName))) { + policyItem.getRoles().add(generateGravitinoRoleName(ownerRoleName)); } }); @@ -509,8 +511,8 @@ protected void updatePolicyOwnerRole(RangerPolicy policy, String ownerRoleName) policyItem .getAccesses() .add(new RangerPolicy.RangerPolicyItemAccess(ownerPrivilege.getName())); - if (!policyItem.getRoles().contains(rangerRoleName(ownerRoleName))) { - policyItem.getRoles().add(rangerRoleName(ownerRoleName)); + if (!policyItem.getRoles().contains(generateGravitinoRoleName(ownerRoleName))) { + policyItem.getRoles().add(generateGravitinoRoleName(ownerRoleName)); } policy.getPolicyItems().add(policyItem); }); diff --git a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java index 2f22a28712e..b2db383498c 100644 --- a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java +++ b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java @@ -1187,7 +1187,7 @@ public boolean hasGravitinoManagedPolicyItemAccess( .anyMatch( i -> i.getRoles().stream() - .anyMatch(r -> r.equals(RangerHelper.GRAVITINO_ROLE_PREFIX + roleName))); + .anyMatch(r -> r.equals(rangerHelper.generateGravitinoRoleName(roleName)))); } private static class MockOwner implements Owner { diff --git a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java index 0f8d895f03f..0c6dd580324 100644 --- a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java +++ b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java @@ -377,7 +377,7 @@ protected static void verifyRoleInRanger( try { rangerRole = RangerITEnv.rangerClient.getRole( - RangerHelper.GRAVITINO_ROLE_PREFIX + role.name(), + rangerHelper.generateGravitinoRoleName(role.name()), rangerAuthPlugin.rangerAdminName, RangerITEnv.RANGER_HIVE_REPO_NAME); LOG.info("rangerRole: " + rangerRole.toString()); From ddcd2c43961bddc928720410770479f76e007e02 Mon Sep 17 00:00:00 2001 From: theoryxu Date: Fri, 22 Nov 2024 17:39:50 +0800 Subject: [PATCH 05/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- .../authorization/ranger/RangerHelper.java | 27 ++-- .../ranger/integration/test/RangerHiveIT.java | 119 ++++++++++++++---- 2 files changed, 105 insertions(+), 41 deletions(-) diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java index c7871780142..490ee22069a 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java @@ -86,6 +86,9 @@ public RangerHelper( */ void checkPolicyItemAccess(RangerPolicy.RangerPolicyItem policyItem) throws AuthorizationPluginException { + if (!isGravitinoManagedPolicyItemAccess(policyItem)) { + return; + } if (policyItem.getAccesses().size() != 1) { throw new AuthorizationPluginException( "The access type only have one in the delegate Gravitino management policy"); @@ -124,7 +127,9 @@ void addPolicyItem(RangerPolicy policy, String roleName, RangerSecurableObject s policyItem -> { return policyItem.getAccesses().stream() .anyMatch( - access -> access.getType().equals(rangerPrivilege.getName())); + access -> + access.getType().equals(rangerPrivilege.getName()) + && isGravitinoManagedPolicyItemAccess(policyItem)); }) .collect(Collectors.toList()); } else { @@ -134,7 +139,9 @@ void addPolicyItem(RangerPolicy policy, String roleName, RangerSecurableObject s policyItem -> { return policyItem.getAccesses().stream() .anyMatch( - access -> access.getType().equals(rangerPrivilege.getName())); + access -> + access.getType().equals(rangerPrivilege.getName()) + && isGravitinoManagedPolicyItemAccess(policyItem)); }) .collect(Collectors.toList()); } @@ -238,18 +245,10 @@ public RangerPolicy findManagedPolicy(RangerMetadataObject rangerMetadataObject) RangerPolicy policy = policies.get(0); // Delegating Gravitino management policies cannot contain duplicate privilege - policy.getPolicyItems().stream() - .filter(this::isGravitinoManagedPolicyItemAccess) - .forEach(this::checkPolicyItemAccess); - policy.getDenyPolicyItems().stream() - .filter(this::isGravitinoManagedPolicyItemAccess) - .forEach(this::checkPolicyItemAccess); - policy.getRowFilterPolicyItems().stream() - .filter(this::isGravitinoManagedPolicyItemAccess) - .forEach(this::checkPolicyItemAccess); - policy.getDataMaskPolicyItems().stream() - .filter(this::isGravitinoManagedPolicyItemAccess) - .forEach(this::checkPolicyItemAccess); + policy.getPolicyItems().forEach(this::checkPolicyItemAccess); + policy.getDenyPolicyItems().forEach(this::checkPolicyItemAccess); + policy.getRowFilterPolicyItems().forEach(this::checkPolicyItemAccess); + policy.getDataMaskPolicyItems().forEach(this::checkPolicyItemAccess); return policy; } diff --git a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java index b2db383498c..461ec457902 100644 --- a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java +++ b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java @@ -33,7 +33,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; import org.apache.gravitino.MetadataObject; import org.apache.gravitino.MetadataObjects; import org.apache.gravitino.authorization.MetadataObjectChange; @@ -355,7 +357,7 @@ public void testFindManagedPolicy() { } @Test - public void testGravitinoAddItemsToExistingPolicy() { + public void testGravitinoCompatibleWithExistingPolicy() { RoleEntity role = mock3TableRole(currentFunName()); role.securableObjects().stream() .forEach( @@ -368,8 +370,66 @@ public void testGravitinoAddItemsToExistingPolicy() { // Manual create the Ranger Policy createHivePolicy(Lists.newArrayList(names), DOT_JOINER.join(names)); }); - // Use role to create Ranger Policy - Assertions.assertTrue(rangerAuthHivePlugin.onRoleCreated(role)); + List existingPolicyNames = + findRoleResourceRelatedPolicies(role).stream() + .map(RangerPolicy::getName) + .collect(Collectors.toList()); + rangerAuthHivePlugin.onRoleCreated(role); + findRoleResourceRelatedPolicies(role) + .forEach( + policy -> { + List policyItems = new ArrayList<>(); + policyItems.addAll(policy.getPolicyItems()); + policyItems.addAll(policy.getDenyPolicyItems()); + policyItems.addAll(policy.getRowFilterPolicyItems()); + policyItems.addAll(policy.getDataMaskPolicyItems()); + + if (existingPolicyNames.contains(policy.getName())) { + Assertions.assertTrue( + policyItems.stream() + .anyMatch( + i -> + !i.getRoles() + .contains( + rangerHelper.generateGravitinoRoleName(role.name())))); + } + Assertions.assertTrue( + policyItems.stream() + .anyMatch( + i -> + i.getRoles() + .contains(rangerHelper.generateGravitinoRoleName(role.name())))); + }); + + rangerAuthHivePlugin.onRoleDeleted(role); + findRoleResourceRelatedPolicies(role) + .forEach( + policy -> { + List policyItems = new ArrayList<>(); + policyItems.addAll(policy.getPolicyItems()); + policyItems.addAll(policy.getDenyPolicyItems()); + policyItems.addAll(policy.getRowFilterPolicyItems()); + policyItems.addAll(policy.getDataMaskPolicyItems()); + + if (existingPolicyNames.contains(policy.getName())) { + Assertions.assertFalse( + policyItems.stream() + .anyMatch( + i -> + i.getRoles() + .contains( + rangerHelper.generateGravitinoRoleName(role.name())))); + Assertions.assertTrue( + policyItems.stream() + .anyMatch( + i -> + !i.getRoles() + .contains( + rangerHelper.generateGravitinoRoleName(role.name())))); + } else { + Assertions.assertEquals(0, policyItems.size()); + } + }); } static void createHivePolicy(List metaObjects, String roleName) { @@ -1151,34 +1211,39 @@ public void testOnRevokedRolesFromGroup() { } private void assertFindManagedPolicyItems(Role role, boolean gravitinoPolicyItemExist) { - role.securableObjects().stream() - .forEach( + List policies = findRoleResourceRelatedPolicies(role); + + if (gravitinoPolicyItemExist) { + Assertions.assertTrue(policies.size() > 0); + } + + policies.forEach( + policy -> { + List policyItems = new ArrayList<>(); + policyItems.addAll(policy.getPolicyItems()); + policyItems.addAll(policy.getDenyPolicyItems()); + policyItems.addAll(policy.getRowFilterPolicyItems()); + policyItems.addAll(policy.getDataMaskPolicyItems()); + if (gravitinoPolicyItemExist) { + Assertions.assertTrue(hasGravitinoManagedPolicyItemAccess(policyItems, role.name())); + } else { + Assertions.assertFalse(hasGravitinoManagedPolicyItemAccess(policyItems, role.name())); + } + }); + } + + private List findRoleResourceRelatedPolicies(Role role) { + return role.securableObjects().stream() + .flatMap( securableObject -> rangerAuthHivePlugin.translatePrivilege(securableObject).stream() - .forEach( + .map( rangerSecurableObject -> { LOG.info("rangerSecurableObject: " + rangerSecurableObject); - RangerPolicy policy = - rangerHelper.findManagedPolicy(rangerSecurableObject); - List policyItems = new ArrayList<>(); - if (policy != null) { - policyItems.addAll(policy.getPolicyItems()); - policyItems.addAll(policy.getDenyPolicyItems()); - policyItems.addAll(policy.getRowFilterPolicyItems()); - policyItems.addAll(policy.getDataMaskPolicyItems()); - } - - if (gravitinoPolicyItemExist) { - Assertions.assertNotNull(policy); - Assertions.assertTrue( - hasGravitinoManagedPolicyItemAccess(policyItems, role.name())); - } else { - Assertions.assertTrue( - (policy == null) - || !hasGravitinoManagedPolicyItemAccess( - policyItems, role.name())); - } - })); + return rangerHelper.findManagedPolicy(rangerSecurableObject); + })) + .filter(Objects::nonNull) + .collect(Collectors.toList()); } public boolean hasGravitinoManagedPolicyItemAccess( From 81a55339cd422d7c7c5d732c08a074fe1d3115ab Mon Sep 17 00:00:00 2001 From: theoryxu Date: Mon, 25 Nov 2024 10:04:11 +0800 Subject: [PATCH 06/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- .../apache/gravitino/authorization/ranger/RangerHelper.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java index 490ee22069a..00b90081785 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java @@ -79,7 +79,9 @@ public RangerHelper( } /** - * For easy management, each privilege will create one RangerPolicyItemAccess in the policy. + * There are two types of policy items: + * Gravitino managed policy items: They contain one privilege each. + * User-defined policy items: They could contain multiple privileges and not be managed and checked by Gravitino. * * @param policyItem The policy item to check * @throws AuthorizationPluginException If the policy item contains more than one access type From b8063aadc28bdb6bf5df03aa60996cefd37ed9d0 Mon Sep 17 00:00:00 2001 From: theoryxu Date: Mon, 25 Nov 2024 10:13:42 +0800 Subject: [PATCH 07/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- .../apache/gravitino/authorization/ranger/RangerHelper.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java index 00b90081785..2b9ab2b8645 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java @@ -79,9 +79,9 @@ public RangerHelper( } /** - * There are two types of policy items: - * Gravitino managed policy items: They contain one privilege each. - * User-defined policy items: They could contain multiple privileges and not be managed and checked by Gravitino. + * There are two types of policy items. Gravitino managed policy items: They contain one privilege + * each. User-defined policy items: They could contain multiple privileges and not be managed and + * checked by Gravitino. * * @param policyItem The policy item to check * @throws AuthorizationPluginException If the policy item contains more than one access type From fbd9fc4b51749f228afd756ca6a05d385f57a9b4 Mon Sep 17 00:00:00 2001 From: theoryxu Date: Mon, 25 Nov 2024 15:48:35 +0800 Subject: [PATCH 08/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- .../authorization/ranger/RangerHelper.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java index 2b9ab2b8645..a6b980ba963 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java @@ -21,7 +21,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Sets; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -95,18 +94,6 @@ void checkPolicyItemAccess(RangerPolicy.RangerPolicyItem policyItem) throw new AuthorizationPluginException( "The access type only have one in the delegate Gravitino management policy"); } - Set setAccesses = new HashSet<>(); - policyItem - .getAccesses() - .forEach( - access -> { - if (setAccesses.contains(access.getType())) { - throw new AuthorizationPluginException( - "Contain duplicate privilege(%s) in the delegate Gravitino management policy ", - access.getType()); - } - setAccesses.add(access.getType()); - }); } /** From 552f08e900430e1daea3daaf582304d4aa5586a1 Mon Sep 17 00:00:00 2001 From: theoryxu Date: Tue, 26 Nov 2024 17:22:03 +0800 Subject: [PATCH 09/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- .../ranger/RangerAuthorizationPlugin.java | 38 ++----------------- .../authorization/ranger/RangerHelper.java | 33 ++++++++++++++++ 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java index 5381ca8a6e3..79afae38a87 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java @@ -800,15 +800,8 @@ private void doRemoveSchemaMetadataObject(RangerMetadataObject rangerMetadataObj try { List policies = rangerClient.getPoliciesInService(rangerServiceName); policies.stream() - .forEach( - policy -> { - try { - rangerClient.deletePolicy(policy.getId()); - } catch (RangerServiceException e) { - LOG.error("Failed to rename the policy {}!", policy); - throw new RuntimeException(e); - } - }); + .filter(rangerHelper::hasGravitinoManagedPolicyItem) + .forEach(rangerHelper::removeAllGravitinoManagedPolicyItem); } catch (RangerServiceException e) { throw new RuntimeException(e); } @@ -977,32 +970,7 @@ private void removePolicyByMetadataObject(List metadataNames) { .getValues() .contains(preciseFilters.get(entry.getKey())))) .collect(Collectors.toList()); - policies.stream() - .forEach( - policy -> { - try { - policy.setPolicyItems( - policy.getPolicyItems().stream() - .filter(i -> !rangerHelper.isGravitinoManagedPolicyItemAccess(i)) - .collect(Collectors.toList())); - policy.setDenyPolicyItems( - policy.getDenyPolicyItems().stream() - .filter(i -> !rangerHelper.isGravitinoManagedPolicyItemAccess(i)) - .collect(Collectors.toList())); - policy.setRowFilterPolicyItems( - policy.getRowFilterPolicyItems().stream() - .filter(i -> !rangerHelper.isGravitinoManagedPolicyItemAccess(i)) - .collect(Collectors.toList())); - policy.setDataMaskPolicyItems( - policy.getDataMaskPolicyItems().stream() - .filter(i -> !rangerHelper.isGravitinoManagedPolicyItemAccess(i)) - .collect(Collectors.toList())); - rangerClient.updatePolicy(policy.getId(), policy); - } catch (RangerServiceException e) { - LOG.error("Failed to rename the policy {}!", policy); - throw new RuntimeException(e); - } - }); + policies.forEach(rangerHelper::removeAllGravitinoManagedPolicyItem); } private void updatePolicyByMetadataObject( diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java index a6b980ba963..8f249c35244 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java @@ -246,6 +246,39 @@ public boolean isGravitinoManagedPolicyItemAccess(RangerPolicy.RangerPolicyItem return policyItem.getRoles().stream().anyMatch(role -> role.startsWith(GRAVITINO_ROLE_PREFIX)); } + public boolean hasGravitinoManagedPolicyItem(RangerPolicy policy) { + List policyItems = policy.getPolicyItems(); + policyItems.addAll(policy.getDenyPolicyItems()); + policyItems.addAll(policy.getRowFilterPolicyItems()); + policyItems.addAll(policy.getDataMaskPolicyItems()); + return policyItems.stream().anyMatch(this::isGravitinoManagedPolicyItemAccess); + } + + public void removeAllGravitinoManagedPolicyItem(RangerPolicy policy) { + try { + policy.setPolicyItems( + policy.getPolicyItems().stream() + .filter(i -> !isGravitinoManagedPolicyItemAccess(i)) + .collect(Collectors.toList())); + policy.setDenyPolicyItems( + policy.getDenyPolicyItems().stream() + .filter(i -> !isGravitinoManagedPolicyItemAccess(i)) + .collect(Collectors.toList())); + policy.setRowFilterPolicyItems( + policy.getRowFilterPolicyItems().stream() + .filter(i -> !isGravitinoManagedPolicyItemAccess(i)) + .collect(Collectors.toList())); + policy.setDataMaskPolicyItems( + policy.getDataMaskPolicyItems().stream() + .filter(i -> !isGravitinoManagedPolicyItemAccess(i)) + .collect(Collectors.toList())); + rangerClient.updatePolicy(policy.getId(), policy); + } catch (RangerServiceException e) { + LOG.error("Failed to update the policy {}!", policy); + throw new RuntimeException(e); + } + } + protected boolean checkRangerRole(String roleName) throws AuthorizationPluginException { roleName = generateGravitinoRoleName(roleName); try { From c773e5b2a59cb869d1ba3f662ea434a184c40984 Mon Sep 17 00:00:00 2001 From: theoryxu Date: Tue, 26 Nov 2024 17:51:05 +0800 Subject: [PATCH 10/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- .../authorization/ranger/integration/test/RangerHiveIT.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java index 461ec457902..8035086ebd9 100644 --- a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java +++ b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java @@ -895,7 +895,10 @@ void metadataObjectChangeRemoveMetalakeOrCatalog(String funcName, MetadataObject assertFindManagedPolicyItems(role, false); Assertions.assertEquals( - 0, rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).size()); + 6, rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).size()); + rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).forEach(policy -> { + Assertions.assertFalse(rangerHelper.hasGravitinoManagedPolicyItem(policy)); + }); } @Test From 03e2c39725f9e7ae4cf391deae4ac664ee3f3f5d Mon Sep 17 00:00:00 2001 From: theoryxu Date: Tue, 26 Nov 2024 17:52:34 +0800 Subject: [PATCH 11/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- .../ranger/integration/test/RangerHiveIT.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java index 8035086ebd9..ff87009e393 100644 --- a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java +++ b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java @@ -896,9 +896,12 @@ void metadataObjectChangeRemoveMetalakeOrCatalog(String funcName, MetadataObject Assertions.assertEquals( 6, rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).size()); - rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).forEach(policy -> { - Assertions.assertFalse(rangerHelper.hasGravitinoManagedPolicyItem(policy)); - }); + rangerClient + .getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME) + .forEach( + policy -> { + Assertions.assertFalse(rangerHelper.hasGravitinoManagedPolicyItem(policy)); + }); } @Test From c59749f9484448e4d98d509a268e62d95782e3ff Mon Sep 17 00:00:00 2001 From: theoryxu Date: Wed, 27 Nov 2024 10:26:09 +0800 Subject: [PATCH 12/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- .../ranger/RangerAuthorizationPlugin.java | 3 +- .../authorization/ranger/RangerHelper.java | 52 +++++++++++-------- .../ranger/integration/test/RangerHiveIT.java | 20 +++---- 3 files changed, 42 insertions(+), 33 deletions(-) diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java index 79afae38a87..1b2c924d262 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java @@ -334,8 +334,7 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n ownerRoleName = RangerHelper.GRAVITINO_CATALOG_OWNER_ROLE; } rangerHelper.createRangerRoleIfNotExists(ownerRoleName, true); - rangerHelper.createRangerRoleIfNotExists( - RangerHelper.GRAVITINO_PLACEHOLDER_OWNER_ROLE, true); + rangerHelper.createRangerRoleIfNotExists(RangerHelper.GRAVITINO_OWNER_ROLE, true); try { if (preOwnerUserName != null || preOwnerGroupName != null) { GrantRevokeRoleRequest revokeRoleRequest = diff --git a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java index 8f249c35244..cb74ad02621 100644 --- a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java +++ b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java @@ -56,13 +56,14 @@ public class RangerHelper { private final RangerClient rangerClient; private final String rangerAdminName; private final String rangerServiceName; - public static final String GRAVITINO_METALAKE_OWNER_ROLE = "GRAVITINO_METALAKE_OWNER_ROLE"; - public static final String GRAVITINO_CATALOG_OWNER_ROLE = "GRAVITINO_CATALOG_OWNER_ROLE"; - - // marking owner policy items - public static final String GRAVITINO_PLACEHOLDER_OWNER_ROLE = "GRAVITINO_PLACEHOLDER_OWNER_ROLE"; public static final String GRAVITINO_ROLE_PREFIX = "GRAVITINO_"; + public static final String GRAVITINO_METALAKE_OWNER_ROLE = + GRAVITINO_ROLE_PREFIX + "METALAKE_OWNER_ROLE"; + public static final String GRAVITINO_CATALOG_OWNER_ROLE = + GRAVITINO_ROLE_PREFIX + "CATALOG_OWNER_ROLE"; + // marking owner policy items + public static final String GRAVITINO_OWNER_ROLE = GRAVITINO_ROLE_PREFIX + "OWNER_ROLE"; public RangerHelper( RangerClient rangerClient, @@ -258,19 +259,19 @@ public void removeAllGravitinoManagedPolicyItem(RangerPolicy policy) { try { policy.setPolicyItems( policy.getPolicyItems().stream() - .filter(i -> !isGravitinoManagedPolicyItemAccess(i)) + .filter(item -> !isGravitinoManagedPolicyItemAccess(item)) .collect(Collectors.toList())); policy.setDenyPolicyItems( policy.getDenyPolicyItems().stream() - .filter(i -> !isGravitinoManagedPolicyItemAccess(i)) + .filter(item -> !isGravitinoManagedPolicyItemAccess(item)) .collect(Collectors.toList())); policy.setRowFilterPolicyItems( policy.getRowFilterPolicyItems().stream() - .filter(i -> !isGravitinoManagedPolicyItemAccess(i)) + .filter(item -> !isGravitinoManagedPolicyItemAccess(item)) .collect(Collectors.toList())); policy.setDataMaskPolicyItems( policy.getDataMaskPolicyItems().stream() - .filter(i -> !isGravitinoManagedPolicyItemAccess(i)) + .filter(item -> !isGravitinoManagedPolicyItemAccess(item)) .collect(Collectors.toList())); rangerClient.updatePolicy(policy.getId(), policy); } catch (RangerServiceException e) { @@ -329,13 +330,18 @@ protected RangerRole createRangerRoleIfNotExists(String roleName, boolean isOwne Preconditions.checkArgument( roleName.equalsIgnoreCase(GRAVITINO_METALAKE_OWNER_ROLE) || roleName.equalsIgnoreCase(GRAVITINO_CATALOG_OWNER_ROLE) - || roleName.equalsIgnoreCase(GRAVITINO_PLACEHOLDER_OWNER_ROLE), - "The role name should be GRAVITINO_METALAKE_OWNER_ROLE or GRAVITINO_CATALOG_OWNER_ROLE or GRAVITINO_PLACEHOLDER_OWNER_ROLE"); + || roleName.equalsIgnoreCase(GRAVITINO_OWNER_ROLE), + String.format( + "The role name should be %s or %s or %s", + GRAVITINO_METALAKE_OWNER_ROLE, GRAVITINO_CATALOG_OWNER_ROLE, GRAVITINO_OWNER_ROLE)); } else { Preconditions.checkArgument( !roleName.equalsIgnoreCase(GRAVITINO_METALAKE_OWNER_ROLE) - && !roleName.equalsIgnoreCase(GRAVITINO_CATALOG_OWNER_ROLE), - "The role name should not be GRAVITINO_METALAKE_OWNER_ROLE or GRAVITINO_CATALOG_OWNER_ROLE"); + && !roleName.equalsIgnoreCase(GRAVITINO_CATALOG_OWNER_ROLE) + && !roleName.equalsIgnoreCase(GRAVITINO_OWNER_ROLE), + String.format( + "The role name should not be %s or %s or %s", + GRAVITINO_METALAKE_OWNER_ROLE, GRAVITINO_CATALOG_OWNER_ROLE, GRAVITINO_OWNER_ROLE)); } RangerRole rangerRole = null; @@ -426,7 +432,7 @@ protected void updatePolicyOwner(RangerPolicy policy, Owner preOwner, Owner newO policyItem.getGroups().add(newOwner.name()); } // mark the policy item is created by Gravitino - policyItem.getRoles().add(GRAVITINO_PLACEHOLDER_OWNER_ROLE); + addRoleToPolicyItemIfNoExists(policyItem, GRAVITINO_OWNER_ROLE); } policy.getPolicyItems().add(policyItem); }); @@ -462,7 +468,7 @@ protected RangerPolicy addOwnerToNewPolicy(RangerMetadataObject metadataObject, policyItem.getGroups().add(newOwner.name()); } // mark the policy item is created by Gravitino - policyItem.getRoles().add(GRAVITINO_PLACEHOLDER_OWNER_ROLE); + policyItem.getRoles().add(GRAVITINO_OWNER_ROLE); } policy.getPolicyItems().add(policyItem); }); @@ -506,9 +512,7 @@ protected void updatePolicyOwnerRole(RangerPolicy policy, String ownerRoleName) // Add or remove the owner role in the policy item matchPolicyItems.forEach( policyItem -> { - if (!policyItem.getRoles().contains(generateGravitinoRoleName(ownerRoleName))) { - policyItem.getRoles().add(generateGravitinoRoleName(ownerRoleName)); - } + addRoleToPolicyItemIfNoExists(policyItem, ownerRoleName); }); // If the policy item is not equal to owner's privileges, then update the policy @@ -532,10 +536,16 @@ protected void updatePolicyOwnerRole(RangerPolicy policy, String ownerRoleName) policyItem .getAccesses() .add(new RangerPolicy.RangerPolicyItemAccess(ownerPrivilege.getName())); - if (!policyItem.getRoles().contains(generateGravitinoRoleName(ownerRoleName))) { - policyItem.getRoles().add(generateGravitinoRoleName(ownerRoleName)); - } + addRoleToPolicyItemIfNoExists(policyItem, ownerRoleName); policy.getPolicyItems().add(policyItem); }); } + + private void addRoleToPolicyItemIfNoExists( + RangerPolicy.RangerPolicyItem policyItem, String roleName) { + String gravitinoRoleName = generateGravitinoRoleName(roleName); + if (!policyItem.getRoles().contains(gravitinoRoleName)) { + policyItem.getRoles().add(gravitinoRoleName); + } + } } diff --git a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java index ff87009e393..7c45ff9b086 100644 --- a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java +++ b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java @@ -388,16 +388,16 @@ public void testGravitinoCompatibleWithExistingPolicy() { Assertions.assertTrue( policyItems.stream() .anyMatch( - i -> - !i.getRoles() + item -> + !item.getRoles() .contains( rangerHelper.generateGravitinoRoleName(role.name())))); } Assertions.assertTrue( policyItems.stream() .anyMatch( - i -> - i.getRoles() + item -> + item.getRoles() .contains(rangerHelper.generateGravitinoRoleName(role.name())))); }); @@ -415,15 +415,15 @@ public void testGravitinoCompatibleWithExistingPolicy() { Assertions.assertFalse( policyItems.stream() .anyMatch( - i -> - i.getRoles() + item -> + item.getRoles() .contains( rangerHelper.generateGravitinoRoleName(role.name())))); Assertions.assertTrue( policyItems.stream() .anyMatch( - i -> - !i.getRoles() + item -> + !item.getRoles() .contains( rangerHelper.generateGravitinoRoleName(role.name())))); } else { @@ -1256,8 +1256,8 @@ public boolean hasGravitinoManagedPolicyItemAccess( List items, String roleName) { return items.stream() .anyMatch( - i -> - i.getRoles().stream() + item -> + item.getRoles().stream() .anyMatch(r -> r.equals(rangerHelper.generateGravitinoRoleName(roleName)))); } From a67359bf650b6a80dbec086a8f035e06c02b7bf8 Mon Sep 17 00:00:00 2001 From: theoryxu Date: Wed, 27 Nov 2024 10:48:29 +0800 Subject: [PATCH 13/13] [#5336]feat(auth-ranger): Remove MANAGED_BY_GRAVITINO limit and compatible for existing ranger policy --- docs/security/authorization-pushdown.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/security/authorization-pushdown.md b/docs/security/authorization-pushdown.md index dbcaa8d80d7..43c1096bd4d 100644 --- a/docs/security/authorization-pushdown.md +++ b/docs/security/authorization-pushdown.md @@ -28,6 +28,12 @@ In order to use the Ranger Hadoop SQL Plugin, you need to configure the followin Once you have used the correct configuration, you can perform authorization operations by calling Gravitino [authorization RESTful API](https://gravitino.apache.org/docs/latest/api/rest/grant-roles-to-a-user). +Gravitino will initially create three roles in Apache Ranger: + +- GRAVITINO_METALAKE_OWNER_ROLE: Includes users and user groups designated as metalake owners, corresponding to the owner's privileges in Ranger policies. +- GRAVITINO_CATALOG_OWNER_ROLE: Includes users and user groups designated as catalog owners, corresponding to the owner's privileges in Ranger policies. +- GRAVITINO_OWNER_ROLE: Used to label Ranger policy items related to schema and table owner privileges. It does not include any users or user groups. + #### Example of using the Ranger Hadoop SQL Plugin Suppose you have an Apache Hive service in your datacenter and have created a `hiveRepo` in Apache Ranger to manage its permissions.