diff --git a/api/src/ProposalSystem.Api/Controllers/AuthController.cs b/api/src/ProposalSystem.Api/Controllers/AuthController.cs index a3a1512..0c16e87 100644 --- a/api/src/ProposalSystem.Api/Controllers/AuthController.cs +++ b/api/src/ProposalSystem.Api/Controllers/AuthController.cs @@ -31,6 +31,17 @@ public class AuthController : ControllerBase [HttpPost("callback")] public async Task> Callback([FromBody] AuthCallbackRequest request, CancellationToken ct) { + // Fix: API-H2 — validate redirectUri against allowlist to prevent open-redirect attacks + var allowedRedirectUris = new HashSet(StringComparer.OrdinalIgnoreCase) + { + "https://proposals.seahaven.com/callback", + }; + if (_config.GetValue("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"]; diff --git a/api/src/ProposalSystem.Infrastructure/Services/LineItemService.cs b/api/src/ProposalSystem.Infrastructure/Services/LineItemService.cs index e3f53d2..189f533 100644 --- a/api/src/ProposalSystem.Infrastructure/Services/LineItemService.cs +++ b/api/src/ProposalSystem.Infrastructure/Services/LineItemService.cs @@ -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 _logger; - public LineItemService(ProposalDbContext db, IAuditService audit) + public LineItemService(ProposalDbContext db, IAuditService audit, ILogger logger) { _db = db; _audit = audit; + _logger = logger; } public async Task> 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); } diff --git a/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs b/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs index d1ed846..065f4fe 100644 --- a/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs +++ b/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs @@ -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 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 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,