@@ -126,7 +126,9 @@ public static Set<AssumeRoleRequest.OzoneGrant> resolve(String policyJson, Strin
126126
127127 validateInputParameters (policyJson , volumeName , authorizerType );
128128
129- final Set <AssumeRoleRequest .OzoneGrant > result = new LinkedHashSet <>();
129+ // Accumulate ACLs across ALL statements using a single map to allow
130+ // cross-statement deduplication and ALL-permission collapsing.
131+ final Map <IOzoneObj , Set <ACLType >> objToAclsMap = new LinkedHashMap <>();
130132
131133 // Parse JSON into set of statements
132134 final Set <JsonNode > statements = parseJsonAndRetrieveStatements (policyJson );
@@ -151,13 +153,11 @@ public static Set<AssumeRoleRequest.OzoneGrant> resolve(String policyJson, Strin
151153 final Set <ResourceSpec > resourceSpecs = validateAndCategorizeResources (authorizerType , resources );
152154
153155 // For each action, map to Ozone objects (paths) and acls based on resource specs and prefixes
154- final Set <AssumeRoleRequest .OzoneGrant > stmtResults = createPathsAndPermissions (
155- volumeName , authorizerType , mappedS3Actions , resourceSpecs , prefixes );
156-
157- result .addAll (stmtResults );
156+ createPathsAndPermissions (volumeName , authorizerType , mappedS3Actions , resourceSpecs , prefixes , objToAclsMap );
158157 }
159158
160- return result ;
159+ // Group accumulated objects by their ACL sets to create final result
160+ return groupObjectsByAcls (objToAclsMap );
161161 }
162162
163163 /**
@@ -418,24 +418,19 @@ static Set<ResourceSpec> validateAndCategorizeResources(AuthorizerType authorize
418418 * entries pairing sets of IOzoneObjs with the requisite permissions granted (if any).
419419 */
420420 @ VisibleForTesting
421- static Set <AssumeRoleRequest .OzoneGrant > createPathsAndPermissions (String volumeName , AuthorizerType authorizerType ,
422- Set <S3Action > mappedS3Actions , Set <ResourceSpec > resourceSpecs , Set <String > prefixes ) {
423- // Create map to collect IOzoneObj to ACLType mappings
424- final Map <IOzoneObj , Set <ACLType >> objToAclsMap = new LinkedHashMap <>();
425-
421+ static void createPathsAndPermissions (String volumeName , AuthorizerType authorizerType , Set <S3Action > mappedS3Actions ,
422+ Set <ResourceSpec > resourceSpecs , Set <String > prefixes , Map <IOzoneObj , Set <ACLType >> objToAclsMap ) {
426423 // Process each resource spec with the given actions
427424 for (ResourceSpec resourceSpec : resourceSpecs ) {
428425 processResourceSpecWithActions (volumeName , authorizerType , mappedS3Actions , resourceSpec , prefixes , objToAclsMap );
429426 }
430-
431- // Group objects by their ACL sets to create proper entries
432- return groupObjectsByAcls (objToAclsMap );
433427 }
434428
435429 /**
436430 * Groups objects by their ACL sets.
437431 */
438- private static Set <AssumeRoleRequest .OzoneGrant > groupObjectsByAcls (Map <IOzoneObj , Set <ACLType >> objToAclsMap ) {
432+ @ VisibleForTesting
433+ static Set <AssumeRoleRequest .OzoneGrant > groupObjectsByAcls (Map <IOzoneObj , Set <ACLType >> objToAclsMap ) {
439434 final Map <Set <ACLType >, Set <IOzoneObj >> groupMap = new LinkedHashMap <>();
440435
441436 // Group objects by their ACL sets only (across resource types)
@@ -526,24 +521,34 @@ private static void processBucketResource(String volumeName, Set<S3Action> mappe
526521 // bucket name of "*". To align with AWS, make sure that in this
527522 // specific case we also grant the volume-level permissions for volume-scoped
528523 // actions (currently s3:ListAllMyBuckets).
529- if (action .kind == ActionKind .BUCKET || action == S3Action . ALL_S3 ||
530- action .kind == ActionKind .VOLUME && "*" .equals (resourceSpec .bucket )) { // this handles s3:ListAllMyBuckets
524+ if (action .kind == ActionKind .BUCKET ||
525+ ( action .kind == ActionKind .VOLUME && "*" .equals (resourceSpec .bucket ) )) { // this handles s3:ListAllMyBuckets
531526 addAclsForObj (objToAclsMap , volumeObj (volumeName ), action .volumePerms );
532527 addAclsForObj (objToAclsMap , bucketObj (volumeName , resourceSpec .bucket ), action .bucketPerms );
528+ } else if (action == S3Action .ALL_S3 ) {
529+ // For s3:*, ALL should only apply at the bucket level; grant READ at volume for navigation
530+ // However, resource "arn:aws:s3:::*" can apply to volume as well (as explained above)
531+ // If the bucket is "*", include the volumePerms, otherwise just include READ for navigation.
532+ if ("*" .equals (resourceSpec .bucket )) {
533+ addAclsForObj (objToAclsMap , volumeObj (volumeName ), action .volumePerms );
534+ } else {
535+ addAclsForObj (objToAclsMap , volumeObj (volumeName ), EnumSet .of (READ ));
536+ }
537+ addAclsForObj (objToAclsMap , bucketObj (volumeName , resourceSpec .bucket ), action .bucketPerms );
533538 }
534539
535- if (action == S3Action .LIST_BUCKET ) {
540+ if (action == S3Action .LIST_BUCKET || action == S3Action . ALL_S3 ) {
536541 // If condition prefixes are present, these would constrain the object permissions if the action
537- // is s3:ListBucket
542+ // is s3:ListBucket or s3:* (which includes s3:ListBucket)
538543 if (prefixes != null && !prefixes .isEmpty ()) {
539544 for (String prefix : prefixes ) {
540545 createObjectResourcesFromConditionPrefix (
541- volumeName , authorizerType , resourceSpec , prefix , objToAclsMap , action . objectPerms );
546+ volumeName , authorizerType , resourceSpec , prefix , objToAclsMap , EnumSet . of ( READ ) );
542547 }
543548 } else {
544549 // No condition prefixes, but we need READ access to all objects, so use "*" as the prefix
545550 createObjectResourcesFromConditionPrefix (
546- volumeName , authorizerType , resourceSpec , "*" , objToAclsMap , action . objectPerms );
551+ volumeName , authorizerType , resourceSpec , "*" , objToAclsMap , EnumSet . of ( READ ) );
547552 }
548553 }
549554 }
@@ -556,11 +561,12 @@ private static void processBucketResource(String volumeName, Set<S3Action> mappe
556561 private static void processObjectExactResource (String volumeName , Set <S3Action > mappedS3Actions ,
557562 ResourceSpec resourceSpec , Map <IOzoneObj , Set <ACLType >> objToAclsMap ) {
558563 for (S3Action action : mappedS3Actions ) {
559- addAclsForObj (objToAclsMap , volumeObj (volumeName ), action .volumePerms );
560564 if (action .kind == ActionKind .OBJECT ) {
565+ addAclsForObj (objToAclsMap , volumeObj (volumeName ), action .volumePerms );
561566 addAclsForObj (objToAclsMap , bucketObj (volumeName , resourceSpec .bucket ), action .bucketPerms );
562567 addAclsForObj (objToAclsMap , keyObj (volumeName , resourceSpec .bucket , resourceSpec .key ), action .objectPerms );
563568 } else if (action == S3Action .ALL_S3 ) {
569+ addAclsForObj (objToAclsMap , volumeObj (volumeName ), EnumSet .of (READ ));
564570 // For s3:*, ALL should only apply at the object level; grant READ at bucket level for navigation
565571 addAclsForObj (objToAclsMap , bucketObj (volumeName , resourceSpec .bucket ), EnumSet .of (READ ));
566572 addAclsForObj (objToAclsMap , keyObj (volumeName , resourceSpec .bucket , resourceSpec .key ), action .objectPerms );
@@ -577,10 +583,11 @@ private static void processObjectPrefixResource(String volumeName, AuthorizerTyp
577583 Set <S3Action > mappedS3Actions , ResourceSpec resourceSpec , Map <IOzoneObj , Set <ACLType >> objToAclsMap ) {
578584 for (S3Action action : mappedS3Actions ) {
579585 // Object actions apply to prefix/key resources
580- addAclsForObj (objToAclsMap , volumeObj (volumeName ), action .volumePerms );
581586 if (action .kind == ActionKind .OBJECT ) {
587+ addAclsForObj (objToAclsMap , volumeObj (volumeName ), action .volumePerms );
582588 addAclsForObj (objToAclsMap , bucketObj (volumeName , resourceSpec .bucket ), action .bucketPerms );
583589 } else if (action == S3Action .ALL_S3 ) {
590+ addAclsForObj (objToAclsMap , volumeObj (volumeName ), EnumSet .of (READ ));
584591 // For s3:*, ALL should only apply at the object/prefix level; grant READ at bucket level for navigation
585592 addAclsForObj (objToAclsMap , bucketObj (volumeName , resourceSpec .bucket ), EnumSet .of (READ ));
586593 }
@@ -633,11 +640,26 @@ private static void createObjectResourcesFromConditionPrefix(String volumeName,
633640
634641 /**
635642 * Helper method to add ACLs for an IOzoneObj, merging with existing ACLs if present.
643+ * If ALL permission is present, no other permissions are added.
636644 */
637645 private static void addAclsForObj (Map <IOzoneObj , Set <ACLType >> objToAclsMap , IOzoneObj obj , Set <ACLType > acls ) {
638646 if (acls != null && !acls .isEmpty ()) {
639647 final OzoneObj ozoneObj = (OzoneObj ) obj ;
640- objToAclsMap .computeIfAbsent (ozoneObj , k -> EnumSet .noneOf (ACLType .class )).addAll (acls );
648+ final Set <ACLType > existingAcls = objToAclsMap .computeIfAbsent (ozoneObj , k -> EnumSet .noneOf (ACLType .class ));
649+
650+ // If ALL is already present, don't add other permissions
651+ if (existingAcls .contains (ACLType .ALL )) {
652+ return ;
653+ }
654+
655+ // If we're about to add ALL, remove all other permissions first
656+ if (acls .contains (ACLType .ALL )) {
657+ existingAcls .clear ();
658+ existingAcls .add (ACLType .ALL );
659+ } else {
660+ // Only add permissions if ALL is not already present
661+ existingAcls .addAll (acls );
662+ }
641663 }
642664 }
643665
@@ -808,7 +830,7 @@ enum S3Action {
808830 EnumSet .of (ACLType .WRITE )),
809831
810832 // Wildcard all
811- ALL_S3 ("s3:*" , ActionKind .ALL , EnumSet .of (READ ), EnumSet .of (ACLType .ALL ), EnumSet .of (ACLType .ALL ));
833+ ALL_S3 ("s3:*" , ActionKind .ALL , EnumSet .of (READ , LIST ), EnumSet .of (ACLType .ALL ), EnumSet .of (ACLType .ALL ));
812834
813835 private final String name ;
814836 private final ActionKind kind ;
0 commit comments