diff --git a/.changes/next-release/bugfix-IAMPolicyBuilder-60dd1f5.json b/.changes/next-release/bugfix-IAMPolicyBuilder-60dd1f5.json new file mode 100644 index 000000000000..d16337350275 --- /dev/null +++ b/.changes/next-release/bugfix-IAMPolicyBuilder-60dd1f5.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "IAM Policy Builder", + "contributor": "", + "description": "Allow integer AWS account IDs and boolean values when reading IAM policies from JSON with `IamPolicyReader`." +} diff --git a/services-custom/iam-policy-builder/src/main/java/software/amazon/awssdk/policybuilder/iam/internal/DefaultIamPolicyReader.java b/services-custom/iam-policy-builder/src/main/java/software/amazon/awssdk/policybuilder/iam/internal/DefaultIamPolicyReader.java index 51f217ad7dfb..4592e8c1c7d2 100644 --- a/services-custom/iam-policy-builder/src/main/java/software/amazon/awssdk/policybuilder/iam/internal/DefaultIamPolicyReader.java +++ b/services-custom/iam-policy-builder/src/main/java/software/amazon/awssdk/policybuilder/iam/internal/DefaultIamPolicyReader.java @@ -111,10 +111,10 @@ private IamStatement readStatement(Map statementObject) { .effect(getString(statementObject, "Effect")) .principals(readPrincipals(statementObject, "Principal")) .notPrincipals(readPrincipals(statementObject, "NotPrincipal")) - .actionIds(readStringOrArrayAsList(statementObject, "Action")) - .notActionIds(readStringOrArrayAsList(statementObject, "NotAction")) - .resourceIds(readStringOrArrayAsList(statementObject, "Resource")) - .notResourceIds(readStringOrArrayAsList(statementObject, "NotResource")) + .actionIds(readStringOrArrayAsList(statementObject, "Action", false)) + .notActionIds(readStringOrArrayAsList(statementObject, "NotAction", false)) + .resourceIds(readStringOrArrayAsList(statementObject, "Resource", false)) + .notResourceIds(readStringOrArrayAsList(statementObject, "NotResource", false)) .conditions(readConditions(statementObject.get("Condition"))) .build(); } @@ -134,7 +134,7 @@ private List readPrincipals(Map statementObject, List result = new ArrayList<>(); Map principalsNodeObject = principalsNode.asObject(); principalsNodeObject.keySet().forEach( - k -> result.addAll(IamPrincipal.createAll(k, readStringOrArrayAsList(principalsNodeObject, k))) + k -> result.addAll(IamPrincipal.createAll(k, readStringOrArrayAsList(principalsNodeObject, k, true))) ); return result; } @@ -154,15 +154,15 @@ private List readConditions(JsonNode conditionNode) { conditionObject.forEach((operator, keyValueNode) -> { Map keyValueObject = expectObject(keyValueNode, "Condition key"); keyValueObject.forEach((key, value) -> { - if (value.isString()) { - result.add(IamCondition.create(operator, key, value.asString())); - } else if (value.isArray()) { + if (value.isArray()) { List values = value.asArray() .stream() - .map(valueNode -> expectString(valueNode, "Condition values entry")) + .map(valueNode -> expectConditionValue(valueNode, "Condition values entry")) .collect(toList()); result.addAll(IamCondition.createAll(operator, key, values)); + } else { + result.add(IamCondition.create(operator, key, expectConditionValue(value, "Condition value entry"))); } }); @@ -171,7 +171,7 @@ private List readConditions(JsonNode conditionNode) { return result; } - private List readStringOrArrayAsList(Map statementObject, String nodeKey) { + private List readStringOrArrayAsList(Map statementObject, String nodeKey, boolean allowAccountIds) { JsonNode node = statementObject.get(nodeKey); if (node == null) { @@ -185,7 +185,12 @@ private List readStringOrArrayAsList(Map statementObje if (node.isArray()) { return node.asArray() .stream() - .map(n -> expectString(n, nodeKey + " entry")) + .map(n -> { + if (allowAccountIds) { + return expectStringOrAccountId(n, nodeKey + " entry"); + } + return expectString(n, nodeKey + " entry"); + }) .collect(toList()); } @@ -206,6 +211,26 @@ private String expectString(JsonNode node, String name) { return node.asString(); } + private String expectStringOrAccountId(JsonNode node, String name) { + if (node.isNumber()) { + return node.asNumber(); + } + Validate.isTrue(node.isString(), "%s was not a string", name); + return node.asString(); + } + + // 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()) { + return node.asNumber(); + } + if (node.isBoolean()) { + return Boolean.toString(node.asBoolean()); + } + Validate.isTrue(node.isString(), "%s was not a string", name); + return node.asString(); + } + private Map expectObject(JsonNode node, String name) { Validate.isTrue(node.isObject(), "%s was not an object", name); return node.asObject(); diff --git a/services-custom/iam-policy-builder/src/test/java/software/amazon/awssdk/policybuilder/iam/IamPolicyReaderTest.java b/services-custom/iam-policy-builder/src/test/java/software/amazon/awssdk/policybuilder/iam/IamPolicyReaderTest.java index 27ff0cd3544a..f828016b1f26 100644 --- a/services-custom/iam-policy-builder/src/test/java/software/amazon/awssdk/policybuilder/iam/IamPolicyReaderTest.java +++ b/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 { @@ -89,6 +92,34 @@ class IamPolicyReaderTest { .statements(singletonList(ONE_ELEMENT_LISTS_STATEMENT)) .build(); + private static final IamPolicy INTEGER_ACCOUNT_ID_POLICY = + IamPolicy.builder() + .version("Version") + .statements(singletonList( + IamStatement + .builder() + .effect("Allow") + .principals( + IamPrincipal.createAll("AWS", asList("123456789012", "555555555555"))) + .addCondition( + IamCondition.create("StringNotEquals", "aws:PrincipalAccount", "123456789012")) + .build() + )) + .build(); + + private static final IamPolicy BOOLEAN_CONDITION_POLICY = + IamPolicy.builder() + .version("Version") + .statements(singletonList( + IamStatement + .builder() + .effect("Allow") + .addCondition( + IamCondition.create("Bool", "aws:SecureTransport", "true")) + .build() + )) + .build(); + private static final IamStatement COMPOUND_PRINCIPAL_STATEMENT = IamStatement.builder() .effect(ALLOW) @@ -174,6 +205,67 @@ public void readMinimalPolicyWorks() { .isEqualTo(MINIMAL_POLICY); } + @Test + public void readPolicyWithIntegerAccountsWorks() { + assertThat(READER.read("{\n" + + " \"Version\" : \"Version\",\n" + + " \"Statement\" : {\n" + + " \"Effect\" : \"Allow\",\n" + + " \"Principal\" : { \n" + + " \"AWS\": [ \n" + + " 123456789012,\n" + + " \"555555555555\" \n" + + " ]\n" + + " },\n" + + " \"Condition\" : {\n" + + " \"StringNotEquals\": {\n" + + " \"aws:PrincipalAccount\": [\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" + + " \"Version\" : \"Version\",\n" + + " \"Statement\" : {\n" + + " \"Effect\" : \"Allow\",\n" + + " \"Condition\" : {\n" + + " \"Bool\": {\n" + + " \"aws:SecureTransport\": true\n" + + " }\n" + + " }\n" + + " }\n" + + "}")).isEqualTo(BOOLEAN_CONDITION_POLICY); + } + @Test public void readCompoundPrincipalsWorks() { assertThat(READER.read("{\n" +