From ce31a907ae19472681e31c9e34ee74270daa0199 Mon Sep 17 00:00:00 2001 From: Ismael Leon Date: Mon, 21 Sep 2026 21:58:01 -0600 Subject: [PATCH 1/5] Stop materializing password hash; replace reflection mapper with explicit mapping The reflection-based mapper in Authorization.Common copied every matching property from the database entity into the domain model, which carried the user's password hash into the business and middleware layers even though nothing there needs it. The package only ever mapped two trivial types. - Remove Authorization.Common (Mapper, Converter) and their tests. - Move persistence entities into DataAccess as internal types without a password hash property; Dapper ignores the unmatched column. - Remove PasswordHash from the public User model. - Map entities to models explicitly in SecurityRepository. Refs #3 --- Authorization.sln | 15 ---- .../Entities/Profile.cs | 13 --- .../Entities/User.cs | 19 ---- src/Authorization.Abstractions/Models/User.cs | 5 -- .../Authorization.Common.csproj | 8 -- src/Authorization.Common/Converter.cs | 46 ---------- src/Authorization.Common/Mapper.cs | 87 ------------------- .../Authorization.DataAccess.csproj | 1 - .../Entities/Profile.cs | 8 ++ src/Authorization.DataAccess/Entities/User.cs | 13 +++ .../SecurityRepository.cs | 34 ++++---- .../Authorization.Middleware.csproj | 1 - .../Authorization.UnitTests.csproj | 1 - .../Authorization.UnitTests/ConverterTests.cs | 50 ----------- tests/Authorization.UnitTests/MapperTests.cs | 64 -------------- 15 files changed, 37 insertions(+), 328 deletions(-) delete mode 100644 src/Authorization.Abstractions/Entities/Profile.cs delete mode 100644 src/Authorization.Abstractions/Entities/User.cs delete mode 100644 src/Authorization.Common/Authorization.Common.csproj delete mode 100644 src/Authorization.Common/Converter.cs delete mode 100644 src/Authorization.Common/Mapper.cs create mode 100644 src/Authorization.DataAccess/Entities/Profile.cs create mode 100644 src/Authorization.DataAccess/Entities/User.cs delete mode 100644 tests/Authorization.UnitTests/ConverterTests.cs delete mode 100644 tests/Authorization.UnitTests/MapperTests.cs diff --git a/Authorization.sln b/Authorization.sln index 4e844bf..c6d9e3a 100644 --- a/Authorization.sln +++ b/Authorization.sln @@ -7,8 +7,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{827E0CD3-B72 EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Authorization.Abstractions", "src\Authorization.Abstractions\Authorization.Abstractions.csproj", "{60BA81F8-B279-47E3-8786-6FAE86C81FD3}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Authorization.Common", "src\Authorization.Common\Authorization.Common.csproj", "{E8AF7B80-0A65-45E5-8F73-357A0FFFF699}" -EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Authorization.DataAccess", "src\Authorization.DataAccess\Authorization.DataAccess.csproj", "{03B4B2E4-7A31-44B1-87B4-3A939DAD29E0}" EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Authorization.Business", "src\Authorization.Business\Authorization.Business.csproj", "{8B921C4E-CCBD-49DE-91F8-0955695BC2D7}" @@ -39,18 +37,6 @@ Global {60BA81F8-B279-47E3-8786-6FAE86C81FD3}.Release|x64.Build.0 = Release|Any CPU {60BA81F8-B279-47E3-8786-6FAE86C81FD3}.Release|x86.ActiveCfg = Release|Any CPU {60BA81F8-B279-47E3-8786-6FAE86C81FD3}.Release|x86.Build.0 = Release|Any CPU - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699}.Debug|Any CPU.Build.0 = Debug|Any CPU - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699}.Debug|x64.ActiveCfg = Debug|Any CPU - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699}.Debug|x64.Build.0 = Debug|Any CPU - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699}.Debug|x86.ActiveCfg = Debug|Any CPU - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699}.Debug|x86.Build.0 = Debug|Any CPU - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699}.Release|Any CPU.ActiveCfg = Release|Any CPU - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699}.Release|Any CPU.Build.0 = Release|Any CPU - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699}.Release|x64.ActiveCfg = Release|Any CPU - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699}.Release|x64.Build.0 = Release|Any CPU - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699}.Release|x86.ActiveCfg = Release|Any CPU - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699}.Release|x86.Build.0 = Release|Any CPU {03B4B2E4-7A31-44B1-87B4-3A939DAD29E0}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {03B4B2E4-7A31-44B1-87B4-3A939DAD29E0}.Debug|Any CPU.Build.0 = Debug|Any CPU {03B4B2E4-7A31-44B1-87B4-3A939DAD29E0}.Debug|x64.ActiveCfg = Debug|Any CPU @@ -105,7 +91,6 @@ Global EndGlobalSection GlobalSection(NestedProjects) = preSolution {60BA81F8-B279-47E3-8786-6FAE86C81FD3} = {827E0CD3-B72D-47B6-A68D-7590B98EB39B} - {E8AF7B80-0A65-45E5-8F73-357A0FFFF699} = {827E0CD3-B72D-47B6-A68D-7590B98EB39B} {03B4B2E4-7A31-44B1-87B4-3A939DAD29E0} = {827E0CD3-B72D-47B6-A68D-7590B98EB39B} {8B921C4E-CCBD-49DE-91F8-0955695BC2D7} = {827E0CD3-B72D-47B6-A68D-7590B98EB39B} {C3F978CB-98CB-46B9-82FB-D795B1D76DA4} = {827E0CD3-B72D-47B6-A68D-7590B98EB39B} diff --git a/src/Authorization.Abstractions/Entities/Profile.cs b/src/Authorization.Abstractions/Entities/Profile.cs deleted file mode 100644 index b966ead..0000000 --- a/src/Authorization.Abstractions/Entities/Profile.cs +++ /dev/null @@ -1,13 +0,0 @@ -namespace Authorization.Abstractions.Entities; - -/// -/// Database-facing representation of a security profile (role) a user can hold. -/// -public sealed class Profile -{ - /// Unique identifier of the profile. - public int Id { get; set; } - - /// Display name of the profile. - public string? Name { get; set; } -} diff --git a/src/Authorization.Abstractions/Entities/User.cs b/src/Authorization.Abstractions/Entities/User.cs deleted file mode 100644 index d6ab7f9..0000000 --- a/src/Authorization.Abstractions/Entities/User.cs +++ /dev/null @@ -1,19 +0,0 @@ -namespace Authorization.Abstractions.Entities; - -/// -/// Database-facing representation of a user, as returned by the security data store. -/// -public sealed class User -{ - /// Unique identifier of the user. - public Guid Id { get; set; } - - /// Login name of the user. - public string? UserName { get; set; } - - /// Hashed password. Never expose this outside the data layer. - public string? PasswordHash { get; set; } - - /// Email address of the user. - public string? Email { get; set; } -} diff --git a/src/Authorization.Abstractions/Models/User.cs b/src/Authorization.Abstractions/Models/User.cs index 4cb82ea..b98a840 100644 --- a/src/Authorization.Abstractions/Models/User.cs +++ b/src/Authorization.Abstractions/Models/User.cs @@ -2,8 +2,6 @@ namespace Authorization.Abstractions.Models; /// /// Domain model of a user, used by the business and middleware layers. -/// Kept separate from the database entity so persistence changes never leak -/// into the pipeline contract. /// public sealed class User { @@ -13,9 +11,6 @@ public sealed class User /// Login name of the user. public string? UserName { get; set; } - /// Hashed password. Never surfaced in claims, logs or responses. - public string? PasswordHash { get; set; } - /// Email address of the user. public string? Email { get; set; } } diff --git a/src/Authorization.Common/Authorization.Common.csproj b/src/Authorization.Common/Authorization.Common.csproj deleted file mode 100644 index fc3d216..0000000 --- a/src/Authorization.Common/Authorization.Common.csproj +++ /dev/null @@ -1,8 +0,0 @@ - - - - Authorization.Common - Shared helpers for the JWT authorization middleware, including a cached reflection-based object mapper. - - - diff --git a/src/Authorization.Common/Converter.cs b/src/Authorization.Common/Converter.cs deleted file mode 100644 index 2435761..0000000 --- a/src/Authorization.Common/Converter.cs +++ /dev/null @@ -1,46 +0,0 @@ -namespace Authorization.Common; - -/// -/// Convenience facade over for single objects and sequences. -/// -public static class Converter -{ - /// Creates a shallow copy of . - public static TModel? Clone(TModel? source) - where TModel : class, new() - => Mapper.Map(source); - - /// Converts a single object to . - public static TDestination? Convert( - TSource? source, - Action? transform = null) - where TSource : class - where TDestination : new() - => Mapper.Map(source, transform); - - /// - /// Converts a sequence, skipping any elements that map to null. - /// Materialised to a list so the (already-executed) source query is only - /// enumerated once. - /// - public static IReadOnlyList ConvertList( - IEnumerable source, - Action? transform = null) - where TSource : class - where TDestination : new() - { - ArgumentNullException.ThrowIfNull(source); - - var result = new List(); - foreach (var element in source) - { - var mapped = Mapper.Map(element, transform); - if (mapped is not null) - { - result.Add(mapped); - } - } - - return result; - } -} diff --git a/src/Authorization.Common/Mapper.cs b/src/Authorization.Common/Mapper.cs deleted file mode 100644 index 0784255..0000000 --- a/src/Authorization.Common/Mapper.cs +++ /dev/null @@ -1,87 +0,0 @@ -using System.Collections.Concurrent; -using System.Reflection; - -namespace Authorization.Common; - -/// -/// Lightweight convention-based object mapper: copies matching public -/// properties (by name and type) from a source object to a new destination -/// instance. -/// -/// -/// This runs on the request hot path, so the reflection work (discovering -/// which source/destination properties line up) is computed once per -/// type-pair and cached. Only value types, strings and arrays are copied, -/// mirroring the original shallow-copy behaviour so nested reference graphs -/// are never shared by accident. -/// -public static class Mapper -{ - private static readonly ConcurrentDictionary<(Type Source, Type Destination), PropertyPair[]> PropertyMapCache = new(); - - private readonly record struct PropertyPair(PropertyInfo Source, PropertyInfo Destination); - - /// - /// Maps onto a new instance of - /// . - /// - /// Object to read values from. May be null. - /// Optional hook to apply custom rules after the automatic copy. - /// The populated destination, or the type default when is null. - public static TDestination? Map( - TSource? source, - Action? transform = null) - where TSource : class - where TDestination : new() - { - if (source is null) - { - return default; - } - - var destination = new TDestination(); - foreach (var pair in GetPropertyMap(typeof(TSource), typeof(TDestination))) - { - pair.Destination.SetValue(destination, pair.Source.GetValue(source)); - } - - transform?.Invoke(source, destination); - return destination; - } - - private static PropertyPair[] GetPropertyMap(Type source, Type destination) => - PropertyMapCache.GetOrAdd((source, destination), static key => BuildPropertyMap(key.Source, key.Destination)); - - private static PropertyPair[] BuildPropertyMap(Type source, Type destination) - { - var destinationProperties = destination - .GetProperties(BindingFlags.Public | BindingFlags.Instance) - .ToDictionary(p => p.Name, StringComparer.Ordinal); - - var pairs = new List(); - foreach (var sourceProperty in source.GetProperties(BindingFlags.Public | BindingFlags.Instance)) - { - if (!destinationProperties.TryGetValue(sourceProperty.Name, out var destinationProperty)) - { - continue; - } - - if (!destinationProperty.CanWrite || - destinationProperty.GetIndexParameters().Length != 0 || - destinationProperty.PropertyType != sourceProperty.PropertyType || - !IsCopyable(destinationProperty.PropertyType)) - { - continue; - } - - pairs.Add(new PropertyPair(sourceProperty, destinationProperty)); - } - - return pairs.ToArray(); - } - - // Copy value types, strings and arrays only; skip complex reference types - // to avoid sharing mutable nested objects between source and destination. - private static bool IsCopyable(Type type) => - !type.IsClass || type == typeof(string) || type.IsArray; -} diff --git a/src/Authorization.DataAccess/Authorization.DataAccess.csproj b/src/Authorization.DataAccess/Authorization.DataAccess.csproj index b3485c2..6bf0230 100644 --- a/src/Authorization.DataAccess/Authorization.DataAccess.csproj +++ b/src/Authorization.DataAccess/Authorization.DataAccess.csproj @@ -14,7 +14,6 @@ - diff --git a/src/Authorization.DataAccess/Entities/Profile.cs b/src/Authorization.DataAccess/Entities/Profile.cs new file mode 100644 index 0000000..3511863 --- /dev/null +++ b/src/Authorization.DataAccess/Entities/Profile.cs @@ -0,0 +1,8 @@ +namespace Authorization.DataAccess.Entities; + +internal sealed class Profile +{ + public int Id { get; set; } + + public string? Name { get; set; } +} diff --git a/src/Authorization.DataAccess/Entities/User.cs b/src/Authorization.DataAccess/Entities/User.cs new file mode 100644 index 0000000..955d54c --- /dev/null +++ b/src/Authorization.DataAccess/Entities/User.cs @@ -0,0 +1,13 @@ +namespace Authorization.DataAccess.Entities; + +// Row shape returned by the user stored procedure. Columns without a matching +// property (e.g. the password hash) are ignored by Dapper, so secrets are never +// materialized in memory. +internal sealed class User +{ + public Guid Id { get; set; } + + public string? UserName { get; set; } + + public string? Email { get; set; } +} diff --git a/src/Authorization.DataAccess/SecurityRepository.cs b/src/Authorization.DataAccess/SecurityRepository.cs index a624d76..9a015f5 100644 --- a/src/Authorization.DataAccess/SecurityRepository.cs +++ b/src/Authorization.DataAccess/SecurityRepository.cs @@ -1,10 +1,9 @@ using System.Data; using Authorization.Abstractions.DataAccess; using Authorization.Abstractions.Options; -using Authorization.Common; using Dapper; using Microsoft.Extensions.Options; -using Entities = Authorization.Abstractions.Entities; +using Entities = Authorization.DataAccess.Entities; using Models = Authorization.Abstractions.Models; namespace Authorization.DataAccess; @@ -29,19 +28,11 @@ public SecurityRepository(IDbConnectionFactory connectionFactory, IOptions( + CreateCommand(_options.GetUserProcedure, user, cancellationToken)); - var command = new CommandDefinition( - _options.GetUserProcedure, - new { user.Email, user.UserName }, - commandType: CommandType.StoredProcedure, - cancellationToken: cancellationToken); - - var entity = await connection.QueryFirstOrDefaultAsync(command); - return Converter.Convert(entity); + return entity is null ? null : ToModel(entity); } /// @@ -50,14 +41,21 @@ public SecurityRepository(IDbConnectionFactory connectionFactory, IOptions( + CreateCommand(_options.GetProfilesProcedure, user, cancellationToken)); - var command = new CommandDefinition( - _options.GetProfilesProcedure, + return entities.Select(ToModel).ToList(); + } + + private static CommandDefinition CreateCommand(string procedure, Models.User user, CancellationToken cancellationToken) => + new(procedure, new { user.Email, user.UserName }, commandType: CommandType.StoredProcedure, cancellationToken: cancellationToken); - var entities = await connection.QueryAsync(command); - return Converter.ConvertList(entities); - } + private static Models.User ToModel(Entities.User entity) => + new() { Id = entity.Id, UserName = entity.UserName, Email = entity.Email }; + + private static Models.Profile ToModel(Entities.Profile entity) => + new() { Id = entity.Id, Name = entity.Name }; } diff --git a/src/Authorization.Middleware/Authorization.Middleware.csproj b/src/Authorization.Middleware/Authorization.Middleware.csproj index e744ddd..6b8e7f0 100644 --- a/src/Authorization.Middleware/Authorization.Middleware.csproj +++ b/src/Authorization.Middleware/Authorization.Middleware.csproj @@ -16,7 +16,6 @@ - diff --git a/tests/Authorization.UnitTests/Authorization.UnitTests.csproj b/tests/Authorization.UnitTests/Authorization.UnitTests.csproj index dc14bcf..9e67b5d 100644 --- a/tests/Authorization.UnitTests/Authorization.UnitTests.csproj +++ b/tests/Authorization.UnitTests/Authorization.UnitTests.csproj @@ -25,7 +25,6 @@ - diff --git a/tests/Authorization.UnitTests/ConverterTests.cs b/tests/Authorization.UnitTests/ConverterTests.cs deleted file mode 100644 index f3554e6..0000000 --- a/tests/Authorization.UnitTests/ConverterTests.cs +++ /dev/null @@ -1,50 +0,0 @@ -using Authorization.Common; -using Entities = Authorization.Abstractions.Entities; -using Models = Authorization.Abstractions.Models; - -namespace Authorization.UnitTests; - -public class ConverterTests -{ - [Fact] - public void ConvertList_MapsEveryElement() - { - var source = new[] - { - new Entities.Profile { Id = 1, Name = "admin" }, - new Entities.Profile { Id = 2, Name = "user" }, - }; - - var result = Converter.ConvertList(source); - - Assert.Equal(2, result.Count); - Assert.Equal("admin", result[0].Name); - Assert.Equal("user", result[1].Name); - } - - [Fact] - public void ConvertList_EmptySource_ReturnsEmpty() - { - var result = Converter.ConvertList(Array.Empty()); - Assert.Empty(result); - } - - [Fact] - public void ConvertList_NullSource_Throws() - { - Assert.Throws( - () => Converter.ConvertList(null!)); - } - - [Fact] - public void Clone_ProducesIndependentCopy() - { - var original = new Models.User { Id = Guid.NewGuid(), UserName = "jdoe" }; - - var clone = Converter.Clone(original); - - Assert.NotNull(clone); - Assert.NotSame(original, clone); - Assert.Equal(original.UserName, clone!.UserName); - } -} diff --git a/tests/Authorization.UnitTests/MapperTests.cs b/tests/Authorization.UnitTests/MapperTests.cs deleted file mode 100644 index 2736756..0000000 --- a/tests/Authorization.UnitTests/MapperTests.cs +++ /dev/null @@ -1,64 +0,0 @@ -using Authorization.Common; -using Entities = Authorization.Abstractions.Entities; -using Models = Authorization.Abstractions.Models; - -namespace Authorization.UnitTests; - -public class MapperTests -{ - [Fact] - public void Map_CopiesMatchingProperties() - { - var id = Guid.NewGuid(); - var source = new Entities.User - { - Id = id, - UserName = "jdoe", - Email = "jdoe@example.com", - PasswordHash = "hash", - }; - - var result = Mapper.Map(source); - - Assert.NotNull(result); - Assert.Equal(id, result!.Id); - Assert.Equal("jdoe", result.UserName); - Assert.Equal("jdoe@example.com", result.Email); - Assert.Equal("hash", result.PasswordHash); - } - - [Fact] - public void Map_NullSource_ReturnsDefault() - { - var result = Mapper.Map(null); - Assert.Null(result); - } - - [Fact] - public void Map_AppliesTransformAfterCopy() - { - var source = new Entities.Profile { Id = 7, Name = "admin" }; - - var result = Mapper.Map( - source, - (src, dest) => dest.Name = src.Name!.ToUpperInvariant()); - - Assert.NotNull(result); - Assert.Equal(7, result!.Id); - Assert.Equal("ADMIN", result.Name); - } - - [Fact] - public void Map_IsConsistentAcrossCachedCalls() - { - // Exercises the per-type-pair property-map cache: a second call must - // produce the same result as the first. - var first = Mapper.Map(new Entities.Profile { Id = 1, Name = "a" }); - var second = Mapper.Map(new Entities.Profile { Id = 2, Name = "b" }); - - Assert.Equal(1, first!.Id); - Assert.Equal("a", first.Name); - Assert.Equal(2, second!.Id); - Assert.Equal("b", second.Name); - } -} From be6f98132680495589299141a525b5af1dce8603 Mon Sep 17 00:00:00 2001 From: Ismael Leon Date: Mon, 21 Sep 2026 21:58:43 -0600 Subject: [PATCH 2/5] Use the ASP.NET Core shared framework in the middleware package Microsoft.AspNetCore.Http.Abstractions 2.3.0 is the legacy ASP.NET Core 2.x package; on net8.0 it ships its own copies of types that the host already provides through the shared framework. Reference Microsoft.AspNetCore.App instead, which also covers the Microsoft.Extensions.* packages that were listed explicitly (Configuration.Abstractions was unused). Refs #3 --- .../Authorization.Middleware.csproj | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Authorization.Middleware/Authorization.Middleware.csproj b/src/Authorization.Middleware/Authorization.Middleware.csproj index 6b8e7f0..b1179b3 100644 --- a/src/Authorization.Middleware/Authorization.Middleware.csproj +++ b/src/Authorization.Middleware/Authorization.Middleware.csproj @@ -6,11 +6,7 @@ - - - - - + From 7a33e68cbe4b49b7a4be2087b93f18c33a5d6e3b Mon Sep 17 00:00:00 2001 From: Ismael Leon Date: Mon, 21 Sep 2026 21:59:54 -0600 Subject: [PATCH 3/5] Validate claims-enrichment configuration at startup SqlConnectionFactory throws when the connection string is missing, but it is resolved while binding the middleware's scoped dependencies, outside the middleware's graceful-degradation block. A misconfigured host therefore returned 500 on every authenticated request. Validate the options and the connection string presence with ValidateOnStart so the host fails fast. Refs #3 --- .../ServiceCollectionExtensions.cs | 27 ++++++--- .../ServiceCollectionExtensionsTests.cs | 56 +++++++++++++++++++ 2 files changed, 75 insertions(+), 8 deletions(-) create mode 100644 tests/Authorization.UnitTests/ServiceCollectionExtensionsTests.cs diff --git a/src/Authorization.Middleware/ServiceCollectionExtensions.cs b/src/Authorization.Middleware/ServiceCollectionExtensions.cs index 042c11e..c8acefe 100644 --- a/src/Authorization.Middleware/ServiceCollectionExtensions.cs +++ b/src/Authorization.Middleware/ServiceCollectionExtensions.cs @@ -3,14 +3,13 @@ using Authorization.Abstractions.Options; using Authorization.Business; using Authorization.DataAccess; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; namespace Authorization.Middleware; /// -/// Dependency-injection registration for the authorization stack. A single -/// call wires the connection factory, repository and business manager so -/// consumers no longer have to assemble the graph by hand. +/// Dependency-injection registration for the authorization stack. /// public static class ServiceCollectionExtensions { @@ -25,11 +24,17 @@ public static IServiceCollection AddAuthorizationClaims( { ArgumentNullException.ThrowIfNull(services); - var optionsBuilder = services.AddOptions(); - if (configure is not null) - { - optionsBuilder.Configure(configure); - } + // Validated at host start: the connection factory is resolved while + // binding the middleware's scoped dependencies, outside its graceful + // degradation, so a misconfiguration would otherwise fail every request. + services.AddOptions() + .Configure(options => configure?.Invoke(options)) + .Validate(HasRequiredSettings, + "ClaimsEnrichmentOptions requires a connection string name, user name claim type and stored procedure names.") + .Validate( + (options, configuration) => !string.IsNullOrWhiteSpace(configuration.GetConnectionString(options.ConnectionStringName)), + "The security database connection string was not found under \"ConnectionStrings\". Check ClaimsEnrichmentOptions.ConnectionStringName (default: SecurityDb).") + .ValidateOnStart(); // The factory only caches an immutable connection string, so it is safe // as a singleton; it still hands out a fresh connection per call. @@ -39,4 +44,10 @@ public static IServiceCollection AddAuthorizationClaims( return services; } + + private static bool HasRequiredSettings(ClaimsEnrichmentOptions options) => + !string.IsNullOrWhiteSpace(options.ConnectionStringName) && + !string.IsNullOrWhiteSpace(options.UserNameClaimType) && + !string.IsNullOrWhiteSpace(options.GetUserProcedure) && + !string.IsNullOrWhiteSpace(options.GetProfilesProcedure); } diff --git a/tests/Authorization.UnitTests/ServiceCollectionExtensionsTests.cs b/tests/Authorization.UnitTests/ServiceCollectionExtensionsTests.cs new file mode 100644 index 0000000..f6a2b00 --- /dev/null +++ b/tests/Authorization.UnitTests/ServiceCollectionExtensionsTests.cs @@ -0,0 +1,56 @@ +using Authorization.Abstractions.Options; +using Authorization.Middleware; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; + +namespace Authorization.UnitTests; + +public class ServiceCollectionExtensionsTests +{ + [Fact] + public void StartupValidation_MissingConnectionString_Throws() + { + using var provider = BuildProvider(new Dictionary()); + + var exception = Assert.Throws( + () => provider.GetRequiredService().Validate()); + + Assert.Contains("ConnectionStrings", exception.Message); + } + + [Fact] + public void StartupValidation_BlankClaimType_Throws() + { + using var provider = BuildProvider(ValidConnectionString, options => options.UserNameClaimType = " "); + + Assert.Throws( + () => provider.GetRequiredService().Validate()); + } + + [Fact] + public void StartupValidation_ValidConfiguration_Passes() + { + using var provider = BuildProvider(ValidConnectionString); + + var exception = Record.Exception(() => provider.GetRequiredService().Validate()); + + Assert.Null(exception); + } + + private static readonly Dictionary ValidConnectionString = new() + { + ["ConnectionStrings:SecurityDb"] = "Server=localhost;Database=Security;", + }; + + private static ServiceProvider BuildProvider( + Dictionary settings, + Action? configure = null) + { + var configuration = new ConfigurationBuilder().AddInMemoryCollection(settings).Build(); + return new ServiceCollection() + .AddSingleton(configuration) + .AddAuthorizationClaims(configure) + .BuildServiceProvider(); + } +} From 3d52a98f215738497e7f0d0ccc4cf8cf3a3d862f Mon Sep 17 00:00:00 2001 From: Ismael Leon Date: Mon, 21 Sep 2026 22:01:03 -0600 Subject: [PATCH 4/5] Expose the user-id claim type and tidy middleware, options and tests - Add ClaimsEnrichmentMiddleware.UserIdClaimType so consumers and tests stop repeating the "IdUsuario" literal. - Make AddProfileClaimsAsync static; it uses no instance state. - Drop historical wording from option docs and package descriptions. - Update the README for the removed Common package and startup validation. - Share middleware construction in tests and cover the aborted-request path. Refs #3 --- README.md | 9 ++--- .../Authorization.Abstractions.csproj | 2 +- .../Options/ClaimsEnrichmentOptions.cs | 9 ++--- .../ClaimsEnrichmentMiddleware.cs | 7 +++- .../ClaimsEnrichmentMiddlewareTests.cs | 40 +++++++++++++++---- 5 files changed, 45 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 7b52be0..895bc52 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ Once a request has been authenticated (by JWT bearer auth, for example), the mid 1. Reads the configured user-name claim from the incoming principal. 2. Looks up the matching user in the security database (via a stored procedure). -3. Adds `Email`, `Name` and `IdUsuario` claims. +3. Adds `Email`, `Name` and `IdUsuario` (`ClaimsEnrichmentMiddleware.UserIdClaimType`) claims. 4. Looks up the user's profiles and adds a `Role` claim for each one. If anything goes wrong resolving that data (missing claim, unknown user, database outage), the request **degrades gracefully**: it continues unenriched instead of crashing the pipeline. @@ -29,9 +29,8 @@ The solution is layered so each concern is isolated and independently testable: | Project | Responsibility | | --- | --- | -| `Authorization.Abstractions` | Contracts: entities, models, options and interfaces. No external dependencies. | -| `Authorization.Common` | Cached, reflection-based object mapper used to map entities → models. | -| `Authorization.DataAccess` | Dapper + `Microsoft.Data.SqlClient` access to stored procedures. | +| `Authorization.Abstractions` | Contracts: models, options and interfaces. No external dependencies. | +| `Authorization.DataAccess` | Dapper + `Microsoft.Data.SqlClient` access to stored procedures; maps database rows to models. | | `Authorization.Business` | Thin business layer orchestrating identity resolution. | | `Authorization.Middleware` | The ASP.NET Core middleware plus DI and pipeline extensions. | @@ -92,7 +91,7 @@ app.UseAuthorization(); ## ⚙️ Configuration -Everything that used to be hard-coded is now configurable through `ClaimsEnrichmentOptions`: +Claim and database settings are configurable through `ClaimsEnrichmentOptions`. They are validated at startup, so a missing connection string or blank setting fails the host instead of individual requests. ```csharp builder.Services.AddAuthorizationClaims(options => diff --git a/src/Authorization.Abstractions/Authorization.Abstractions.csproj b/src/Authorization.Abstractions/Authorization.Abstractions.csproj index acdd200..b5e6512 100644 --- a/src/Authorization.Abstractions/Authorization.Abstractions.csproj +++ b/src/Authorization.Abstractions/Authorization.Abstractions.csproj @@ -2,7 +2,7 @@ Authorization.Abstractions - Contracts (entities, models, options and interfaces) for the JWT authorization middleware. Has no external dependencies. + Contracts (models, options and interfaces) for the JWT authorization middleware. Has no external dependencies. diff --git a/src/Authorization.Abstractions/Options/ClaimsEnrichmentOptions.cs b/src/Authorization.Abstractions/Options/ClaimsEnrichmentOptions.cs index a8f2515..249c914 100644 --- a/src/Authorization.Abstractions/Options/ClaimsEnrichmentOptions.cs +++ b/src/Authorization.Abstractions/Options/ClaimsEnrichmentOptions.cs @@ -2,9 +2,6 @@ namespace Authorization.Abstractions.Options; /// /// Configuration for the claims-enrichment middleware and the security store. -/// Everything that used to be hard-coded (the inbound claim to read the user -/// name from, and the stored-procedure names) is configurable here so the -/// package can be reused without recompiling. /// public sealed class ClaimsEnrichmentOptions { @@ -16,19 +13,19 @@ public sealed class ClaimsEnrichmentOptions /// /// The inbound JWT claim type that carries the user name used to look the - /// user up. Defaults to usuario to preserve the historical contract. + /// user up. Defaults to usuario. /// public string UserNameClaimType { get; set; } = "usuario"; /// /// Stored procedure that returns a single user by user name / email. - /// Defaults to ObtenerUsuario (the existing database object name). + /// Defaults to ObtenerUsuario. /// public string GetUserProcedure { get; set; } = "ObtenerUsuario"; /// /// Stored procedure that returns the profiles for a user. - /// Defaults to ObtenerPerfilesxUsuario (the existing database object name). + /// Defaults to ObtenerPerfilesxUsuario. /// public string GetProfilesProcedure { get; set; } = "ObtenerPerfilesxUsuario"; } diff --git a/src/Authorization.Middleware/ClaimsEnrichmentMiddleware.cs b/src/Authorization.Middleware/ClaimsEnrichmentMiddleware.cs index cb42eef..ec3d173 100644 --- a/src/Authorization.Middleware/ClaimsEnrichmentMiddleware.cs +++ b/src/Authorization.Middleware/ClaimsEnrichmentMiddleware.cs @@ -21,6 +21,9 @@ namespace Authorization.Middleware; /// public sealed partial class ClaimsEnrichmentMiddleware { + /// Claim type carrying the resolved user's identifier. + public const string UserIdClaimType = "IdUsuario"; + private readonly RequestDelegate _next; private readonly ILogger _logger; private readonly ClaimsEnrichmentOptions _options; @@ -102,10 +105,10 @@ private static void AddUserClaims(ICollection claims, User user) claims.Add(new Claim(ClaimTypes.Name, user.UserName)); } - claims.Add(new Claim("IdUsuario", user.Id.ToString())); + claims.Add(new Claim(UserIdClaimType, user.Id.ToString())); } - private async Task AddProfileClaimsAsync( + private static async Task AddProfileClaimsAsync( ICollection claims, User user, IAuthorizationManager authorizationManager, diff --git a/tests/Authorization.UnitTests/ClaimsEnrichmentMiddlewareTests.cs b/tests/Authorization.UnitTests/ClaimsEnrichmentMiddlewareTests.cs index 61e1afc..1404f28 100644 --- a/tests/Authorization.UnitTests/ClaimsEnrichmentMiddlewareTests.cs +++ b/tests/Authorization.UnitTests/ClaimsEnrichmentMiddlewareTests.cs @@ -22,7 +22,7 @@ public async Task Invoke_AnonymousUser_CallsNextWithoutEnrichment() var context = new DefaultHttpContext(); var nextCalled = false; - var middleware = new ClaimsEnrichmentMiddleware(_ => { nextCalled = true; return Task.CompletedTask; }, NullLogger.Instance, Options); + var middleware = CreateMiddleware(_ => { nextCalled = true; return Task.CompletedTask; }); await middleware.InvokeAsync(context, manager.Object); @@ -45,14 +45,14 @@ public async Task Invoke_AuthenticatedUser_AddsUserAndRoleClaims() var context = BuildAuthenticatedContext("jdoe"); var nextCalled = false; - var middleware = new ClaimsEnrichmentMiddleware(_ => { nextCalled = true; return Task.CompletedTask; }, NullLogger.Instance, Options); + var middleware = CreateMiddleware(_ => { nextCalled = true; return Task.CompletedTask; }); await middleware.InvokeAsync(context, manager.Object); Assert.True(nextCalled); Assert.Equal("jdoe@example.com", context.User.FindFirst(ClaimTypes.Email)?.Value); Assert.Equal("jdoe", context.User.FindFirst(ClaimTypes.Name)?.Value); - Assert.Equal(userId.ToString(), context.User.FindFirst("IdUsuario")?.Value); + Assert.Equal(userId.ToString(), context.User.FindFirst(ClaimsEnrichmentMiddleware.UserIdClaimType)?.Value); var roles = context.User.FindAll(ClaimTypes.Role).Select(c => c.Value).ToArray(); Assert.Equal(new[] { "10", "20" }, roles); } @@ -62,7 +62,7 @@ public async Task Invoke_MissingUserNameClaim_DoesNotEnrich() { var manager = new Mock(MockBehavior.Strict); var context = BuildAuthenticatedContext(userName: null); - var middleware = new ClaimsEnrichmentMiddleware(_ => Task.CompletedTask, NullLogger.Instance, Options); + var middleware = CreateMiddleware(_ => Task.CompletedTask); await middleware.InvokeAsync(context, manager.Object); @@ -78,11 +78,11 @@ public async Task Invoke_UserNotFound_DoesNotAddClaims() .ReturnsAsync((User?)null); var context = BuildAuthenticatedContext("ghost"); - var middleware = new ClaimsEnrichmentMiddleware(_ => Task.CompletedTask, NullLogger.Instance, Options); + var middleware = CreateMiddleware(_ => Task.CompletedTask); await middleware.InvokeAsync(context, manager.Object); - Assert.Null(context.User.FindFirst("IdUsuario")); + Assert.Null(context.User.FindFirst(ClaimsEnrichmentMiddleware.UserIdClaimType)); manager.Verify(m => m.GetProfilesForUserAsync(It.IsAny(), It.IsAny()), Times.Never); } @@ -98,15 +98,39 @@ public async Task Invoke_StoreThrows_DegradesGracefullyAndCallsNext() var context = BuildAuthenticatedContext("jdoe"); var nextCalled = false; - var middleware = new ClaimsEnrichmentMiddleware(_ => { nextCalled = true; return Task.CompletedTask; }, NullLogger.Instance, Options); + var middleware = CreateMiddleware(_ => { nextCalled = true; return Task.CompletedTask; }); var exception = await Record.ExceptionAsync(() => middleware.InvokeAsync(context, manager.Object)); Assert.Null(exception); Assert.True(nextCalled); - Assert.Null(context.User.FindFirst("IdUsuario")); + Assert.Null(context.User.FindFirst(ClaimsEnrichmentMiddleware.UserIdClaimType)); } + [Fact] + public async Task Invoke_RequestAborted_SkipsEnrichmentAndCallsNext() + { + using var aborted = new CancellationTokenSource(); + aborted.Cancel(); + var manager = new Mock(); + manager + .Setup(m => m.GetUserAsync(It.IsAny(), It.IsAny())) + .ThrowsAsync(new OperationCanceledException(aborted.Token)); + + var context = BuildAuthenticatedContext("jdoe"); + context.RequestAborted = aborted.Token; + var nextCalled = false; + var middleware = CreateMiddleware(_ => { nextCalled = true; return Task.CompletedTask; }); + + await middleware.InvokeAsync(context, manager.Object); + + Assert.True(nextCalled); + Assert.Null(context.User.FindFirst(ClaimsEnrichmentMiddleware.UserIdClaimType)); + } + + private static ClaimsEnrichmentMiddleware CreateMiddleware(RequestDelegate next) => + new(next, NullLogger.Instance, Options); + private static DefaultHttpContext BuildAuthenticatedContext(string? userName) { var claims = new List(); From 6c0d72f4f81f727a42458d76a26ac1dc2b4f558a Mon Sep 17 00:00:00 2001 From: Ismael Leon Date: Mon, 21 Sep 2026 22:01:24 -0600 Subject: [PATCH 5/5] Simplify CI trigger and restrict workflow token to read access An empty branches-ignore list is equivalent to an unfiltered push trigger. The CI job only needs to read the repository, so grant contents: read. Refs #3 --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9f320cc..62b5134 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,11 +2,13 @@ name: CI on: push: - branches-ignore: [] # build every branch push pull_request: branches: [ "main" ] workflow_dispatch: +permissions: + contents: read + # Cancel superseded runs on the same ref to save minutes. concurrency: group: ci-${{ github.ref }}