Skip to content

Commit f01e4e7

Browse files
abvaidyaabvaidya
authored
resource ownership override idempotency (#2901)
Signed-off-by: Abhijeet V <31417623+abvaidya@users.noreply.github.com>
1 parent 0f0d596 commit f01e4e7

File tree

2 files changed

+87
-31
lines changed

2 files changed

+87
-31
lines changed

‎libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/util/ResourceOwnership.java

+31-31
Original file line numberDiff line numberDiff line change
@@ -215,9 +215,9 @@ public static ResourceDomainOwnership verifyDomainMetaResourceOwnership(Domain d
215215
// if the object has no meta owner then we need to set it
216216
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
217217
return requestOwnership;
218-
} else if (!resourceOwner.equalsIgnoreCase(metaOwner)) {
218+
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(metaOwner)) {
219219
throw Utils.conflictError("Invalid resource owner for domain: " + domain.getName()
220-
+ ", " + metaOwner + " vs. " + resourceOwner, caller);
220+
+ ", " + metaOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
221221
} else {
222222
// no changes needed
223223
return null;
@@ -261,23 +261,23 @@ public static ResourceRoleOwnership verifyRoleResourceOwnership(Role role, boole
261261
boolean bUpdateRequired = false;
262262
if (resourceOwnership.getObjectOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getObjectOwner())) {
263263
bUpdateRequired = true;
264-
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getObjectOwner())) {
264+
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getObjectOwner())) {
265265
throw Utils.conflictError("Invalid resource owner for role: " + role.getName()
266-
+ ", " + resourceOwnership.getObjectOwner() + " vs. " + resourceOwner, caller);
266+
+ ", " + resourceOwnership.getObjectOwner() + " vs. " + resourceOwnerWithoutForceSuffix, caller);
267267
}
268268

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

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

283283
return bUpdateRequired ? requestOwnership : null;
@@ -344,9 +344,9 @@ public static ResourceRoleOwnership verifyRoleMetaResourceOwnership(Role role, f
344344
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
345345
requestOwnership.setMembersOwner(resourceOwnership.getMembersOwner());
346346
return requestOwnership;
347-
} else if (!resourceOwner.equalsIgnoreCase(metaOwner)) {
347+
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(metaOwner)) {
348348
throw Utils.conflictError("Invalid resource meta owner for role: " + role.getName()
349-
+ ", " + metaOwner + " vs. " + resourceOwner, caller);
349+
+ ", " + metaOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
350350
} else {
351351
// no changes needed
352352
return null;
@@ -389,9 +389,9 @@ public static ResourceRoleOwnership verifyRoleMembersResourceOwnership(Role role
389389
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
390390
requestOwnership.setMetaOwner(resourceOwnership.getMetaOwner());
391391
return requestOwnership;
392-
} else if (!resourceOwner.equalsIgnoreCase(membersOwner)) {
392+
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(membersOwner)) {
393393
throw Utils.conflictError("Invalid resource member owner for role: " + role.getName()
394-
+ ", " + membersOwner + " vs. " + resourceOwner, caller);
394+
+ ", " + membersOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
395395
} else {
396396
// no changes needed
397397
return null;
@@ -434,23 +434,23 @@ public static ResourceGroupOwnership verifyGroupResourceOwnership(Group group, b
434434
boolean bUpdateRequired = false;
435435
if (resourceOwnership.getObjectOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getObjectOwner())) {
436436
bUpdateRequired = true;
437-
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getObjectOwner())) {
437+
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getObjectOwner())) {
438438
throw Utils.conflictError("Invalid resource owner for group: " + group.getName()
439439
+ ", " + resourceOwnership.getObjectOwner() + " vs. " + resourceOwner, caller);
440440
}
441441

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

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

456456
return bUpdateRequired ? requestOwnership : null;
@@ -508,9 +508,9 @@ public static ResourceGroupOwnership verifyGroupMetaResourceOwnership(Group grou
508508
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
509509
requestOwnership.setMembersOwner(resourceOwnership.getMembersOwner());
510510
return requestOwnership;
511-
} else if (!resourceOwner.equalsIgnoreCase(metaOwner)) {
511+
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(metaOwner)) {
512512
throw Utils.conflictError("Invalid resource meta owner for group: " + group.getName()
513-
+ ", " + metaOwner + " vs. " + resourceOwner, caller);
513+
+ ", " + metaOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
514514
} else {
515515
// no changes needed
516516
return null;
@@ -553,9 +553,9 @@ public static ResourceGroupOwnership verifyGroupMembersResourceOwnership(Group g
553553
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
554554
requestOwnership.setMetaOwner(resourceOwnership.getMetaOwner());
555555
return requestOwnership;
556-
} else if (!resourceOwner.equalsIgnoreCase(membersOwner)) {
556+
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(membersOwner)) {
557557
throw Utils.conflictError("Invalid resource member owner for group: " + group.getName()
558-
+ ", " + membersOwner + " vs. " + resourceOwner, caller);
558+
+ ", " + membersOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
559559
} else {
560560
// no changes needed
561561
return null;
@@ -597,16 +597,16 @@ public static ResourcePolicyOwnership verifyPolicyResourceOwnership(Policy polic
597597
boolean bUpdateRequired = false;
598598
if (resourceOwnership.getObjectOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getObjectOwner())) {
599599
bUpdateRequired = true;
600-
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getObjectOwner())) {
600+
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getObjectOwner())) {
601601
throw Utils.conflictError("Invalid resource owner for policy: " + policy.getName()
602-
+ ", " + resourceOwnership.getObjectOwner() + " vs. " + resourceOwner, caller);
602+
+ ", " + resourceOwnership.getObjectOwner() + " vs. " + resourceOwnerWithoutForceSuffix, caller);
603603
}
604604

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

612612
return bUpdateRequired ? requestOwnership : null;
@@ -668,9 +668,9 @@ public static ResourcePolicyOwnership verifyPolicyAssertionsResourceOwnership(Po
668668
// if the object has no assertions owner then we need to set it
669669
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
670670
return requestOwnership;
671-
} else if (!resourceOwner.equalsIgnoreCase(assertionsOwner)) {
671+
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(assertionsOwner)) {
672672
throw Utils.conflictError("Invalid resource member owner for policy: " + policy.getName()
673-
+ ", " + assertionsOwner + " vs. " + resourceOwner, caller);
673+
+ ", " + assertionsOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
674674
} else {
675675
// no changes needed
676676
return null;
@@ -717,21 +717,21 @@ public static ResourceServiceIdentityOwnership verifyServiceResourceOwnership(Se
717717
boolean bUpdateRequired = false;
718718
if (resourceOwnership.getObjectOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getObjectOwner())) {
719719
bUpdateRequired = true;
720-
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getObjectOwner())) {
720+
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getObjectOwner())) {
721721
throw Utils.conflictError("Invalid resource owner for service: " + service.getName()
722722
+ ", " + resourceOwnership.getObjectOwner() + " vs. " + resourceOwner, caller);
723723
}
724724

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

732732
if (resourceOwnership.getHostsOwner() == null || isResourceOwnershipOverrideAllowed(resourceOwner, resourceOwnership.getHostsOwner())) {
733733
bUpdateRequired = true;
734-
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwner, resourceOwnership.getHostsOwner())) {
734+
} else if (ownershipCheckFailure(bOwnerSpecified, resourceOwnerWithoutForceSuffix, resourceOwnership.getHostsOwner())) {
735735
throw Utils.conflictError("Invalid hosts owner for service: " + service.getName()
736736
+ ", " + resourceOwnership.getHostsOwner() + " vs. " + resourceOwner, caller);
737737
}
@@ -788,9 +788,9 @@ public static ResourceServiceIdentityOwnership verifyServicePublicKeysResourceOw
788788
requestOwnership.setObjectOwner(resourceOwnership.getObjectOwner());
789789
requestOwnership.setHostsOwner(resourceOwnership.getHostsOwner());
790790
return requestOwnership;
791-
} else if (!resourceOwner.equalsIgnoreCase(publicKeysOwner)) {
791+
} else if (!resourceOwnerWithoutForceSuffix.equalsIgnoreCase(publicKeysOwner)) {
792792
throw Utils.conflictError("Invalid resource member owner for service: " + service.getName()
793-
+ ", " + publicKeysOwner + " vs. " + resourceOwner, caller);
793+
+ ", " + publicKeysOwner + " vs. " + resourceOwnerWithoutForceSuffix, caller);
794794
} else {
795795
// no changes needed
796796
return null;

‎libs/java/server_common/src/test/java/com/yahoo/athenz/common/server/util/ResourceOwnershipTest.java

+56
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,16 @@ public void testVerifyPolicyResourceOwnership() {
638638
fail();
639639
}
640640

641+
// no-op needed when resource ownership is same and force overridden
642+
policy = new Policy().setName("policy1").setResourceOwnership(new ResourcePolicyOwnership().setObjectOwner("TF2"));
643+
try {
644+
ownership = ResourceOwnership.verifyPolicyResourceOwnership(policy, true, "TF2:force", "unit-test");
645+
assertNotNull(ownership);
646+
assertEquals(ownership.getAssertionsOwner(), "TF2");
647+
} catch (ServerResourceException ignored) {
648+
fail();
649+
}
650+
641651
// exception is thrown when resource ownership is different and not force overridden
642652
policy = new Policy().setName("policy1").setResourceOwnership(new ResourcePolicyOwnership().setObjectOwner("TF1"));
643653
try {
@@ -776,6 +786,15 @@ public void testVerifyGroupMetaResourceOwnership() {
776786
fail();
777787
}
778788

789+
// no-op when resource ownership is same and force overridden
790+
group = new Group().setName("group1").setResourceOwnership(new ResourceGroupOwnership().setMetaOwner("TF2"));
791+
try {
792+
ownership = ResourceOwnership.verifyGroupMetaResourceOwnership(group, "TF2:force", "unit-test");
793+
assertNull(ownership);
794+
} catch (ServerResourceException ignored) {
795+
fail();
796+
}
797+
779798
// exception is thrown when resource ownership is different and not force overridden
780799
group = new Group().setName("group1").setResourceOwnership(new ResourceGroupOwnership().setMetaOwner("TF1"));
781800
try {
@@ -845,6 +864,16 @@ public void testVerifyGroupResourceOwnership() {
845864
fail();
846865
}
847866

867+
// no-op when resource ownership is force overridden with the same value
868+
group = new Group().setName("group1").setResourceOwnership(new ResourceGroupOwnership().setObjectOwner("TF2"));
869+
try {
870+
ownership = ResourceOwnership.verifyGroupResourceOwnership(group, true, "TF2:force", "unit-test");
871+
assertNotNull(ownership);
872+
assertEquals(ownership.getObjectOwner(), "TF2");
873+
} catch (ServerResourceException ignored) {
874+
fail();
875+
}
876+
848877
// exception is thrown when resource ownership is different and not force overridden
849878
group = new Group().setName("group1").setResourceOwnership(new ResourceGroupOwnership().setObjectOwner("TF1"));
850879
try {
@@ -914,6 +943,16 @@ public void testVerifyRoleResourceOwnership() {
914943
fail();
915944
}
916945

946+
// no-op when resource ownership is force overridden with the same value
947+
role = new Role().setName("role1").setResourceOwnership(new ResourceRoleOwnership().setObjectOwner("TF2"));
948+
try {
949+
ownership = ResourceOwnership.verifyRoleResourceOwnership(role, true, "TF2:force", "unit-test");
950+
assertNotNull(ownership);
951+
assertEquals(ownership.getObjectOwner(), "TF2");
952+
} catch (ServerResourceException ignored) {
953+
fail();
954+
}
955+
917956
// exception is thrown when resource ownership is different and not force overridden
918957
role = new Role().setName("role1").setResourceOwnership(new ResourceRoleOwnership().setObjectOwner("TF1"));
919958
try {
@@ -983,6 +1022,15 @@ public void testVerifyRoleMemberResourceOwnership() {
9831022
fail();
9841023
}
9851024

1025+
// no-op when second time called with same owner
1026+
role = new Role().setName("role1").setResourceOwnership(new ResourceRoleOwnership().setMembersOwner("TF2"));
1027+
try {
1028+
ownership = ResourceOwnership.verifyRoleMembersResourceOwnership(role, "TF2:force", "unit-test");
1029+
assertNull(ownership);
1030+
} catch (ServerResourceException ignored) {
1031+
fail();
1032+
}
1033+
9861034
// exception is thrown when resource ownership is different and not force overridden
9871035
role = new Role().setName("role1").setResourceOwnership(new ResourceRoleOwnership().setMembersOwner("TF1"));
9881036
try {
@@ -1052,6 +1100,14 @@ public void testVerifyRoleMetaResourceOwnership() {
10521100
fail();
10531101
}
10541102

1103+
// no-op when second time called with same owner
1104+
role = new Role().setName("role1").setResourceOwnership(new ResourceRoleOwnership().setMetaOwner("TF1"));
1105+
try {
1106+
ResourceOwnership.verifyRoleMetaResourceOwnership(role, "TF2:force", "unit-test");
1107+
} catch (ServerResourceException sre) {
1108+
fail();
1109+
}
1110+
10551111
// exception is thrown when resource ownership is different and not force overridden
10561112
role = new Role().setName("role1").setResourceOwnership(new ResourceRoleOwnership().setMetaOwner("TF1"));
10571113
try {

0 commit comments

Comments
 (0)