Skip to content

Commit a550fe1

Browse files
joshkempnerjoshkempner
authored
Roles are case-insensitive (#121)
- Trim role names before persisting. - Role name comparisons are always case-insensitive.
1 parent 9b5344a commit a550fe1

File tree

8 files changed

+140
-81
lines changed

8 files changed

+140
-81
lines changed

‎src/ReactiveDomain.Policy/Policy.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public bool TryGetRole(string roleName, out Role role) {
3838
public bool Equals(Policy other)
3939
{
4040
if (other is null) return false;
41-
return string.Equals(PolicyName, other.PolicyName);
41+
return string.Equals(PolicyName, other.PolicyName, StringComparison.OrdinalIgnoreCase);
4242
}
4343

4444
public override bool Equals(object obj) => Equals(obj as Policy);

‎src/ReactiveDomain.Policy/Role.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,14 @@ public Role(string roleName, params Permission[] permissions) :
2222
}
2323
public Role(string roleName)
2424
{
25-
Ensure.NotNullOrEmpty(roleName, nameof(roleName));
26-
RoleName = roleName;
25+
Ensure.NotNullOrEmpty(roleName.Trim(), nameof(roleName));
26+
RoleName = roleName.Trim();
2727
}
2828
#region IEquatable<T> Implementation
2929
public bool Equals(Role other)
3030
{
3131
if (other is null) return false;
32-
return string.Equals(RoleName, other.RoleName);
32+
return string.Equals(RoleName.ToLowerInvariant(), other.RoleName.ToLowerInvariant());
3333
}
3434

3535
public override bool Equals(object obj) => Equals(obj as Role);

‎src/ReactiveDomain.Policy/UserPolicy.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public UserPolicy(UserDTO user, HashSet<Role> grantedRoles)
4141
public void AddRole(Role role)
4242
{
4343
_roles.Add(role);
44-
_roleNames.Add(role.RoleName);
44+
_roleNames.Add(role.RoleName.Trim().ToLowerInvariant());
4545
foreach (var permission in role.Permissions)
4646
{
4747
_permissions.Add(permission);
@@ -56,7 +56,7 @@ public void AddRole(Role role)
5656
public bool HasRole(string roleName)
5757
{
5858
if (string.IsNullOrWhiteSpace(roleName)) { return false; }
59-
return _roleNames.Contains(roleName);
59+
return _roleNames.Contains(roleName.Trim().ToLowerInvariant());
6060
}
6161
public bool HasRole(Role role)
6262
{

‎src/ReactiveDomain.PolicyStorage.Tests/with_policy_user.cs‎

Lines changed: 72 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,11 @@ namespace ReactiveDomain.PolicyStorage.Tests
99
{
1010
public class with_policy_user
1111
{
12-
Guid _policyId = Guid.NewGuid();
13-
Guid _userId = Guid.NewGuid();
14-
Guid _id = Guid.NewGuid();
12+
private readonly Guid _policyId = Guid.NewGuid();
13+
private readonly Guid _userId = Guid.NewGuid();
14+
private readonly Guid _id = Guid.NewGuid();
1515
private readonly ICorrelatedMessage _command = MessageBuilder.New(() => new TestMessages.RootCommand());
1616

17-
public with_policy_user()
18-
{
19-
20-
}
2117
[Fact]
2218
public void can_create_policy_user()
2319
{
@@ -27,8 +23,7 @@ public void can_create_policy_user()
2723
_policyId,
2824
_userId,
2925
false,
30-
_command)
31-
{ };
26+
_command);
3227

3328
var events = user.TakeEvents();
3429
Assert.Collection(
@@ -57,8 +52,7 @@ public void can_add_role()
5752
_policyId,
5853
_userId,
5954
false,
60-
_command)
61-
{ };
55+
_command);
6256

6357
user.TakeEvents();
6458
//add role
@@ -102,8 +96,7 @@ public void can_remove_role()
10296
_policyId,
10397
_userId,
10498
false,
105-
_command)
106-
{ };
99+
_command);
107100

108101

109102
//add role
@@ -145,10 +138,10 @@ public void can_remove_role()
145138
[Fact]
146139
public void can_deactivate()
147140
{
148-
Guid role1_Id = Guid.NewGuid();
149-
string role1_name = "role1";
150-
Guid role2_Id = Guid.NewGuid();
151-
string role2_name = "role2";
141+
Guid role1Id = Guid.NewGuid();
142+
string role1Name = "role1";
143+
Guid role2Id = Guid.NewGuid();
144+
string role2Name = "role2";
152145

153146
var user = new PolicyUser(
154147
_id,
@@ -158,8 +151,8 @@ public void can_deactivate()
158151
_command)
159152
{ };
160153
//add role
161-
user.AddRole(role1_name, role1_Id);
162-
user.AddRole(role2_name, role2_Id);
154+
user.AddRole(role1Name, role1Id);
155+
user.AddRole(role2Name, role2Id);
163156
user.TakeEvents();
164157

165158
//deactivate
@@ -184,8 +177,8 @@ public void can_deactivate()
184177
if (e is PolicyUserMsgs.RoleRemoved removed)
185178
{
186179
Assert.Equal(_id, removed.PolicyUserId);
187-
Assert.Equal(role1_Id, removed.RoleId);
188-
Assert.Equal(role1_name, removed.RoleName);
180+
Assert.Equal(role1Id, removed.RoleId);
181+
Assert.Equal(role1Name, removed.RoleName);
189182
}
190183
else
191184
{
@@ -197,8 +190,8 @@ public void can_deactivate()
197190
if (e is PolicyUserMsgs.RoleRemoved removed)
198191
{
199192
Assert.Equal(_id, removed.PolicyUserId);
200-
Assert.Equal(role2_Id, removed.RoleId);
201-
Assert.Equal(role2_name, removed.RoleName);
193+
Assert.Equal(role2Id, removed.RoleId);
194+
Assert.Equal(role2Name, removed.RoleName);
202195
}
203196
else
204197
{
@@ -215,21 +208,20 @@ public void can_deactivate()
215208
[Fact]
216209
public void can_reactivate()
217210
{
218-
Guid role1_Id = Guid.NewGuid();
219-
string role1_name = "role1";
220-
Guid role2_Id = Guid.NewGuid();
221-
string role2_name = "role2";
211+
Guid role1Id = Guid.NewGuid();
212+
string role1Name = "role1";
213+
Guid role2Id = Guid.NewGuid();
214+
string role2Name = "role2";
222215

223216
var user = new PolicyUser(
224217
_id,
225218
_policyId,
226219
_userId,
227220
false,
228-
_command)
229-
{ };
221+
_command);
230222
//add roles
231-
user.AddRole(role1_name, role1_Id);
232-
user.AddRole(role2_name, role2_Id);
223+
user.AddRole(role1Name, role1Id);
224+
user.AddRole(role2Name, role2Id);
233225
//deactivate
234226
user.Deactivate();
235227
user.TakeEvents();
@@ -256,8 +248,8 @@ public void can_reactivate()
256248
if (e is PolicyUserMsgs.RoleAdded added)
257249
{
258250
Assert.Equal(_id, added.PolicyUserId);
259-
Assert.Equal(role1_Id, added.RoleId);
260-
Assert.Equal(role1_name, added.RoleName);
251+
Assert.Equal(role1Id, added.RoleId);
252+
Assert.Equal(role1Name, added.RoleName);
261253
}
262254
else
263255
{
@@ -269,8 +261,8 @@ public void can_reactivate()
269261
if (e is PolicyUserMsgs.RoleAdded added)
270262
{
271263
Assert.Equal(_id, added.PolicyUserId);
272-
Assert.Equal(role2_Id, added.RoleId);
273-
Assert.Equal(role2_name, added.RoleName);
264+
Assert.Equal(role2Id, added.RoleId);
265+
Assert.Equal(role2Name, added.RoleName);
274266
}
275267
else
276268
{
@@ -283,5 +275,50 @@ public void can_reactivate()
283275
events = user.TakeEvents();
284276
Assert.Empty(events);
285277
}
278+
279+
[Fact]
280+
public void role_names_are_case_insensitive()
281+
{
282+
var roleId = Guid.NewGuid();
283+
const string roleName = "admin";
284+
const string roleName2 = "Admin";
285+
286+
var user = new PolicyUser(
287+
_id,
288+
_policyId,
289+
_userId,
290+
false,
291+
_command);
292+
user.AddRole(roleName, roleId);
293+
user.AddRole(roleName2, roleId); // case-insensitive, idempotent
294+
user.RemoveRole(roleName2, roleId); // remove using same ID, different case
295+
var events = user.TakeEvents();
296+
Assert.Collection(
297+
events,
298+
e => Assert.IsType<PolicyUserMsgs.PolicyUserAdded>(e),
299+
e => Assert.IsType<PolicyUserMsgs.RoleAdded>(e),
300+
e => Assert.IsType<PolicyUserMsgs.RoleRemoved>(e));
301+
}
302+
303+
[Fact]
304+
public void cannot_add_same_named_role_with_different_id()
305+
{
306+
var roleId = Guid.NewGuid();
307+
const string roleName = "admin";
308+
309+
var user = new PolicyUser(
310+
_id,
311+
_policyId,
312+
_userId,
313+
false,
314+
_command);
315+
user.AddRole(roleName, roleId);
316+
Assert.Throws<ArgumentException>(() => user.AddRole(roleName, Guid.NewGuid()));
317+
var events = user.TakeEvents();
318+
Assert.Collection(
319+
events,
320+
e => Assert.IsType<PolicyUserMsgs.PolicyUserAdded>(e),
321+
e => Assert.IsType<PolicyUserMsgs.RoleAdded>(e));
322+
}
286323
}
287324
}

‎src/ReactiveDomain.PolicyStorage/Domain/PolicyUser.cs‎

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,17 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Linq;
4-
using System.Runtime.CompilerServices;
54
using ReactiveDomain.Messaging;
65
using ReactiveDomain.Policy.Messages;
76
using ReactiveDomain.Util;
87

9-
108
namespace ReactiveDomain.Policy.Domain
119
{
1210
/// <summary>
1311
/// Aggregate for a User in an application policy.
1412
/// </summary>
15-
public class PolicyUser : AggregateRoot {
16-
13+
public class PolicyUser : AggregateRoot
14+
{
1715
private Guid _userId;
1816
private Guid _policyId;
1917
private readonly HashSet<(string name, Guid id)> _roles = new HashSet<(string name, Guid id)>();
@@ -39,27 +37,32 @@ private void RegisterEvents()
3937
Register<PolicyUserMsgs.UserReactivated>(Apply);
4038
}
4139
//Apply State Changes
42-
private void Apply(PolicyUserMsgs.PolicyUserAdded @event) {
40+
private void Apply(PolicyUserMsgs.PolicyUserAdded @event)
41+
{
4342
Id = @event.PolicyUserId;
4443
_policyId = @event.PolicyId;
4544
_userId = @event.UserId;
4645
_active = true;
4746
}
4847

49-
private void Apply(PolicyUserMsgs.RoleAdded @event) {
50-
_roles.Add((@event.RoleName, @event.RoleId));
48+
private void Apply(PolicyUserMsgs.RoleAdded @event)
49+
{
50+
_roles.Add((@event.RoleName.Trim().ToLowerInvariant(), @event.RoleId));
5151
}
5252

53-
private void Apply(PolicyUserMsgs.RoleRemoved @event) {
54-
_roles.Remove((@event.RoleName, @event.RoleId));
53+
private void Apply(PolicyUserMsgs.RoleRemoved @event)
54+
{
55+
_roles.Remove((@event.RoleName.Trim().ToLowerInvariant(), @event.RoleId));
5556
}
5657

57-
private void Apply(PolicyUserMsgs.UserDeactivated @event) {
58+
private void Apply(PolicyUserMsgs.UserDeactivated @event)
59+
{
5860
_active = false;
5961
_rolesAtDeactivation = new HashSet<(string name, Guid id)>(_roles);
6062
}
6163

62-
private void Apply(PolicyUserMsgs.UserReactivated @event) {
64+
private void Apply(PolicyUserMsgs.UserReactivated @event)
65+
{
6366
_active = true;
6467
}
6568

@@ -88,28 +91,41 @@ public PolicyUser(
8891
singleRole));
8992
}
9093

91-
public void AddRole(string roleName, Guid roleId) {
92-
if(_roles.Contains((roleName,roleId))){ return; }
93-
Raise(new PolicyUserMsgs.RoleAdded(Id, roleId, roleName));
94+
public void AddRole(string roleName, Guid roleId)
95+
{
96+
Ensure.NotNullOrEmpty(roleName.Trim(), nameof(roleName));
97+
var testRoleName = roleName.Trim().ToLowerInvariant();
98+
if (_roles.Contains((testRoleName, roleId))) { return; }
99+
if (_roles.Any(x => x.name == testRoleName)) { throw new ArgumentException($"A role with name {roleName} already exists with a different ID.", nameof(roleId)); }
100+
Raise(new PolicyUserMsgs.RoleAdded(Id, roleId, roleName.Trim()));
94101
}
95-
public void RemoveRole(string roleName, Guid roleId) {
96-
if(!_roles.Contains((roleName,roleId))){ return; }
97-
Raise(new PolicyUserMsgs.RoleRemoved(Id, roleId, roleName));
102+
103+
public void RemoveRole(string roleName, Guid roleId)
104+
{
105+
Ensure.NotNullOrEmpty(roleName.Trim(), nameof(roleName));
106+
var testRoleName = roleName.Trim().ToLowerInvariant();
107+
if (!_roles.Contains((testRoleName, roleId))) { return; }
108+
Raise(new PolicyUserMsgs.RoleRemoved(Id, roleId, roleName.Trim()));
98109
}
99110

100-
public void Deactivate() {
111+
public void Deactivate()
112+
{
101113
if (!_active) { return; }
102114
Raise(new PolicyUserMsgs.UserDeactivated(Id));
103-
foreach (var role in _roles.ToArray()) {
115+
foreach (var role in _roles.ToArray())
116+
{
104117
Raise(new PolicyUserMsgs.RoleRemoved(Id, role.id, role.name));
105118
}
106119
}
107120

108-
public void Reactivate() {
121+
public void Reactivate()
122+
{
109123
if (_active) { return; }
110124
Raise(new PolicyUserMsgs.UserReactivated(Id));
111-
if (_rolesAtDeactivation?.Any() ?? false) {
112-
foreach (var role in _rolesAtDeactivation) {
125+
if (_rolesAtDeactivation?.Any() ?? false)
126+
{
127+
foreach (var role in _rolesAtDeactivation)
128+
{
113129
Raise(new PolicyUserMsgs.RoleAdded(Id, role.id, role.name));
114130
}
115131
}

0 commit comments

Comments
 (0)