Skip to content

Commit

Permalink
enforce resource ownership for delete assertion operation (#2710)
Browse files Browse the repository at this point in the history
* enforce resource ownership for delete assertion operation

Signed-off-by: craman <chandrasekhar.raman@yahooinc.com>

* Update ResourceOwnershipTest.java

---------

Signed-off-by: craman <chandrasekhar.raman@yahooinc.com>
Signed-off-by: Henry Avetisyan <hga@yahooinc.com>
Co-authored-by: craman <chandrasekhar.raman@yahooinc.com>
Co-authored-by: Henry Avetisyan <hga@yahooinc.com>
Co-authored-by: Henry Avetisyan <havetisy@yahoo.com>
  • Loading branch information
4 people authored Sep 5, 2024
1 parent 96cd7ad commit 0248759
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 3 deletions.
22 changes: 22 additions & 0 deletions servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ public void testVerifyRoleMembersDeleteResourceOwnership() {
fail();
}
}

@Test
public void testVerifyGroupMembersDeleteResourceOwnership() {

Expand All @@ -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();
}
}
}

0 comments on commit 0248759

Please sign in to comment.