Skip to content

Commit f8f09a8

Browse files
jskeetjskeet
committed
Update IAM helper methods to protect against IAM conditions
The methods throw an exception if: - The policy version is greater than 1 - Any bindings have conditions The exception message will need to be changed to include a link with more information.
1 parent af47eb8 commit f8f09a8

File tree

2 files changed

+56
-0
lines changed

2 files changed

+56
-0
lines changed

‎apis/Google.Cloud.Iam.V1/Google.Cloud.Iam.V1.Tests/PolicyTest.cs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
using System;
1516
using Xunit;
1617

1718
namespace Google.Cloud.Iam.V1.Tests
@@ -154,5 +155,42 @@ public void RemoveRoleMember_NoRole()
154155
Assert.False(policy.RemoveRoleMember("target", "d"));
155156
Assert.Equal(expected, policy);
156157
}
158+
159+
[Fact]
160+
public void AddRoleMember_LaterVersion()
161+
{
162+
var policy = new Policy { Version = 2 };
163+
Assert.Throws<InvalidOperationException>(() => policy.AddRoleMember("role", "member"));
164+
}
165+
166+
[Fact]
167+
public void RemoveRoleMember_LaterVersion()
168+
{
169+
var policy = new Policy { Version = 2 };
170+
Assert.Throws<InvalidOperationException>(() => policy.RemoveRoleMember("role", "member"));
171+
}
172+
173+
[Fact]
174+
public void AddRoleMember_ContainsBinding()
175+
{
176+
var policy = new Policy
177+
{
178+
Bindings = { new Binding { Role = "role", Members = { "member" }, Condition = new Type.Expr { Description = "condition" } } }
179+
};
180+
Assert.Throws<InvalidOperationException>(() => policy.AddRoleMember("role", "member2"));
181+
Assert.Throws<InvalidOperationException>(() => policy.AddRoleMember("role2", "member2"));
182+
}
183+
184+
[Fact]
185+
public void RemoveRoleMember_ContainsBinding()
186+
{
187+
var policy = new Policy
188+
{
189+
Bindings = { new Binding { Role = "role", Members = { "member" }, Condition = new Type.Expr { Description = "condition" } } }
190+
};
191+
Assert.Throws<InvalidOperationException>(() => policy.RemoveRoleMember("role", "member"));
192+
Assert.Throws<InvalidOperationException>(() => policy.RemoveRoleMember("role", "member2"));
193+
Assert.Throws<InvalidOperationException>(() => policy.RemoveRoleMember("role2", "member"));
194+
}
157195
}
158196
}

‎apis/Google.Cloud.Iam.V1/Google.Cloud.Iam.V1/PolicyPartial.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,21 @@ public partial class Policy
2626
/// <summary>
2727
/// Adds the specified member to the specified role. If the role does
2828
/// not already exist, it is created.
29+
/// This method will fail with an <see cref="InvalidOperationException"/>
30+
/// if it is called on a Policy with a <see cref="Version"/> greater than 1,
31+
/// or if any of the bindings contain conditions,
32+
/// as that indicates a more complicated policy than this method is prepared
33+
/// to handle. Changes to such policies must be made manually.
2934
/// </summary>
3035
/// <param name="role">The role to add the member to. Must not be null or empty.</param>
3136
/// <param name="member">The member to add to the role. Must not be null or empty.</param>
3237
/// <returns><c>true</c> if the policy was changed; <c>false</c> if the member already existed in the role.</returns>
38+
/// <exception cref="InvalidOperationException">The <see cref="Version"/> is greater than 1.</exception>
3339
public bool AddRoleMember(string role, string member)
3440
{
3541
GaxPreconditions.CheckNotNullOrEmpty(role, nameof(role));
3642
GaxPreconditions.CheckNotNullOrEmpty(member, nameof(member));
43+
ValidatePolicyVersion();
3744
var binding = FindRole(role);
3845
if (binding == null)
3946
{
@@ -51,15 +58,22 @@ public bool AddRoleMember(string role, string member)
5158
/// <summary>
5259
/// Removes the specified member to the specified role, if they belong to it. If the role becomes empty after
5360
/// removing the member, it is removed from the policy.
61+
/// This method will fail with an <see cref="InvalidOperationException"/>
62+
/// if it is called on a Policy with a <see cref="Version"/> greater than 1,
63+
/// or if any of the bindings contain conditions,
64+
/// as that indicates a more complicated policy than this method is prepared
65+
/// to handle. Changes to such policies must be made manually.
5466
/// </summary>
5567
/// <param name="role">The role to remove the member from. Must not be null or empty.</param>
5668
/// <param name="member">The member to remove from the role. Must not be null or empty.</param>
5769
/// <returns><c>true</c> if the policy was changed; <c>false</c> if the member didn't exist in the role
5870
/// or the role didn't exist.</returns>
71+
/// <exception cref="InvalidOperationException">The <see cref="Version"/> is greater than 1.</exception>
5972
public bool RemoveRoleMember(string role, string member)
6073
{
6174
GaxPreconditions.CheckNotNullOrEmpty(role, nameof(role));
6275
GaxPreconditions.CheckNotNullOrEmpty(member, nameof(member));
76+
ValidatePolicyVersion();
6377
var binding = FindRole(role);
6478
if (binding == null)
6579
{
@@ -77,5 +91,9 @@ public bool RemoveRoleMember(string role, string member)
7791
}
7892

7993
private Binding FindRole(string role) => Bindings.FirstOrDefault(binding => binding.Role == role);
94+
95+
private void ValidatePolicyVersion() =>
96+
GaxPreconditions.CheckState(Version <= 1 && Bindings.All(b => b.Condition == null),
97+
"Helper methods cannot be invoked on policies with version {0} or with conditional bindings. For more information, see https://cloud.google.com/iam/docs/policies.", Version);
8098
}
8199
}

0 commit comments

Comments
 (0)