This is an automated email from the ASF dual-hosted git repository.
pradeepagrawal8184 pushed a commit to branch ranger-2.10
in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/ranger-2.10 by this push:
new df1001251 RANGER-5793: Align delegated-admin checks for policy update
and delete when access grants are unchanged Backport of #1223 to ranger-2.10
(#1254)
df1001251 is described below
commit df100125109f981737e5eddeb0174741b66b5382
Author: Vyom Mani Tiwari <[email protected]>
AuthorDate: Fri Sep 25 09:38:49 2026 +0530
RANGER-5793: Align delegated-admin checks for policy update and delete when
access grants are unchanged Backport of #1223 to ranger-2.10 (#1254)
---
.../apache/ranger/biz/RangerPolicyAdminImpl.java | 87 +++++++++++++++++-----
.../org/apache/ranger/rest/TestServiceREST.java | 34 +++++++++
2 files changed, 101 insertions(+), 20 deletions(-)
diff --git
a/security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminImpl.java
b/security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminImpl.java
index 55a1a01cc..7997b0918 100644
---
a/security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminImpl.java
+++
b/security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyAdminImpl.java
@@ -56,10 +56,12 @@
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
public class RangerPolicyAdminImpl implements RangerPolicyAdmin {
@@ -222,7 +224,7 @@ boolean isDelegatedAdminAccessAllowed(RangerPolicy policy,
String user, Set<Stri
String oldResourceSignature =
getResourceSignature(oldPolicy);
String newResourceSignature =
getResourceSignature(policy);
- if (StringUtils.equals(oldResourceSignature,
newResourceSignature)) {
+ if (StringUtils.equals(oldResourceSignature,
newResourceSignature) && !isPolicyScopeChanged(oldPolicy, policy)) {
Set<String> modifiedAccessTypes =
getAllModifiedAccessTypes(oldPolicy, policy, getServiceDef());
ret =
isDelegatedAdminAccessAllowedForPolicy(matchedRepository, policy, user,
userGroups, roles, modifiedAccessTypes, false, evalContext);
} else {
@@ -858,7 +860,9 @@ private Set<String> getAllModifiedAccessTypes(RangerPolicy
oldPolicy, RangerPoli
ret.addAll(getAccessTypesDiff(newRoleAccesses, oldRoleAccesses));
if (ret.isEmpty()) {
- ret.add(RangerPolicyEngine.ADMIN_ACCESS);
+ // Empty access-type delta (delete, or updates that do not
add/remove grants) must be authorized against
+ // every access-type in the stored policy, not a generic _admin
token.
+ ret.addAll(getAllAccessTypes(oldPolicy, serviceDef));
}
return ret;
}
@@ -894,31 +898,18 @@ private void collectAccessTypes(Map<String,
Collection<String>> expandedAccesses
accessTypes.addAll(expandedAccesses.get(access.getType()));
}
+ // each principal must get its own copy: a shared instance would
let a later addAll() for one principal
+ // leak access-types into every other principal of this item and
make the diff miss real changes
for (String user : item.getUsers()) {
- Set<String> oldAccesses = userAccesses.get(user);
- if (oldAccesses != null) {
- oldAccesses.addAll(accessTypes);
- } else {
- userAccesses.put(user, accessTypes);
- }
+ userAccesses.computeIfAbsent(user, k -> new
HashSet<>()).addAll(accessTypes);
}
for (String group : item.getGroups()) {
- Set<String> oldAccesses = groupAccesses.get(group);
- if (oldAccesses != null) {
- oldAccesses.addAll(accessTypes);
- } else {
- groupAccesses.put(group, accessTypes);
- }
+ groupAccesses.computeIfAbsent(group, k -> new
HashSet<>()).addAll(accessTypes);
}
for (String role : item.getRoles()) {
- Set<String> oldAccesses = roleAccesses.get(role);
- if (oldAccesses != null) {
- oldAccesses.addAll(accessTypes);
- } else {
- roleAccesses.put(role, accessTypes);
- }
+ roleAccesses.computeIfAbsent(role, k -> new
HashSet<>()).addAll(accessTypes);
}
}
}
@@ -947,6 +938,62 @@ private Set<String> getAccessTypesDiff(Map<String,
Set<String>> newAccessesMap,
return ret;
}
+ private boolean isPolicyScopeChanged(RangerPolicy oldPolicy, RangerPolicy
policy) {
+ boolean ret = !Objects.equals(oldPolicy.getIsEnabled() == null ||
oldPolicy.getIsEnabled(), policy.getIsEnabled() == null ||
policy.getIsEnabled())
+ || !Objects.equals(oldPolicy.getPolicyPriority() == null ?
RangerPolicy.POLICY_PRIORITY_NORMAL : oldPolicy.getPolicyPriority(),
policy.getPolicyPriority() == null ? RangerPolicy.POLICY_PRIORITY_NORMAL :
policy.getPolicyPriority())
+ ||
!Objects.equals(Boolean.TRUE.equals(oldPolicy.getIsDenyAllElse()),
Boolean.TRUE.equals(policy.getIsDenyAllElse()))
+ || !Objects.equals(nullToEmpty(oldPolicy.getConditions()),
nullToEmpty(policy.getConditions()))
+ ||
!Objects.equals(toStrings(oldPolicy.getValiditySchedules()),
toStrings(policy.getValiditySchedules()))
+ || !Objects.equals(getDelegateAdminPrincipals(oldPolicy),
getDelegateAdminPrincipals(policy));
+ if (ret) {
+ LOG.debug("isPolicyScopeChanged(policy-id:[{}]):
isEnabled/policyPriority/isDenyAllElse/conditions/validitySchedules/delegateAdmin
changed", policy.getId());
+ }
+ return ret;
+ }
+
+ private static <T> List<T> nullToEmpty(List<T> list) {
+ return list == null ? Collections.emptyList() : list;
+ }
+
+ private static List<String> toStrings(List<?> list) {
+ List<String> ret = new ArrayList<>();
+ if (list != null) {
+ for (Object obj : list) {
+ ret.add(String.valueOf(obj));
+ }
+ }
+ return ret;
+ }
+
+ private static Set<String> getDelegateAdminPrincipals(RangerPolicy policy)
{
+ Set<String> ret = new HashSet<>();
+ collectDelegateAdminPrincipals(policy.getPolicyItems(), ret);
+ collectDelegateAdminPrincipals(policy.getDenyPolicyItems(), ret);
+ collectDelegateAdminPrincipals(policy.getAllowExceptions(), ret);
+ collectDelegateAdminPrincipals(policy.getDenyExceptions(), ret);
+ collectDelegateAdminPrincipals(policy.getDataMaskPolicyItems(), ret);
+ collectDelegateAdminPrincipals(policy.getRowFilterPolicyItems(), ret);
+ return ret;
+ }
+
+ private static void collectDelegateAdminPrincipals(List<? extends
RangerPolicy.RangerPolicyItem> policyItems, Set<String> ret) {
+ if (policyItems != null) {
+ for (RangerPolicy.RangerPolicyItem item : policyItems) {
+ if (Boolean.TRUE.equals(item.getDelegateAdmin())) {
+ for (String user : nullToEmpty(item.getUsers())) {
+ ret.add("user:" + user);
+ }
+ for (String group : nullToEmpty(item.getGroups())) {
+ ret.add("group:" + group);
+ }
+ for (String role : nullToEmpty(item.getRoles())) {
+ ret.add("role:" + role);
+ }
+ }
+ }
+ }
+ }
+
private String getResourceSignature(final RangerPolicy policy) {
return
RangerPolicyResourceSignature.toSignatureString(policy.getResources(),
policy.getAdditionalResources());
}
diff --git
a/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
b/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
index f70fe27dd..026d10c57 100644
--- a/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
+++ b/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
@@ -1032,6 +1032,40 @@ public void test18deletePolicyFalse() throws Exception {
Mockito.verify(validatorFactory).getPolicyValidator(svcStore);
}
+ @Test
+ public void
testDeletePolicy_delegatedAdminWithoutModifyAccessIsForbidden() throws
Exception {
+ // DELETE /service/plugins/policies/{id} by a non-admin
delegated-admin whose grants do not
+ // cover the policy's access-types. deletePolicy() must route
the stored policy through the modify check and
+ // must not delete when that check fails.
+ RangerPolicy rangerPolicy = rangerPolicy();
+ ServiceREST spy = Mockito.spy(serviceREST);
+
+
Mockito.when(validatorFactory.getPolicyValidator(svcStore)).thenReturn(policyValidator);
+ Mockito.when(svcStore.getPolicy(Id)).thenReturn(rangerPolicy);
+
Mockito.when(bizUtil.getCurrentUserLoginId()).thenReturn("alice");
+ Mockito.when(bizUtil.isAdmin()).thenReturn(false);
+ Mockito.when(bizUtil.isKeyAdmin()).thenReturn(false);
+ Mockito.when(svcStore.isServiceAdminUser(Mockito.anyString(),
Mockito.anyString())).thenReturn(false);
+
Mockito.when(userMgr.getGroupsForUser(Mockito.anyString())).thenReturn(new
HashSet<String>());
+
+ RangerPolicyAdmin policyAdmin =
Mockito.mock(RangerPolicyAdmin.class);
+
Mockito.doReturn(policyAdmin).when(spy).getPolicyAdminForDelegatedAdmin(Mockito.anyString());
+
Mockito.when(policyAdmin.getRolesFromUserAndGroups(Mockito.anyString(),
Mockito.anySet())).thenReturn(new HashSet<String>());
+
Mockito.when(policyAdmin.isDelegatedAdminAccessAllowedForModify(Mockito.same(rangerPolicy),
Mockito.eq("alice"), Mockito.anySet(), Mockito.anySet(),
Mockito.anyMap())).thenReturn(false);
+
Mockito.when(restErrorUtil.createRESTException(Mockito.eq(HttpServletResponse.SC_FORBIDDEN),
Mockito.anyString(), Mockito.eq(true)))
+ .thenReturn(new
WebApplicationException(HttpServletResponse.SC_FORBIDDEN));
+
+ try {
+ spy.deletePolicy(Id);
+ Assert.fail("deletePolicy() must throw when the modify
check fails");
+ } catch (WebApplicationException expected) {
+ // expected
+ }
+
+
Mockito.verify(policyAdmin).isDelegatedAdminAccessAllowedForModify(Mockito.same(rangerPolicy),
Mockito.eq("alice"), Mockito.anySet(), Mockito.anySet(), Mockito.anyMap());
+ Mockito.verify(svcStore,
Mockito.never()).deletePolicy(Mockito.any(RangerPolicy.class));
+ }
+
@Test
public void test19getPolicyFalse() throws Exception {
RangerPolicy rangerPolicy = rangerPolicy();
