feat(api): PermissionAuthorizationHandler resuelve overrides desde DB por request [UDT-009]
This commit is contained in:
@@ -1,26 +1,32 @@
|
||||
using System.IdentityModel.Tokens.Jwt;
|
||||
using Microsoft.AspNetCore.Authorization;
|
||||
using Microsoft.AspNetCore.Http;
|
||||
using SIGCM2.Application.Abstractions.Persistence;
|
||||
using SIGCM2.Application.Common;
|
||||
|
||||
namespace SIGCM2.Api.Authorization;
|
||||
|
||||
/// <summary>
|
||||
/// Authorization handler for <see cref="RequirePermissionAttribute"/>.
|
||||
/// Reads the "rol" claim from the authenticated user, queries <see cref="IRolPermisoRepository"/>
|
||||
/// for the role's assigned permissions, and succeeds if at least one matches (OR semantics).
|
||||
/// No caching — UDT-006 design decision D1: always authoritative from DB.
|
||||
/// UDT-009: Reads "rol" + "sub" claims, queries both IRolPermisoRepository
|
||||
/// and IUsuarioRepository, resolves effective permissions via PermisoResolver,
|
||||
/// and succeeds if at least one required permission matches (OR semantics).
|
||||
/// No caching — always authoritative from DB (UDT-006 D1, UDT-009 D3).
|
||||
/// </summary>
|
||||
public sealed class PermissionAuthorizationHandler
|
||||
: AuthorizationHandler<RequirePermissionAttribute>
|
||||
{
|
||||
private readonly IRolPermisoRepository _rolPermisoRepo;
|
||||
private readonly IUsuarioRepository _usuarioRepo;
|
||||
private readonly ILogger<PermissionAuthorizationHandler> _logger;
|
||||
|
||||
public PermissionAuthorizationHandler(
|
||||
IRolPermisoRepository rolPermisoRepo,
|
||||
IUsuarioRepository usuarioRepo,
|
||||
ILogger<PermissionAuthorizationHandler> logger)
|
||||
{
|
||||
_rolPermisoRepo = rolPermisoRepo;
|
||||
_usuarioRepo = usuarioRepo;
|
||||
_logger = logger;
|
||||
}
|
||||
|
||||
@@ -28,13 +34,11 @@ public sealed class PermissionAuthorizationHandler
|
||||
AuthorizationHandlerContext context,
|
||||
RequirePermissionAttribute requirement)
|
||||
{
|
||||
// 1. Must be authenticated — defense-in-depth (AuthorizeAttribute already requires it)
|
||||
// 1. Must be authenticated — defense-in-depth
|
||||
if (context.User?.Identity?.IsAuthenticated != true)
|
||||
{
|
||||
return; // implicit Fail — nothing Succeeded
|
||||
}
|
||||
return; // implicit Fail
|
||||
|
||||
// 2. Extract "rol" claim — JwtBearer is configured with RoleClaimType="rol"
|
||||
// 2. Extract "rol" claim
|
||||
var rolCodigo = context.User.FindFirst("rol")?.Value;
|
||||
if (string.IsNullOrWhiteSpace(rolCodigo))
|
||||
{
|
||||
@@ -45,13 +49,32 @@ public sealed class PermissionAuthorizationHandler
|
||||
return;
|
||||
}
|
||||
|
||||
// 3. Load permissions assigned to this role — no cache (UDT-006 D1)
|
||||
var permisos = await _rolPermisoRepo.GetByRolCodigoAsync(rolCodigo);
|
||||
var permisoCodes = permisos.Select(p => p.Codigo).ToHashSet(StringComparer.Ordinal);
|
||||
// 3. Extract "sub" claim — MapInboundClaims=false so it stays as "sub" (NOT NameIdentifier)
|
||||
var subClaim = context.User.FindFirst(JwtRegisteredClaimNames.Sub)?.Value
|
||||
?? context.User.FindFirst("sub")?.Value;
|
||||
|
||||
// 4. OR semantics — any single match is enough
|
||||
var matched = requirement.PermissionCodes
|
||||
.FirstOrDefault(code => permisoCodes.Contains(code));
|
||||
if (string.IsNullOrWhiteSpace(subClaim) || !int.TryParse(subClaim, out var userId))
|
||||
{
|
||||
_logger.LogWarning(
|
||||
"Authorization failed — token missing or non-numeric 'sub' claim for user {User}",
|
||||
context.User.Identity?.Name);
|
||||
context.Fail(new AuthorizationFailureReason(this, "missing_sub_claim"));
|
||||
return;
|
||||
}
|
||||
|
||||
// 4. Load role permissions — no cache (UDT-006 D1)
|
||||
var rolPermisoEntities = await _rolPermisoRepo.GetByRolCodigoAsync(rolCodigo);
|
||||
var rolPermisos = rolPermisoEntities.Select(p => p.Codigo);
|
||||
|
||||
// 5. Load user overrides — no cache (UDT-009 D3); null usuario → no overrides
|
||||
var usuario = await _usuarioRepo.GetByIdAsync(userId);
|
||||
var overrides = PermisosOverride.FromJson(usuario?.PermisosJson);
|
||||
|
||||
// 6. Resolve effective permissions
|
||||
var effective = PermisoResolver.Resolve(rolPermisos, overrides);
|
||||
|
||||
// 7. OR semantics — any single match is enough
|
||||
var matched = requirement.PermissionCodes.FirstOrDefault(effective.Contains);
|
||||
|
||||
if (matched is not null)
|
||||
{
|
||||
@@ -59,11 +82,9 @@ public sealed class PermissionAuthorizationHandler
|
||||
return;
|
||||
}
|
||||
|
||||
// 5. Stash required permission for ForbiddenProblemDetailsHandler (Batch 3)
|
||||
// 8. Stash required permission for ForbiddenProblemDetailsHandler
|
||||
if (context.Resource is HttpContext httpContext)
|
||||
{
|
||||
httpContext.Items["RequiredPermission"] = requirement.PermissionCodes[0];
|
||||
}
|
||||
|
||||
context.Fail(new AuthorizationFailureReason(this,
|
||||
$"missing_permission:{string.Join('|', requirement.PermissionCodes)}"));
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
using System.IdentityModel.Tokens.Jwt;
|
||||
using System.Security.Claims;
|
||||
using Microsoft.AspNetCore.Authorization;
|
||||
using Microsoft.AspNetCore.Http;
|
||||
@@ -10,27 +11,38 @@ using SIGCM2.Domain.Entities;
|
||||
namespace SIGCM2.Api.Tests.Authorization;
|
||||
|
||||
/// <summary>
|
||||
/// Unit tests for PermissionAuthorizationHandler — SUITE-B-01 (UDT-006).
|
||||
/// Tests isolated from DB: IRolPermisoRepository is mocked via NSubstitute.
|
||||
/// Unit tests for PermissionAuthorizationHandler — SUITE-B-01 (UDT-006) + SUITE-B-AUTHZ-HANDLER (UDT-009).
|
||||
/// Tests isolated from DB: IRolPermisoRepository and IUsuarioRepository mocked via NSubstitute.
|
||||
/// </summary>
|
||||
public sealed class PermissionAuthorizationHandlerTests
|
||||
{
|
||||
private readonly IRolPermisoRepository _rolPermisoRepo = Substitute.For<IRolPermisoRepository>();
|
||||
private readonly IUsuarioRepository _usuarioRepo = Substitute.For<IUsuarioRepository>();
|
||||
private readonly PermissionAuthorizationHandler _handler;
|
||||
|
||||
public PermissionAuthorizationHandlerTests()
|
||||
{
|
||||
// Default: usuario repo returns null (no overrides) unless overridden in individual tests
|
||||
_usuarioRepo.GetByIdAsync(Arg.Any<int>(), Arg.Any<CancellationToken>())
|
||||
.Returns((Usuario?)null);
|
||||
|
||||
_handler = new PermissionAuthorizationHandler(
|
||||
_rolPermisoRepo,
|
||||
_usuarioRepo,
|
||||
NullLogger<PermissionAuthorizationHandler>.Instance);
|
||||
}
|
||||
|
||||
// ── Helpers ──────────────────────────────────────────────────────────────
|
||||
|
||||
private static ClaimsPrincipal AuthenticatedUserWithRol(string rolValue)
|
||||
/// <summary>Creates an authenticated user with rol claim and sub=42 (needed by UDT-009 handler).</summary>
|
||||
private static ClaimsPrincipal AuthenticatedUserWithRol(string rolValue, int userId = 42)
|
||||
{
|
||||
var identity = new ClaimsIdentity(
|
||||
new[] { new Claim("rol", rolValue) },
|
||||
new[]
|
||||
{
|
||||
new Claim("rol", rolValue),
|
||||
new Claim(JwtRegisteredClaimNames.Sub, userId.ToString()),
|
||||
},
|
||||
authenticationType: "TestAuth");
|
||||
return new ClaimsPrincipal(identity);
|
||||
}
|
||||
@@ -38,7 +50,19 @@ public sealed class PermissionAuthorizationHandlerTests
|
||||
private static ClaimsPrincipal AuthenticatedUserWithoutRolClaim()
|
||||
{
|
||||
var identity = new ClaimsIdentity(
|
||||
new[] { new Claim(ClaimTypes.Name, "someuser") },
|
||||
new[]
|
||||
{
|
||||
new Claim(ClaimTypes.Name, "someuser"),
|
||||
new Claim(JwtRegisteredClaimNames.Sub, "42"),
|
||||
},
|
||||
authenticationType: "TestAuth");
|
||||
return new ClaimsPrincipal(identity);
|
||||
}
|
||||
|
||||
private static ClaimsPrincipal AuthenticatedUserWithoutSubClaim()
|
||||
{
|
||||
var identity = new ClaimsIdentity(
|
||||
new[] { new Claim("rol", "cajero") },
|
||||
authenticationType: "TestAuth");
|
||||
return new ClaimsPrincipal(identity);
|
||||
}
|
||||
@@ -243,4 +267,149 @@ public sealed class PermissionAuthorizationHandlerTests
|
||||
Assert.False(context.HasSucceeded);
|
||||
Assert.Equal("administracion:usuarios:gestionar", httpContext.Items["RequiredPermission"]);
|
||||
}
|
||||
|
||||
// ── UDT-009: SUITE-B-AUTHZ-HANDLER (A-01 a A-07) ────────────────────────
|
||||
|
||||
// A-01: Cajero sin override, endpoint requiere permiso ajeno → HasSucceeded == false
|
||||
[Fact]
|
||||
public async Task A01_Cajero_NoOverride_LacksPermission_Fails()
|
||||
{
|
||||
var user = AuthenticatedUserWithRol("cajero", userId: 42);
|
||||
var requirement = new RequirePermissionAttribute("administracion:usuarios:gestionar");
|
||||
|
||||
_rolPermisoRepo.GetByRolCodigoAsync("cajero", Arg.Any<CancellationToken>())
|
||||
.Returns(new List<Permiso> { MakePermiso(10, "ventas:contado:crear") }
|
||||
.AsReadOnly() as IReadOnlyList<Permiso>);
|
||||
_usuarioRepo.GetByIdAsync(42, Arg.Any<CancellationToken>())
|
||||
.Returns(MakeUsuario(42, "cajero", """{"grant":[],"deny":[]}"""));
|
||||
|
||||
var context = MakeContext(user, requirement);
|
||||
await _handler.HandleAsync(context);
|
||||
|
||||
Assert.False(context.HasSucceeded);
|
||||
}
|
||||
|
||||
// A-02: Cajero con grant del permiso requerido → HasSucceeded == true
|
||||
[Fact]
|
||||
public async Task A02_Cajero_WithGrant_RequiredPermiso_Succeeds()
|
||||
{
|
||||
var user = AuthenticatedUserWithRol("cajero", userId: 42);
|
||||
var requirement = new RequirePermissionAttribute("administracion:usuarios:gestionar");
|
||||
|
||||
_rolPermisoRepo.GetByRolCodigoAsync("cajero", Arg.Any<CancellationToken>())
|
||||
.Returns(new List<Permiso> { MakePermiso(10, "ventas:contado:crear") }
|
||||
.AsReadOnly() as IReadOnlyList<Permiso>);
|
||||
_usuarioRepo.GetByIdAsync(42, Arg.Any<CancellationToken>())
|
||||
.Returns(MakeUsuario(42, "cajero", """{"grant":["administracion:usuarios:gestionar"],"deny":[]}"""));
|
||||
|
||||
var context = MakeContext(user, requirement);
|
||||
await _handler.HandleAsync(context);
|
||||
|
||||
Assert.True(context.HasSucceeded);
|
||||
}
|
||||
|
||||
// A-03: Admin (tiene el permiso) + deny del permiso requerido → HasSucceeded == false
|
||||
[Fact]
|
||||
public async Task A03_Admin_WithDeny_RequiredPermiso_Fails()
|
||||
{
|
||||
var user = AuthenticatedUserWithRol("admin", userId: 1);
|
||||
var requirement = new RequirePermissionAttribute("administracion:permisos:ver");
|
||||
|
||||
_rolPermisoRepo.GetByRolCodigoAsync("admin", Arg.Any<CancellationToken>())
|
||||
.Returns(new List<Permiso> { MakePermiso(21, "administracion:permisos:ver") }
|
||||
.AsReadOnly() as IReadOnlyList<Permiso>);
|
||||
_usuarioRepo.GetByIdAsync(1, Arg.Any<CancellationToken>())
|
||||
.Returns(MakeUsuario(1, "admin", """{"grant":[],"deny":["administracion:permisos:ver"]}"""));
|
||||
|
||||
var context = MakeContext(user, requirement);
|
||||
await _handler.HandleAsync(context);
|
||||
|
||||
Assert.False(context.HasSucceeded);
|
||||
}
|
||||
|
||||
// A-04: Token sin claim 'permisos' (post-UDT-009) → handler resuelve desde DB
|
||||
[Fact]
|
||||
public async Task A04_TokenWithoutPermisosClaim_HandlerResolvesFromDB()
|
||||
{
|
||||
// Token has sub=42 but no 'permisos' claim (post-UDT-009 JWT)
|
||||
var user = AuthenticatedUserWithRol("cajero", userId: 42);
|
||||
var requirement = new RequirePermissionAttribute("ventas:contado:crear");
|
||||
|
||||
_rolPermisoRepo.GetByRolCodigoAsync("cajero", Arg.Any<CancellationToken>())
|
||||
.Returns(new List<Permiso> { MakePermiso(10, "ventas:contado:crear") }
|
||||
.AsReadOnly() as IReadOnlyList<Permiso>);
|
||||
_usuarioRepo.GetByIdAsync(42, Arg.Any<CancellationToken>())
|
||||
.Returns(MakeUsuario(42, "cajero", """{"grant":[],"deny":[]}"""));
|
||||
|
||||
var context = MakeContext(user, requirement);
|
||||
await _handler.HandleAsync(context);
|
||||
|
||||
// Handler correctly resolves from DB (no 'permisos' claim needed)
|
||||
Assert.True(context.HasSucceeded);
|
||||
await _usuarioRepo.Received(1).GetByIdAsync(42, Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
// A-05: IUsuarioRepository.GetByIdAsync called with sub from token
|
||||
[Fact]
|
||||
public async Task A05_GetByIdAsync_CalledWithSubFromToken()
|
||||
{
|
||||
var user = AuthenticatedUserWithRol("cajero", userId: 42);
|
||||
var requirement = new RequirePermissionAttribute("ventas:contado:crear");
|
||||
|
||||
_rolPermisoRepo.GetByRolCodigoAsync("cajero", Arg.Any<CancellationToken>())
|
||||
.Returns(new List<Permiso> { MakePermiso(10, "ventas:contado:crear") }
|
||||
.AsReadOnly() as IReadOnlyList<Permiso>);
|
||||
_usuarioRepo.GetByIdAsync(42, Arg.Any<CancellationToken>())
|
||||
.Returns(MakeUsuario(42, "cajero", """{"grant":[],"deny":[]}"""));
|
||||
|
||||
var context = MakeContext(user, requirement);
|
||||
await _handler.HandleAsync(context);
|
||||
|
||||
await _usuarioRepo.Received(1).GetByIdAsync(42, Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
// A-06: sub claim absent → Fail, repo NOT called
|
||||
[Fact]
|
||||
public async Task A06_SubClaimAbsent_Fails_RepoNotCalled()
|
||||
{
|
||||
var user = AuthenticatedUserWithoutSubClaim();
|
||||
var requirement = new RequirePermissionAttribute("ventas:contado:crear");
|
||||
|
||||
_rolPermisoRepo.GetByRolCodigoAsync(Arg.Any<string>(), Arg.Any<CancellationToken>())
|
||||
.Returns(new List<Permiso> { MakePermiso(10, "ventas:contado:crear") }
|
||||
.AsReadOnly() as IReadOnlyList<Permiso>);
|
||||
|
||||
var context = MakeContext(user, requirement);
|
||||
await _handler.HandleAsync(context);
|
||||
|
||||
Assert.False(context.HasSucceeded);
|
||||
await _usuarioRepo.DidNotReceive().GetByIdAsync(Arg.Any<int>(), Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
// A-07: Usuario not found in DB (null) → Fail, no exception
|
||||
[Fact]
|
||||
public async Task A07_UsuarioNotFoundInDB_FailsSafely_NoException()
|
||||
{
|
||||
var user = AuthenticatedUserWithRol("cajero", userId: 9999);
|
||||
var requirement = new RequirePermissionAttribute("ventas:contado:crear");
|
||||
|
||||
_rolPermisoRepo.GetByRolCodigoAsync("cajero", Arg.Any<CancellationToken>())
|
||||
.Returns(new List<Permiso> { MakePermiso(10, "ventas:contado:crear") }
|
||||
.AsReadOnly() as IReadOnlyList<Permiso>);
|
||||
_usuarioRepo.GetByIdAsync(9999, Arg.Any<CancellationToken>())
|
||||
.Returns((Usuario?)null);
|
||||
|
||||
var context = MakeContext(user, requirement);
|
||||
|
||||
// Should not throw — null usuario → no overrides → resolve with Empty (rol permisos only)
|
||||
await _handler.HandleAsync(context);
|
||||
|
||||
// With no overrides, cajero with ventas:contado:crear should succeed
|
||||
Assert.True(context.HasSucceeded);
|
||||
}
|
||||
|
||||
// ── helpers ───────────────────────────────────────────────────────────────
|
||||
|
||||
private static Usuario MakeUsuario(int id, string rol, string permisosJson)
|
||||
=> new(id, "user" + id, "$2a$12$hash", "Test", "User", null, rol, permisosJson, true);
|
||||
}
|
||||
|
||||
@@ -225,7 +225,8 @@ public sealed class CreateUsuarioEndpointTests : IAsyncLifetime
|
||||
new { Username = newUsername });
|
||||
|
||||
Assert.True(row.Activo, "Activo should be true");
|
||||
Assert.Equal("[]", row.PermisosJson);
|
||||
// V009 (UDT-009): ForCreation now defaults to canonical shape {"grant":[],"deny":[]}
|
||||
Assert.Equal("""{"grant":[],"deny":[]}""", row.PermisosJson);
|
||||
}
|
||||
finally
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user