Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-IAMPolicyBuilder-60dd1f5.json
Original file line number Diff line number Diff line change
@@ -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`."
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,10 @@ private IamStatement readStatement(Map<String, JsonNode> 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();
}
Expand All @@ -134,7 +134,7 @@ private List<IamPrincipal> readPrincipals(Map<String, JsonNode> statementObject,
List<IamPrincipal> result = new ArrayList<>();
Map<String, JsonNode> 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;
}
Expand All @@ -154,15 +154,15 @@ private List<IamCondition> readConditions(JsonNode conditionNode) {
conditionObject.forEach((operator, keyValueNode) -> {
Map<String, JsonNode> 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<String> 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")));
}
});

Expand All @@ -171,7 +171,7 @@ private List<IamCondition> readConditions(JsonNode conditionNode) {
return result;
}

private List<String> readStringOrArrayAsList(Map<String, JsonNode> statementObject, String nodeKey) {
private List<String> readStringOrArrayAsList(Map<String, JsonNode> statementObject, String nodeKey, boolean allowAccountIds) {
JsonNode node = statementObject.get(nodeKey);

if (node == null) {
Expand All @@ -185,7 +185,12 @@ private List<String> readStringOrArrayAsList(Map<String, JsonNode> 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());
}

Expand All @@ -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<String, JsonNode> expectObject(JsonNode node, String name) {
Validate.isTrue(node.isObject(), "%s was not an object", name);
return node.asObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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" +
Expand Down
Loading