Skip to content

Commit 884c8a7

Browse files
Fix policy reader to properly consume object principles and populate the principles collection
1 parent 41a9184 commit 884c8a7

File tree

3 files changed

+118
-19
lines changed

3 files changed

+118
-19
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"core": {
3+
"updateMinimum": true,
4+
"type": "Patch",
5+
"changeLogMessages": [
6+
"Fix regression where `Amazon.Auth.AccessControlPolicy.Policy.FromJson` was not correctly handling objects in the `Principal` property (https://github.com/aws/aws-sdk-net/issues/3791)"
7+
]
8+
}
9+
}

sdk/src/Core/Amazon.Auth/AccessControlPolicy/Internal/JsonPolicyReader.cs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -97,24 +97,19 @@ private static void convertPrincipals(Statement statement, JsonElement jStatemen
9797
{
9898
if (jPrincipal.Value.ValueKind == JsonValueKind.String)
9999
{
100-
if (jPrincipal.Value.GetString().Equals("*"))
101-
{
102-
statement.Principals.Add(Principal.Anonymous);
103-
}
104-
else
105-
{
106-
// Don't strip '-' and assume the policy being deserialized is already valid.
107-
Principal principal = new Principal(jPrincipal.Name, jPrincipal.Value.GetString());
108-
}
100+
// Don't strip '-' and assume the policy being deserialized is already valid.
101+
Principal principal = new Principal(jPrincipal.Name, jPrincipal.Value.GetString());
102+
statement.Principals.Add(principal);
109103
}
110104
else if (jPrincipal.Value.ValueKind == JsonValueKind.Array)
111105
{
112-
foreach (JsonElement arrayElement in jPrincipals.EnumerateArray())
106+
foreach (JsonElement arrayElement in jPrincipal.Value.EnumerateArray())
113107
{
114108
if (arrayElement.ValueKind == JsonValueKind.String)
115109
{
116110
// Don't strip '-' and assume the policy being deserialized is already valid.
117111
Principal principal = new Principal(jPrincipal.Name, arrayElement.GetString(), false);
112+
statement.Principals.Add(principal);
118113
}
119114
}
120115
}

sdk/test/UnitTests/Custom/PolicyTests.cs

Lines changed: 104 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,17 @@
1-
using System;
2-
using System.Collections.Generic;
3-
using System.Linq;
4-
using System.Text;
5-
using System.Threading.Tasks;
6-
using Microsoft.VisualStudio.TestTools.UnitTesting;
7-
8-
using Amazon.Auth.AccessControlPolicy;
1+
using Amazon.Auth.AccessControlPolicy;
92
using AWSSDK_DotNet.IntegrationTests.Utils;
3+
using Microsoft.VisualStudio.TestTools.UnitTesting;
4+
using System;
5+
using System.Linq;
106
using static Amazon.Auth.AccessControlPolicy.ConditionFactory;
117

128
namespace AWSSDK_DotNet.UnitTests
139
{
1410
[TestClass]
11+
[TestCategory("Runtime")]
1512
public class PolicyTests
1613
{
1714
[TestMethod]
18-
[TestCategory("Runtime")]
1915
public void TestIfStatementAlreadyExists()
2016
{
2117
var policy = new Policy();
@@ -154,5 +150,104 @@ public void LookForStringComparisonTypeChanges()
154150
expectedHash,
155151
"The Amazon.Auth.AccessControlPolicy.ConditionFactory.ToString(DateComparisonType) method implementation may need to be updated.");
156152
}
153+
154+
[TestMethod]
155+
public void HandleObjectsWhenConvertingPrincipals()
156+
{
157+
string testPolicy = @"{
158+
""Version"": ""2012-10-17"",
159+
""Statement"": [
160+
{
161+
""Effect"": ""Deny"",
162+
""Action"": ""s3:*"",
163+
""Principal"": {
164+
""AWS"": [
165+
""arn:aws:iam::123456789012:root"",
166+
""999999999999""
167+
],
168+
""CanonicalUser"": ""79a59df900b949e55d96a1e698fbacedfd6e09d98eacf8f8d5218e7cd47ef2be""
169+
},
170+
""Resource"": [
171+
""arn:aws:s3:::amzn-s3-demo-bucket/*"",
172+
""arn:aws:s3:::amzn-s3-demo-bucket""
173+
],
174+
""Condition"": {
175+
""ArnNotEquals"": {
176+
""aws:PrincipalArn"": ""arn:aws:iam::444455556666:user/user-name""
177+
}
178+
}
179+
}
180+
]
181+
}";
182+
183+
var policy = Policy.FromJson(testPolicy);
184+
Assert.IsNotNull(policy);
185+
Assert.AreEqual(1, policy.Statements.Count);
186+
187+
var statement = policy.Statements.First();
188+
Assert.AreEqual(statement.Actions.Count, 1);
189+
Assert.AreEqual(statement.Principals.Count, 3);
190+
Assert.AreEqual(statement.Conditions.Count, 1);
191+
}
192+
193+
[TestMethod]
194+
public void HandleSingleValueWhenConvertingPrincipals()
195+
{
196+
string testPolicy = @"{
197+
""Version"": ""2012-10-17"",
198+
""Statement"": [
199+
{
200+
""Effect"": ""Allow"",
201+
""Action"": ""s3:ListBucket"",
202+
""Principal"": {
203+
""AWS"": ""123456789012""
204+
}
205+
}
206+
]
207+
}";
208+
209+
var policy = Policy.FromJson(testPolicy);
210+
Assert.IsNotNull(policy);
211+
Assert.AreEqual(1, policy.Statements.Count);
212+
213+
var statement = policy.Statements.First();
214+
Assert.AreEqual(1, statement.Principals.Count);
215+
216+
var principal = statement.Principals.First();
217+
Assert.AreEqual(Principal.AWS_PROVIDER, principal.Provider);
218+
Assert.AreEqual("123456789012", principal.Id);
219+
}
220+
221+
[TestMethod]
222+
public void HandleAnonymousWhenConvertingPrincipals()
223+
{
224+
string testPolicy = @"{
225+
""Version"": ""2012-10-17"",
226+
""Statement"": [
227+
{
228+
""Effect"": ""Deny"",
229+
""Action"": ""s3:*"",
230+
""Principal"": ""*"",
231+
""Resource"": [
232+
""arn:aws:s3:::amzn-s3-demo-bucket/*"",
233+
""arn:aws:s3:::amzn-s3-demo-bucket""
234+
],
235+
""Condition"": {
236+
""ArnNotEquals"": {
237+
""aws:PrincipalArn"": ""arn:aws:iam::444455556666:user/user-name""
238+
}
239+
}
240+
}
241+
]
242+
}";
243+
244+
var policy = Policy.FromJson(testPolicy);
245+
Assert.IsNotNull(policy);
246+
Assert.AreEqual(1, policy.Statements.Count);
247+
248+
var statement = policy.Statements.First();
249+
Assert.AreEqual(1, statement.Principals.Count);
250+
Assert.AreEqual(Principal.Anonymous, statement.Principals.First());
251+
}
157252
}
158253
}

0 commit comments

Comments
 (0)