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.

Sign up for GitHub

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

resource ownership override idempotency #2901

Merged
merged 1 commit into from
Mar 7, 2025
Merged

resource ownership override idempotency #2901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 31 additions & 31 deletions ...va/server_common/src/main/java/com/yahoo/athenz/common/server/util/ResourceOwnership.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,9 @@ public static ResourceDomainOwnership verifyDomainMetaResourceOwnership(Domain d
// if the object has no meta owner then we need to set it
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
return requestOwnership;
} else if (!resourceOwner.equalsIgnoreCase(metaOwner)) {
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(metaOwner)) {
throw Utils.conflictError("Invalid resource owner for domain: " + domain.getName()
+ ", " + metaOwner + " vs. " + resourceOwner, caller);
+ ", " + metaOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
} else {
// no changes needed
return null;
Expand Down Expand Up @@ -261,23 +261,23 @@ public static ResourceRoleOwnership verifyRoleResourceOwnership(Role role, boole
boolean bUpdateRequired = false;
if (resourceOwnership.getObjectOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getObjectOwner())) {
bUpdateRequired = true;
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getObjectOwner())) {
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getObjectOwner())) {
throw Utils.conflictError("Invalid resource owner for role: " + role.getName()
+ ", " + resourceOwnership.getObjectOwner() + " vs. " + resourceOwner, caller);
+ ", " + resourceOwnership.getObjectOwner() + " vs. " + resourceOwnerWithoutForceSuffix, caller);
}

if (resourceOwnership.getMembersOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getMembersOwner())) {
bUpdateRequired = true;
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getMembersOwner())) {
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getMembersOwner())) {
throw Utils.conflictError("Invalid members owner for role: " + role.getName()
+ ", " + resourceOwnership.getMembersOwner() + " vs. " + resourceOwner, caller);
+ ", " + resourceOwnership.getMembersOwner() + " vs. " + resourceOwnerWithoutForceSuffix, caller);
}

if (resourceOwnership.getMetaOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getMetaOwner())) {
bUpdateRequired = true;
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getMetaOwner())) {
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getMetaOwner())) {
throw Utils.conflictError("Invalid meta owner for role: " + role.getName()
+ ", " + resourceOwnership.getMetaOwner() + " vs. " + resourceOwner, caller);
+ ", " + resourceOwnership.getMetaOwner() + " vs. " + resourceOwnerWithoutForceSuffix, caller);
}

return bUpdateRequired ? requestOwnership : null;
Expand Down Expand Up @@ -344,9 +344,9 @@ public static ResourceRoleOwnership verifyRoleMetaResourceOwnership(Role role, f
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
requestOwnership.setMembersOwner(resourceOwnership.getMembersOwner());
return requestOwnership;
} else if (!resourceOwner.equalsIgnoreCase(metaOwner)) {
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(metaOwner)) {
throw Utils.conflictError("Invalid resource meta owner for role: " + role.getName()
+ ", " + metaOwner + " vs. " + resourceOwner, caller);
+ ", " + metaOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
} else {
// no changes needed
return null;
Expand Down Expand Up @@ -389,9 +389,9 @@ public static ResourceRoleOwnership verifyRoleMembersResourceOwnership(Role role
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
requestOwnership.setMetaOwner(resourceOwnership.getMetaOwner());
return requestOwnership;
} else if (!resourceOwner.equalsIgnoreCase(membersOwner)) {
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(membersOwner)) {
throw Utils.conflictError("Invalid resource member owner for role: " + role.getName()
+ ", " + membersOwner + " vs. " + resourceOwner, caller);
+ ", " + membersOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
} else {
// no changes needed
return null;
Expand Down Expand Up @@ -434,23 +434,23 @@ public static ResourceGroupOwnership verifyGroupResourceOwnership(Group group, b
boolean bUpdateRequired = false;
if (resourceOwnership.getObjectOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getObjectOwner())) {
bUpdateRequired = true;
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getObjectOwner())) {
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getObjectOwner())) {
throw Utils.conflictError("Invalid resource owner for group: " + group.getName()
+ ", " + resourceOwnership.getObjectOwner() + " vs. " + resourceOwner, caller);
}

if (resourceOwnership.getMembersOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getMembersOwner())) {
bUpdateRequired = true;
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getMembersOwner())) {
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getMembersOwner())) {
throw Utils.conflictError("Invalid members owner for group: " + group.getName()
+ ", " + resourceOwnership.getMembersOwner() + " vs. " + resourceOwner, caller);
}

if (resourceOwnership.getMetaOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getMetaOwner())) {
bUpdateRequired = true;
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getMetaOwner())) {
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getMetaOwner())) {
throw Utils.conflictError("Invalid meta owner for group: " + group.getName()
+ ", " + resourceOwnership.getMetaOwner() + " vs. " + resourceOwner, caller);
+ ", " + resourceOwnership.getMetaOwner() + " vs. " + resourceOwnerWithoutForceSuffix, caller);
}

return bUpdateRequired ? requestOwnership : null;
Expand Down Expand Up @@ -508,9 +508,9 @@ public static ResourceGroupOwnership verifyGroupMetaResourceOwnership(Group grou
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
requestOwnership.setMembersOwner(resourceOwnership.getMembersOwner());
return requestOwnership;
} else if (!resourceOwner.equalsIgnoreCase(metaOwner)) {
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(metaOwner)) {
throw Utils.conflictError("Invalid resource meta owner for group: " + group.getName()
+ ", " + metaOwner + " vs. " + resourceOwner, caller);
+ ", " + metaOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
} else {
// no changes needed
return null;
Expand Down Expand Up @@ -553,9 +553,9 @@ public static ResourceGroupOwnership verifyGroupMembersResourceOwnership(Group g
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
requestOwnership.setMetaOwner(resourceOwnership.getMetaOwner());
return requestOwnership;
} else if (!resourceOwner.equalsIgnoreCase(membersOwner)) {
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(membersOwner)) {
throw Utils.conflictError("Invalid resource member owner for group: " + group.getName()
+ ", " + membersOwner + " vs. " + resourceOwner, caller);
+ ", " + membersOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
} else {
// no changes needed
return null;
Expand Down Expand Up @@ -597,16 +597,16 @@ public static ResourcePolicyOwnership verifyPolicyResourceOwnership(Policy polic
boolean bUpdateRequired = false;
if (resourceOwnership.getObjectOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getObjectOwner())) {
bUpdateRequired = true;
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getObjectOwner())) {
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getObjectOwner())) {
throw Utils.conflictError("Invalid resource owner for policy: " + policy.getName()
+ ", " + resourceOwnership.getObjectOwner() + " vs. " + resourceOwner, caller);
+ ", " + resourceOwnership.getObjectOwner() + " vs. " + resourceOwnerWithoutForceSuffix, caller);
}

if (resourceOwnership.getAssertionsOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getAssertionsOwner())) {
bUpdateRequired = true;
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getAssertionsOwner())) {
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getAssertionsOwner())) {
throw Utils.conflictError("Invalid assertions owner for policy: " + policy.getName()
+ ", " + resourceOwnership.getAssertionsOwner() + " vs. " + resourceOwner, caller);
+ ", " + resourceOwnership.getAssertionsOwner() + " vs. " + resourceOwnerWithoutForceSuffix, caller);
}

return bUpdateRequired ? requestOwnership : null;
Expand Down Expand Up @@ -668,9 +668,9 @@ public static ResourcePolicyOwnership verifyPolicyAssertionsResourceOwnership(Po
// if the object has no assertions owner then we need to set it
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
return requestOwnership;
} else if (!resourceOwner.equalsIgnoreCase(assertionsOwner)) {
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(assertionsOwner)) {
throw Utils.conflictError("Invalid resource member owner for policy: " + policy.getName()
+ ", " + assertionsOwner + " vs. " + resourceOwner, caller);
+ ", " + assertionsOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
} else {
// no changes needed
return null;
Expand Down Expand Up @@ -717,21 +717,21 @@ public static ResourceServiceIdentityOwnership verifyServiceResourceOwnership(Se
boolean bUpdateRequired = false;
if (resourceOwnership.getObjectOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getObjectOwner())) {
bUpdateRequired = true;
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getObjectOwner())) {
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getObjectOwner())) {
throw Utils.conflictError("Invalid resource owner for service: " + service.getName()
+ ", " + resourceOwnership.getObjectOwner() + " vs. " + resourceOwner, caller);
}

if (resourceOwnership.getPublicKeysOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getPublicKeysOwner())) {
bUpdateRequired = true;
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getPublicKeysOwner())) {
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getPublicKeysOwner())) {
throw Utils.conflictError("Invalid public-keys owner for service: " + service.getName()
+ ", " + resourceOwnership.getPublicKeysOwner() + " vs. " + resourceOwner, caller);
}

if (resourceOwnership.getHostsOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getHostsOwner())) {
bUpdateRequired = true;
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getHostsOwner())) {
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getHostsOwner())) {
throw Utils.conflictError("Invalid hosts owner for service: " + service.getName()
+ ", " + resourceOwnership.getHostsOwner() + " vs. " + resourceOwner, caller);
}
Expand Down Expand Up @@ -788,9 +788,9 @@ public static ResourceServiceIdentityOwnership verifyServicePublicKeysResourceOw
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
requestOwnership.setHostsOwner(resourceOwnership.getHostsOwner());
return requestOwnership;
} else if (!resourceOwner.equalsIgnoreCase(publicKeysOwner)) {
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(publicKeysOwner)) {
throw Utils.conflictError("Invalid resource member owner for service: " + service.getName()
+ ", " + publicKeysOwner + " vs. " + resourceOwner, caller);
+ ", " + publicKeysOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
} else {
// no changes needed
return null;
Expand Down
56 changes: 56 additions & 0 deletions ...erver_common/src/test/java/com/yahoo/athenz/common/server/util/ResourceOwnershipTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,16 @@ public void testVerifyPolicyResourceOwnership() {
fail();
}

// no-op needed when resource ownership is same and force overridden
policy = new Policy().setName("policy1").setResourceOwnership(new ResourcePolicyOwnership().setObjectOwner("TF2"));
try {
ownership = ResourceOwnership.verifyPolicyResourceOwnership(policy, true, "TF2:force", "unit-test");
assertNotNull(ownership);
assertEquals(ownership.getAssertionsOwner(), "TF2");
} catch (ServerResourceException ignored) {
fail();
}

// exception is thrown when resource ownership is different and not force overridden
policy = new Policy().setName("policy1").setResourceOwnership(new ResourcePolicyOwnership().setObjectOwner("TF1"));
try {
Expand Down Expand Up @@ -776,6 +786,15 @@ public void testVerifyGroupMetaResourceOwnership() {
fail();
}

// no-op when resource ownership is same and force overridden
group = new Group().setName("group1").setResourceOwnership(new ResourceGroupOwnership().setMetaOwner("TF2"));
try {
ownership = ResourceOwnership.verifyGroupMetaResourceOwnership(group, "TF2:force", "unit-test");
assertNull(ownership);
} catch (ServerResourceException ignored) {
fail();
}

// exception is thrown when resource ownership is different and not force overridden
group = new Group().setName("group1").setResourceOwnership(new ResourceGroupOwnership().setMetaOwner("TF1"));
try {
Expand Down Expand Up @@ -845,6 +864,16 @@ public void testVerifyGroupResourceOwnership() {
fail();
}

// no-op when resource ownership is force overridden with the same value
group = new Group().setName("group1").setResourceOwnership(new ResourceGroupOwnership().setObjectOwner("TF2"));
try {
ownership = ResourceOwnership.verifyGroupResourceOwnership(group, true, "TF2:force", "unit-test");
assertNotNull(ownership);
assertEquals(ownership.getObjectOwner(), "TF2");
} catch (ServerResourceException ignored) {
fail();
}

// exception is thrown when resource ownership is different and not force overridden
group = new Group().setName("group1").setResourceOwnership(new ResourceGroupOwnership().setObjectOwner("TF1"));
try {
Expand Down Expand Up @@ -914,6 +943,16 @@ public void testVerifyRoleResourceOwnership() {
fail();
}

// no-op when resource ownership is force overridden with the same value
role = new Role().setName("role1").setResourceOwnership(new ResourceRoleOwnership().setObjectOwner("TF2"));
try {
ownership = ResourceOwnership.verifyRoleResourceOwnership(role, true, "TF2:force", "unit-test");
assertNotNull(ownership);
assertEquals(ownership.getObjectOwner(), "TF2");
} catch (ServerResourceException ignored) {
fail();
}

// exception is thrown when resource ownership is different and not force overridden
role = new Role().setName("role1").setResourceOwnership(new ResourceRoleOwnership().setObjectOwner("TF1"));
try {
Expand Down Expand Up @@ -983,6 +1022,15 @@ public void testVerifyRoleMemberResourceOwnership() {
fail();
}

// no-op when second time called with same owner
role = new Role().setName("role1").setResourceOwnership(new ResourceRoleOwnership().setMembersOwner("TF2"));
try {
ownership = ResourceOwnership.verifyRoleMembersResourceOwnership(role, "TF2:force", "unit-test");
assertNull(ownership);
} catch (ServerResourceException ignored) {
fail();
}

// exception is thrown when resource ownership is different and not force overridden
role = new Role().setName("role1").setResourceOwnership(new ResourceRoleOwnership().setMembersOwner("TF1"));
try {
Expand Down Expand Up @@ -1052,6 +1100,14 @@ public void testVerifyRoleMetaResourceOwnership() {
fail();
}

// no-op when second time called with same owner
role = new Role().setName("role1").setResourceOwnership(new ResourceRoleOwnership().setMetaOwner("TF1"));
try {
ResourceOwnership.verifyRoleMetaResourceOwnership(role, "TF2:force", "unit-test");
} catch (ServerResourceException sre) {
fail();
}

// exception is thrown when resource ownership is different and not force overridden
role = new Role().setName("role1").setResourceOwnership(new ResourceRoleOwnership().setMetaOwner("TF1"));
try {
Expand Down