Skip to content

Commit

Permalink
merge #2702 into jetty 1.12.x branch
Browse files Browse the repository at this point in the history 
Signed-off-by: Henry Avetisyan <hga@yahooinc.com>
  • Loading branch information
@havetisyan
havetisyan committed Sep 5, 2024
1 parent 3f3221f commit d6956a7
Show file tree
Hide file tree
Showing 2 changed files with 140 additions and 24 deletions.
44 changes: 24 additions & 20 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 @@ -9981,34 +9981,36 @@ private void validatePutMembershipDecisionAuthorization(final Principal principa
// out the authorization in the sys.auth.audit domains

if (role.getAuditEnabled() == Boolean.TRUE) {

if (!isAllowedAuditRoleMembershipApproval(principal, domain)) {
throw ZMSUtils.forbiddenError("principal " + principal.getFullName()
+ " is not authorized to approve / reject members", caller);
}
return;
}

// otherwise, we're going to do a standard check if the principal
// is authorized to update the domain role membership
} else {

// otherwise, we're going to do a standard check if the principal
// is authorized to update the domain role membership

if (!isAllowedPutMembershipAccess(principal, domain, role.getName())) {
throw ZMSUtils.forbiddenError("principal " + principal.getFullName()
+ " is not authorized to approve / reject members", caller);
if (!isAllowedPutMembershipAccess(principal, domain, role.getName())) {
throw ZMSUtils.forbiddenError("principal " + principal.getFullName()
+ " is not authorized to approve / reject members", caller);
}
}

// if the user is allowed to make changes in the domain but
// the role is review enabled then we need to make sure
// the role is review or audit enabled then we need to make sure
// the approver cannot be the same as the requester

if (role.getReviewEnabled() == Boolean.TRUE) {
if (role.getReviewEnabled() == Boolean.TRUE || role.getAuditEnabled() == Boolean.TRUE) {

Membership pendingMember = dbService.getMembership(domain.getName(),
ZMSUtils.extractRoleName(domain.getName(), role.getName()),
roleMember.getMemberName(), 0, true);

if (principal.getFullName().equalsIgnoreCase(pendingMember.getRequestPrincipal())) {
throw ZMSUtils.forbiddenError("principal " + principal.getFullName()
+ " cannot approve his/her own request", caller);
+ " cannot approve / reject own request", caller);
}
}
}
Expand All @@ -10022,34 +10024,36 @@ private void validatePutGroupMembershipDecisionAuthorization(final Principal pri
// out the authorization in the sys.auth.audit domains

if (group.getAuditEnabled() == Boolean.TRUE) {

if (!isAllowedAuditRoleMembershipApproval(principal, domain)) {
throw ZMSUtils.forbiddenError("principal " + principal.getFullName()
+ " is not authorized to approve / reject members", caller);
}
return;
}

// otherwise, we're going to do a standard check if the principal
// is authorized to update the domain group membership
} else {

// otherwise, we're going to do a standard check if the principal
// is authorized to update the domain group membership

if (!isAllowedPutMembershipAccess(principal, domain, group.getName())) {
throw ZMSUtils.forbiddenError("principal " + principal.getFullName()
+ " is not authorized to approve / reject members", caller);
if (!isAllowedPutMembershipAccess(principal, domain, group.getName())) {
throw ZMSUtils.forbiddenError("principal " + principal.getFullName()
+ " is not authorized to approve / reject members", caller);
}
}

// if the user is allowed to make changes in the domain but
// the role is review enabled then we need to make sure
// the role is review or audit enabled then we need to make sure
// the approver cannot be the same as the requester

if (group.getReviewEnabled() == Boolean.TRUE) {
if (group.getReviewEnabled() == Boolean.TRUE || group.getAuditEnabled() == Boolean.TRUE) {

GroupMembership pendingMember = dbService.getGroupMembership(domain.getName(),
ZMSUtils.extractGroupName(domain.getName(), group.getName()),
groupMember.getMemberName(), 0, true);

if (principal.getFullName().equalsIgnoreCase(pendingMember.getRequestPrincipal())) {
throw ZMSUtils.forbiddenError("principal " + principal.getFullName()
+ " cannot approve his/her own request", caller);
+ " cannot approve / reject own request", caller);
}
}
}
Expand Down
120 changes: 116 additions & 4 deletions servers/zms/src/test/java/com/yahoo/athenz/zms/ZMSImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21581,7 +21581,7 @@ public void testPutMembershipDecisionReviewEnabledRoleApproveAddState() {
zmsImpl.putMembershipDecision(ctx, domainName, roleName, "user.bob", auditRef, mbr);
fail();
} catch (ResourceException ex) {
assertTrue(ex.getMessage().contains("cannot approve his/her own request"));
assertTrue(ex.getMessage().contains("cannot approve / reject own request"));
}

// revert back to admin principal
Expand Down Expand Up @@ -21681,7 +21681,7 @@ public void testPutMembershipDecisionReviewEnabledRoleApproveDeleteState() {
zmsImpl.putMembershipDecision(ctx, domainName, roleName, "user.bob", auditRef, mbr);
fail();
} catch (ResourceException ex) {
assertTrue(ex.getMessage().contains("cannot approve his/her own request"));
assertTrue(ex.getMessage().contains("cannot approve / reject own request"));
}

// revert back to admin principal
Expand Down Expand Up @@ -21864,6 +21864,62 @@ public void testPutMembershipDecisionAuditEnabledRoleByOrg() {
zmsImpl.deleteTopLevelDomain(ctx, domainName, auditRef, null);
}

@Test
public void testPutMembershipDecisionAuditEnabledRejectSelfApproval() {

ZMSImpl zmsImpl = zmsTestInitializer.getZms();
RsrcCtxWrapper ctx = zmsTestInitializer.getMockDomRsrcCtx();
final String auditRef = zmsTestInitializer.getAuditRef();

final String domainName = "testdomain1";
final String roleName = "testrole1";

TopLevelDomain dom1 = zmsTestInitializer.createTopLevelDomainObject(domainName, "Approval Test Domain1",
"testOrg", "user.user1");
zmsImpl.postTopLevelDomain(ctx, auditRef, null, dom1);
DomainMeta meta = zmsTestInitializer.createDomainMetaObject("Domain Meta for approval test", "testOrg",
true, true, "12345", 1001);
zmsImpl.putDomainMeta(ctx, domainName, auditRef, null, meta);
zmsImpl.putDomainSystemMeta(ctx, domainName, "auditenabled", auditRef, meta);

Role auditedRole = zmsTestInitializer.createRoleObject(domainName, roleName, null, "user.john", "user.jane");
zmsImpl.putRole(ctx, domainName, roleName, auditRef, false, null, auditedRole);
RoleSystemMeta rsm = ZMSTestUtils.createRoleSystemMetaObject(true);
zmsImpl.putRoleSystemMeta(ctx, domainName, roleName, "auditenabled", auditRef, rsm);

Membership mbr = new Membership();
mbr.setMemberName("user.bob");
mbr.setActive(false);
mbr.setApproved(false);
zmsImpl.putMembership(ctx, domainName, roleName, "user.bob", auditRef, false, null, mbr);

Role resrole = zmsImpl.getRole(ctx, domainName, roleName, false, false, true);
assertEquals(resrole.getRoleMembers().size(), 3);
for (RoleMember rmem : resrole.getRoleMembers()) {
if ("user.bob".equals(rmem.getMemberName())) {
assertFalse(rmem.getApproved());
}
}

setupPrincipalAuditedRoleApprovalByOrg(zmsImpl, "user.user1", "testOrg");

mbr = new Membership();
mbr.setMemberName("user.bob");
mbr.setActive(true);
mbr.setApproved(true);

try {
zmsImpl.putMembershipDecision(ctx, domainName, roleName, "user.bob", auditRef, mbr);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), ResourceException.FORBIDDEN);
assertTrue(ex.getMessage().contains("cannot approve / reject own request"));
}

cleanupPrincipalAuditedRoleApprovalByOrg(zmsImpl, "testOrg");
zmsImpl.deleteTopLevelDomain(ctx, domainName, auditRef, null);
}

@Test
public void testPutMembershipDecisionAuditEnabledRoleByDomain() {

Expand Down Expand Up @@ -26221,6 +26277,62 @@ public void testPutGroupMembershipDecisionAuditEnabledGroupByDomain() {
zmsImpl.deleteTopLevelDomain(ctx, domainName, auditRef, null);
}

@Test
public void testPutGroupMembershipDecisionAuditEnabledGroupRejectSelfApproval() {

ZMSImpl zmsImpl = zmsTestInitializer.getZms();
RsrcCtxWrapper ctx = zmsTestInitializer.getMockDomRsrcCtx();
final String auditRef = zmsTestInitializer.getAuditRef();

final String domainName = "group-dec-by-domain";
final String groupName = "group1";

TopLevelDomain dom1 = zmsTestInitializer.createTopLevelDomainObject(domainName, "Approval Test Domain1",
"testOrg", "user.user1");
zmsImpl.postTopLevelDomain(ctx, auditRef, null, dom1);
DomainMeta meta = zmsTestInitializer.createDomainMetaObject("Domain Meta for approval test", "testOrg",
true, true, "12345", 1001);
zmsImpl.putDomainMeta(ctx, domainName, auditRef, null, meta);
zmsImpl.putDomainSystemMeta(ctx, domainName, "auditenabled", auditRef, meta);

Group auditedGroup = zmsTestInitializer.createGroupObject(domainName, groupName, "user.john", "user.jane");
zmsImpl.putGroup(ctx, domainName, groupName, auditRef, false, null, auditedGroup);
GroupSystemMeta rsm = createGroupSystemMetaObject(true);
zmsImpl.putGroupSystemMeta(ctx, domainName, groupName, "auditenabled", auditRef, rsm);

GroupMembership mbr = new GroupMembership();
mbr.setMemberName("user.bob");
mbr.setActive(false);
mbr.setApproved(false);
zmsImpl.putGroupMembership(ctx, domainName, groupName, "user.bob", auditRef, false, null, mbr);

Group resgroup = zmsImpl.getGroup(ctx, domainName, groupName, false, true);
assertEquals(resgroup.getGroupMembers().size(), 3);
for (GroupMember rmem : resgroup.getGroupMembers()) {
if ("user.bob".equals(rmem.getMemberName())) {
assertFalse(rmem.getApproved());
}
}

setupPrincipalAuditedRoleApprovalByDomain(zmsImpl, "user.user1", domainName);

mbr = new GroupMembership();
mbr.setMemberName("user.bob");
mbr.setActive(true);
mbr.setApproved(true);

try {
zmsImpl.putGroupMembershipDecision(ctx, domainName, groupName, "user.bob", auditRef, mbr);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), ResourceException.FORBIDDEN);
assertTrue(ex.getMessage().contains("cannot approve / reject own request"));
}

cleanupPrincipalAuditedRoleApprovalByDomain(zmsImpl, domainName);
zmsImpl.deleteTopLevelDomain(ctx, domainName, auditRef, null);
}

@Test
public void testPutGroupMembershipDecisionAuditEnabledGroupInvalidUser() {

Expand Down Expand Up @@ -27327,7 +27439,7 @@ public void testPutGroupMembershipDecisionReviewEnabledGroupApproveAddState() {
zmsImpl.putGroupMembershipDecision(ctx, domainName, groupName, "user.bob", auditRef, mbr);
fail();
} catch (ResourceException ex) {
assertTrue(ex.getMessage().contains("cannot approve his/her own request"));
assertTrue(ex.getMessage().contains("cannot approve / reject own request"));
}

// revert back to admin principal
Expand Down Expand Up @@ -27428,7 +27540,7 @@ public void testPutGroupMembershipDecisionReviewEnabledGroupApproveDeleteState()
zmsImpl.putGroupMembershipDecision(ctx, domainName, groupName, "user.bob", auditRef, mbr);
fail();
} catch (ResourceException ex) {
assertTrue(ex.getMessage().contains("cannot approve his/her own request"));
assertTrue(ex.getMessage().contains("cannot approve / reject own request"));
}

// revert back to admin principal
Expand Down

0 comments on commit d6956a7

Please sign in to comment.