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

Enable deletion of Assertion with delete {domain}:assertion.{assertionId} permission #2902

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
8 changes: 4 additions & 4 deletions clients/go/zms/zms_schema.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions core/zms/src/main/java/com/yahoo/athenz/zms/ZMSSchema.java
Original file line number Diff line number Diff line change
Expand Up @@ -2264,13 +2264,13 @@ private static Schema build() {
;

sb.resource("Assertion", "DELETE", "/domain/{domainName}/policy/{policyName}/assertion/{assertionId}")
.comment("Delete the specified policy assertion. Upon successful completion of this delete request, the server will return NO_CONTENT status code without any data (no object will be returned).")
.comment("Delete the specified policy assertion. Upon successful completion of this delete request, the server will return NO_CONTENT status code without any data (no object will be returned). The required authorization includes three options: 1. (\"update\", \"{domainName}:policy.{policyName}\") 2. (\"delete\", \"{domainName}:policy.{policyName}.assertion.{assertionId}\")")
.pathParam("domainName", "DomainName", "name of the domain")
.pathParam("policyName", "EntityName", "name of the policy")
.pathParam("assertionId", "Int64", "assertion id")
.headerParam("Y-Audit-Ref", "auditRef", "String", null, "Audit param required(not empty) if domain auditEnabled is true.")
.headerParam("Athenz-Resource-Owner", "resourceOwner", "String", null, "Resource owner for the request")
.auth("update", "{domainName}:policy.{policyName}")
.auth("", "", true)
.expected("NO_CONTENT")
.exception("BAD_REQUEST", "ResourceError", "")

Expand All @@ -2286,15 +2286,15 @@ private static Schema build() {
;

sb.resource("Assertion", "DELETE", "/domain/{domainName}/policy/{policyName}/version/{version}/assertion/{assertionId}")
.comment("Delete the specified policy version assertion. Upon successful completion of this delete request, the server will return NO_CONTENT status code without any data (no object will be returned).")
.comment("Delete the specified policy version assertion. Upon successful completion of this delete request, the server will return NO_CONTENT status code without any data (no object will be returned). The required authorization includes three options: 1. (\"update\", \"{domainName}:policy.{policyName}\") 2. (\"delete\", \"{domainName}:policy.{policyName}.assertion.{assertionId}\")")
.name("deleteAssertionPolicyVersion")
.pathParam("domainName", "DomainName", "name of the domain")
.pathParam("policyName", "EntityName", "name of the policy")
.pathParam("version", "SimpleName", "name of the version")
.pathParam("assertionId", "Int64", "assertion id")
.headerParam("Y-Audit-Ref", "auditRef", "String", null, "Audit param required(not empty) if domain auditEnabled is true.")
.headerParam("Athenz-Resource-Owner", "resourceOwner", "String", null, "Resource owner for the request")
.auth("update", "{domainName}:policy.{policyName}")
.auth("", "", true)
.expected("NO_CONTENT")
.exception("BAD_REQUEST", "ResourceError", "")

Expand Down
12 changes: 8 additions & 4 deletions core/zms/src/main/rdl/Policy.rdli
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,16 @@ resource Assertion PUT "/domain/{domainName}/policy/{policyName}/version/{versio

//Delete the specified policy assertion. Upon successful completion of this delete
//request, the server will return NO_CONTENT status code without any data (no
//object will be returned).
//object will be returned). The required authorization includes three options:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two options and not three

// 1. ("update", "{domainName}:policy.{policyName}")
// 2. ("delete", "{domainName}:policy.{policyName}.assertion.{assertionId}")
resource Assertion DELETE "/domain/{domainName}/policy/{policyName}/assertion/{assertionId}" {
DomainName domainName; //name of the domain
EntityName policyName; //name of the policy
Int64 assertionId; //assertion id
String auditRef (header="Y-Audit-Ref"); //Audit param required(not empty) if domain auditEnabled is true.
String resourceOwner (header="Athenz-Resource-Owner"); //Resource owner for the request
authorize ("update", "{domainName}:policy.{policyName}");
authenticate;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please update the comment section for the block to include what type of authorization statements will be carried out within the code.

expected NO_CONTENT;
exceptions {
ResourceError NOT_FOUND;
Expand All @@ -173,15 +175,17 @@ resource Assertion DELETE "/domain/{domainName}/policy/{policyName}/assertion/{a

//Delete the specified policy version assertion. Upon successful completion of this delete
//request, the server will return NO_CONTENT status code without any data (no
//object will be returned).
//object will be returned). The required authorization includes three options:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two options

// 1. ("update", "{domainName}:policy.{policyName}")
// 2. ("delete", "{domainName}:policy.{policyName}.assertion.{assertionId}")
resource Assertion DELETE "/domain/{domainName}/policy/{policyName}/version/{version}/assertion/{assertionId}" (name=deleteAssertionPolicyVersion) {
DomainName domainName; //name of the domain
EntityName policyName; //name of the policy
SimpleName version; //name of the version
Int64 assertionId; //assertion id
String auditRef (header="Y-Audit-Ref"); //Audit param required(not empty) if domain auditEnabled is true.
String resourceOwner (header="Athenz-Resource-Owner"); //Resource owner for the request
authorize ("update", "{domainName}:policy.{policyName}");
authenticate;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please update the comment section for the block to include what type of authorization statements will be carried out within the code.

expected NO_CONTENT;
exceptions {
ResourceError NOT_FOUND;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public final class ServerCommonConsts {
public static final String OBJECT_ROLE = "role";
public static final String OBJECT_GROUP = "group";
public static final String OBJECT_POLICY = "policy";
public static final String OBJECT_ASSERTION = "assertion";
public static final String OBJECT_ENTITY = "entity";
public static final String USER_DOMAIN = "user";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ public static String policyResourceName(String domainName, String policyName) {
return generateResourceName(domainName, policyName, ServerCommonConsts.OBJECT_POLICY);
}

public static String assertionResourceName(String domainName, String policyName, Long assertionId) {
return domainName + ":" + ServerCommonConsts.OBJECT_POLICY + "." + policyName + "." + ServerCommonConsts.OBJECT_ASSERTION + "." + assertionId;
}

public static String entityResourceName(String domainName, String entityName) {
return generateResourceName(domainName, entityName, ServerCommonConsts.OBJECT_ENTITY);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,10 @@ public void testPolicyResourceName() {
assertEquals(ResourceUtils.policyResourceName("athenz", "policy1"), "athenz:policy.policy1");
assertEquals(ResourceUtils.policyResourceName("athenz.api", "policy1"), "athenz.api:policy.policy1");
}

@Test
public void testAssertionResourceName() {
assertEquals(ResourceUtils.assertionResourceName("athenz", "policy1", 123l), "athenz:policy.policy1.assertion.123");
assertEquals(ResourceUtils.assertionResourceName("athenz.api", "policy1", 123l), "athenz.api:policy.policy1.assertion.123");
}
}
28 changes: 28 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 @@ -109,6 +109,7 @@ public class ZMSImpl implements Authorizer, KeyStore, ZMSHandler {

private static final String ROLE_PREFIX = "role.";
private static final String POLICY_PREFIX = "policy.";
private static final String ASSERTION_PREFIX = "assertion.";

private static final String ADMIN_POLICY_NAME = "admin";
private static final String ADMIN_ROLE_NAME = "admin";
Expand Down Expand Up @@ -6073,6 +6074,8 @@ public void deleteAssertion(ResourceContext ctx, String domainName, String polic
setRequestDomain(ctx, domainName);
policyName = policyName.toLowerCase();

final Principal principal = ((RsrcCtxWrapper) ctx).principal();

// we are not going to allow any user to update
// the admin policy since that is required
// for standard domain operations */
Expand All @@ -6094,6 +6097,12 @@ public void deleteAssertion(ResourceContext ctx, String domainName, String polic
throw ZMSUtils.notFoundError("Invalid policy name specified", caller);
}

// authorization check which also automatically updates
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the "automatically update the active and approved flags for the request" referring to in the comments?

// the active and approved flags for the request
if (!isAllowedDeleteAssertion(principal, domain, policyName, assertionId)) {
throw ZMSUtils.forbiddenError("deleteAssertion: principal is not authorized to delete assertion", caller);
}

try {
ResourceOwnership.verifyPolicyAssertionsDeleteResourceOwnership(policy, resourceOwner, caller);
} catch (ServerResourceException ex) {
Expand Down Expand Up @@ -6128,6 +6137,8 @@ public void deleteAssertionPolicyVersion(ResourceContext ctx, String domainName,
policyName = policyName.toLowerCase();
version = version.toLowerCase();

final Principal principal = ((RsrcCtxWrapper) ctx).principal();

// we are not going to allow any user to update
// the admin policy since that is required
// for standard domain operations */
Expand All @@ -6139,6 +6150,13 @@ public void deleteAssertionPolicyVersion(ResourceContext ctx, String domainName,
// verify that request is properly authenticated for this request

verifyAuthorizedServiceOperation(((RsrcCtxWrapper) ctx).principal().getAuthorizedService(), caller);
AthenzDomain domain = getAthenzDomain(domainName, false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on line 6152 since we already have a principal object can simplify the verifyAuthorizedServiceOperation call and use the principal object directly


// authorization check which also automatically updates
// the active and approved flags for the request
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment here - not sure what the comment is referring to

if (!isAllowedDeleteAssertion(principal, domain, policyName, assertionId)) {
throw ZMSUtils.forbiddenError("deleteAssertion: principal is not authorized to delete assertios", caller);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor spelling fix in the error - assertions

}

dbService.executeDeleteAssertion(ctx, domainName, policyName, version, assertionId, auditRef, caller);
}
Expand Down Expand Up @@ -10589,6 +10607,16 @@ boolean isAllowedDeletePendingMembership(Principal principal, final String domai
return pendingMember != null && principal.getFullName().equals(pendingMember.getRequestPrincipal());
}

boolean isAllowedDeleteAssertion(Principal principal, final AthenzDomain domain, final String policyName, final Long assertionId) {
if (hasAccess(domain, "update", ResourceUtils.policyResourceName(domain.getName(), policyName), principal, null) == AccessStatus.ALLOWED) {
return true;
}
if (hasAccess(domain, "delete", ResourceUtils.assertionResourceName(domain.getName(), policyName, assertionId), principal, null) == AccessStatus.ALLOWED) {
return true;
}
return false;
}

@Override
public DomainRoleMembership getPendingDomainRoleMembersList(ResourceContext ctx, String principal, String domainName) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2514,7 +2514,7 @@ public Assertion putAssertionPolicyVersion(
@DELETE
@Path("/domain/{domainName}/policy/{policyName}/assertion/{assertionId}")
@Produces(MediaType.APPLICATION_JSON)
@Operation(description = "Delete the specified policy assertion. Upon successful completion of this delete request, the server will return NO_CONTENT status code without any data (no object will be returned).")
@Operation(description = "Delete the specified policy assertion. Upon successful completion of this delete request, the server will return NO_CONTENT status code without any data (no object will be returned). The required authorization includes three options: 1. (\"update\", \"{domainName}:policy.{policyName}\") 2. (\"delete\", \"{domainName}:policy.{policyName}.assertion.{assertionId}\")")
public void deleteAssertion(
@Parameter(description = "name of the domain", required = true) @PathParam("domainName") String domainName,
@Parameter(description = "name of the policy", required = true) @PathParam("policyName") String policyName,
Expand All @@ -2525,7 +2525,7 @@ public void deleteAssertion(
ResourceContext context = null;
try {
context = this.delegate.newResourceContext(this.servletContext, this.request, this.response, "deleteAssertion");
context.authorize("update", "" + domainName + ":policy." + policyName + "", null);
context.authenticate();
this.delegate.deleteAssertion(context, domainName, policyName, assertionId, auditRef, resourceOwner);
} catch (ResourceException e) {
code = e.getCode();
Expand Down Expand Up @@ -2555,7 +2555,7 @@ public void deleteAssertion(
@DELETE
@Path("/domain/{domainName}/policy/{policyName}/version/{version}/assertion/{assertionId}")
@Produces(MediaType.APPLICATION_JSON)
@Operation(description = "Delete the specified policy version assertion. Upon successful completion of this delete request, the server will return NO_CONTENT status code without any data (no object will be returned).")
@Operation(description = "Delete the specified policy version assertion. Upon successful completion of this delete request, the server will return NO_CONTENT status code without any data (no object will be returned). The required authorization includes three options: 1. (\"update\", \"{domainName}:policy.{policyName}\") 2. (\"delete\", \"{domainName}:policy.{policyName}.assertion.{assertionId}\")")
public void deleteAssertionPolicyVersion(
@Parameter(description = "name of the domain", required = true) @PathParam("domainName") String domainName,
@Parameter(description = "name of the policy", required = true) @PathParam("policyName") String policyName,
Expand All @@ -2567,7 +2567,7 @@ public void deleteAssertionPolicyVersion(
ResourceContext context = null;
try {
context = this.delegate.newResourceContext(this.servletContext, this.request, this.response, "deleteAssertionPolicyVersion");
context.authorize("update", "" + domainName + ":policy." + policyName + "", null);
context.authenticate();
this.delegate.deleteAssertionPolicyVersion(context, domainName, policyName, version, assertionId, auditRef, resourceOwner);
} catch (ResourceException e) {
code = e.getCode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2074,7 +2074,7 @@ public void testResourcePolicyOwnershipAssertions() {
final String roleName = "role1";
final String policyName = "policy1";
TopLevelDomain dom1 = zmsTestInitializer.createTopLevelDomainObject(domainName,
"Test Domain1", "testOrg", zmsTestInitializer.getAdminUser());
"Test Domain1", "testOrg", "user.user1");
zmsImpl.postTopLevelDomain(ctx, auditRef, null, dom1);

Role role1 = zmsTestInitializer.createRoleObject(domainName, roleName, null, null);
Expand Down
Loading