diff --git a/servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java b/servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java index 55fe7d50739..cf6c5ba01f3 100644 --- a/servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java +++ b/servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java @@ -5845,6 +5845,16 @@ public void deleteAssertion(ResourceContext ctx, String domainName, String polic verifyAuthorizedServiceOperation(((RsrcCtxWrapper) ctx).principal().getAuthorizedService(), caller); + // extract our policy object to get its attributes + + AthenzDomain domain = getAthenzDomain(domainName, false); + Policy policy = getPolicyFromDomain(policyName, domain); + + if (policy == null) { + throw ZMSUtils.notFoundError("Invalid policy name specified", caller); + } + + ResourceOwnership.verifyPolicyAssertionsDeleteResourceOwnership(policy, resourceOwner, caller); dbService.executeDeleteAssertion(ctx, domainName, policyName, null, assertionId, auditRef, caller); } @@ -10115,6 +10125,18 @@ Group getGroupFromDomain(final String groupName, AthenzDomain domain) { return null; } + Policy getPolicyFromDomain(final String policyName, AthenzDomain domain) { + if (domain != null && domain.getPolicies() != null) { + final String resourcePolicyName = ResourceUtils.policyResourceName(domain.getName(), policyName); + for (Policy policy : domain.getPolicies()) { + if (policy.getName().equalsIgnoreCase(resourcePolicyName)) { + return policy; + } + } + } + return null; + } + boolean isAllowedPutMembershipAccess(Principal principal, final AthenzDomain domain, final String resourceName) { // evaluate our domain's roles and policies to see if either update or update_members diff --git a/servers/zms/src/main/java/com/yahoo/athenz/zms/utils/ResourceOwnership.java b/servers/zms/src/main/java/com/yahoo/athenz/zms/utils/ResourceOwnership.java index 1765c4efb13..8c29dbfae08 100644 --- a/servers/zms/src/main/java/com/yahoo/athenz/zms/utils/ResourceOwnership.java +++ b/servers/zms/src/main/java/com/yahoo/athenz/zms/utils/ResourceOwnership.java @@ -774,6 +774,19 @@ public static boolean ownershipCheckFailure(boolean bOwnerSpecified, final Strin (!bOwnerSpecified && !StringUtil.isEmpty(objectOwner))); } + public static void verifyPolicyAssertionsDeleteResourceOwnership(Policy policy, final String resourceOwner, + final String caller) { + + // if the object has no owner then we're good for the enforcement check + + ResourcePolicyOwnership resourceOwnership = policy.getResourceOwnership(); + if (resourceOwnership == null || resourceOwnership.getAssertionsOwner() == null) { + return; + } + + verifyDeleteResourceOwnership(resourceOwner, resourceOwnership.getAssertionsOwner(), caller); + } + public static void verifyGroupMembersDeleteResourceOwnership(Group group, final String resourceOwner, final String caller) { diff --git a/servers/zms/src/test/java/com/yahoo/athenz/zms/ResourceOwnershipTest.java b/servers/zms/src/test/java/com/yahoo/athenz/zms/ResourceOwnershipTest.java index b44c0603138..c3f372c4dd9 100644 --- a/servers/zms/src/test/java/com/yahoo/athenz/zms/ResourceOwnershipTest.java +++ b/servers/zms/src/test/java/com/yahoo/athenz/zms/ResourceOwnershipTest.java @@ -2007,8 +2007,7 @@ public void testResourcePolicyOwnershipAssertions() { // apply the assertion with the same ownership which should be ok - zmsImpl.putAssertion(ctx, domainName, policyName, auditRef, "TF1", assertion); - + Assertion assertion1 = zmsImpl.putAssertion(ctx, domainName, policyName, auditRef, "TF1", assertion); // apply the assertion with a different value and verify it's rejected try { @@ -2018,6 +2017,39 @@ public void testResourcePolicyOwnershipAssertions() { assertEquals(ResourceException.CONFLICT, ex.getCode()); } + // delete the assertion with different ownership and it should be rejected + + try { + zmsImpl.deleteAssertion(ctx, domainName, policyName, assertion1.getId(), auditRef, "TF2"); + fail(); + } catch (ResourceException ex) { + assertEquals(ResourceException.CONFLICT, ex.getCode()); + } + + // delete the assertion with null ownership and it should be rejected + + try { + zmsImpl.deleteAssertion(ctx, domainName, policyName, assertion1.getId(), auditRef, null); + fail(); + } catch (ResourceException ex) { + assertEquals(ResourceException.CONFLICT, ex.getCode()); + } + + // delete the assertion with same ownership and it should be ok + try { + zmsImpl.deleteAssertion(ctx, domainName, policyName, assertion1.getId(), auditRef, "TF1"); + } catch (ResourceException ex) { + fail(); + } + + // apply the assertion again with resource owner TF1 + assertion1 = zmsImpl.putAssertion(ctx, domainName, policyName, auditRef, "TF1", assertion); + policy = zmsImpl.getPolicy(ctx, domainName, policyName); + resourceOwnership = policy.getResourceOwnership(); + assertNotNull(resourceOwnership); + assertNull(resourceOwnership.getObjectOwner()); + assertEquals(resourceOwnership.getAssertionsOwner(), "TF1"); + // apply the assertion with a null value and verify it's rejected try { @@ -2037,6 +2069,14 @@ public void testResourcePolicyOwnershipAssertions() { assertNull(resourceOwnership.getObjectOwner()); assertEquals(resourceOwnership.getAssertionsOwner(), "TF1"); + // delete the assertion with the ignore ownership flag which should be ok + zmsImpl.deleteAssertion(ctx, domainName, policyName, assertion1.getId(), auditRef, "ignore"); + policy = zmsImpl.getPolicy(ctx, domainName, policyName); + resourceOwnership = policy.getResourceOwnership(); + assertNotNull(resourceOwnership); + assertNull(resourceOwnership.getObjectOwner()); + assertEquals(resourceOwnership.getAssertionsOwner(), "TF1"); + // create a new policy without any assertions final String policyName2 = "policy2"; diff --git a/servers/zms/src/test/java/com/yahoo/athenz/zms/utils/ResourceOwnershipTest.java b/servers/zms/src/test/java/com/yahoo/athenz/zms/utils/ResourceOwnershipTest.java index 9dfb02cc2c1..82f5aba37bc 100644 --- a/servers/zms/src/test/java/com/yahoo/athenz/zms/utils/ResourceOwnershipTest.java +++ b/servers/zms/src/test/java/com/yahoo/athenz/zms/utils/ResourceOwnershipTest.java @@ -308,7 +308,7 @@ public void testVerifyRoleMembersDeleteResourceOwnership() { fail(); } } - + @Test public void testVerifyGroupMembersDeleteResourceOwnership() { @@ -331,4 +331,27 @@ public void testVerifyGroupMembersDeleteResourceOwnership() { fail(); } } + + @Test + public void testVerifyPolicyAssertionsDeleteResourceOwnership() { + + ResourceOwnership.verifyPolicyAssertionsDeleteResourceOwnership(new Policy(), "resourceOwner", "unit-test"); + ResourceOwnership.verifyPolicyAssertionsDeleteResourceOwnership(new Policy() + .setResourceOwnership(new ResourcePolicyOwnership()), "resourceOwner", "unit-test"); + + Policy assertionsOwnerPolicy = new Policy().setResourceOwnership(new ResourcePolicyOwnership().setAssertionsOwner("assertions-owner")); + try { + ResourceOwnership.verifyPolicyAssertionsDeleteResourceOwnership(assertionsOwnerPolicy, "resourceOwner", "unit-test"); + fail(); + } catch (ResourceException ex) { + assertEquals(ex.getCode(), 409); + } + + Policy objectOwnerPolicy = new Policy().setResourceOwnership(new ResourcePolicyOwnership().setObjectOwner("object-owner")); + try { + ResourceOwnership.verifyPolicyAssertionsDeleteResourceOwnership(objectOwnerPolicy, "resourceOwner", "unit-test"); + } catch (ResourceException ex) { + fail(); + } + } }