Skip to content

Fixed bug in IAM policy builder where actions were being written instead of resources. #4223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 24, 2023
Merged

Fixed bug in IAM policy builder where actions were being written instead of resources. #4223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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-AWSSDKforJavav2-3bb4487.json
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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