Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enforce resource ownership for delete assertion operation #2710

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}
}
}