Remove support for unquoted accountIds with leading zeros (IAM rejects these)

Author: alextwoods

services-custom/iam-policy-builder/src/main/java/software/amazon/awssdk/policybuilder/iam/internal/DefaultIamPolicyReader.java

@@ -32,9 +32,6 @@
 import software.amazon.awssdk.policybuilder.iam.IamStatement;
 import software.amazon.awssdk.protocols.jsoncore.JsonNode;
 import software.amazon.awssdk.protocols.jsoncore.JsonNodeParser;
-import software.amazon.awssdk.thirdparty.jackson.core.JsonFactory;
-import software.amazon.awssdk.thirdparty.jackson.core.StreamReadFeature;
-import software.amazon.awssdk.thirdparty.jackson.core.json.JsonReadFeature;
 import software.amazon.awssdk.utils.Validate;
 
 /**
@@ -44,16 +41,7 @@
  */
 @SdkInternalApi
 public final class DefaultIamPolicyReader implements IamPolicyReader {
-    private static final JsonNodeParser JSON_NODE_PARSER = JsonNodeParser
-        .builder()
-        .jsonFactory(JsonFactory
-                         .builder()
-                         .enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION)
-                         .configure(JsonReadFeature.ALLOW_JAVA_COMMENTS, true)
-                         // required to handle integer accountIDs with leading zeros
-                         .configure(JsonReadFeature.ALLOW_LEADING_ZEROS_FOR_NUMBERS, true)
-                         .build())
-        .build();
+    private static final JsonNodeParser JSON_NODE_PARSER = JsonNodeParser.create();
 
     @Override
     public IamPolicy read(String policy) {
@@ -225,10 +213,7 @@ private String expectString(JsonNode node, String name) {
 
     private String expectStringOrAccountId(JsonNode node, String name) {
         if (node.isNumber()) {
-            // treat numbers like accountIDs and return a zero padded 12 digit string
-            if (node.asNumber().length() <= 12) {
-                return  String.format("%012d", Long.parseLong(node.asNumber()));
-            }
+            return node.asNumber();
         }
         Validate.isTrue(node.isString(), "%s was not a string", name);
         return node.asString();
@@ -237,10 +222,7 @@ private String expectStringOrAccountId(JsonNode node, String name) {
     // condition values are generally String, however in some cases they may be an AWS accountID or a boolean.
     private String expectConditionValue(JsonNode node, String name) {
         if (node.isNumber()) {
-            // treat numbers like accountIDs and return a zero padded 12 digit string
-            if (node.asNumber().length() <= 12) {
-                return String.format("%012d", Long.parseLong(node.asNumber()));
-            }
+            return node.asNumber();
         }
         if (node.isBoolean()) {
             return Boolean.toString(node.asBoolean());

services-custom/iam-policy-builder/src/test/java/software/amazon/awssdk/policybuilder/iam/IamPolicyReaderTest.java

@@ -18,8 +18,11 @@
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static software.amazon.awssdk.policybuilder.iam.IamEffect.ALLOW;
 
+import java.io.UncheckedIOException;
 import org.junit.jupiter.api.Test;
 
 class IamPolicyReaderTest {
@@ -97,9 +100,9 @@ class IamPolicyReaderTest {
                          .builder()
                          .effect("Allow")
                          .principals(
-                             IamPrincipal.createAll("AWS", asList("001234567890", "555555555555")))
+                             IamPrincipal.createAll("AWS", asList("123456789012", "555555555555")))
                          .addCondition(
-                             IamCondition.create("StringNotEquals", "aws:PrincipalAccount", "001234567890"))
+                             IamCondition.create("StringNotEquals", "aws:PrincipalAccount", "123456789012"))
                          .build()
                  ))
                  .build();
@@ -210,21 +213,44 @@ public void readPolicyWithIntegerAccountsWorks() {
                                   + "    \"Effect\" : \"Allow\",\n"
                                   + "    \"Principal\" : { \n"
                                   + "      \"AWS\": [ \n"
-                                  + "        001234567890,\n"
+                                  + "        123456789012,\n"
                                   + "        \"555555555555\" \n"
                                   + "        ]\n"
                                   + "    },\n"
                                   + "    \"Condition\" : {\n"
                                   + "      \"StringNotEquals\": {\n"
                                   + "          \"aws:PrincipalAccount\": [\n"
-                                  + "              001234567890\n"
+                                  + "              123456789012\n"
                                   + "          ]\n"
                                   + "      }\n"
                                   + "    }\n"
                                   + "  }\n"
                                   + "}")).isEqualTo(INTEGER_ACCOUNT_ID_POLICY);
     }
 
+    @Test
+    public void readPolicyWithLeadingZeroIntegerFails() {
+        // while accountIds may have leading zeros, IAM rejects policy documents with
+        // unquoted accountIDs with leading zeros because these are invalid json.
+        Exception e = assertThrows(Exception.class, () ->
+        {
+            READER.read("{\n"
+                        + "  \"Version\" : \"Version\",\n"
+                        + "  \"Statement\" : {\n"
+                        + "    \"Effect\" : \"Allow\",\n"
+                        + "    \"Condition\" : {\n"
+                        + "      \"StringNotEquals\": {\n"
+                        + "          \"aws:PrincipalAccount\": [\n"
+                        + "              001234567890\n"
+                        + "          ]\n"
+                        + "      }\n"
+                        + "    }\n"
+                        + "  }\n"
+                        + "}");
+        });
+        assertThat(e.getMessage()).contains("Invalid numeric value: Leading zeroes not allowed");
+    }
+
     @Test
     public void readPolicyWithBooleanConditionWorks() {
         assertThat(READER.read("{\n"