Skip to content

Commit

Permalink
Fixed bug in IAM policy builder where actions were being written inst…
Browse files Browse the repository at this point in the history 
…ead of resources. (#4223)

Also fixed javadoc issue, where create() doesn't exist on the IAM client.
  • Loading branch information
@millems
millems committed Jul 24, 2023
1 parent a48f2fc commit 87d2ffc
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 84 deletions.
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-3bb4487.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "AWS IAM Policy Builder",
"contributor": "",
"description": "Fixed bug where actions were written instead of resources."
}
4 changes: 2 additions & 2 deletions .../iam-policy-builder/src/main/java/software/amazon/awssdk/policybuilder/iam/IamPolicy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
* <b>Create a new IAM identity policy that allows a role to write items to an Amazon DynamoDB table.</b>
* {@snippet :
* // IamClient requires a dependency on software.amazon.awssdk:iam
* try (IamClient iam = IamClient.create()) {
* try (IamClient iam = IamClient.builder().region(Region.AWS_GLOBAL).build()) {
* IamPolicy policy =
* IamPolicy.builder()
* .addStatement(IamStatement.builder()
Expand All @@ -73,7 +73,7 @@
* <b>Download the policy uploaded in the previous example and create a new policy with "read" access added to it.</b>
* {@snippet :
* // IamClient requires a dependency on software.amazon.awssdk:iam
* try (IamClient iam = IamClient.create()) {
* try (IamClient iam = IamClient.builder().region(Region.AWS_GLOBAL).build()) {
* String policyArn = "arn:aws:iam::123456789012:policy/AllowWriteBookMetadata";
* GetPolicyResponse getPolicyResponse = iam.getPolicy(r -> r.policyArn(policyArn));
*
Expand Down
2 changes: 1 addition & 1 deletion ...olicy-builder/src/main/java/software/amazon/awssdk/policybuilder/iam/IamPolicyReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
* <b>Log the number of statements in a policy downloaded from IAM.</b>
* {@snippet :
* // IamClient requires a dependency on software.amazon.awssdk:iam
* try (IamClient iam = IamClient.create()) {
* try (IamClient iam = IamClient.builder().region(Region.AWS_GLOBAL).build()) {
* String policyArn = "arn:aws:iam::123456789012:policy/AllowWriteBookMetadata";
* GetPolicyResponse getPolicyResponse = iam.getPolicy(r -> r.policyArn(policyArn));
*
Expand Down
2 changes: 1 addition & 1 deletion ...olicy-builder/src/main/java/software/amazon/awssdk/policybuilder/iam/IamPolicyWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* <b>Create a new IAM identity policy that allows a role to write items to an Amazon DynamoDB table.</b>
* {@snippet :
* // IamClient requires a dependency on software.amazon.awssdk:iam
* try (IamClient iam = IamClient.create()) {
* try (IamClient iam = IamClient.builder().region(Region.AWS_GLOBAL).build()) {
* IamPolicy policy =
* IamPolicy.builder()
* .addStatement(IamStatement.builder()
Expand Down
2 changes: 1 addition & 1 deletion ...c/main/java/software/amazon/awssdk/policybuilder/iam/internal/DefaultIamPolicyWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ private void writeStatement(JsonWriter writer, IamStatement statement) {
writePrincipals(writer, "NotPrincipal", statement.notPrincipals());
writeValueArrayField(writer, "Action", statement.actions());
writeValueArrayField(writer, "NotAction", statement.notActions());
writeValueArrayField(writer, "Resource", statement.actions());
writeValueArrayField(writer, "Resource", statement.resources());
writeValueArrayField(writer, "NotResource", statement.notResources());
writeConditions(writer, statement.conditions());
writer.writeEndObject();
Expand Down
87 changes: 41 additions & 46 deletions ...y-builder/src/test/java/software/amazon/awssdk/policybuilder/iam/IamPolicyReaderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,18 @@
import org.junit.jupiter.api.Test;

class IamPolicyReaderTest {
private static final IamPrincipal PRINCIPAL_1 = IamPrincipal.ALL;
private static final IamPrincipal PRINCIPAL_2 = IamPrincipal.create("2", "*");
private static final IamResource RESOURCE_1 = IamResource.create("1");
private static final IamResource RESOURCE_2 = IamResource.create("2");
private static final IamAction ACTION_1 = IamAction.create("1");
private static final IamAction ACTION_2 = IamAction.create("2");
private static final IamPrincipal PRINCIPAL_1 = IamPrincipal.create("P1", "*");
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_2 = IamPrincipal.create("NP2", "*");
private static final IamResource RESOURCE_1 = IamResource.create("R1");
private static final IamResource RESOURCE_2 = IamResource.create("R2");
private static final IamResource NOT_RESOURCE_1 = IamResource.create("NR1");
private static final IamResource NOT_RESOURCE_2 = IamResource.create("NR2");
private static final IamAction ACTION_1 = IamAction.create("A1");
private static final IamAction ACTION_2 = IamAction.create("A2");
private static final IamAction NOT_ACTION_1 = IamAction.create("NA1");
private static final IamAction NOT_ACTION_2 = IamAction.create("NA2");
private static final IamCondition CONDITION_1 = IamCondition.create("1", "K1", "V1");
private static final IamCondition CONDITION_2 = IamCondition.create("1", "K2", "V1");
private static final IamCondition CONDITION_3 = IamCondition.create("1", "K2", "V2");
Expand All @@ -39,11 +45,11 @@ class IamPolicyReaderTest {
.effect(ALLOW)
.sid("Sid")
.principals(asList(PRINCIPAL_1, PRINCIPAL_2))
.notPrincipals(asList(PRINCIPAL_1, PRINCIPAL_2))
.notPrincipals(asList(NOT_PRINCIPAL_1, NOT_PRINCIPAL_2))
.resources(asList(RESOURCE_1, RESOURCE_2))
.notResources(asList(RESOURCE_1, RESOURCE_2))
.notResources(asList(NOT_RESOURCE_1, NOT_RESOURCE_2))
.actions(asList(ACTION_1, ACTION_2))
.notActions(asList(ACTION_1, ACTION_2))
.notActions(asList(NOT_ACTION_1, NOT_ACTION_2))
.conditions(asList(CONDITION_1, CONDITION_2, CONDITION_3, CONDITION_4))
.build();

Expand All @@ -66,12 +72,12 @@ class IamPolicyReaderTest {
IamStatement.builder()
.effect(ALLOW)
.sid("Sid")
.principals(singletonList(PRINCIPAL_1))
.notPrincipals(singletonList(PRINCIPAL_1))
.principals(singletonList(IamPrincipal.ALL))
.notPrincipals(singletonList(IamPrincipal.ALL))
.resources(singletonList(RESOURCE_1))
.notResources(singletonList(RESOURCE_1))
.notResources(singletonList(NOT_RESOURCE_1))
.actions(singletonList(ACTION_1))
.notActions(singletonList(ACTION_1))
.notActions(singletonList(NOT_ACTION_1))
.conditions(singletonList(CONDITION_1))
.build();

Expand All @@ -85,35 +91,24 @@ class IamPolicyReaderTest {

@Test
public void readFullPolicyWorks() {
assertThat(READER.read("{\"Version\":\"Version\","
+ "\"Id\":\"Id\","
+ "\"Statement\":["
+ "{\"Sid\":\"Sid\",\"Effect\":\"Allow\",\"Principal\":{\"*\":\"*\",\"2\":\"*\"},\"NotPrincipal\":{\"*\":\"*\",\"2\":\"*\"},\"Action\":[\"1\",\"2\"],\"NotAction\":[\"1\",\"2\"],\"Resource\":[\"1\",\"2\"],\"NotResource\":[\"1\",\"2\"],\"Condition\":{\"1\":{\"K1\":\"V1\",\"K2\":[\"V1\",\"V2\"]},\"2\":{\"K1\":\"V1\"}}},"
+ "{\"Sid\":\"Sid\",\"Effect\":\"Allow\",\"Principal\":{\"*\":\"*\",\"2\":\"*\"},\"NotPrincipal\":{\"*\":\"*\",\"2\":\"*\"},\"Action\":[\"1\",\"2\"],\"NotAction\":[\"1\",\"2\"],\"Resource\":[\"1\",\"2\"],\"NotResource\":[\"1\",\"2\"],\"Condition\":{\"1\":{\"K1\":\"V1\",\"K2\":[\"V1\",\"V2\"]},\"2\":{\"K1\":\"V1\"}}}"
+ "]}"))
.isEqualTo(FULL_POLICY);
}

@Test
public void prettyWriteFullPolicyWorks() {
assertThat(READER.read("{\n"
+ " \"Version\" : \"Version\",\n"
+ " \"Id\" : \"Id\",\n"
+ " \"Statement\" : [ {\n"
+ " \"Sid\" : \"Sid\",\n"
+ " \"Effect\" : \"Allow\",\n"
+ " \"Principal\" : {\n"
+ " \"*\" : \"*\",\n"
+ " \"2\" : \"*\"\n"
+ " \"P1\" : \"*\",\n"
+ " \"P2\" : \"*\"\n"
+ " },\n"
+ " \"NotPrincipal\" : {\n"
+ " \"*\" : \"*\",\n"
+ " \"2\" : \"*\"\n"
+ " \"NP1\" : \"*\",\n"
+ " \"NP2\" : \"*\"\n"
+ " },\n"
+ " \"Action\" : [ \"1\", \"2\" ],\n"
+ " \"NotAction\" : [ \"1\", \"2\" ],\n"
+ " \"Resource\" : [ \"1\", \"2\" ],\n"
+ " \"NotResource\" : [ \"1\", \"2\" ],\n"
+ " \"Action\" : [ \"A1\", \"A2\" ],\n"
+ " \"NotAction\" : [ \"NA1\", \"NA2\" ],\n"
+ " \"Resource\" : [ \"R1\", \"R2\" ],\n"
+ " \"NotResource\" : [ \"NR1\", \"NR2\" ],\n"
+ " \"Condition\" : {\n"
+ " \"1\" : {\n"
+ " \"K1\" : \"V1\",\n"
Expand All @@ -127,17 +122,17 @@ public void prettyWriteFullPolicyWorks() {
+ " \"Sid\" : \"Sid\",\n"
+ " \"Effect\" : \"Allow\",\n"
+ " \"Principal\" : {\n"
+ " \"*\" : \"*\",\n"
+ " \"2\" : \"*\"\n"
+ " \"P1\" : \"*\",\n"
+ " \"P2\" : \"*\"\n"
+ " },\n"
+ " \"NotPrincipal\" : {\n"
+ " \"*\" : \"*\",\n"
+ " \"2\" : \"*\"\n"
+ " \"NP1\" : \"*\",\n"
+ " \"NP2\" : \"*\"\n"
+ " },\n"
+ " \"Action\" : [ \"1\", \"2\" ],\n"
+ " \"NotAction\" : [ \"1\", \"2\" ],\n"
+ " \"Resource\" : [ \"1\", \"2\" ],\n"
+ " \"NotResource\" : [ \"1\", \"2\" ],\n"
+ " \"Action\" : [ \"A1\", \"A2\" ],\n"
+ " \"NotAction\" : [ \"NA1\", \"NA2\" ],\n"
+ " \"Resource\" : [ \"R1\", \"R2\" ],\n"
+ " \"NotResource\" : [ \"NR1\", \"NR2\" ],\n"
+ " \"Condition\" : {\n"
+ " \"1\" : {\n"
+ " \"K1\" : \"V1\",\n"
Expand All @@ -153,7 +148,7 @@ public void prettyWriteFullPolicyWorks() {
}

@Test
public void writeMinimalPolicyWorks() {
public void readMinimalPolicyWorks() {
assertThat(READER.read("{\n"
+ " \"Version\" : \"Version\",\n"
+ " \"Statement\" : {\n"
Expand All @@ -164,18 +159,18 @@ public void writeMinimalPolicyWorks() {
}

@Test
public void singleElementListsAreWrittenAsNonArrays() {
public void singleElementListsAreSupported() {
assertThat(READER.read("{\n"
+ " \"Version\" : \"Version\",\n"
+ " \"Statement\" : {\n"
+ " \"Sid\" : \"Sid\",\n"
+ " \"Effect\" : \"Allow\",\n"
+ " \"Principal\" : \"*\",\n"
+ " \"NotPrincipal\" : \"*\",\n"
+ " \"Action\" : \"1\",\n"
+ " \"NotAction\" : \"1\",\n"
+ " \"Resource\" : \"1\",\n"
+ " \"NotResource\" : \"1\",\n"
+ " \"Action\" : \"A1\",\n"
+ " \"NotAction\" : \"NA1\",\n"
+ " \"Resource\" : \"R1\",\n"
+ " \"NotResource\" : \"NR1\",\n"
+ " \"Condition\" : {\n"
+ " \"1\" : {\n"
+ " \"K1\" : \"V1\"\n"
Expand Down
Loading

0 comments on commit 87d2ffc

Please sign in to comment.