Skip to content

Commit eec5bd1

Browse files
JasonMWebbjasonmwebb-lv
JasonMWebb
and
jasonmwebb-lv
authored
Breaking: Change ICurrentUser.Id from Guid? to string? (#167)
Aligns security abstractions with .NET claims identity standards: - ICurrentUser.Id is now string? (was Guid?), matching how claims work - IsAuthenticated delegates to ClaimsIdentity.IsAuthenticated - FindUserId() returns raw string claim value (no Guid parsing) - ClaimTypesConst uses configure-once freeze pattern via Configure(Action<ClaimTypesOptions>) - Removed FindClaimValue<T> (consumers parse claim strings themselves) - Updated website documentation, examples, and fixed incorrect GitHub URL Co-authored-by: jasonmwebb-lv <jason.webb@leadventure.com>
1 parent 249a6eb commit eec5bd1

19 files changed

Lines changed: 1366 additions & 128 deletions

File tree

‎Examples/CleanWithCQRS/HR.LeaveManagement.Persistence/AuditableDbContext.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public override Task<int> SaveChangesAsync(bool acceptAllChangesOnSuccess, Cance
3535
foreach (var entry in base.ChangeTracker.Entries<BaseDomainEntity>()
3636
.Where(q => q.State == EntityState.Added || q.State == EntityState.Modified))
3737
{
38-
string userId = (_currentUser == null || _currentUser.Id == null ? "System" : _currentUser.Id.ToString());
38+
string userId = _currentUser?.Id ?? "System";
3939

4040
entry.Entity.DateLastModified = _systemTime.Now;
4141
entry.Entity.LastModifiedBy = userId;

‎Src/RCommon.Security/Claims/ClaimTypesConst.cs‎

Lines changed: 55 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,76 @@
1-
using System;
2-
using System.Collections.Generic;
3-
using System.Linq;
1+
using System;
42
using System.Security.Claims;
5-
using System.Text;
6-
using System.Threading.Tasks;
73

84
namespace RCommon.Security.Claims
95
{
106
/// <summary>
11-
/// Provides configurable constants for standard claim type URIs used throughout the security subsystem.
12-
/// Each property defaults to the corresponding <see cref="ClaimTypes"/> value and can be overridden at startup.
7+
/// Provides claim type URI constants used throughout the security subsystem.
8+
/// Call <see cref="Configure"/> once at startup to override defaults.
9+
/// After configuration (or first property access), values are frozen.
1310
/// </summary>
1411
public static class ClaimTypesConst
1512
{
16-
/// <summary>
17-
/// Default: <see cref="ClaimTypes.Name"/>
18-
/// </summary>
19-
public static string UserName { get; set; } = ClaimTypes.Name;
13+
private static ClaimTypesOptions? _options;
14+
private static bool _frozen;
15+
private static readonly object _lock = new();
2016

21-
/// <summary>
22-
/// Default: <see cref="ClaimTypes.GivenName"/>
23-
/// </summary>
24-
public static string Name { get; set; } = ClaimTypes.GivenName;
17+
public static string UserName => GetOptions().UserName;
18+
public static string Name => GetOptions().Name;
19+
public static string SurName => GetOptions().SurName;
20+
public static string UserId => GetOptions().UserId;
21+
public static string Role => GetOptions().Role;
22+
public static string Email => GetOptions().Email;
23+
public static string TenantId => GetOptions().TenantId;
24+
public static string ClientId => GetOptions().ClientId;
2525

2626
/// <summary>
27-
/// Default: <see cref="ClaimTypes.Surname"/>
27+
/// Configures claim type mappings. May only be called once, before any property is accessed.
2828
/// </summary>
29-
public static string SurName { get; set; } = ClaimTypes.Surname;
29+
public static void Configure(Action<ClaimTypesOptions> configure)
30+
{
31+
Guard.IsNotNull(configure, nameof(configure));
3032

31-
/// <summary>
32-
/// Default: <see cref="ClaimTypes.NameIdentifier"/>
33-
/// </summary>
34-
public static string UserId { get; set; } = ClaimTypes.NameIdentifier;
33+
lock (_lock)
34+
{
35+
if (_frozen)
36+
{
37+
throw new InvalidOperationException(
38+
"ClaimTypesConst has already been configured or accessed. Configure may only be called once, before any property is read.");
39+
}
3540

36-
/// <summary>
37-
/// Default: <see cref="ClaimTypes.Role"/>
38-
/// </summary>
39-
public static string Role { get; set; } = ClaimTypes.Role;
41+
var options = new ClaimTypesOptions();
42+
configure(options);
43+
_options = options;
44+
_frozen = true;
45+
}
46+
}
4047

41-
/// <summary>
42-
/// Default: <see cref="ClaimTypes.Email"/>
43-
/// </summary>
44-
public static string Email { get; set; } = ClaimTypes.Email;
48+
private static ClaimTypesOptions GetOptions()
49+
{
50+
if (_options != null)
51+
return _options;
4552

46-
/// <summary>
47-
/// Default: "tenantid".
48-
/// </summary>
49-
public static string TenantId { get; set; } = "tenantid";
53+
lock (_lock)
54+
{
55+
if (_options != null)
56+
return _options;
57+
58+
_options = new ClaimTypesOptions();
59+
_frozen = true;
60+
return _options;
61+
}
62+
}
5063

5164
/// <summary>
52-
/// Default: "client_id".
65+
/// Resets configuration to allow reconfiguration. Internal — for test isolation only.
5366
/// </summary>
54-
public static string ClientId { get; set; } = "client_id";
67+
internal static void Reset()
68+
{
69+
lock (_lock)
70+
{
71+
_options = null;
72+
_frozen = false;
73+
}
74+
}
5575
}
5676
}

‎Src/RCommon.Security/Claims/ClaimTypesOptions.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
using System.Security.Claims;
2+
3+
namespace RCommon.Security.Claims
4+
{
5+
/// <summary>
6+
/// Configurable options for claim type URI mappings.
7+
/// Set properties to match the claim types issued by your identity provider.
8+
/// </summary>
9+
public class ClaimTypesOptions
10+
{
11+
public string UserName { get; set; } = ClaimTypes.Name;
12+
public string Name { get; set; } = ClaimTypes.GivenName;
13+
public string SurName { get; set; } = ClaimTypes.Surname;
14+
public string UserId { get; set; } = ClaimTypes.NameIdentifier;
15+
public string Role { get; set; } = ClaimTypes.Role;
16+
public string Email { get; set; } = ClaimTypes.Email;
17+
public string TenantId { get; set; } = "tenantid";
18+
public string ClientId { get; set; } = "client_id";
19+
}
20+
}

‎Src/RCommon.Security/ClaimsIdentityExtensions.cs‎

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ namespace RCommon.Security
1616
public static class ClaimsIdentityExtensions
1717
{
1818
/// <summary>
19-
/// Extracts the user identifier from the principal's claims as a <see cref="Guid"/>.
19+
/// Extracts the user identifier from the principal's claims as a raw string value.
2020
/// </summary>
2121
/// <param name="principal">The claims principal to search.</param>
22-
/// <returns>The parsed user ID, or <c>null</c> if the claim is missing or not a valid GUID.</returns>
23-
public static Guid? FindUserId(this ClaimsPrincipal principal)
22+
/// <returns>The user ID string, or <c>null</c> if the claim is missing or empty.</returns>
23+
public static string? FindUserId(this ClaimsPrincipal principal)
2424
{
2525
Guard.IsNotNull(principal, nameof(principal));
2626

@@ -30,20 +30,15 @@ public static class ClaimsIdentityExtensions
3030
return null;
3131
}
3232

33-
if (Guid.TryParse(userIdOrNull.Value, out Guid guid))
34-
{
35-
return guid;
36-
}
37-
38-
return null;
33+
return userIdOrNull.Value;
3934
}
4035

4136
/// <summary>
42-
/// Extracts the user identifier from the identity's claims as a <see cref="Guid"/>.
37+
/// Extracts the user identifier from the identity's claims as a raw string value.
4338
/// </summary>
4439
/// <param name="identity">The identity to search. Must be castable to <see cref="ClaimsIdentity"/>.</param>
45-
/// <returns>The parsed user ID, or <c>null</c> if the claim is missing or not a valid GUID.</returns>
46-
public static Guid? FindUserId(this IIdentity identity)
40+
/// <returns>The user ID string, or <c>null</c> if the claim is missing or empty.</returns>
41+
public static string? FindUserId(this IIdentity identity)
4742
{
4843
Guard.IsNotNull(identity, nameof(identity));
4944

@@ -55,12 +50,7 @@ public static class ClaimsIdentityExtensions
5550
return null;
5651
}
5752

58-
if (Guid.TryParse(userIdOrNull.Value, out var guid))
59-
{
60-
return guid;
61-
}
62-
63-
return null;
53+
return userIdOrNull.Value;
6454
}
6555

6656
/// <summary>

‎Src/RCommon.Security/RCommon.Security.csproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,8 @@
3939
<PackageReference Update="MinVer" Version="7.0.0" />
4040
</ItemGroup>
4141

42+
<ItemGroup>
43+
<InternalsVisibleTo Include="RCommon.Security.Tests" />
44+
</ItemGroup>
45+
4246
</Project>

‎Src/RCommon.Security/Users/CurrentUser.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ public CurrentUser(ICurrentPrincipalAccessor principalAccessor)
3131
}
3232

3333
/// <inheritdoc />
34-
public virtual bool IsAuthenticated => Id.HasValue;
34+
public virtual bool IsAuthenticated => _principalAccessor.Principal?.Identity?.IsAuthenticated ?? false;
3535

3636
/// <inheritdoc />
37-
public virtual Guid? Id => _principalAccessor.Principal?.FindUserId();
37+
public virtual string? Id => _principalAccessor.Principal?.FindUserId();
3838

3939
/// <inheritdoc />
4040
public virtual string? TenantId => _principalAccessor.Principal?.FindTenantId();

‎Src/RCommon.Security/Users/CurrentUserExtensions.cs‎

Lines changed: 5 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,4 @@
1-
using RCommon.Security.Claims;
2-
using System;
3-
using System.Collections.Generic;
4-
using System.Diagnostics;
5-
using System.Globalization;
6-
using System.Linq;
7-
using System.Text;
8-
using System.Threading.Tasks;
1+
using System;
92

103
namespace RCommon.Security.Users
114
{
@@ -26,38 +19,16 @@ public static class CurrentUserExtensions
2619
return currentUser.FindClaim(claimType)?.Value;
2720
}
2821

29-
/// <summary>
30-
/// Finds a claim of the specified type and converts its value to <typeparamref name="T"/>.
31-
/// </summary>
32-
/// <typeparam name="T">The target value type to convert the claim value to.</typeparam>
33-
/// <param name="currentUser">The current user instance.</param>
34-
/// <param name="claimType">The claim type URI to search for.</param>
35-
/// <returns>The converted claim value, or <c>default</c> if the claim is not found.</returns>
36-
/// <remarks>Uses <see cref="Convert.ChangeType(object, Type, IFormatProvider)"/> with <see cref="CultureInfo.InvariantCulture"/>.</remarks>
37-
public static T FindClaimValue<T>(this ICurrentUser currentUser, string claimType)
38-
where T : struct
39-
{
40-
var value = currentUser.FindClaimValue(claimType);
41-
if (value == null)
42-
{
43-
return default;
44-
}
45-
return (T)Convert.ChangeType(value, typeof(T), CultureInfo.InvariantCulture);
46-
}
47-
4822
/// <summary>
4923
/// Gets the current user's ID, asserting that it is not <c>null</c>.
5024
/// </summary>
5125
/// <param name="currentUser">The current user instance.</param>
52-
/// <returns>The user's <see cref="Guid"/> identifier.</returns>
26+
/// <returns>The user's string identifier.</returns>
5327
/// <exception cref="InvalidOperationException">Thrown if <see cref="ICurrentUser.Id"/> is <c>null</c>.</exception>
54-
public static Guid GetId(this ICurrentUser currentUser)
28+
public static string GetId(this ICurrentUser currentUser)
5529
{
56-
Debug.Assert(currentUser.Id != null, "currentUser.Id != null");
57-
58-
return currentUser.Id.Value;
30+
return currentUser.Id
31+
?? throw new InvalidOperationException("The current user ID is null. Ensure the user is authenticated and has a NameIdentifier claim.");
5932
}
60-
61-
6233
}
6334
}

‎Src/RCommon.Security/Users/ICurrentUser.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
using System;
2-
using System.Security.Claims;
1+
using System.Security.Claims;
32

43
namespace RCommon.Security.Users
54
{
@@ -11,10 +10,10 @@ public interface ICurrentUser
1110
/// <summary>
1211
/// Gets the unique identifier of the current user, or <c>null</c> if no user is authenticated.
1312
/// </summary>
14-
Guid? Id { get; }
13+
string? Id { get; }
1514

1615
/// <summary>
17-
/// Gets a value indicating whether the current user is authenticated (i.e., <see cref="Id"/> is not <c>null</c>).
16+
/// Gets a value indicating whether the current user is authenticated, as reported by the underlying <see cref="System.Security.Principal.IIdentity"/>.
1817
/// </summary>
1918
bool IsAuthenticated { get; }
2019

‎Tests/RCommon.Security.Tests/ClaimTypesConstTests.cs‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
using System.Security.Claims;
2+
using FluentAssertions;
3+
using RCommon.Security.Claims;
4+
using Xunit;
5+
6+
namespace RCommon.Security.Tests;
7+
8+
public class ClaimTypesConstTests : IDisposable
9+
{
10+
public ClaimTypesConstTests()
11+
{
12+
ClaimTypesConst.Reset();
13+
}
14+
15+
public void Dispose()
16+
{
17+
ClaimTypesConst.Reset();
18+
}
19+
20+
[Fact]
21+
public void Defaults_MatchStandardClaimTypes()
22+
{
23+
ClaimTypesConst.UserId.Should().Be(ClaimTypes.NameIdentifier);
24+
ClaimTypesConst.UserName.Should().Be(ClaimTypes.Name);
25+
ClaimTypesConst.Name.Should().Be(ClaimTypes.GivenName);
26+
ClaimTypesConst.SurName.Should().Be(ClaimTypes.Surname);
27+
ClaimTypesConst.Role.Should().Be(ClaimTypes.Role);
28+
ClaimTypesConst.Email.Should().Be(ClaimTypes.Email);
29+
ClaimTypesConst.TenantId.Should().Be("tenantid");
30+
ClaimTypesConst.ClientId.Should().Be("client_id");
31+
}
32+
33+
[Fact]
34+
public void Configure_AppliesCustomValues()
35+
{
36+
ClaimTypesConst.Configure(options =>
37+
{
38+
options.UserId = "sub";
39+
options.Role = "roles";
40+
options.TenantId = "tenant_id";
41+
});
42+
43+
ClaimTypesConst.UserId.Should().Be("sub");
44+
ClaimTypesConst.Role.Should().Be("roles");
45+
ClaimTypesConst.TenantId.Should().Be("tenant_id");
46+
// Unchanged values keep defaults
47+
ClaimTypesConst.Email.Should().Be(ClaimTypes.Email);
48+
}
49+
50+
[Fact]
51+
public void Configure_CalledTwice_ThrowsInvalidOperationException()
52+
{
53+
ClaimTypesConst.Configure(options => { options.UserId = "sub"; });
54+
55+
var action = () => ClaimTypesConst.Configure(options => { options.Role = "roles"; });
56+
57+
action.Should().Throw<InvalidOperationException>();
58+
}
59+
60+
[Fact]
61+
public void Configure_WithNullAction_ThrowsArgumentNullException()
62+
{
63+
var action = () => ClaimTypesConst.Configure(null!);
64+
65+
action.Should().Throw<ArgumentNullException>();
66+
}
67+
}

0 commit comments

Comments
 (0)