Allow IamPolicy to read json policies where the principal value itsel…

…f an array (#4406) * Allow IamPolicy to read json policies where the principal value is itself an array * Change name of method that parses strings or arrays
aws · Sep 12, 2023 · 38075ec · 38075ec
1 parent 68d6b53
commit 38075ec
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 8 deletions.
diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-72bd387.json b/.changes/next-release/bugfix-AWSSDKforJavav2-72bd387.json
@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "AWS SDK for Java v2",
+    "contributor": "",
+    "description": "Allow IamPolicy to read json policies where the principal value is itself an array"
+}
diff --git a/...c/main/java/software/amazon/awssdk/policybuilder/iam/internal/DefaultIamPolicyReader.java b/...c/main/java/software/amazon/awssdk/policybuilder/iam/internal/DefaultIamPolicyReader.java
@@ -111,10 +111,10 @@ private IamStatement readStatement(Map<String, JsonNode> statementObject) {
                            .effect(getString(statementObject, "Effect"))
                            .principals(readPrincipals(statementObject, "Principal"))
                            .notPrincipals(readPrincipals(statementObject, "NotPrincipal"))
-                           .actionIds(readStringArray(statementObject, "Action"))
-                           .notActionIds(readStringArray(statementObject, "NotAction"))
-                           .resourceIds(readStringArray(statementObject, "Resource"))
-                           .notResourceIds(readStringArray(statementObject, "NotResource"))
+                           .actionIds(readStringOrArrayAsList(statementObject, "Action"))
+                           .notActionIds(readStringOrArrayAsList(statementObject, "NotAction"))
+                           .resourceIds(readStringOrArrayAsList(statementObject, "Resource"))
+                           .notResourceIds(readStringOrArrayAsList(statementObject, "NotResource"))
                            .conditions(readConditions(statementObject.get("Condition")))
                            .build();
     }
@@ -132,9 +132,10 @@ private List<IamPrincipal> readPrincipals(Map<String, JsonNode> statementObject,
 
         if (principalsNode.isObject()) {
             List<IamPrincipal> result = new ArrayList<>();
-            principalsNode.asObject().forEach((id, value) -> {
-                result.add(IamPrincipal.create(id, expectString(value, name + " entry value")));
-            });
+            Map<String, JsonNode> principalsNodeObject = principalsNode.asObject();
+            principalsNodeObject.keySet().forEach(
+                k -> result.addAll(IamPrincipal.createAll(k, readStringOrArrayAsList(principalsNodeObject, k)))
+            );
             return result;
         }
 
@@ -170,7 +171,7 @@ private List<IamCondition> readConditions(JsonNode conditionNode) {
         return result;
     }
 
-    private List<String> readStringArray(Map<String, JsonNode> statementObject, String nodeKey) {
+    private List<String> readStringOrArrayAsList(Map<String, JsonNode> statementObject, String nodeKey) {
         JsonNode node = statementObject.get(nodeKey);
 
         if (node == null) {

diff --git a/...y-builder/src/test/java/software/amazon/awssdk/policybuilder/iam/IamPolicyReaderTest.java b/...y-builder/src/test/java/software/amazon/awssdk/policybuilder/iam/IamPolicyReaderTest.java
@@ -24,8 +24,10 @@
 
 class IamPolicyReaderTest {
     private static final IamPrincipal PRINCIPAL_1 = IamPrincipal.create("P1", "*");
+    private static final IamPrincipal PRINCIPAL_1B = IamPrincipal.create("P1", "B");
     private static final IamPrincipal PRINCIPAL_2 = IamPrincipal.create("P2", "*");
     private static final IamPrincipal NOT_PRINCIPAL_1 = IamPrincipal.create("NP1", "*");
+    private static final IamPrincipal NOT_PRINCIPAL_1B = IamPrincipal.create("NP1", "B");
     private static final IamPrincipal NOT_PRINCIPAL_2 = IamPrincipal.create("NP2", "*");
     private static final IamResource RESOURCE_1 = IamResource.create("R1");
     private static final IamResource RESOURCE_2 = IamResource.create("R2");
@@ -87,6 +89,20 @@ class IamPolicyReaderTest {
                  .statements(singletonList(ONE_ELEMENT_LISTS_STATEMENT))
                  .build();
 
+    private static final IamStatement COMPOUND_PRINCIPAL_STATEMENT =
+        IamStatement.builder()
+                    .effect(ALLOW)
+                    .sid("Sid")
+                    .principals(asList(PRINCIPAL_1, PRINCIPAL_1B))
+                    .notPrincipals(asList(NOT_PRINCIPAL_1, NOT_PRINCIPAL_1B))
+                    .build();
+
+    private static final IamPolicy COMPOUND_PRINCIPAL_POLICY =
+        IamPolicy.builder()
+                 .version("Version")
+                 .statements(singletonList(COMPOUND_PRINCIPAL_STATEMENT))
+                 .build();
+
     private static final IamPolicyReader READER = IamPolicyReader.create();
 
     @Test
@@ -158,6 +174,31 @@ public void readMinimalPolicyWorks() {
             .isEqualTo(MINIMAL_POLICY);
     }
 
+    @Test
+    public void readCompoundPrincipalsWorks() {
+        assertThat(READER.read("{\n" +
+                           "    \"Version\": \"Version\",\n" +
+                           "    \"Statement\": [\n" +
+                           "        {\n" +
+                           "            \"Sid\": \"Sid\",\n" +
+                           "            \"Effect\": \"Allow\",\n" +
+                           "            \"Principal\": {\n" +
+                           "                \"P1\": [\n" +
+                           "                    \"*\",\n" +
+                           "                    \"B\"\n" +
+                           "                ]\n" +
+                           "            },\n" +
+                           "            \"NotPrincipal\": {\n" +
+                           "                \"NP1\": [\n" +
+                           "                    \"*\",\n" +
+                           "                    \"B\"\n" +
+                           "                ]\n" +
+                           "            }\n" +
+                           "        }\n" +
+                           "    ]\n" +
+                           "}")).isEqualTo(COMPOUND_PRINCIPAL_POLICY);
+    }
+
     @Test
     public void singleElementListsAreSupported() {
         assertThat(READER.read("{\n"