Skip to content

Commit

Permalink
handle assertion conditions in putPolicy api (#2300)
Browse files Browse the repository at this point in the history 
Signed-off-by: mshneorson <[email protected]>
Co-authored-by: mshneorson <[email protected]>
  • Loading branch information
@mendi160
mendi160 and mshneorson authored Sep 6, 2023
1 parent 175b268 commit aac1d20
Show file tree
Hide file tree
Showing 4 changed files with 403 additions and 2 deletions.
105 changes: 103 additions & 2 deletions servers/zms/src/main/java/com/yahoo/athenz/zms/DBService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,12 @@ boolean processPolicy(ObjectStoreConnection con, Policy originalPolicy, String d
if (!con.insertAssertion(domainName, policyName, policy.getVersion(), assertion)) {
return false;
}
// insert the new assertion conditions if any
if (assertion.getConditions() != null) {
if (!con.insertAssertionConditions(assertion.getId(), assertion.getConditions())) {
return false;
}
}
}
auditLogAssertions(auditDetails, "added-assertions", newAssertions);
}
Expand All @@ -506,7 +512,15 @@ boolean processPolicy(ObjectStoreConnection con, Policy originalPolicy, String d
}
List<Assertion> addAssertions = new ArrayList<>();
List<Assertion> delAssertions = new ArrayList<>();
policyAssertionChanges(newAssertions, curAssertions, addAssertions, delAssertions);

// those lists are used to check for assertion condition changes for those assertions that are unchanged
List<Assertion> newMatchedAssertions = new ArrayList<>();
List<Assertion> oldMatchedAssertions = new ArrayList<>(curAssertions);

policyAssertionChanges(newAssertions, curAssertions, addAssertions, delAssertions, newMatchedAssertions);

// delAssertion are the ones that are in curr assertions but not in new assertions, means they are not matched
oldMatchedAssertions.removeAll(delAssertions);

if (!ignoreDeletes) {
for (Assertion assertion : delAssertions) {
Expand All @@ -521,8 +535,32 @@ boolean processPolicy(ObjectStoreConnection con, Policy originalPolicy, String d
if (!con.insertAssertion(domainName, policyName, policy.getVersion(), assertion)) {
return false;
}
if (assertion.getConditions() != null) {
if (!con.insertAssertionConditions(assertion.getId(), assertion.getConditions())) {
return false;
}
}
}
auditLogAssertions(auditDetails, "added-assertions", addAssertions);

Map<Long, List<AssertionCondition>> addConditions = new HashMap<>();
Map<Long, List<AssertionCondition>> delConditions = new HashMap<>();
policyAssertionConditionsChanges(oldMatchedAssertions, newMatchedAssertions, addConditions, delConditions);

for (Map.Entry<Long, List<AssertionCondition>> entry : delConditions.entrySet()) {
Long assertionId = entry.getKey();
if (!con.deleteAssertionConditions(assertionId)) {
return false;
}
}

for (Map.Entry<Long, List<AssertionCondition>> entry : addConditions.entrySet()) {
Long assertionId = entry.getKey();
List<AssertionCondition> conditionsList = entry.getValue();
if (!con.insertAssertionConditions(assertionId, new AssertionConditions().setConditionsList(conditionsList))) {
return false;
}
}
}

if (!processPolicyTags(policy, policyName, domainName, originalPolicy, con)) {
Expand Down Expand Up @@ -575,6 +613,9 @@ boolean removeMatchedAssertion(Assertion assertion, List<Assertion> assertions,
continue;
}

// we add the id to the assertion so that we can use it later to check for assertion conditions changes
assertion.setId(checkAssertion.getId());

itr.remove();
matchedAssertions.add(checkAssertion);
return true;
Expand All @@ -583,8 +624,66 @@ boolean removeMatchedAssertion(Assertion assertion, List<Assertion> assertions,
return false;
}

void mapAssertionsToConditions(List<Assertion> assertions, Map<Long, List<AssertionCondition>> assertionConditionsMap) {
if (assertions == null) {
return;
}
for (Assertion assertion : assertions) {
if (assertion.getConditions() != null) {
assertionConditionsMap.put(assertion.getId(), new ArrayList<>(assertion.getConditions().getConditionsList()));
}
}
}

boolean assertionConditionEqualsIgnoreConditionId(AssertionCondition c1, AssertionCondition c2) {
return Objects.equals(c1.conditionsMap, c2.conditionsMap);
}

boolean isAssertionConditionsHasChanged(List<AssertionCondition> list1, List<AssertionCondition> list2) {
if (list1 == null || list1.isEmpty()) {
return list2 != null && !list2.isEmpty();
}
if (list2 == null) {
return true;
}
if (list1.size() != list2.size()) {
return true;
}
for (AssertionCondition ac1: list1) {
if (list2.stream().noneMatch(ac2 -> assertionConditionEqualsIgnoreConditionId(ac1, ac2))) {
return true;
}
}
return false;
}

void policyAssertionConditionsChanges(List<Assertion> oldAssertions, List<Assertion> currentAssertions, Map<Long, List<AssertionCondition>> addConditions,
Map<Long, List<AssertionCondition>> delConditions) {

Set<Long> keysToRemove = new HashSet<>();

mapAssertionsToConditions(oldAssertions, delConditions);
mapAssertionsToConditions(currentAssertions, addConditions);

// Iterate over old assertion conditions and check if they are changed
for (Map.Entry<Long, List<AssertionCondition>> entry : delConditions.entrySet()) {
Long assertionId = entry.getKey();
List<AssertionCondition> delAcSet = entry.getValue();
// if new assertion conditions has the same conditions then we need remove it from the maps, since no action is needed
if (!isAssertionConditionsHasChanged(delAcSet, addConditions.getOrDefault(assertionId, new ArrayList<>()))) {
keysToRemove.add(assertionId);
}

}

for (Long assertionId : keysToRemove) {
delConditions.remove(assertionId);
addConditions.remove(assertionId);
}
}

void policyAssertionChanges(List<Assertion> newAssertions, List<Assertion> curAssertions,
List<Assertion> addAssertions, List<Assertion> delAssertions) {
List<Assertion> addAssertions, List<Assertion> delAssertions, List<Assertion> newMatchedAssertions) {

// let's iterate through the new list and the ones that are
// not in the current list should be added to the add list
Expand All @@ -594,6 +693,8 @@ void policyAssertionChanges(List<Assertion> newAssertions, List<Assertion> curAs
for (Assertion assertion : newAssertions) {
if (!removeMatchedAssertion(assertion, curAssertions, matchedAssertions)) {
addAssertions.add(assertion);
} else {
newMatchedAssertions.add(assertion);
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5405,6 +5405,17 @@ void validatePolicyAssertions(List<Assertion> assertions, final String roleDomai
}
}

void validatePolicyAssertionConditions(List<Assertion> assertions, String caller) {
if (assertions == null) {
return;
}
for (Assertion a : assertions) {
if (a.getConditions() != null) {
validateAssertionConditions(a.getConditions(), caller);
}
}
}

void validatePolicyAssertion(Assertion assertion, final String roleDomainName, Set<String> roleNamesSet, final String caller) {

// extract the domain name from the resource
Expand Down Expand Up @@ -5537,6 +5548,8 @@ public Response putPolicy(ResourceContext ctx, String domainName, String policyN

validatePolicyAssertions(policy.getAssertions(), domainName, caller);

validatePolicyAssertionConditions(policy.getAssertions(), caller);

Policy dbPolicy = dbService.executePutPolicy(ctx, domainName, policyName, policy, auditRef, caller, returnObj);

return ZMSUtils.returnPutResponse(returnObj, dbPolicy);
Expand Down
119 changes: 119 additions & 0 deletions servers/zms/src/test/java/com/yahoo/athenz/zms/DBServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12332,6 +12332,72 @@ public void testProcessNoTags() {
assertTrue(zms.dbService.processUpdateTags(Collections.emptyMap(), Collections.emptyMap(), null, null, null));
}

private AssertionCondition createAssertionConditionObject(Integer conditionId, String key, String value) {
Map<String, AssertionConditionData> map = new HashMap<>();
AssertionConditionData cd = new AssertionConditionData().setOperator(AssertionConditionOperator.EQUALS).setValue(value);
map.put(key, cd);
return new AssertionCondition().setId(conditionId).setConditionsMap(map);
}

@Test
public void testMapAssertionsToConditions() {
Map<Long, List<AssertionCondition>> assertionConditionsMap = new HashMap<>();
zms.dbService.mapAssertionsToConditions(null, assertionConditionsMap);
assertTrue(assertionConditionsMap.isEmpty());
List<Assertion> assertions = new ArrayList<>();
zms.dbService.mapAssertionsToConditions(assertions, assertionConditionsMap);
assertTrue(assertionConditionsMap.isEmpty());

Assertion a1 = new Assertion().setId(1L);
assertions.add(a1);

AssertionCondition ac1 = createAssertionConditionObject(1, "test1", "test1");
AssertionCondition ac2 = createAssertionConditionObject(2, "test2", "test2");
Assertion a2 = new Assertion().setId(2L).setConditions(
new AssertionConditions()
.setConditionsList(List.of(ac1, ac2))
);
assertions.add(a2);

AssertionCondition ac3 = createAssertionConditionObject(1, "test3", "test3");
Assertion a3 = new Assertion().setId(3L).setConditions(
new AssertionConditions()
.setConditionsList(List.of(ac3))
);
assertions.add(a3);

zms.dbService.mapAssertionsToConditions(assertions, assertionConditionsMap);
assertEquals(assertionConditionsMap.size(), 2);
assertEquals(assertionConditionsMap.get(2L), List.of(ac1, ac2));
assertEquals(assertionConditionsMap.get(3L), List.of(ac3));

}

@Test
public void testIsAssertionConditionsHasChanged() {
List<AssertionCondition> list1 = null;
List<AssertionCondition> list2 = null;
assertFalse(zms.dbService.isAssertionConditionsHasChanged(list1, list2));

list1 = new ArrayList<>();
list1.add(createAssertionConditionObject(1, "test1", "test1"));
assertTrue(zms.dbService.isAssertionConditionsHasChanged(list1, list2));

list2 = new ArrayList<>();
list2.add(createAssertionConditionObject(null, "test1", "test1"));
assertFalse(zms.dbService.isAssertionConditionsHasChanged(list1, list2));

list1.add(createAssertionConditionObject(2, "test2", "test2"));
list2.add(createAssertionConditionObject(null, "test2", "test2"));
assertFalse(zms.dbService.isAssertionConditionsHasChanged(list1, list2));

list1.add(createAssertionConditionObject(3, "test3", "test3"));
assertTrue(zms.dbService.isAssertionConditionsHasChanged(list1, list2));

list2.add(createAssertionConditionObject(null, "test4", "test4"));
assertTrue(zms.dbService.isAssertionConditionsHasChanged(list1, list2));
}

@Test
public void testProcessPolicyConReturnFalse() {
ObjectStoreConnection conn = Mockito.mock(ObjectStoreConnection.class);
Expand All @@ -12350,6 +12416,59 @@ public void testProcessPolicyConReturnFalse() {

assertFalse(success);

}

@Test
public void testProcessPolicyAssertionConditionsConReturnFalse() {
ObjectStoreConnection conn = Mockito.mock(ObjectStoreConnection.class);

Policy policy = new Policy().setName("newPolicy");

Assertion policyAssertion = new Assertion()
.setId(1L)
.setAction("testAction")
.setResource("sys.auth:testResource")
.setRole("sys.auth:role.testRole")
.setEffect(AssertionEffect.ALLOW)
.setConditions(
new AssertionConditions()
.setConditionsList(List.of(createAssertionConditionObject(1, "test1", "test1")))
);

policy.setAssertions(new LinkedList<>(Collections.singletonList(policyAssertion)));
Mockito.when(conn.insertAssertionConditions(1L, policyAssertion.getConditions())).thenReturn(false).thenReturn(true);
Mockito.when(conn.insertAssertion("sys.auth", "newPolicy", null, policyAssertion)).thenReturn(true).thenReturn(true);
Mockito.when(conn.insertPolicy("sys.auth", policy)).thenReturn(true).thenReturn(true);

StringBuilder auditDetails = new StringBuilder("testAudit");
boolean success = zms.dbService.processPolicy(conn, null, "sys.auth", "newPolicy",
policy, false, auditDetails);
assertFalse(success);

Policy oldPolicy = new Policy().setName("oldPolicy");
Assertion oldPolicyAssertion = new Assertion()
.setId(1L)
.setAction("testAction")
.setResource("sys.auth:testResource")
.setRole("sys.auth:role.testRole")
.setEffect(AssertionEffect.ALLOW);
oldPolicy.setAssertions(new LinkedList<>(Collections.singletonList(oldPolicyAssertion)));
Mockito.when(conn.updatePolicy("sys.auth", policy)).thenReturn(true).thenReturn(true);
Mockito.when(conn.insertAssertion("sys.auth", "newPolicy", null, policyAssertion)).thenReturn(true).thenReturn(true);
Mockito.when(conn.insertAssertionConditions(1L, policyAssertion.getConditions())).thenReturn(false).thenReturn(true);
success = zms.dbService.processPolicy(conn, oldPolicy, "sys.auth", "newPolicy",
policy, false, auditDetails);
assertFalse(success);

oldPolicy.setAssertions(policy.getAssertions());
policy.setAssertions(new LinkedList<>(Collections.singletonList(oldPolicyAssertion)));
Mockito.when(conn.updatePolicy("sys.auth", policy)).thenReturn(true).thenReturn(true);
Mockito.when(conn.deleteAssertionConditions(1L)).thenReturn(false).thenReturn(true);
success = zms.dbService.processPolicy(conn, oldPolicy, "sys.auth", "newPolicy",
policy, false, auditDetails);
assertFalse(success);


}
@Test
public void testProcessPolicyWithTagsInsert() {
Expand Down
Loading

0 comments on commit aac1d20

Please sign in to comment.