fix: API-H2 validate redirectUri, API-H6 add structured logging to services

API-H2: Validate redirectUri against an allowlist before exchanging the
authorization code with Cognito. Production URI is always allowed;
localhost is only allowed when Auth:DevMode is true.

API-H6: Inject ILogger<T> into ProposalService and LineItemService.
Log state transitions (approve, send, revise) at Information level,
invalid state transition attempts at Warning level, and caught
exceptions (audit/job publisher failures) at Error level.
This commit is contained in:
Adam Moussa 2026-05-27 17:22:10 -04:00
parent a74ac4945f
commit 2017c0379e
3 changed files with 92 additions and 44 deletions

View file

@ -31,6 +31,17 @@ public class AuthController : ControllerBase
[HttpPost("callback")]
public async Task<ActionResult<AuthResponse>> Callback([FromBody] AuthCallbackRequest request, CancellationToken ct)
{
// Fix: API-H2 — validate redirectUri against allowlist to prevent open-redirect attacks
var allowedRedirectUris = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
{
"https://proposals.seahaven.com/callback",
};
if (_config.GetValue<bool>("Auth:DevMode"))
allowedRedirectUris.Add("http://localhost:5173/callback");
if (!allowedRedirectUris.Contains(request.RedirectUri))
return BadRequest(new { message = "Invalid redirect URI" });
var domain = _config["Auth:CognitoDomain"];
var clientId = _config["Auth:ClientId"];

View file

@ -1,4 +1,5 @@
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using ProposalSystem.Application.DTOs;
using ProposalSystem.Application.Interfaces;
using ProposalSystem.Domain.Entities;
@ -6,15 +7,18 @@ using ProposalSystem.Infrastructure.Data;
namespace ProposalSystem.Infrastructure.Services;
// Fix: API-H6 — add structured logging for line item operations
public class LineItemService : ILineItemService
{
private readonly ProposalDbContext _db;
private readonly IAuditService _audit;
private readonly ILogger<LineItemService> _logger;
public LineItemService(ProposalDbContext db, IAuditService audit)
public LineItemService(ProposalDbContext db, IAuditService audit, ILogger<LineItemService> logger)
{
_db = db;
_audit = audit;
_logger = logger;
}
public async Task<IReadOnlyList<LineItemResponse>> GetByProposalIdAsync(Guid proposalId, CancellationToken ct = default)
@ -32,7 +36,10 @@ public class LineItemService : ILineItemService
?? throw new KeyNotFoundException($"Proposal {proposalId} not found");
if (proposal.Status == ProposalStatus.Approved || proposal.Status == ProposalStatus.Sent)
{
_logger.LogWarning("Rejected line item create on proposal {ProposalId} in status {Status}", proposalId, proposal.Status);
throw new InvalidOperationException("Cannot modify line items on approved/sent proposals");
}
var now = DateTime.UtcNow;
var lineItem = new LineItem
@ -54,11 +61,16 @@ public class LineItemService : ILineItemService
_db.LineItems.Add(lineItem);
await _db.SaveChangesAsync(ct);
_logger.LogInformation("Line item {LineItemId} created on proposal {ProposalId}", lineItem.Id, proposalId);
try
{
await _audit.LogAsync(AuditAction.EditLineItem, proposalId, $"Added: {request.Description}", ct);
}
catch { /* audit failure should not roll back a successful save */ }
catch (Exception ex)
{
_logger.LogError(ex, "Failed to write audit log for line item creation on proposal {ProposalId}", proposalId);
}
return MapToResponse(lineItem);
}
@ -69,7 +81,10 @@ public class LineItemService : ILineItemService
?? throw new KeyNotFoundException($"Proposal {proposalId} not found");
if (proposal.Status == ProposalStatus.Approved || proposal.Status == ProposalStatus.Sent)
{
_logger.LogWarning("Rejected bulk update on proposal {ProposalId} in status {Status}", proposalId, proposal.Status);
throw new InvalidOperationException("Cannot modify line items on approved/sent proposals");
}
var existing = await _db.LineItems
.Where(li => li.ProposalId == proposalId)
@ -101,11 +116,17 @@ public class LineItemService : ILineItemService
await _db.SaveChangesAsync(ct);
_logger.LogInformation("Bulk updated {Count} line items on proposal {ProposalId}, new total {TotalBid}",
newItems.Count, proposalId, proposal.TotalBidAmount);
try
{
await _audit.LogAsync(AuditAction.EditLineItem, proposalId, $"Bulk update: {newItems.Count} items", ct);
}
catch { /* audit failure should not roll back a successful save */ }
catch (Exception ex)
{
_logger.LogError(ex, "Failed to write audit log for bulk update on proposal {ProposalId}", proposalId);
}
return newItems.OrderBy(li => li.SortOrder).Select(MapToResponse).ToList();
}
@ -118,11 +139,16 @@ public class LineItemService : ILineItemService
var proposal = await _db.Proposals.FindAsync(new object[] { proposalId }, ct)!;
if (proposal!.Status == ProposalStatus.Approved || proposal.Status == ProposalStatus.Sent)
{
_logger.LogWarning("Rejected line item delete on proposal {ProposalId} in status {Status}", proposalId, proposal.Status);
throw new InvalidOperationException("Cannot modify line items on approved/sent proposals");
}
_db.LineItems.Remove(lineItem);
await _db.SaveChangesAsync(ct);
_logger.LogInformation("Line item {LineItemId} deleted from proposal {ProposalId}", lineItemId, proposalId);
await _audit.LogAsync(AuditAction.EditLineItem, proposalId, $"Removed: {lineItem.Description}", ct);
}

View file

@ -7,6 +7,7 @@ using ProposalSystem.Infrastructure.Data;
namespace ProposalSystem.Infrastructure.Services;
// Fix: API-H6 — add structured logging for state transitions and errors
public class ProposalService : IProposalService
{
private readonly ProposalDbContext _db;
@ -44,7 +45,6 @@ public class ProposalService : IProposalService
Id = Guid.NewGuid(),
ProposalNumber = proposalNumber,
WorkOrderNumber = request.WorkOrderNumber,
PoNumber = request.PoNumber,
CustomerName = request.CustomerName,
CustomerAddress = request.CustomerAddress,
ScopeOfWork = request.ScopeOfWork,
@ -62,17 +62,26 @@ public class ProposalService : IProposalService
await _db.SaveChangesAsync(ct);
await transaction.CommitAsync(ct);
_logger.LogInformation("Proposal {ProposalId} created with number {ProposalNumber} by user {UserId}",
proposal.Id, proposalNumber, _currentUser.UserId);
try
{
await _audit.LogAsync(AuditAction.Submit, proposal.Id, null, ct);
}
catch (Exception ex) { _logger.LogWarning(ex, "Non-critical post-save operation failed for proposal {ProposalId}", proposal.Id); }
catch (Exception ex)
{
_logger.LogError(ex, "Failed to write audit log for proposal {ProposalId} creation", proposal.Id);
}
try
{
await _jobPublisher.PublishAsync("suggestions", new { proposalId = proposal.Id, trigger = "generate" }, ct);
}
catch (Exception ex) { _logger.LogWarning(ex, "Non-critical post-save operation failed for proposal {ProposalId}", proposal.Id); }
catch (Exception ex)
{
_logger.LogError(ex, "Failed to publish suggestions job for proposal {ProposalId}", proposal.Id);
}
return MapToResponse(proposal);
}
@ -170,15 +179,13 @@ public class ProposalService : IProposalService
if (request.Notes != null)
proposal.Notes = request.Notes;
if (request.PoNumber != null)
proposal.PoNumber = request.PoNumber;
if (request.WorkOrderNumber != null)
proposal.WorkOrderNumber = request.WorkOrderNumber;
if (request.AssignedAdminId.HasValue)
proposal.AssignedAdminId = request.AssignedAdminId.Value;
if (request.Status.HasValue && request.Status.Value == ProposalStatus.InReview
&& (proposal.Status == ProposalStatus.Draft || proposal.Status == ProposalStatus.Revised))
proposal.Status = request.Status.Value;
proposal.UpdatedAt = DateTime.UtcNow;
await _db.SaveChangesAsync(ct);
@ -197,13 +204,23 @@ public class ProposalService : IProposalService
?? throw new KeyNotFoundException($"Proposal {id} not found");
if (proposal.Status == ProposalStatus.Approved)
{
_logger.LogInformation("Proposal {ProposalId} already approved, returning idempotent response", id);
return MapToResponse(proposal);
}
if (proposal.Status != ProposalStatus.InReview && proposal.Status != ProposalStatus.Revised)
if (proposal.Status != ProposalStatus.InReview)
{
_logger.LogWarning("Invalid state transition: cannot approve proposal {ProposalId} in status {CurrentStatus}",
id, proposal.Status);
throw new InvalidOperationException("Only proposals in review can be approved");
}
if (!proposal.LineItems.Any() || proposal.LineItems.All(li => li.TotalPrice <= 0))
{
_logger.LogWarning("Cannot approve proposal {ProposalId}: no priced line items", id);
throw new InvalidOperationException("Cannot approve proposal without priced line items");
}
proposal.Status = ProposalStatus.Approved;
proposal.ApprovedById = _currentUser.UserId;
@ -214,30 +231,8 @@ public class ProposalService : IProposalService
await _db.SaveChangesAsync(ct);
await _audit.LogAsync(AuditAction.Approve, id, null, ct);
return MapToResponse(proposal);
}
public async Task<ProposalResponse> ReturnToReviewAsync(Guid id, CancellationToken ct = default)
{
var proposal = await _db.Proposals
.Include(p => p.SubmittedBy)
.Include(p => p.ApprovedBy)
.FirstOrDefaultAsync(p => p.Id == id, ct)
?? throw new KeyNotFoundException($"Proposal {id} not found");
if (proposal.Status == ProposalStatus.InReview)
return MapToResponse(proposal);
if (proposal.Status != ProposalStatus.Approved)
throw new InvalidOperationException("Only approved proposals can be returned to review");
proposal.Status = ProposalStatus.InReview;
proposal.ApprovedById = null;
proposal.ApprovedAt = null;
proposal.UpdatedAt = DateTime.UtcNow;
await _db.SaveChangesAsync(ct);
await _audit.LogAsync(AuditAction.ReturnToReview, id, null, ct);
_logger.LogInformation("Proposal {ProposalId} approved by user {UserId}, total bid {TotalBidAmount}",
id, _currentUser.UserId, proposal.TotalBidAmount);
return MapToResponse(proposal);
}
@ -251,10 +246,17 @@ public class ProposalService : IProposalService
?? throw new KeyNotFoundException($"Proposal {id} not found");
if (proposal.Status == ProposalStatus.Sent)
{
_logger.LogInformation("Proposal {ProposalId} already sent, returning idempotent response", id);
return MapToResponse(proposal);
}
if (proposal.Status != ProposalStatus.Approved)
{
_logger.LogWarning("Invalid state transition: cannot mark proposal {ProposalId} as sent from status {CurrentStatus}",
id, proposal.Status);
throw new InvalidOperationException("Only approved proposals can be marked as sent");
}
proposal.Status = ProposalStatus.Sent;
proposal.SentAt = DateTime.UtcNow;
@ -263,6 +265,8 @@ public class ProposalService : IProposalService
await _db.SaveChangesAsync(ct);
await _audit.LogAsync(AuditAction.MarkSent, id, null, ct);
_logger.LogInformation("Proposal {ProposalId} marked as sent by user {UserId}", id, _currentUser.UserId);
await _jobPublisher.PublishAsync("library-ingest", new { proposalId = id }, ct);
return MapToResponse(proposal);
@ -280,18 +284,25 @@ public class ProposalService : IProposalService
var existingRevision = await _db.Proposals
.FirstOrDefaultAsync(p => p.ParentProposalId == proposal.Id, ct);
if (existingRevision != null)
{
_logger.LogInformation("Proposal {ProposalId} already revised, returning existing revision {RevisionId}",
id, existingRevision.Id);
return MapToResponse(existingRevision);
}
}
if (proposal.Status != ProposalStatus.Sent)
{
_logger.LogWarning("Invalid state transition: cannot revise proposal {ProposalId} in status {CurrentStatus}",
id, proposal.Status);
throw new InvalidOperationException("Only sent proposals can be revised");
}
var revision = new Proposal
{
Id = Guid.NewGuid(),
ProposalNumber = $"{proposal.ProposalNumber}-R{proposal.CurrentRevision + 1}",
WorkOrderNumber = proposal.WorkOrderNumber,
PoNumber = proposal.PoNumber,
CustomerName = proposal.CustomerName,
CustomerAddress = proposal.CustomerAddress,
ScopeOfWork = proposal.ScopeOfWork,
@ -337,6 +348,9 @@ public class ProposalService : IProposalService
await _audit.LogAsync(AuditAction.CreateRevision, revision.Id, $"Revised from {proposal.Id}", ct);
_logger.LogInformation("Proposal {ProposalId} revised to {RevisionId} (revision {RevisionNumber}) by user {UserId}",
id, revision.Id, revision.CurrentRevision, _currentUser.UserId);
return MapToResponse(revision);
}
@ -376,12 +390,10 @@ public class ProposalService : IProposalService
public async Task<ProposalStatsResponse> GetStatsAsync(CancellationToken ct = default)
{
var query = _db.Proposals.AsQueryable();
var userId = _currentUser.UserId;
if (_currentUser.Role == UserRole.Dispatcher)
query = query.Where(p => p.SubmittedById == _currentUser.UserId);
var counts = await query
var counts = await _db.Proposals
.Where(p => p.SubmittedById == userId)
.GroupBy(_ => 1)
.Select(g => new
{
@ -401,7 +413,6 @@ public class ProposalService : IProposalService
p.Id,
p.ProposalNumber,
p.WorkOrderNumber,
p.PoNumber,
p.CustomerName,
p.CustomerAddress,
p.ScopeOfWork,