fmorg-git commented on code in PR #9306:
URL: https://github.com/apache/ozone/pull/9306#discussion_r2566388931
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/iam/IamSessionPolicyResolver.java:
##########
@@ -352,21 +369,255 @@ static Set<S3Action>
mapPolicyActionsToS3Actions(Set<String> actions) {
* <p>
* It also validates that the Resource Arn(s) are valid and supported.
*/
- private static Set<ResourceSpec>
validateAndCategorizeResources(AuthorizerType authorizerType,
+ @VisibleForTesting
+ static Set<ResourceSpec> validateAndCategorizeResources(AuthorizerType
authorizerType,
Set<String> resources) throws OMException {
- // TODO implement in future PR
- return Collections.emptySet();
+ final Set<ResourceSpec> resourceSpecs = new HashSet<>();
+ if (resources.isEmpty()) {
+ throw new OMException("No Resource(s) found in policy", INVALID_REQUEST);
+ }
+ for (String resource : resources) {
+ if ("*".equals(resource)) {
Review Comment:
yes, that's correct. "*" is used for s3:ListAllMyBuckets
(https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies.html#policies_session).
But I just learned that apparently "arn:aws:s3:::*" is also used for
s3:ListAllMyBuckets (according to
https://docs.aws.amazon.com/AmazonS3/latest/userguide/example-policies-s3.html).
I updated the code to handle the "arn:aws:s3:::*" edge case.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/iam/IamSessionPolicyResolver.java:
##########
@@ -352,21 +369,255 @@ static Set<S3Action>
mapPolicyActionsToS3Actions(Set<String> actions) {
* <p>
* It also validates that the Resource Arn(s) are valid and supported.
*/
- private static Set<ResourceSpec>
validateAndCategorizeResources(AuthorizerType authorizerType,
+ @VisibleForTesting
+ static Set<ResourceSpec> validateAndCategorizeResources(AuthorizerType
authorizerType,
Set<String> resources) throws OMException {
- // TODO implement in future PR
- return Collections.emptySet();
+ final Set<ResourceSpec> resourceSpecs = new HashSet<>();
+ if (resources.isEmpty()) {
+ throw new OMException("No Resource(s) found in policy", INVALID_REQUEST);
+ }
+ for (String resource : resources) {
+ if ("*".equals(resource)) {
Review Comment:
yes, that's correct. `"*"` is used for s3:ListAllMyBuckets
(https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies.html#policies_session).
But I just learned that apparently `"arn:aws:s3:::*"` is also used for
s3:ListAllMyBuckets (according to
https://docs.aws.amazon.com/AmazonS3/latest/userguide/example-policies-s3.html).
I updated the code to handle the `"arn:aws:s3:::*"` edge case.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/iam/IamSessionPolicyResolver.java:
##########
@@ -352,21 +369,255 @@ static Set<S3Action>
mapPolicyActionsToS3Actions(Set<String> actions) {
* <p>
* It also validates that the Resource Arn(s) are valid and supported.
*/
- private static Set<ResourceSpec>
validateAndCategorizeResources(AuthorizerType authorizerType,
+ @VisibleForTesting
+ static Set<ResourceSpec> validateAndCategorizeResources(AuthorizerType
authorizerType,
Set<String> resources) throws OMException {
- // TODO implement in future PR
- return Collections.emptySet();
+ final Set<ResourceSpec> resourceSpecs = new HashSet<>();
+ if (resources.isEmpty()) {
+ throw new OMException("No Resource(s) found in policy", INVALID_REQUEST);
+ }
+ for (String resource : resources) {
+ if ("*".equals(resource)) {
+ validateNativeAuthorizerBucketPattern(authorizerType, "*");
+ resourceSpecs.add(ResourceSpec.any());
+ continue;
+ }
+
+ if (!resource.startsWith(AWS_S3_ARN_PREFIX)) {
+ throw new OMException("Unsupported Resource Arn - " + resource,
NOT_SUPPORTED_OPERATION);
+ }
+
+ final String suffix = resource.substring(AWS_S3_ARN_PREFIX.length());
+ if (suffix.isEmpty()) {
+ throw new OMException("Invalid Resource Arn - " + resource,
INVALID_REQUEST);
+ }
+
+ ResourceSpec spec = parseResourceSpec(suffix);
+ if (spec.type == S3ResourceType.BUCKET_WILDCARD) {
+ validateNativeAuthorizerBucketPattern(authorizerType, spec.bucket);
+ }
+
+ // This scenario can happen in the case of arn:aws:s3:::*/* or
arn:aws:s3:::*/test.txt for
+ // examples
+ validateNativeAuthorizerBucketPattern(authorizerType, spec.bucket);
+
+ if (authorizerType == AuthorizerType.NATIVE && spec.type ==
S3ResourceType.OBJECT_PREFIX_WILDCARD) {
+ if (spec.prefix.endsWith("*")) {
Review Comment:
I didn't follow - could you please give an example? Is there an edge case
the unit tests aren't covering?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
