From bc6570afc5fb3e433212290c3a680da97f526e1a Mon Sep 17 00:00:00 2001 From: jasonmwebb-lv Date: Sun, 5 Apr 2026 12:37:47 -0600 Subject: [PATCH] Breaking: Change ICurrentUser.Id from Guid? to string? 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) - Removed FindClaimValue (consumers parse claim strings themselves) - Updated website documentation, examples, and fixed incorrect GitHub URL --- .../AuditableDbContext.cs | 2 +- .../Claims/ClaimTypesConst.cs | 90 +- .../Claims/ClaimTypesOptions.cs | 20 + .../ClaimsIdentityExtensions.cs | 26 +- Src/RCommon.Security/RCommon.Security.csproj | 4 + Src/RCommon.Security/Users/CurrentUser.cs | 4 +- .../Users/CurrentUserExtensions.cs | 39 +- Src/RCommon.Security/Users/ICurrentUser.cs | 7 +- .../ClaimTypesConstTests.cs | 67 ++ .../ClaimsIdentityExtensionsTests.cs | 109 +++ .../CurrentUserTests.cs | 111 ++- ...6-04-05-security-abstractions-string-id.md | 822 ++++++++++++++++++ ...-security-abstractions-string-id-design.md | 135 +++ website/docs/getting-started/overview.mdx | 2 +- website/docs/security-web/authorization.mdx | 25 +- website/docs/security-web/web-utilities.mdx | 2 +- .../getting-started/overview.mdx | 2 +- .../security-web/authorization.mdx | 25 +- .../security-web/web-utilities.mdx | 2 +- 19 files changed, 1366 insertions(+), 128 deletions(-) create mode 100644 Src/RCommon.Security/Claims/ClaimTypesOptions.cs create mode 100644 Tests/RCommon.Security.Tests/ClaimTypesConstTests.cs create mode 100644 Tests/RCommon.Security.Tests/ClaimsIdentityExtensionsTests.cs create mode 100644 docs/superpowers/plans/2026-04-05-security-abstractions-string-id.md create mode 100644 docs/superpowers/specs/2026-04-05-security-abstractions-string-id-design.md diff --git a/Examples/CleanWithCQRS/HR.LeaveManagement.Persistence/AuditableDbContext.cs b/Examples/CleanWithCQRS/HR.LeaveManagement.Persistence/AuditableDbContext.cs index 4d66f242..b39bad67 100644 --- a/Examples/CleanWithCQRS/HR.LeaveManagement.Persistence/AuditableDbContext.cs +++ b/Examples/CleanWithCQRS/HR.LeaveManagement.Persistence/AuditableDbContext.cs @@ -35,7 +35,7 @@ public override Task SaveChangesAsync(bool acceptAllChangesOnSuccess, Cance foreach (var entry in base.ChangeTracker.Entries() .Where(q => q.State == EntityState.Added || q.State == EntityState.Modified)) { - string userId = (_currentUser == null || _currentUser.Id == null ? "System" : _currentUser.Id.ToString()); + string userId = _currentUser?.Id ?? "System"; entry.Entity.DateLastModified = _systemTime.Now; entry.Entity.LastModifiedBy = userId; diff --git a/Src/RCommon.Security/Claims/ClaimTypesConst.cs b/Src/RCommon.Security/Claims/ClaimTypesConst.cs index 95a7c83b..0675faf7 100644 --- a/Src/RCommon.Security/Claims/ClaimTypesConst.cs +++ b/Src/RCommon.Security/Claims/ClaimTypesConst.cs @@ -1,56 +1,76 @@ -using System; -using System.Collections.Generic; -using System.Linq; +using System; using System.Security.Claims; -using System.Text; -using System.Threading.Tasks; namespace RCommon.Security.Claims { /// - /// Provides configurable constants for standard claim type URIs used throughout the security subsystem. - /// Each property defaults to the corresponding value and can be overridden at startup. + /// Provides claim type URI constants used throughout the security subsystem. + /// Call once at startup to override defaults. + /// After configuration (or first property access), values are frozen. /// public static class ClaimTypesConst { - /// - /// Default: - /// - public static string UserName { get; set; } = ClaimTypes.Name; + private static ClaimTypesOptions? _options; + private static bool _frozen; + private static readonly object _lock = new(); - /// - /// Default: - /// - public static string Name { get; set; } = ClaimTypes.GivenName; + public static string UserName => GetOptions().UserName; + public static string Name => GetOptions().Name; + public static string SurName => GetOptions().SurName; + public static string UserId => GetOptions().UserId; + public static string Role => GetOptions().Role; + public static string Email => GetOptions().Email; + public static string TenantId => GetOptions().TenantId; + public static string ClientId => GetOptions().ClientId; /// - /// Default: + /// Configures claim type mappings. May only be called once, before any property is accessed. /// - public static string SurName { get; set; } = ClaimTypes.Surname; + public static void Configure(Action configure) + { + Guard.IsNotNull(configure, nameof(configure)); - /// - /// Default: - /// - public static string UserId { get; set; } = ClaimTypes.NameIdentifier; + lock (_lock) + { + if (_frozen) + { + throw new InvalidOperationException( + "ClaimTypesConst has already been configured or accessed. Configure may only be called once, before any property is read."); + } - /// - /// Default: - /// - public static string Role { get; set; } = ClaimTypes.Role; + var options = new ClaimTypesOptions(); + configure(options); + _options = options; + _frozen = true; + } + } - /// - /// Default: - /// - public static string Email { get; set; } = ClaimTypes.Email; + private static ClaimTypesOptions GetOptions() + { + if (_options != null) + return _options; - /// - /// Default: "tenantid". - /// - public static string TenantId { get; set; } = "tenantid"; + lock (_lock) + { + if (_options != null) + return _options; + + _options = new ClaimTypesOptions(); + _frozen = true; + return _options; + } + } /// - /// Default: "client_id". + /// Resets configuration to allow reconfiguration. Internal — for test isolation only. /// - public static string ClientId { get; set; } = "client_id"; + internal static void Reset() + { + lock (_lock) + { + _options = null; + _frozen = false; + } + } } } diff --git a/Src/RCommon.Security/Claims/ClaimTypesOptions.cs b/Src/RCommon.Security/Claims/ClaimTypesOptions.cs new file mode 100644 index 00000000..b5f1ecf8 --- /dev/null +++ b/Src/RCommon.Security/Claims/ClaimTypesOptions.cs @@ -0,0 +1,20 @@ +using System.Security.Claims; + +namespace RCommon.Security.Claims +{ + /// + /// Configurable options for claim type URI mappings. + /// Set properties to match the claim types issued by your identity provider. + /// + public class ClaimTypesOptions + { + public string UserName { get; set; } = ClaimTypes.Name; + public string Name { get; set; } = ClaimTypes.GivenName; + public string SurName { get; set; } = ClaimTypes.Surname; + public string UserId { get; set; } = ClaimTypes.NameIdentifier; + public string Role { get; set; } = ClaimTypes.Role; + public string Email { get; set; } = ClaimTypes.Email; + public string TenantId { get; set; } = "tenantid"; + public string ClientId { get; set; } = "client_id"; + } +} diff --git a/Src/RCommon.Security/ClaimsIdentityExtensions.cs b/Src/RCommon.Security/ClaimsIdentityExtensions.cs index 24cb7053..7c15b0f9 100644 --- a/Src/RCommon.Security/ClaimsIdentityExtensions.cs +++ b/Src/RCommon.Security/ClaimsIdentityExtensions.cs @@ -16,11 +16,11 @@ namespace RCommon.Security public static class ClaimsIdentityExtensions { /// - /// Extracts the user identifier from the principal's claims as a . + /// Extracts the user identifier from the principal's claims as a raw string value. /// /// The claims principal to search. - /// The parsed user ID, or null if the claim is missing or not a valid GUID. - public static Guid? FindUserId(this ClaimsPrincipal principal) + /// The user ID string, or null if the claim is missing or empty. + public static string? FindUserId(this ClaimsPrincipal principal) { Guard.IsNotNull(principal, nameof(principal)); @@ -30,20 +30,15 @@ public static class ClaimsIdentityExtensions return null; } - if (Guid.TryParse(userIdOrNull.Value, out Guid guid)) - { - return guid; - } - - return null; + return userIdOrNull.Value; } /// - /// Extracts the user identifier from the identity's claims as a . + /// Extracts the user identifier from the identity's claims as a raw string value. /// /// The identity to search. Must be castable to . - /// The parsed user ID, or null if the claim is missing or not a valid GUID. - public static Guid? FindUserId(this IIdentity identity) + /// The user ID string, or null if the claim is missing or empty. + public static string? FindUserId(this IIdentity identity) { Guard.IsNotNull(identity, nameof(identity)); @@ -55,12 +50,7 @@ public static class ClaimsIdentityExtensions return null; } - if (Guid.TryParse(userIdOrNull.Value, out var guid)) - { - return guid; - } - - return null; + return userIdOrNull.Value; } /// diff --git a/Src/RCommon.Security/RCommon.Security.csproj b/Src/RCommon.Security/RCommon.Security.csproj index 5e21e874..0f32212c 100644 --- a/Src/RCommon.Security/RCommon.Security.csproj +++ b/Src/RCommon.Security/RCommon.Security.csproj @@ -39,4 +39,8 @@ + + + + diff --git a/Src/RCommon.Security/Users/CurrentUser.cs b/Src/RCommon.Security/Users/CurrentUser.cs index 15713d36..c096571e 100644 --- a/Src/RCommon.Security/Users/CurrentUser.cs +++ b/Src/RCommon.Security/Users/CurrentUser.cs @@ -31,10 +31,10 @@ public CurrentUser(ICurrentPrincipalAccessor principalAccessor) } /// - public virtual bool IsAuthenticated => Id.HasValue; + public virtual bool IsAuthenticated => _principalAccessor.Principal?.Identity?.IsAuthenticated ?? false; /// - public virtual Guid? Id => _principalAccessor.Principal?.FindUserId(); + public virtual string? Id => _principalAccessor.Principal?.FindUserId(); /// public virtual string? TenantId => _principalAccessor.Principal?.FindTenantId(); diff --git a/Src/RCommon.Security/Users/CurrentUserExtensions.cs b/Src/RCommon.Security/Users/CurrentUserExtensions.cs index 2a6cc9b5..37f5e712 100644 --- a/Src/RCommon.Security/Users/CurrentUserExtensions.cs +++ b/Src/RCommon.Security/Users/CurrentUserExtensions.cs @@ -1,11 +1,4 @@ -using RCommon.Security.Claims; -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.Globalization; -using System.Linq; -using System.Text; -using System.Threading.Tasks; +using System; namespace RCommon.Security.Users { @@ -26,38 +19,16 @@ public static class CurrentUserExtensions return currentUser.FindClaim(claimType)?.Value; } - /// - /// Finds a claim of the specified type and converts its value to . - /// - /// The target value type to convert the claim value to. - /// The current user instance. - /// The claim type URI to search for. - /// The converted claim value, or default if the claim is not found. - /// Uses with . - public static T FindClaimValue(this ICurrentUser currentUser, string claimType) - where T : struct - { - var value = currentUser.FindClaimValue(claimType); - if (value == null) - { - return default; - } - return (T)Convert.ChangeType(value, typeof(T), CultureInfo.InvariantCulture); - } - /// /// Gets the current user's ID, asserting that it is not null. /// /// The current user instance. - /// The user's identifier. + /// The user's string identifier. /// Thrown if is null. - public static Guid GetId(this ICurrentUser currentUser) + public static string GetId(this ICurrentUser currentUser) { - Debug.Assert(currentUser.Id != null, "currentUser.Id != null"); - - return currentUser.Id.Value; + return currentUser.Id + ?? throw new InvalidOperationException("The current user ID is null. Ensure the user is authenticated and has a NameIdentifier claim."); } - - } } diff --git a/Src/RCommon.Security/Users/ICurrentUser.cs b/Src/RCommon.Security/Users/ICurrentUser.cs index 8f4172cf..8b4a15e2 100644 --- a/Src/RCommon.Security/Users/ICurrentUser.cs +++ b/Src/RCommon.Security/Users/ICurrentUser.cs @@ -1,5 +1,4 @@ -using System; -using System.Security.Claims; +using System.Security.Claims; namespace RCommon.Security.Users { @@ -11,10 +10,10 @@ public interface ICurrentUser /// /// Gets the unique identifier of the current user, or null if no user is authenticated. /// - Guid? Id { get; } + string? Id { get; } /// - /// Gets a value indicating whether the current user is authenticated (i.e., is not null). + /// Gets a value indicating whether the current user is authenticated, as reported by the underlying . /// bool IsAuthenticated { get; } diff --git a/Tests/RCommon.Security.Tests/ClaimTypesConstTests.cs b/Tests/RCommon.Security.Tests/ClaimTypesConstTests.cs new file mode 100644 index 00000000..efeb5ba3 --- /dev/null +++ b/Tests/RCommon.Security.Tests/ClaimTypesConstTests.cs @@ -0,0 +1,67 @@ +using System.Security.Claims; +using FluentAssertions; +using RCommon.Security.Claims; +using Xunit; + +namespace RCommon.Security.Tests; + +public class ClaimTypesConstTests : IDisposable +{ + public ClaimTypesConstTests() + { + ClaimTypesConst.Reset(); + } + + public void Dispose() + { + ClaimTypesConst.Reset(); + } + + [Fact] + public void Defaults_MatchStandardClaimTypes() + { + ClaimTypesConst.UserId.Should().Be(ClaimTypes.NameIdentifier); + ClaimTypesConst.UserName.Should().Be(ClaimTypes.Name); + ClaimTypesConst.Name.Should().Be(ClaimTypes.GivenName); + ClaimTypesConst.SurName.Should().Be(ClaimTypes.Surname); + ClaimTypesConst.Role.Should().Be(ClaimTypes.Role); + ClaimTypesConst.Email.Should().Be(ClaimTypes.Email); + ClaimTypesConst.TenantId.Should().Be("tenantid"); + ClaimTypesConst.ClientId.Should().Be("client_id"); + } + + [Fact] + public void Configure_AppliesCustomValues() + { + ClaimTypesConst.Configure(options => + { + options.UserId = "sub"; + options.Role = "roles"; + options.TenantId = "tenant_id"; + }); + + ClaimTypesConst.UserId.Should().Be("sub"); + ClaimTypesConst.Role.Should().Be("roles"); + ClaimTypesConst.TenantId.Should().Be("tenant_id"); + // Unchanged values keep defaults + ClaimTypesConst.Email.Should().Be(ClaimTypes.Email); + } + + [Fact] + public void Configure_CalledTwice_ThrowsInvalidOperationException() + { + ClaimTypesConst.Configure(options => { options.UserId = "sub"; }); + + var action = () => ClaimTypesConst.Configure(options => { options.Role = "roles"; }); + + action.Should().Throw(); + } + + [Fact] + public void Configure_WithNullAction_ThrowsArgumentNullException() + { + var action = () => ClaimTypesConst.Configure(null!); + + action.Should().Throw(); + } +} diff --git a/Tests/RCommon.Security.Tests/ClaimsIdentityExtensionsTests.cs b/Tests/RCommon.Security.Tests/ClaimsIdentityExtensionsTests.cs new file mode 100644 index 00000000..2500d09b --- /dev/null +++ b/Tests/RCommon.Security.Tests/ClaimsIdentityExtensionsTests.cs @@ -0,0 +1,109 @@ +using System.Security.Claims; +using FluentAssertions; +using RCommon.Security.Claims; +using Xunit; + +namespace RCommon.Security.Tests; + +public class ClaimsIdentityExtensionsTests : IDisposable +{ + public ClaimsIdentityExtensionsTests() + { + ClaimTypesConst.Reset(); + } + + public void Dispose() + { + ClaimTypesConst.Reset(); + } + + [Fact] + public void FindUserId_WithGuidClaim_ReturnsGuidString() + { + var guid = Guid.NewGuid().ToString(); + var principal = CreatePrincipal(ClaimTypes.NameIdentifier, guid); + + var result = principal.FindUserId(); + + result.Should().Be(guid); + } + + [Fact] + public void FindUserId_WithIntegerClaim_ReturnsIntegerString() + { + var principal = CreatePrincipal(ClaimTypes.NameIdentifier, "12345"); + + var result = principal.FindUserId(); + + result.Should().Be("12345"); + } + + [Fact] + public void FindUserId_WithAuth0StyleClaim_ReturnsFullString() + { + var principal = CreatePrincipal(ClaimTypes.NameIdentifier, "auth0|abc123def456"); + + var result = principal.FindUserId(); + + result.Should().Be("auth0|abc123def456"); + } + + [Fact] + public void FindUserId_WhenClaimMissing_ReturnsNull() + { + var principal = CreatePrincipal("other-claim", "value"); + + var result = principal.FindUserId(); + + result.Should().BeNull(); + } + + [Fact] + public void FindUserId_WhenClaimValueEmpty_ReturnsNull() + { + var principal = CreatePrincipal(ClaimTypes.NameIdentifier, ""); + + var result = principal.FindUserId(); + + result.Should().BeNull(); + } + + [Fact] + public void FindUserId_WhenClaimValueWhitespace_ReturnsNull() + { + var principal = CreatePrincipal(ClaimTypes.NameIdentifier, " "); + + var result = principal.FindUserId(); + + result.Should().BeNull(); + } + + [Fact] + public void FindUserId_OnIIdentity_WithStringClaim_ReturnsString() + { + var identity = new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-42") }, "test"); + + var result = ((System.Security.Principal.IIdentity)identity).FindUserId(); + + result.Should().Be("user-42"); + } + + [Fact] + public void FindUserId_OnIIdentity_WhenClaimMissing_ReturnsNull() + { + var identity = new ClaimsIdentity( + new[] { new Claim("other", "value") }, "test"); + + var result = ((System.Security.Principal.IIdentity)identity).FindUserId(); + + result.Should().BeNull(); + } + + private static ClaimsPrincipal CreatePrincipal(string claimType, string claimValue) + { + var claims = new[] { new Claim(claimType, claimValue) }; + var identity = new ClaimsIdentity(claims, "test"); + return new ClaimsPrincipal(identity); + } +} diff --git a/Tests/RCommon.Security.Tests/CurrentUserTests.cs b/Tests/RCommon.Security.Tests/CurrentUserTests.cs index 1f100592..6a4f9643 100644 --- a/Tests/RCommon.Security.Tests/CurrentUserTests.cs +++ b/Tests/RCommon.Security.Tests/CurrentUserTests.cs @@ -32,7 +32,7 @@ public void Constructor_WithValidAccessor_Succeeds() } [Fact] - public void IsAuthenticated_WhenIdIsNull_ReturnsFalse() + public void IsAuthenticated_WhenPrincipalIsNull_ReturnsFalse() { // Arrange _mockPrincipalAccessor.Setup(x => x.Principal).Returns((ClaimsPrincipal?)null); @@ -46,12 +46,11 @@ public void IsAuthenticated_WhenIdIsNull_ReturnsFalse() } [Fact] - public void IsAuthenticated_WhenIdHasValue_ReturnsTrue() + public void IsAuthenticated_WhenIdentityIsAuthenticated_ReturnsTrue() { - // Arrange - var userId = Guid.NewGuid(); - var claims = new[] { new Claim(ClaimTypes.NameIdentifier, userId.ToString()) }; - var identity = new ClaimsIdentity(claims, "test"); + // Arrange - identity with AuthenticationType set (makes IsAuthenticated true) + var claims = new[] { new Claim(ClaimTypes.NameIdentifier, "user-42") }; + var identity = new ClaimsIdentity(claims, "Bearer"); var principal = new ClaimsPrincipal(identity); _mockPrincipalAccessor.Setup(x => x.Principal).Returns(principal); var currentUser = CreateCurrentUser(); @@ -63,6 +62,40 @@ public void IsAuthenticated_WhenIdHasValue_ReturnsTrue() result.Should().BeTrue(); } + [Fact] + public void IsAuthenticated_WhenAuthenticatedButNoNameIdentifier_ReturnsTrue() + { + // Arrange - authenticated identity with roles but no NameIdentifier + var claims = new[] { new Claim(ClaimTypes.Role, "Admin") }; + var identity = new ClaimsIdentity(claims, "Bearer"); + var principal = new ClaimsPrincipal(identity); + _mockPrincipalAccessor.Setup(x => x.Principal).Returns(principal); + var currentUser = CreateCurrentUser(); + + // Act + var result = currentUser.IsAuthenticated; + + // Assert + result.Should().BeTrue(); + } + + [Fact] + public void IsAuthenticated_WhenIdentityNotAuthenticated_ReturnsFalse() + { + // Arrange - identity without AuthenticationType (unauthenticated) + var claims = new[] { new Claim(ClaimTypes.NameIdentifier, "user-42") }; + var identity = new ClaimsIdentity(claims); // no authenticationType + var principal = new ClaimsPrincipal(identity); + _mockPrincipalAccessor.Setup(x => x.Principal).Returns(principal); + var currentUser = CreateCurrentUser(); + + // Act + var result = currentUser.IsAuthenticated; + + // Assert + result.Should().BeFalse(); + } + [Fact] public void Id_WhenPrincipalIsNull_ReturnsNull() { @@ -77,6 +110,41 @@ public void Id_WhenPrincipalIsNull_ReturnsNull() result.Should().BeNull(); } + [Fact] + public void Id_WithStringIdentifier_ReturnsRawString() + { + // Arrange + var claims = new[] { new Claim(ClaimTypes.NameIdentifier, "auth0|abc123") }; + var identity = new ClaimsIdentity(claims, "test"); + var principal = new ClaimsPrincipal(identity); + _mockPrincipalAccessor.Setup(x => x.Principal).Returns(principal); + var currentUser = CreateCurrentUser(); + + // Act + var result = currentUser.Id; + + // Assert + result.Should().Be("auth0|abc123"); + } + + [Fact] + public void Id_WithGuidIdentifier_ReturnsGuidAsString() + { + // Arrange + var userId = Guid.NewGuid(); + var claims = new[] { new Claim(ClaimTypes.NameIdentifier, userId.ToString()) }; + var identity = new ClaimsIdentity(claims, "test"); + var principal = new ClaimsPrincipal(identity); + _mockPrincipalAccessor.Setup(x => x.Principal).Returns(principal); + var currentUser = CreateCurrentUser(); + + // Act + var result = currentUser.Id; + + // Assert + result.Should().Be(userId.ToString()); + } + [Fact] public void TenantId_WhenPrincipalIsNull_ReturnsNull() { @@ -287,6 +355,37 @@ public void GetAllClaims_WhenPrincipalIsNull_ReturnsEmptyArray() result.Should().BeEmpty(); } + [Fact] + public void GetId_WhenIdIsPresent_ReturnsId() + { + // Arrange + var claims = new[] { new Claim(ClaimTypes.NameIdentifier, "user-42") }; + var identity = new ClaimsIdentity(claims, "test"); + var principal = new ClaimsPrincipal(identity); + _mockPrincipalAccessor.Setup(x => x.Principal).Returns(principal); + var currentUser = CreateCurrentUser(); + + // Act + var result = currentUser.GetId(); + + // Assert + result.Should().Be("user-42"); + } + + [Fact] + public void GetId_WhenIdIsNull_ThrowsInvalidOperationException() + { + // Arrange + _mockPrincipalAccessor.Setup(x => x.Principal).Returns((ClaimsPrincipal?)null); + var currentUser = CreateCurrentUser(); + + // Act + var action = () => currentUser.GetId(); + + // Assert + action.Should().Throw(); + } + [Fact] public void CurrentUser_ImplementsICurrentUser() { diff --git a/docs/superpowers/plans/2026-04-05-security-abstractions-string-id.md b/docs/superpowers/plans/2026-04-05-security-abstractions-string-id.md new file mode 100644 index 00000000..98de97ef --- /dev/null +++ b/docs/superpowers/plans/2026-04-05-security-abstractions-string-id.md @@ -0,0 +1,822 @@ +# Security Abstractions: String-Based User Identity — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Change `ICurrentUser.Id` from `Guid?` to `string?`, align `IsAuthenticated` with `ClaimsIdentity.IsAuthenticated`, replace mutable `ClaimTypesConst` statics with a configure-once freeze pattern, remove `FindClaimValue`, and update all tests, examples, and website docs. + +**Architecture:** This is a breaking change to the `RCommon.Security` public API surface. All changes flow from the interface (`ICurrentUser.Id` type change) outward through the implementation, extensions, tests, examples, and documentation. The `ClaimTypesConst` freeze pattern is an independent but co-shipped change. + +**Tech Stack:** C# / .NET (multi-target net8.0/net9.0/net10.0), xUnit, FluentAssertions, Moq + +**Spec:** `docs/superpowers/specs/2026-04-05-security-abstractions-string-id-design.md` + +--- + +## File Map + +| File | Action | Responsibility | +|---|---|---| +| `Src/RCommon.Security/Claims/ClaimTypesOptions.cs` | Create | Options POCO with settable claim type properties and defaults | +| `Src/RCommon.Security/Claims/ClaimTypesConst.cs` | Modify | Replace mutable statics with `Configure`/freeze pattern backed by `ClaimTypesOptions` | +| `Src/RCommon.Security/Users/ICurrentUser.cs` | Modify | `Id` type: `Guid?` → `string?` | +| `Src/RCommon.Security/ClaimsIdentityExtensions.cs` | Modify | `FindUserId` overloads return `string?` | +| `Src/RCommon.Security/Users/CurrentUser.cs` | Modify | `Id` return type, `IsAuthenticated` derivation | +| `Src/RCommon.Security/Users/CurrentUserExtensions.cs` | Modify | `GetId()` returns `string`, remove `FindClaimValue` | +| `Src/RCommon.Security/RCommon.Security.csproj` | Modify | Add `InternalsVisibleTo` for test project | +| `Tests/RCommon.Security.Tests/ClaimTypesConstTests.cs` | Create | Configure/freeze/reset tests | +| `Tests/RCommon.Security.Tests/ClaimsIdentityExtensionsTests.cs` | Create | `FindUserId` string return tests, non-GUID cases | +| `Tests/RCommon.Security.Tests/CurrentUserTests.cs` | Modify | Update `Id`/`IsAuthenticated` assertions | +| `Examples/CleanWithCQRS/HR.LeaveManagement.Persistence/AuditableDbContext.cs` | Modify | Simplify `Id` usage | +| `website/docs/security-web/authorization.mdx` | Modify | Update code examples and API summary | +| `website/docs/security-web/web-utilities.mdx` | Modify | Update middleware example | +| `website/versioned_docs/version-2.4.1/security-web/authorization.mdx` | Modify | Mirror authorization.mdx changes | +| `website/versioned_docs/version-2.4.1/security-web/web-utilities.mdx` | Modify | Mirror web-utilities.mdx changes | + +--- + +## Task 1: ClaimTypesOptions and ClaimTypesConst freeze pattern + +**Files:** +- Create: `Src/RCommon.Security/Claims/ClaimTypesOptions.cs` +- Modify: `Src/RCommon.Security/Claims/ClaimTypesConst.cs` +- Modify: `Src/RCommon.Security/RCommon.Security.csproj` +- Create: `Tests/RCommon.Security.Tests/ClaimTypesConstTests.cs` + +### Step 1: Write ClaimTypesConst tests + +- [ ] **Create `Tests/RCommon.Security.Tests/ClaimTypesConstTests.cs`** + +```csharp +using System.Security.Claims; +using FluentAssertions; +using RCommon.Security.Claims; +using Xunit; + +namespace RCommon.Security.Tests; + +public class ClaimTypesConstTests : IDisposable +{ + public ClaimTypesConstTests() + { + ClaimTypesConst.Reset(); + } + + public void Dispose() + { + ClaimTypesConst.Reset(); + } + + [Fact] + public void Defaults_MatchStandardClaimTypes() + { + ClaimTypesConst.UserId.Should().Be(ClaimTypes.NameIdentifier); + ClaimTypesConst.UserName.Should().Be(ClaimTypes.Name); + ClaimTypesConst.Name.Should().Be(ClaimTypes.GivenName); + ClaimTypesConst.SurName.Should().Be(ClaimTypes.Surname); + ClaimTypesConst.Role.Should().Be(ClaimTypes.Role); + ClaimTypesConst.Email.Should().Be(ClaimTypes.Email); + ClaimTypesConst.TenantId.Should().Be("tenantid"); + ClaimTypesConst.ClientId.Should().Be("client_id"); + } + + [Fact] + public void Configure_AppliesCustomValues() + { + ClaimTypesConst.Configure(options => + { + options.UserId = "sub"; + options.Role = "roles"; + options.TenantId = "tenant_id"; + }); + + ClaimTypesConst.UserId.Should().Be("sub"); + ClaimTypesConst.Role.Should().Be("roles"); + ClaimTypesConst.TenantId.Should().Be("tenant_id"); + // Unchanged values keep defaults + ClaimTypesConst.Email.Should().Be(ClaimTypes.Email); + } + + [Fact] + public void Configure_CalledTwice_ThrowsInvalidOperationException() + { + ClaimTypesConst.Configure(options => { options.UserId = "sub"; }); + + var action = () => ClaimTypesConst.Configure(options => { options.Role = "roles"; }); + + action.Should().Throw(); + } + + [Fact] + public void Configure_WithNullAction_ThrowsArgumentNullException() + { + var action = () => ClaimTypesConst.Configure(null!); + + action.Should().Throw(); + } +} +``` + +- [ ] **Run tests to verify they fail** + +Run: `dotnet test Tests/RCommon.Security.Tests/ --filter "FullyQualifiedName~ClaimTypesConstTests" --no-restore -v quiet` +Expected: Build errors — `Reset()` and `Configure()` don't exist yet. + +### Step 2: Add InternalsVisibleTo for test project + +- [ ] **Modify `Src/RCommon.Security/RCommon.Security.csproj`** — add a new `` block: + +```xml + + + +``` + +### Step 3: Create ClaimTypesOptions + +- [ ] **Create `Src/RCommon.Security/Claims/ClaimTypesOptions.cs`** + +```csharp +using System.Security.Claims; + +namespace RCommon.Security.Claims +{ + /// + /// Configurable options for claim type URI mappings. + /// Set properties to match the claim types issued by your identity provider. + /// + public class ClaimTypesOptions + { + public string UserName { get; set; } = ClaimTypes.Name; + public string Name { get; set; } = ClaimTypes.GivenName; + public string SurName { get; set; } = ClaimTypes.Surname; + public string UserId { get; set; } = ClaimTypes.NameIdentifier; + public string Role { get; set; } = ClaimTypes.Role; + public string Email { get; set; } = ClaimTypes.Email; + public string TenantId { get; set; } = "tenantid"; + public string ClientId { get; set; } = "client_id"; + } +} +``` + +### Step 4: Rewrite ClaimTypesConst with freeze pattern + +- [ ] **Replace contents of `Src/RCommon.Security/Claims/ClaimTypesConst.cs`** + +```csharp +using System; +using System.Security.Claims; + +namespace RCommon.Security.Claims +{ + /// + /// Provides claim type URI constants used throughout the security subsystem. + /// Call once at startup to override defaults. + /// After configuration (or first property access), values are frozen. + /// + public static class ClaimTypesConst + { + private static ClaimTypesOptions? _options; + private static bool _frozen; + private static readonly object _lock = new(); + + public static string UserName => GetOptions().UserName; + public static string Name => GetOptions().Name; + public static string SurName => GetOptions().SurName; + public static string UserId => GetOptions().UserId; + public static string Role => GetOptions().Role; + public static string Email => GetOptions().Email; + public static string TenantId => GetOptions().TenantId; + public static string ClientId => GetOptions().ClientId; + + /// + /// Configures claim type mappings. May only be called once, before any property is accessed. + /// + public static void Configure(Action configure) + { + Guard.IsNotNull(configure, nameof(configure)); + + lock (_lock) + { + if (_frozen) + { + throw new InvalidOperationException( + "ClaimTypesConst has already been configured or accessed. Configure may only be called once, before any property is read."); + } + + var options = new ClaimTypesOptions(); + configure(options); + _options = options; + _frozen = true; + } + } + + private static ClaimTypesOptions GetOptions() + { + if (_options != null) + return _options; + + lock (_lock) + { + if (_options != null) + return _options; + + _options = new ClaimTypesOptions(); + _frozen = true; + return _options; + } + } + + /// + /// Resets configuration to allow reconfiguration. Internal — for test isolation only. + /// + internal static void Reset() + { + lock (_lock) + { + _options = null; + _frozen = false; + } + } + } +} +``` + +- [ ] **Run tests to verify they pass** + +Run: `dotnet test Tests/RCommon.Security.Tests/ --filter "FullyQualifiedName~ClaimTypesConstTests" --no-restore -v quiet` +Expected: All 4 tests PASS. + +- [ ] **Commit** + +```bash +git add Src/RCommon.Security/Claims/ClaimTypesOptions.cs Src/RCommon.Security/Claims/ClaimTypesConst.cs Src/RCommon.Security/RCommon.Security.csproj Tests/RCommon.Security.Tests/ClaimTypesConstTests.cs +git commit -m "Replace mutable ClaimTypesConst statics with configure-once freeze pattern" +``` + +--- + +## Task 2: FindUserId returns string + +**Files:** +- Modify: `Src/RCommon.Security/ClaimsIdentityExtensions.cs` +- Create: `Tests/RCommon.Security.Tests/ClaimsIdentityExtensionsTests.cs` + +### Step 1: Write FindUserId tests + +- [ ] **Create `Tests/RCommon.Security.Tests/ClaimsIdentityExtensionsTests.cs`** + +```csharp +using System.Security.Claims; +using FluentAssertions; +using RCommon.Security.Claims; +using Xunit; + +namespace RCommon.Security.Tests; + +public class ClaimsIdentityExtensionsTests : IDisposable +{ + public ClaimsIdentityExtensionsTests() + { + ClaimTypesConst.Reset(); + } + + public void Dispose() + { + ClaimTypesConst.Reset(); + } + + [Fact] + public void FindUserId_WithGuidClaim_ReturnsGuidString() + { + var guid = Guid.NewGuid().ToString(); + var principal = CreatePrincipal(ClaimTypes.NameIdentifier, guid); + + var result = principal.FindUserId(); + + result.Should().Be(guid); + } + + [Fact] + public void FindUserId_WithIntegerClaim_ReturnsIntegerString() + { + var principal = CreatePrincipal(ClaimTypes.NameIdentifier, "12345"); + + var result = principal.FindUserId(); + + result.Should().Be("12345"); + } + + [Fact] + public void FindUserId_WithAuth0StyleClaim_ReturnsFullString() + { + var principal = CreatePrincipal(ClaimTypes.NameIdentifier, "auth0|abc123def456"); + + var result = principal.FindUserId(); + + result.Should().Be("auth0|abc123def456"); + } + + [Fact] + public void FindUserId_WhenClaimMissing_ReturnsNull() + { + var principal = CreatePrincipal("other-claim", "value"); + + var result = principal.FindUserId(); + + result.Should().BeNull(); + } + + [Fact] + public void FindUserId_WhenClaimValueEmpty_ReturnsNull() + { + var principal = CreatePrincipal(ClaimTypes.NameIdentifier, ""); + + var result = principal.FindUserId(); + + result.Should().BeNull(); + } + + [Fact] + public void FindUserId_WhenClaimValueWhitespace_ReturnsNull() + { + var principal = CreatePrincipal(ClaimTypes.NameIdentifier, " "); + + var result = principal.FindUserId(); + + result.Should().BeNull(); + } + + [Fact] + public void FindUserId_OnIIdentity_WithStringClaim_ReturnsString() + { + var identity = new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, "user-42") }, "test"); + + var result = ((System.Security.Principal.IIdentity)identity).FindUserId(); + + result.Should().Be("user-42"); + } + + [Fact] + public void FindUserId_OnIIdentity_WhenClaimMissing_ReturnsNull() + { + var identity = new ClaimsIdentity( + new[] { new Claim("other", "value") }, "test"); + + var result = ((System.Security.Principal.IIdentity)identity).FindUserId(); + + result.Should().BeNull(); + } + + private static ClaimsPrincipal CreatePrincipal(string claimType, string claimValue) + { + var claims = new[] { new Claim(claimType, claimValue) }; + var identity = new ClaimsIdentity(claims, "test"); + return new ClaimsPrincipal(identity); + } +} +``` + +- [ ] **Run tests to verify they fail** + +Run: `dotnet test Tests/RCommon.Security.Tests/ --filter "FullyQualifiedName~ClaimsIdentityExtensionsTests" --no-restore -v quiet` +Expected: FAIL — `FindUserId` returns `Guid?` not `string?`, type mismatches. + +### Step 2: Change FindUserId to return string + +- [ ] **Modify `Src/RCommon.Security/ClaimsIdentityExtensions.cs`** — replace the two `FindUserId` methods. + +Replace the `ClaimsPrincipal` overload (lines 23-38) with: + +```csharp +/// +/// Extracts the user identifier from the principal's claims as a raw string value. +/// +/// The claims principal to search. +/// The user ID string, or null if the claim is missing or empty. +public static string? FindUserId(this ClaimsPrincipal principal) +{ + Guard.IsNotNull(principal, nameof(principal)); + + var userIdOrNull = principal.Claims?.FirstOrDefault(c => c.Type == ClaimTypesConst.UserId); + if (userIdOrNull == null || userIdOrNull.Value.IsNullOrWhiteSpace()) + { + return null; + } + + return userIdOrNull.Value; +} +``` + +Replace the `IIdentity` overload (lines 46-63) with: + +```csharp +/// +/// Extracts the user identifier from the identity's claims as a raw string value. +/// +/// The identity to search. Must be castable to . +/// The user ID string, or null if the claim is missing or empty. +public static string? FindUserId(this IIdentity identity) +{ + Guard.IsNotNull(identity, nameof(identity)); + + var claimsIdentity = identity as ClaimsIdentity; + + var userIdOrNull = claimsIdentity?.Claims?.FirstOrDefault(c => c.Type == ClaimTypesConst.UserId); + if (userIdOrNull == null || userIdOrNull.Value.IsNullOrWhiteSpace()) + { + return null; + } + + return userIdOrNull.Value; +} +``` + +- [ ] **Run tests to verify they pass** + +Run: `dotnet test Tests/RCommon.Security.Tests/ --filter "FullyQualifiedName~ClaimsIdentityExtensionsTests" --no-restore -v quiet` +Expected: All 8 tests PASS. + +- [ ] **Commit** + +```bash +git add Src/RCommon.Security/ClaimsIdentityExtensions.cs Tests/RCommon.Security.Tests/ClaimsIdentityExtensionsTests.cs +git commit -m "Change FindUserId to return string instead of Guid" +``` + +--- + +## Task 3: ICurrentUser.Id to string and IsAuthenticated alignment + +**Files:** +- Modify: `Src/RCommon.Security/Users/ICurrentUser.cs` +- Modify: `Src/RCommon.Security/Users/CurrentUser.cs` +- Modify: `Src/RCommon.Security/Users/CurrentUserExtensions.cs` +- Modify: `Tests/RCommon.Security.Tests/CurrentUserTests.cs` + +### Step 1: Update existing tests and add new IsAuthenticated tests + +- [ ] **Modify `Tests/RCommon.Security.Tests/CurrentUserTests.cs`** + +Replace the `IsAuthenticated_WhenIdIsNull_ReturnsFalse` test (line 35-46) with: + +```csharp +[Fact] +public void IsAuthenticated_WhenPrincipalIsNull_ReturnsFalse() +{ + // Arrange + _mockPrincipalAccessor.Setup(x => x.Principal).Returns((ClaimsPrincipal?)null); + var currentUser = CreateCurrentUser(); + + // Act + var result = currentUser.IsAuthenticated; + + // Assert + result.Should().BeFalse(); +} +``` + +Replace the `IsAuthenticated_WhenIdHasValue_ReturnsTrue` test (line 48-64) with: + +```csharp +[Fact] +public void IsAuthenticated_WhenIdentityIsAuthenticated_ReturnsTrue() +{ + // Arrange - identity with AuthenticationType set (makes IsAuthenticated true) + var claims = new[] { new Claim(ClaimTypes.NameIdentifier, "user-42") }; + var identity = new ClaimsIdentity(claims, "Bearer"); + var principal = new ClaimsPrincipal(identity); + _mockPrincipalAccessor.Setup(x => x.Principal).Returns(principal); + var currentUser = CreateCurrentUser(); + + // Act + var result = currentUser.IsAuthenticated; + + // Assert + result.Should().BeTrue(); +} + +[Fact] +public void IsAuthenticated_WhenAuthenticatedButNoNameIdentifier_ReturnsTrue() +{ + // Arrange - authenticated identity with roles but no NameIdentifier + var claims = new[] { new Claim(ClaimTypes.Role, "Admin") }; + var identity = new ClaimsIdentity(claims, "Bearer"); + var principal = new ClaimsPrincipal(identity); + _mockPrincipalAccessor.Setup(x => x.Principal).Returns(principal); + var currentUser = CreateCurrentUser(); + + // Act + var result = currentUser.IsAuthenticated; + + // Assert + result.Should().BeTrue(); +} + +[Fact] +public void IsAuthenticated_WhenIdentityNotAuthenticated_ReturnsFalse() +{ + // Arrange - identity without AuthenticationType (unauthenticated) + var claims = new[] { new Claim(ClaimTypes.NameIdentifier, "user-42") }; + var identity = new ClaimsIdentity(claims); // no authenticationType + var principal = new ClaimsPrincipal(identity); + _mockPrincipalAccessor.Setup(x => x.Principal).Returns(principal); + var currentUser = CreateCurrentUser(); + + // Act + var result = currentUser.IsAuthenticated; + + // Assert + result.Should().BeFalse(); +} +``` + +Update the `Id_WhenPrincipalIsNull_ReturnsNull` test — no changes needed (assertion is `BeNull()` which works for both types). + +Add a new test for string-based Id: + +```csharp +[Fact] +public void Id_WithStringIdentifier_ReturnsRawString() +{ + // Arrange + var claims = new[] { new Claim(ClaimTypes.NameIdentifier, "auth0|abc123") }; + var identity = new ClaimsIdentity(claims, "test"); + var principal = new ClaimsPrincipal(identity); + _mockPrincipalAccessor.Setup(x => x.Principal).Returns(principal); + var currentUser = CreateCurrentUser(); + + // Act + var result = currentUser.Id; + + // Assert + result.Should().Be("auth0|abc123"); +} + +[Fact] +public void Id_WithGuidIdentifier_ReturnsGuidAsString() +{ + // Arrange + var userId = Guid.NewGuid(); + var claims = new[] { new Claim(ClaimTypes.NameIdentifier, userId.ToString()) }; + var identity = new ClaimsIdentity(claims, "test"); + var principal = new ClaimsPrincipal(identity); + _mockPrincipalAccessor.Setup(x => x.Principal).Returns(principal); + var currentUser = CreateCurrentUser(); + + // Act + var result = currentUser.Id; + + // Assert + result.Should().Be(userId.ToString()); +} +``` + +- [ ] **Run tests to verify they fail** + +Run: `dotnet test Tests/RCommon.Security.Tests/ --filter "FullyQualifiedName~CurrentUserTests" --no-restore -v quiet` +Expected: FAIL — `IsAuthenticated` still uses `Id.HasValue`, `Id` still returns `Guid?`. + +### Step 2: Change ICurrentUser interface + +- [ ] **Modify `Src/RCommon.Security/Users/ICurrentUser.cs`** — change line 14: + +From: `Guid? Id { get; }` +To: `string? Id { get; }` + +Remove `using System;` if it becomes unused (it won't be needed after removing `Guid`). + +### Step 3: Change CurrentUser implementation + +- [ ] **Modify `Src/RCommon.Security/Users/CurrentUser.cs`** + +Change `IsAuthenticated` (line 34): +From: `public virtual bool IsAuthenticated => Id.HasValue;` +To: `public virtual bool IsAuthenticated => _principalAccessor.Principal?.Identity?.IsAuthenticated ?? false;` + +Change `Id` (line 37): +From: `public virtual Guid? Id => _principalAccessor.Principal?.FindUserId();` +To: `public virtual string? Id => _principalAccessor.Principal?.FindUserId();` + +The return type now matches because `FindUserId()` was changed to `string?` in Task 2. + +### Step 4: Change CurrentUserExtensions + +- [ ] **Modify `Src/RCommon.Security/Users/CurrentUserExtensions.cs`** + +Replace `GetId()` method (lines 54-59) with: + +```csharp +/// +/// Gets the current user's ID, asserting that it is not null. +/// +/// The current user instance. +/// The user's string identifier. +/// Thrown if is null. +public static string GetId(this ICurrentUser currentUser) +{ + return currentUser.Id + ?? throw new InvalidOperationException("The current user ID is null. Ensure the user is authenticated and has a NameIdentifier claim."); +} +``` + +Remove the `FindClaimValue` method (lines 37-46) entirely. + +Remove `using System.Diagnostics;` (no longer needed after removing `Debug.Assert`). + +- [ ] **Run tests to verify they pass** + +Run: `dotnet test Tests/RCommon.Security.Tests/ --filter "FullyQualifiedName~CurrentUserTests" --no-restore -v quiet` +Expected: All tests PASS. + +- [ ] **Commit** + +```bash +git add Src/RCommon.Security/Users/ICurrentUser.cs Src/RCommon.Security/Users/CurrentUser.cs Src/RCommon.Security/Users/CurrentUserExtensions.cs Tests/RCommon.Security.Tests/CurrentUserTests.cs +git commit -m "Change ICurrentUser.Id to string, align IsAuthenticated with ClaimsIdentity, remove FindClaimValue" +``` + +--- + +## Task 4: Update example code + +**Files:** +- Modify: `Examples/CleanWithCQRS/HR.LeaveManagement.Persistence/AuditableDbContext.cs` + +- [ ] **Modify line 38 of `AuditableDbContext.cs`** + +From: +```csharp +string userId = (_currentUser == null || _currentUser.Id == null ? "System" : _currentUser.Id.ToString()); +``` + +To: +```csharp +string userId = _currentUser?.Id ?? "System"; +``` + +- [ ] **Build the example to verify compilation** + +Run: `dotnet build Examples/CleanWithCQRS/HR.LeaveManagement.Persistence/ --no-restore -v quiet` +Expected: Build succeeded. + +- [ ] **Commit** + +```bash +git add Examples/CleanWithCQRS/HR.LeaveManagement.Persistence/AuditableDbContext.cs +git commit -m "Simplify AuditableDbContext to use string-based ICurrentUser.Id" +``` + +--- + +## Task 5: Run full test suite + +- [ ] **Run all security tests** + +Run: `dotnet test Tests/RCommon.Security.Tests/ --no-restore -v quiet` +Expected: All tests PASS, zero failures. + +- [ ] **Build entire solution to catch any remaining compilation errors** + +Run: `dotnet build RCommon.sln --no-restore -v quiet` +Expected: Build succeeded. If any other projects reference `ICurrentUser.Id` as `Guid?`, they will surface here. + +--- + +## Task 6: Update website documentation + +**Files:** +- Modify: `website/docs/security-web/authorization.mdx` +- Modify: `website/docs/security-web/web-utilities.mdx` +- Modify: `website/versioned_docs/version-2.4.1/security-web/authorization.mdx` +- Modify: `website/versioned_docs/version-2.4.1/security-web/web-utilities.mdx` + +### Step 1: Update authorization.mdx (current docs) + +- [ ] **Modify `website/docs/security-web/authorization.mdx`** — apply the following changes: + +**Line 82-89** — replace the "Overriding claim type URIs" code block: + +From: +```csharp +using RCommon.Security.Claims; + +// Map to the short claim names issued by your OIDC provider. +ClaimTypesConst.UserId = "sub"; +ClaimTypesConst.Role = "roles"; +ClaimTypesConst.Email = "email"; +ClaimTypesConst.TenantId = "tenant_id"; +ClaimTypesConst.ClientId = "client_id"; +``` + +To: +```csharp +using RCommon.Security.Claims; + +// Map to the short claim names issued by your OIDC provider. +// Configure must be called once at startup, before any property is accessed. +ClaimTypesConst.Configure(options => +{ + options.UserId = "sub"; + options.Role = "roles"; + options.Email = "email"; + options.TenantId = "tenant_id"; + options.ClientId = "client_id"; +}); +``` + +**Line 148** — in the "Accessing the current user" example: + +From: `CustomerId = _currentUser.Id!.Value,` +To: `CustomerId = _currentUser.Id!,` + +**Lines 178-181** — in the "Reading arbitrary claims" section, remove: + +```csharp +// Find a claim value converted to a specific struct type. +int employeeNumber = _currentUser.FindClaimValue("custom:employee_number"); +``` + +**Lines 282-284** — in the "Claims identity helpers" section: + +From: +```csharp +Guid? userId = principal.FindUserId(); // parses ClaimTypesConst.UserId as Guid +string? tenant = principal.FindTenantId(); // reads ClaimTypesConst.TenantId +string? client = principal.FindClientId(); // reads ClaimTypesConst.ClientId +``` + +To: +```csharp +string? userId = principal.FindUserId(); // reads ClaimTypesConst.UserId +string? tenant = principal.FindTenantId(); // reads ClaimTypesConst.TenantId +string? client = principal.FindClientId(); // reads ClaimTypesConst.ClientId +``` + +**Line 298** — in the API summary table, `CurrentUserExtensions` row: + +From: `FindClaimValue(string)`, `FindClaimValue(string)`, `GetId()` +To: `FindClaimValue(string)`, `GetId()` + +**Line 301** — in the API summary table, `ClaimTypesConst` row: + +From: `Configurable claim type URIs: UserName, Name, SurName, UserId, Role, Email, TenantId, ClientId` +To: `Configure-once claim type URIs via Configure(Action): UserName, Name, SurName, UserId, Role, Email, TenantId, ClientId` + +### Step 2: Update web-utilities.mdx (current docs) + +- [ ] **Modify `website/docs/security-web/web-utilities.mdx`** — line 139: + +From: `var userId = currentUser.Id?.ToString() ?? "anonymous";` +To: `var userId = currentUser.Id ?? "anonymous";` + +### Step 3: Mirror changes to versioned docs + +- [ ] **Apply the same changes from Step 1 to `website/versioned_docs/version-2.4.1/security-web/authorization.mdx`** + +The versioned file has identical content — apply the same edits. + +- [ ] **Apply the same change from Step 2 to `website/versioned_docs/version-2.4.1/security-web/web-utilities.mdx`** + +### Step 4: Commit + +- [ ] **Commit** + +```bash +git add website/docs/security-web/authorization.mdx website/docs/security-web/web-utilities.mdx website/versioned_docs/version-2.4.1/security-web/authorization.mdx website/versioned_docs/version-2.4.1/security-web/web-utilities.mdx +git commit -m "Update website docs for string-based ICurrentUser.Id and ClaimTypesConst.Configure" +``` + +--- + +## Task 7: Final verification and squash + +- [ ] **Run full test suite one more time** + +Run: `dotnet test Tests/RCommon.Security.Tests/ -v quiet` +Expected: All tests PASS. + +- [ ] **Run full solution build** + +Run: `dotnet build RCommon.sln -v quiet` +Expected: Build succeeded. + +- [ ] **Rebase and squash interim commits into a single commit** + +```bash +git rebase -i main +``` + +Squash all task commits into one with message: + +``` +Breaking: Change ICurrentUser.Id from Guid? to string? + +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 +- Removed FindClaimValue (consumers parse claim strings themselves) +- Updated website documentation and examples +``` diff --git a/docs/superpowers/specs/2026-04-05-security-abstractions-string-id-design.md b/docs/superpowers/specs/2026-04-05-security-abstractions-string-id-design.md new file mode 100644 index 00000000..724bf242 --- /dev/null +++ b/docs/superpowers/specs/2026-04-05-security-abstractions-string-id-design.md @@ -0,0 +1,135 @@ +# Security Abstractions: String-Based User Identity + +**Date:** 2026-04-05 +**Status:** Approved +**Breaking Change:** Yes (alpha) + +## Problem + +`ICurrentUser.Id` exposes `Guid?`, but the underlying claim (`NameIdentifier`) is a string in .NET's claims model. The `FindUserId()` extension silently parses the claim to `Guid` via `Guid.TryParse`, returning `null` for any non-GUID identifier (integers, Auth0 `auth0|abc123`, Firebase UIDs, etc.). This is a lossy, opinionated conversion that doesn't belong in a framework abstraction. + +Additional issues: +- `IsAuthenticated` is derived from `Id.HasValue`, meaning a user with a valid authenticated identity but a non-GUID NameIdentifier appears unauthenticated. +- `FindClaimValue` uses `Convert.ChangeType` which handles many types poorly. +- `ClaimTypesConst` uses mutable static setters, allowing runtime mutation of global claim type mappings at any time. Not thread-safe. +- `ICurrentClient.Id` is already `string?`, making the `Guid?` on `ICurrentUser.Id` inconsistent. + +## Decisions + +| Decision | Choice | Rationale | +|---|---|---| +| `ICurrentUser.Id` type | `string?` | Aligns with claims standard; consumers parse downstream | +| `IsAuthenticated` derivation | `ClaimsIdentity.IsAuthenticated` | Decouples authentication from ID presence; aligns with .NET standard | +| `FindUserId()` return type | `string?` | Consistent with `FindTenantId()` and `FindClientId()` | +| `GetId()` return type | `string` | Convenience stays, type follows `Id` | +| `ClaimTypesConst` mutation | `Configure(Action)` freeze pattern | One-time config at startup, frozen after first use | +| `FindClaimValue` | Remove | YAGNI; consumers parse claim strings themselves | +| `ICurrentClient.IsAuthenticated` | No change (`Id != null`) | Correct for client credential flows | +| Migration approach | Single breaking change | Alpha stage; clean break, no deprecation | + +## Design + +### Interface Changes + +**`ICurrentUser`** +- `Guid? Id` becomes `string? Id` +- `bool IsAuthenticated` signature unchanged; contract changes to reflect `ClaimsIdentity.IsAuthenticated` + +**`ClaimTypesConst` → freeze-on-first-use pattern** + +Replace mutable static `{ get; set; }` properties with a configure-once pattern: + +1. Introduce a `ClaimTypesOptions` class with settable properties for each claim type mapping (`UserId`, `Role`, `Email`, `TenantId`, `ClientId`, `UserName`, `Name`, `SurName`). Defaults match the current `ClaimTypes.*` values. +2. `ClaimTypesConst` gets a `Configure(Action configure)` static method. Calling it applies the action to the options and freezes the configuration. A second call throws `InvalidOperationException`. +3. The existing static getters (`ClaimTypesConst.UserId`, etc.) become read-only properties backed by the frozen options instance. On first access, if `Configure` was never called, they resolve to defaults (same behavior as today for apps that don't customize). + +**Consumer migration:** + +```csharp +// Before (mutable statics): +ClaimTypesConst.UserId = "sub"; +ClaimTypesConst.Role = "roles"; +ClaimTypesConst.TenantId = "tenant_id"; + +// After (configure-once): +ClaimTypesConst.Configure(options => +{ + options.UserId = "sub"; + options.Role = "roles"; + options.TenantId = "tenant_id"; +}); +``` + +No changes to `ICurrentClient`. + +### Implementation Changes + +**`CurrentUser.cs`** +- `Id`: returns `_principalAccessor.Principal?.FindUserId()` (now `string?`) +- `IsAuthenticated`: changes from `Id.HasValue` to `_principalAccessor.Principal?.Identity?.IsAuthenticated ?? false` + +**`ClaimsIdentityExtensions.cs`** +- `FindUserId(this ClaimsPrincipal)`: returns `string?`. Removes `Guid.TryParse`, returns raw claim value. Same pattern as `FindTenantId`/`FindClientId`. +- `FindUserId(this IIdentity)`: same change. + +**`CurrentUserExtensions.cs`** +- `GetId()`: returns `string`. Throws `InvalidOperationException` if `Id` is null (replaces current `Debug.Assert` + `Nullable.Value` pattern with an explicit throw). +- `FindClaimValue()`: removed entirely. +- `FindClaimValue(string)` (non-generic): unchanged. + +**`ClaimTypesConst.cs`** +- Add `ClaimTypesOptions` class with settable properties and defaults. +- Add `Configure(Action)` static method with freeze semantics. +- Replace `{ get; set; }` properties with read-only getters backed by the frozen options. + +### Test Changes + +**`CurrentUserTests.cs`** +- Update `Id` assertions from `Guid` to `string?` +- Add `IsAuthenticated` cases: authenticated principal with no NameIdentifier (expect `true`), unauthenticated principal with NameIdentifier (expect `false`) + +**`ClaimsIdentityExtensionsTests.cs`** (new file) +- `FindUserId` tests asserting `string?` return +- Non-GUID identifier cases (integer IDs, Auth0-style `auth0|abc123`) +- Null/empty claim handling + +**`ClaimTypesConstTests`** (new or updated) +- Test `Configure` applies values correctly +- Test double-call to `Configure` throws `InvalidOperationException` +- Test default values work when `Configure` is never called +- `ClaimTypesConst` needs an internal `Reset()` method (or `[InternalsVisibleTo]` test access) to allow test isolation, since the freeze is process-global static state + +**`AuditableDbContext.cs`** (example) +- Simplify from `_currentUser.Id == null ? "System" : _currentUser.Id.ToString()` to `_currentUser.Id ?? "System"` + +### Website Documentation Changes + +**`website/docs/security-web/authorization.mdx`** (and `versioned_docs/version-2.4.1/` copy) +- "Accessing the current user" example: `_currentUser.Id!.Value` → `_currentUser.Id!` (no `.Value`, no longer `Guid?`) +- "Reading arbitrary claims" section: remove `FindClaimValue` example (method deleted) +- "Claims identity helpers" section: `Guid? userId = principal.FindUserId()` → `string? userId = principal.FindUserId()`; remove "parses as Guid" comment +- "Overriding claim type URIs" section: replace direct assignment examples with `ClaimTypesConst.Configure(...)` pattern +- API summary table: remove `FindClaimValue(string)` from `CurrentUserExtensions` description +- API summary table: update `ClaimTypesConst` description to mention configure-once pattern + +**`website/docs/security-web/web-utilities.mdx`** (and `versioned_docs/version-2.4.1/` copy) +- "Using ICurrentUser in middleware" example: `currentUser.Id?.ToString() ?? "anonymous"` → `currentUser.Id ?? "anonymous"` + +## Files Affected + +| File | Change | +|---|---| +| `Src/RCommon.Security/Users/ICurrentUser.cs` | `Id` type: `Guid?` to `string?` | +| `Src/RCommon.Security/Users/CurrentUser.cs` | `Id` return type, `IsAuthenticated` derivation | +| `Src/RCommon.Security/Users/CurrentUserExtensions.cs` | `GetId()` return type, remove `FindClaimValue` | +| `Src/RCommon.Security/ClaimsIdentityExtensions.cs` | `FindUserId` overloads return `string?` | +| `Src/RCommon.Security/Claims/ClaimTypesConst.cs` | Replace mutable statics with `Configure`/freeze pattern | +| `Src/RCommon.Security/Claims/ClaimTypesOptions.cs` | New: options class for claim type mappings | +| `Tests/RCommon.Security.Tests/CurrentUserTests.cs` | Update assertions | +| `Tests/RCommon.Security.Tests/ClaimsIdentityExtensionsTests.cs` | New: `FindUserId` tests with string return, non-GUID cases | +| `Tests/RCommon.Security.Tests/ClaimTypesConstTests.cs` | New: configure/freeze tests | +| `Examples/CleanWithCQRS/HR.LeaveManagement.Persistence/AuditableDbContext.cs` | Simplify Id usage | +| `website/docs/security-web/authorization.mdx` | Update code examples and API summary | +| `website/docs/security-web/web-utilities.mdx` | Update middleware example | +| `website/versioned_docs/version-2.4.1/security-web/authorization.mdx` | Mirror changes from current docs | +| `website/versioned_docs/version-2.4.1/security-web/web-utilities.mdx` | Mirror changes from current docs | diff --git a/website/docs/getting-started/overview.mdx b/website/docs/getting-started/overview.mdx index 0a95c25e..f97eb3cb 100644 --- a/website/docs/getting-started/overview.mdx +++ b/website/docs/getting-started/overview.mdx @@ -38,7 +38,7 @@ RCommon is released under the [Apache License, Version 2.0](https://www.apache.o ## Source code -The full source is available on GitHub: [https://github.com/jfrog/RCommon](https://github.com/RCommon-Framework/RCommon) +The full source is available on GitHub: [https://github.com/RCommon-Team/RCommon](https://github.com/RCommon-Team/RCommon) ## Design principles diff --git a/website/docs/security-web/authorization.mdx b/website/docs/security-web/authorization.mdx index 2c54158c..e66b48a4 100644 --- a/website/docs/security-web/authorization.mdx +++ b/website/docs/security-web/authorization.mdx @@ -83,11 +83,15 @@ If your identity provider uses non-standard claim type URIs, override the static using RCommon.Security.Claims; // Map to the short claim names issued by your OIDC provider. -ClaimTypesConst.UserId = "sub"; -ClaimTypesConst.Role = "roles"; -ClaimTypesConst.Email = "email"; -ClaimTypesConst.TenantId = "tenant_id"; -ClaimTypesConst.ClientId = "client_id"; +// Configure must be called once at startup, before any property is accessed. +ClaimTypesConst.Configure(options => +{ + options.UserId = "sub"; + options.Role = "roles"; + options.Email = "email"; + options.TenantId = "tenant_id"; + options.ClientId = "client_id"; +}); ``` ### Swashbuckle operation filters @@ -146,7 +150,7 @@ public class OrderCommandHandler var order = new Order { - CustomerId = _currentUser.Id!.Value, + CustomerId = _currentUser.Id!, CustomerName = _currentUser.FindClaimValue(ClaimTypes.GivenName) ?? "Unknown", Total = command.Total }; @@ -176,9 +180,6 @@ Claim[] allDeptClaims = _currentUser.FindClaims("custom:department"); // Find a claim value as a string. string? department = _currentUser.FindClaimValue("custom:department"); - -// Find a claim value converted to a specific struct type. -int employeeNumber = _currentUser.FindClaimValue("custom:employee_number"); ``` ### Accessing the current client @@ -280,7 +281,7 @@ principal.AddIdentityIfNotContains(new ClaimsIdentity(claims, "Cookie")); Extract well-known values directly from a `ClaimsPrincipal`: ```csharp -Guid? userId = principal.FindUserId(); // parses ClaimTypesConst.UserId as Guid +string? userId = principal.FindUserId(); // reads ClaimTypesConst.UserId string? tenant = principal.FindTenantId(); // reads ClaimTypesConst.TenantId string? client = principal.FindClientId(); // reads ClaimTypesConst.ClientId ``` @@ -295,10 +296,10 @@ string? client = principal.FindClientId(); // reads ClaimTypesConst.ClientId | `CurrentPrincipalAccessorExtensions` | `RCommon.Security` | `Change(Claim)`, `Change(IEnumerable)`, `Change(ClaimsIdentity)` overloads | | `ICurrentUser` | `RCommon.Security` | Exposes `Id`, `IsAuthenticated`, `Roles`, `TenantId`, `FindClaim`, `FindClaims`, `GetAllClaims` | | `CurrentUser` | `RCommon.Security` | Default `ICurrentUser` implementation backed by `ICurrentPrincipalAccessor` | -| `CurrentUserExtensions` | `RCommon.Security` | `FindClaimValue(string)`, `FindClaimValue(string)`, `GetId()` | +| `CurrentUserExtensions` | `RCommon.Security` | `FindClaimValue(string)`, `GetId()` | | `ICurrentClient` | `RCommon.Security` | Exposes `Id` and `IsAuthenticated` for OAuth client identities | | `CurrentClient` | `RCommon.Security` | Default `ICurrentClient` backed by `ICurrentPrincipalAccessor` | -| `ClaimTypesConst` | `RCommon.Security` | Configurable claim type URIs: `UserName`, `Name`, `SurName`, `UserId`, `Role`, `Email`, `TenantId`, `ClientId` | +| `ClaimTypesConst` | `RCommon.Security` | Configure-once claim type URIs via `Configure(Action)`: `UserName`, `Name`, `SurName`, `UserId`, `Role`, `Email`, `TenantId`, `ClientId` | | `ClaimsIdentityExtensions` | `RCommon.Security` | `FindUserId`, `FindTenantId`, `FindClientId`, `AddIfNotContains`, `AddOrReplace`, `AddIdentityIfNotContains` | | `AuthorizationException` | `RCommon.Security` | Application-layer exception for access-denied scenarios; carries `Code`, `LogLevel`, and fluent `WithData()` | | `AuthorizeCheckOperationFilter` | `RCommon.Authorization.Web` | Swashbuckle filter: adds 401/403 responses and OAuth2 security requirement to `[Authorize]`-decorated operations | diff --git a/website/docs/security-web/web-utilities.mdx b/website/docs/security-web/web-utilities.mdx index 8195859f..446a1660 100644 --- a/website/docs/security-web/web-utilities.mdx +++ b/website/docs/security-web/web-utilities.mdx @@ -136,7 +136,7 @@ public class RequestAuditMiddleware { if (currentUser.IsAuthenticated) { - var userId = currentUser.Id?.ToString() ?? "anonymous"; + var userId = currentUser.Id ?? "anonymous"; context.Items["AuditUserId"] = userId; } diff --git a/website/versioned_docs/version-2.4.1/getting-started/overview.mdx b/website/versioned_docs/version-2.4.1/getting-started/overview.mdx index 905b40b6..06d3f666 100644 --- a/website/versioned_docs/version-2.4.1/getting-started/overview.mdx +++ b/website/versioned_docs/version-2.4.1/getting-started/overview.mdx @@ -37,7 +37,7 @@ RCommon is released under the [Apache License, Version 2.0](https://www.apache.o ## Source code -The full source is available on GitHub: [https://github.com/jfrog/RCommon](https://github.com/RCommon-Framework/RCommon) +The full source is available on GitHub: [https://github.com/RCommon-Team/RCommon](https://github.com/RCommon-Team/RCommon) ## Design principles diff --git a/website/versioned_docs/version-2.4.1/security-web/authorization.mdx b/website/versioned_docs/version-2.4.1/security-web/authorization.mdx index 20d90f56..132e40c2 100644 --- a/website/versioned_docs/version-2.4.1/security-web/authorization.mdx +++ b/website/versioned_docs/version-2.4.1/security-web/authorization.mdx @@ -82,11 +82,15 @@ If your identity provider uses non-standard claim type URIs, override the static using RCommon.Security.Claims; // Map to the short claim names issued by your OIDC provider. -ClaimTypesConst.UserId = "sub"; -ClaimTypesConst.Role = "roles"; -ClaimTypesConst.Email = "email"; -ClaimTypesConst.TenantId = "tenant_id"; -ClaimTypesConst.ClientId = "client_id"; +// Configure must be called once at startup, before any property is accessed. +ClaimTypesConst.Configure(options => +{ + options.UserId = "sub"; + options.Role = "roles"; + options.Email = "email"; + options.TenantId = "tenant_id"; + options.ClientId = "client_id"; +}); ``` ### Swashbuckle operation filters @@ -145,7 +149,7 @@ public class OrderCommandHandler var order = new Order { - CustomerId = _currentUser.Id!.Value, + CustomerId = _currentUser.Id!, CustomerName = _currentUser.FindClaimValue(ClaimTypes.GivenName) ?? "Unknown", Total = command.Total }; @@ -175,9 +179,6 @@ Claim[] allDeptClaims = _currentUser.FindClaims("custom:department"); // Find a claim value as a string. string? department = _currentUser.FindClaimValue("custom:department"); - -// Find a claim value converted to a specific struct type. -int employeeNumber = _currentUser.FindClaimValue("custom:employee_number"); ``` ### Accessing the current client @@ -279,7 +280,7 @@ principal.AddIdentityIfNotContains(new ClaimsIdentity(claims, "Cookie")); Extract well-known values directly from a `ClaimsPrincipal`: ```csharp -Guid? userId = principal.FindUserId(); // parses ClaimTypesConst.UserId as Guid +string? userId = principal.FindUserId(); // reads ClaimTypesConst.UserId string? tenant = principal.FindTenantId(); // reads ClaimTypesConst.TenantId string? client = principal.FindClientId(); // reads ClaimTypesConst.ClientId ``` @@ -294,10 +295,10 @@ string? client = principal.FindClientId(); // reads ClaimTypesConst.ClientId | `CurrentPrincipalAccessorExtensions` | `RCommon.Security` | `Change(Claim)`, `Change(IEnumerable)`, `Change(ClaimsIdentity)` overloads | | `ICurrentUser` | `RCommon.Security` | Exposes `Id`, `IsAuthenticated`, `Roles`, `TenantId`, `FindClaim`, `FindClaims`, `GetAllClaims` | | `CurrentUser` | `RCommon.Security` | Default `ICurrentUser` implementation backed by `ICurrentPrincipalAccessor` | -| `CurrentUserExtensions` | `RCommon.Security` | `FindClaimValue(string)`, `FindClaimValue(string)`, `GetId()` | +| `CurrentUserExtensions` | `RCommon.Security` | `FindClaimValue(string)`, `GetId()` | | `ICurrentClient` | `RCommon.Security` | Exposes `Id` and `IsAuthenticated` for OAuth client identities | | `CurrentClient` | `RCommon.Security` | Default `ICurrentClient` backed by `ICurrentPrincipalAccessor` | -| `ClaimTypesConst` | `RCommon.Security` | Configurable claim type URIs: `UserName`, `Name`, `SurName`, `UserId`, `Role`, `Email`, `TenantId`, `ClientId` | +| `ClaimTypesConst` | `RCommon.Security` | Configure-once claim type URIs via `Configure(Action)`: `UserName`, `Name`, `SurName`, `UserId`, `Role`, `Email`, `TenantId`, `ClientId` | | `ClaimsIdentityExtensions` | `RCommon.Security` | `FindUserId`, `FindTenantId`, `FindClientId`, `AddIfNotContains`, `AddOrReplace`, `AddIdentityIfNotContains` | | `AuthorizationException` | `RCommon.Security` | Application-layer exception for access-denied scenarios; carries `Code`, `LogLevel`, and fluent `WithData()` | | `AuthorizeCheckOperationFilter` | `RCommon.Authorization.Web` | Swashbuckle filter: adds 401/403 responses and OAuth2 security requirement to `[Authorize]`-decorated operations | diff --git a/website/versioned_docs/version-2.4.1/security-web/web-utilities.mdx b/website/versioned_docs/version-2.4.1/security-web/web-utilities.mdx index 6199ca77..152c0851 100644 --- a/website/versioned_docs/version-2.4.1/security-web/web-utilities.mdx +++ b/website/versioned_docs/version-2.4.1/security-web/web-utilities.mdx @@ -135,7 +135,7 @@ public class RequestAuditMiddleware { if (currentUser.IsAuthenticated) { - var userId = currentUser.Id?.ToString() ?? "anonymous"; + var userId = currentUser.Id ?? "anonymous"; context.Items["AuditUserId"] = userId; }