Skip to content

Commit d6c2516

Browse files
mneethirajmneethiraj
authored
RANGER-5340: fix incorrect isFinal flag in RangerResourceACLs for GDS ACLs (#687)
1 parent 5a9816a commit d6c2516

File tree

9 files changed

+168
-256
lines changed

9 files changed

+168
-256
lines changed

‎agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerResourceACLs.java‎

Lines changed: 49 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -117,46 +117,73 @@ public void finalizeAcls() {
117117

118118
public void setUserAccessInfo(String userName, String accessType, Integer access, RangerPolicy policy) {
119119
Map<String, AccessResult> userAccessInfo = userACLs.computeIfAbsent(userName, k -> new HashMap<>());
120+
AccessResult existingResult = userAccessInfo.get(accessType);
120121

121-
AccessResult accessResult = userAccessInfo.get(accessType);
122-
123-
if (accessResult == null) {
124-
accessResult = new AccessResult(access, policy);
125-
126-
userAccessInfo.put(accessType, accessResult);
122+
if (existingResult == null) {
123+
userAccessInfo.put(accessType, new AccessResult(access, policy));
127124
} else if (!ACCESS_CONDITIONAL.equals(access)) {
128-
accessResult.setResult(access);
129-
accessResult.setPolicy(policy);
125+
existingResult.setResult(access);
126+
existingResult.setPolicy(policy);
130127
}
131128
}
132129

133130
public void setGroupAccessInfo(String groupName, String accessType, Integer access, RangerPolicy policy) {
134131
Map<String, AccessResult> groupAccessInfo = groupACLs.computeIfAbsent(groupName, k -> new HashMap<>());
132+
AccessResult existingResult = groupAccessInfo.get(accessType);
135133

136-
AccessResult accessResult = groupAccessInfo.get(accessType);
137-
138-
if (accessResult == null) {
139-
accessResult = new AccessResult(access, policy);
140-
141-
groupAccessInfo.put(accessType, accessResult);
134+
if (existingResult == null) {
135+
groupAccessInfo.put(accessType, new AccessResult(access, policy));
142136
} else if (!ACCESS_CONDITIONAL.equals(access)) {
143-
accessResult.setResult(access);
144-
accessResult.setPolicy(policy);
137+
existingResult.setResult(access);
138+
existingResult.setPolicy(policy);
145139
}
146140
}
147141

148142
public void setRoleAccessInfo(String roleName, String accessType, Integer access, RangerPolicy policy) {
149143
Map<String, AccessResult> roleAccessInfo = roleACLs.computeIfAbsent(roleName, k -> new HashMap<>());
144+
AccessResult existingResult = roleAccessInfo.get(accessType);
150145

151-
AccessResult accessResult = roleAccessInfo.get(accessType);
146+
if (existingResult == null) {
147+
roleAccessInfo.put(accessType, new AccessResult(access, policy));
148+
} else if (!ACCESS_CONDITIONAL.equals(access)) {
149+
existingResult.setResult(access);
150+
existingResult.setPolicy(policy);
151+
}
152+
}
152153

153-
if (accessResult == null) {
154-
accessResult = new AccessResult(access, policy);
154+
public void setUserAccessInfo(String userName, String accessType, AccessResult accessResult) {
155+
Map<String, AccessResult> userAccessInfo = userACLs.computeIfAbsent(userName, k -> new HashMap<>());
156+
AccessResult existingResult = userAccessInfo.get(accessType);
155157

158+
if (existingResult == null) {
159+
userAccessInfo.put(accessType, accessResult);
160+
} else if (!ACCESS_CONDITIONAL.equals(accessResult.getResult())) {
161+
existingResult.setResult(accessResult.getResult());
162+
existingResult.setPolicy(accessResult.getPolicy());
163+
}
164+
}
165+
166+
public void setGroupAccessInfo(String groupName, String accessType, AccessResult accessResult) {
167+
Map<String, AccessResult> groupAccessInfo = groupACLs.computeIfAbsent(groupName, k -> new HashMap<>());
168+
AccessResult existingResult = groupAccessInfo.get(accessType);
169+
170+
if (existingResult == null) {
171+
groupAccessInfo.put(accessType, accessResult);
172+
} else if (!ACCESS_CONDITIONAL.equals(accessResult.getResult())) {
173+
existingResult.setResult(accessResult.getResult());
174+
existingResult.setPolicy(accessResult.getPolicy());
175+
}
176+
}
177+
178+
public void setRoleAccessInfo(String roleName, String accessType, AccessResult accessResult) {
179+
Map<String, AccessResult> roleAccessInfo = roleACLs.computeIfAbsent(roleName, k -> new HashMap<>());
180+
AccessResult existingResult = roleAccessInfo.get(accessType);
181+
182+
if (existingResult == null) {
156183
roleAccessInfo.put(accessType, accessResult);
157-
} else if (!ACCESS_CONDITIONAL.equals(access)) {
158-
accessResult.setResult(access);
159-
accessResult.setPolicy(policy);
184+
} else if (!ACCESS_CONDITIONAL.equals(accessResult.getResult())) {
185+
existingResult.setResult(accessResult.getResult());
186+
existingResult.setPolicy(accessResult.getPolicy());
160187
}
161188
}
162189

‎agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerPolicyEvaluator.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,17 @@ void finalizeAcls(final boolean isDenyAllElse, final Set<String> allAccessTypeNa
606606
}
607607
}
608608

609+
@Override
610+
public String toString() {
611+
return "PolicyACLSummary{" +
612+
"usersAccessInfo=" + usersAccessInfo +
613+
", groupsAccessInfo=" + groupsAccessInfo +
614+
", rolesAccessInfo=" + rolesAccessInfo +
615+
", rowFilters=" + rowFilters +
616+
", dataMasks=" + dataMasks +
617+
'}';
618+
}
619+
609620
private void addAccess(String accessorName, AccessorType accessorType, String accessType, Integer access, int policyItemType) {
610621
final Map<String, Map<String, AccessResult>> accessorsAccessInfo;
611622

‎agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1447,13 +1447,13 @@ private static void overrideACLs(final RangerResourceACLs chainedResourceACLs, R
14471447

14481448
switch (userType) {
14491449
case USER:
1450-
baseResourceACLs.setUserAccessInfo(name, chainedAccessType, finalAccessResult.getResult(), finalAccessResult.getPolicy());
1450+
baseResourceACLs.setUserAccessInfo(name, chainedAccessType, finalAccessResult);
14511451
break;
14521452
case GROUP:
1453-
baseResourceACLs.setGroupAccessInfo(name, chainedAccessType, finalAccessResult.getResult(), finalAccessResult.getPolicy());
1453+
baseResourceACLs.setGroupAccessInfo(name, chainedAccessType, finalAccessResult);
14541454
break;
14551455
case ROLE:
1456-
baseResourceACLs.setRoleAccessInfo(name, chainedAccessType, finalAccessResult.getResult(), finalAccessResult.getPolicy());
1456+
baseResourceACLs.setRoleAccessInfo(name, chainedAccessType, finalAccessResult);
14571457
break;
14581458
default:
14591459
break;

‎agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestPolicyACLs.java‎

Lines changed: 5 additions & 161 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@
2525
import com.google.gson.JsonDeserializer;
2626
import com.google.gson.JsonElement;
2727
import com.google.gson.JsonParseException;
28-
import org.apache.commons.collections.MapUtils;
29-
import org.apache.commons.lang.StringUtils;
3028
import org.apache.ranger.authorization.hadoop.config.RangerPluginConfig;
3129
import org.apache.ranger.plugin.policyengine.RangerAccessRequest.ResourceMatchingScope;
3230
import org.apache.ranger.plugin.policyengine.RangerResourceACLs.DataMaskResult;
@@ -44,7 +42,6 @@
4442
import java.util.List;
4543
import java.util.Map;
4644
import java.util.Objects;
47-
import java.util.Set;
4845

4946
import static org.junit.Assert.assertEquals;
5047
import static org.junit.Assert.assertTrue;
@@ -127,164 +124,11 @@ private void runTests(InputStreamReader reader, String testName) {
127124

128125
RangerResourceACLs acls = policyEngine.getResourceACLs(request);
129126

130-
boolean userACLsMatched = true;
131-
boolean groupACLsMatched = true;
132-
boolean roleACLsMatched = true;
133-
boolean rowFiltersMatched = true;
134-
boolean dataMaskingMatched = true;
135-
136-
if (MapUtils.isNotEmpty(acls.getUserACLs()) && MapUtils.isNotEmpty(oneTest.userPermissions)) {
137-
assertEquals("getResourceACLs() failed! " + testCase.name + ":" + oneTest.name + " - userACLsMatched", oneTest.userPermissions.size(), acls.getUserACLs().size());
138-
139-
for (Map.Entry<String, Map<String, RangerResourceACLs.AccessResult>> entry :
140-
acls.getUserACLs().entrySet()) {
141-
String userName = entry.getKey();
142-
Map<String, RangerResourceACLs.AccessResult> expected = oneTest.userPermissions.get(userName);
143-
if (MapUtils.isNotEmpty(entry.getValue()) && MapUtils.isNotEmpty(expected)) {
144-
// Compare
145-
for (Map.Entry<String, RangerResourceACLs.AccessResult> privilege : entry.getValue().entrySet()) {
146-
if (StringUtils.equals(RangerPolicyEngine.ADMIN_ACCESS, privilege.getKey())) {
147-
continue;
148-
}
149-
RangerResourceACLs.AccessResult expectedResult = expected.get(privilege.getKey());
150-
if (expectedResult == null) {
151-
userACLsMatched = false;
152-
break;
153-
} else if (!expectedResult.equals(privilege.getValue())) {
154-
userACLsMatched = false;
155-
break;
156-
}
157-
}
158-
} else if (!(MapUtils.isEmpty(entry.getValue()) && MapUtils.isEmpty(expected))) {
159-
Set<String> privileges = entry.getValue().keySet();
160-
161-
userACLsMatched = privileges.size() == 1 && privileges.contains(RangerPolicyEngine.ADMIN_ACCESS);
162-
163-
break;
164-
}
165-
166-
if (!userACLsMatched) {
167-
break;
168-
}
169-
}
170-
} else if (!(MapUtils.isEmpty(acls.getUserACLs()) && MapUtils.isEmpty(oneTest.userPermissions))) {
171-
userACLsMatched = false;
172-
}
173-
174-
if (acls.getDataMasks().isEmpty()) {
175-
dataMaskingMatched = (oneTest.dataMasks == null || oneTest.dataMasks.isEmpty());
176-
} else if (acls.getDataMasks().size() != (oneTest.dataMasks == null ? 0 : oneTest.dataMasks.size())) {
177-
dataMaskingMatched = false;
178-
} else {
179-
for (int i = 0; i < acls.getDataMasks().size(); i++) {
180-
DataMaskResult found = acls.getDataMasks().get(i);
181-
DataMaskResult expected = oneTest.dataMasks.get(i);
182-
183-
dataMaskingMatched = found.equals(expected);
184-
185-
if (!dataMaskingMatched) {
186-
break;
187-
}
188-
}
189-
}
190-
191-
if (acls.getRowFilters().isEmpty()) {
192-
rowFiltersMatched = (oneTest.rowFilters == null || oneTest.rowFilters.isEmpty());
193-
} else if (acls.getRowFilters().size() != (oneTest.rowFilters == null ? 0 : oneTest.rowFilters.size())) {
194-
rowFiltersMatched = false;
195-
} else {
196-
for (int i = 0; i < acls.getRowFilters().size(); i++) {
197-
RowFilterResult found = acls.getRowFilters().get(i);
198-
RowFilterResult expected = oneTest.rowFilters.get(i);
199-
200-
rowFiltersMatched = found.equals(expected);
201-
202-
if (!rowFiltersMatched) {
203-
break;
204-
}
205-
}
206-
}
207-
208-
if (MapUtils.isNotEmpty(acls.getGroupACLs()) && MapUtils.isNotEmpty(oneTest.groupPermissions)) {
209-
assertEquals("getResourceACLs() failed! " + testCase.name + ":" + oneTest.name + " - groupACLsMatched", oneTest.groupPermissions.size(), acls.getGroupACLs().size());
210-
211-
for (Map.Entry<String, Map<String, RangerResourceACLs.AccessResult>> entry :
212-
acls.getGroupACLs().entrySet()) {
213-
String groupName = entry.getKey();
214-
Map<String, RangerResourceACLs.AccessResult> expected = oneTest.groupPermissions.get(groupName);
215-
if (MapUtils.isNotEmpty(entry.getValue()) && MapUtils.isNotEmpty(expected)) {
216-
// Compare
217-
for (Map.Entry<String, RangerResourceACLs.AccessResult> privilege : entry.getValue().entrySet()) {
218-
if (StringUtils.equals(RangerPolicyEngine.ADMIN_ACCESS, privilege.getKey())) {
219-
continue;
220-
}
221-
RangerResourceACLs.AccessResult expectedResult = expected.get(privilege.getKey());
222-
if (expectedResult == null) {
223-
groupACLsMatched = false;
224-
break;
225-
} else if (!expectedResult.equals(privilege.getValue())) {
226-
groupACLsMatched = false;
227-
break;
228-
}
229-
}
230-
} else if (!(MapUtils.isEmpty(entry.getValue()) && MapUtils.isEmpty(expected))) {
231-
Set<String> privileges = entry.getValue().keySet();
232-
233-
groupACLsMatched = privileges.size() == 1 && privileges.contains(RangerPolicyEngine.ADMIN_ACCESS);
234-
235-
break;
236-
}
237-
238-
if (!groupACLsMatched) {
239-
break;
240-
}
241-
}
242-
} else if (!(MapUtils.isEmpty(acls.getGroupACLs()) && MapUtils.isEmpty(oneTest.groupPermissions))) {
243-
groupACLsMatched = false;
244-
}
245-
246-
if (MapUtils.isNotEmpty(acls.getRoleACLs()) && MapUtils.isNotEmpty(oneTest.rolePermissions)) {
247-
assertEquals("getResourceACLs() failed! " + testCase.name + ":" + oneTest.name + " - roleACLsMatched", oneTest.rolePermissions.size(), acls.getRoleACLs().size());
248-
249-
for (Map.Entry<String, Map<String, RangerResourceACLs.AccessResult>> entry :
250-
acls.getRoleACLs().entrySet()) {
251-
String roleName = entry.getKey();
252-
Map<String, RangerResourceACLs.AccessResult> expected = oneTest.rolePermissions.get(roleName);
253-
if (MapUtils.isNotEmpty(entry.getValue()) && MapUtils.isNotEmpty(expected)) {
254-
// Compare
255-
for (Map.Entry<String, RangerResourceACLs.AccessResult> privilege : entry.getValue().entrySet()) {
256-
if (StringUtils.equals(RangerPolicyEngine.ADMIN_ACCESS, privilege.getKey())) {
257-
continue;
258-
}
259-
RangerResourceACLs.AccessResult expectedResult = expected.get(privilege.getKey());
260-
if (expectedResult == null) {
261-
roleACLsMatched = false;
262-
break;
263-
} else if (!expectedResult.equals(privilege.getValue())) {
264-
roleACLsMatched = false;
265-
break;
266-
}
267-
}
268-
} else if (!(MapUtils.isEmpty(entry.getValue()) && MapUtils.isEmpty(expected))) {
269-
Set<String> privileges = entry.getValue().keySet();
270-
271-
roleACLsMatched = privileges.size() == 1 && privileges.contains(RangerPolicyEngine.ADMIN_ACCESS);
272-
273-
break;
274-
}
275-
if (!roleACLsMatched) {
276-
break;
277-
}
278-
}
279-
} else if (!(MapUtils.isEmpty(acls.getRoleACLs()) && MapUtils.isEmpty(oneTest.rolePermissions))) {
280-
roleACLsMatched = false;
281-
}
282-
283-
assertTrue("getResourceACLs() failed! " + testCase.name + ":" + oneTest.name + " - userACLsMatched", userACLsMatched);
284-
assertTrue("getResourceACLs() failed! " + testCase.name + ":" + oneTest.name + " - groupACLsMatched", groupACLsMatched);
285-
assertTrue("getResourceACLs() failed! " + testCase.name + ":" + oneTest.name + " - roleACLsMatched", roleACLsMatched);
286-
assertTrue("getResourceACLs() failed! " + testCase.name + ":" + oneTest.name + " - rowFiltersMatched", rowFiltersMatched);
287-
assertTrue("getResourceACLs() failed! " + testCase.name + ":" + oneTest.name + " - dataMaskingMatched", dataMaskingMatched);
127+
assertEquals(oneTest.name + ": userACLs mismatch", oneTest.userPermissions, acls.getUserACLs());
128+
assertEquals(oneTest.name + ": groupACLs mismatch", oneTest.groupPermissions, acls.getGroupACLs());
129+
assertEquals(oneTest.name + ": roleACLs mismatch", oneTest.rolePermissions, acls.getRoleACLs());
130+
assertEquals(oneTest.name + ": rowFilters mismatch", oneTest.rowFilters, acls.getRowFilters());
131+
assertEquals(oneTest.name + ": dataMasks mismatch", oneTest.dataMasks, acls.getDataMasks());
288132
});
289133
}
290134
}

0 commit comments

Comments
 (0)