diff --git a/AUDIT-REPORT.md b/AUDIT-REPORT.md new file mode 100644 index 0000000..b1f11d8 --- /dev/null +++ b/AUDIT-REPORT.md @@ -0,0 +1,248 @@ +# Proposal System — Production Readiness Audit Report + +**Date:** 2026-05-27 +**Auditor:** Claude Code (6 parallel specialist agents) +**Scope:** Full monorepo — API, Web, Mobile, Lambdas, Infrastructure/CI/CD, QA/Testing + +--- + +## Executive Summary + +The Proposal System has a solid architectural foundation with clean separation of concerns, proper Cognito JWT auth at the API Gateway layer, encrypted RDS, and a working end-to-end flow. However, the audit uncovered **5 Critical**, **36 High**, **75+ Medium**, and **60+ Low** severity findings across all layers. + +**The most urgent issues:** + +1. **Internal API key middleware applies globally** — any endpoint can be accessed with the internal key, bypassing JWT auth entirely (API Critical) +2. **JWT validation skipped when Authority not configured** — tokens are parsed but signatures are never verified (API Critical) +3. **Lambda Function URL has AUTH_NONE** — publicly accessible, bypassing API Gateway authorization (Infra/Lambda Critical) +4. **JWT stored in localStorage** — vulnerable to XSS token theft (Web Critical) +5. **Zero test coverage across entire monorepo** — no test files, no test frameworks, CI passes with zero tests (QA Critical) +6. **DevMode has no environment guard** — if `Auth:DevMode=true` leaks to production, anyone can mint tokens for any role (API High) + +--- + +## Findings by Domain + +### 1. API Security (2 Critical, 8 High, 14 Medium, 13 Low) + +#### Critical + +| ID | Finding | File | Fix | +|----|---------|------|-----| +| API-C1 | Internal API key middleware applies to ALL routes, not scoped to internal paths. Any request with the key bypasses JWT and gets admin role. | `Middleware/InternalApiKeyMiddleware.cs:22-48` | Scope to internal-only paths via path check | +| API-C2 | Auth callback skips JWT signature validation when `Auth:Authority` is empty — tokens are parsed but never verified. `appsettings.json` defaults to `""`. | `Controllers/AuthController.cs:70-73` | Throw on empty authority in non-dev mode; remove unvalidated path | + +#### High + +| ID | Finding | File | +|----|---------|------| +| API-H1 | Invalid API key does not short-circuit — request continues through normal auth pipeline | `InternalApiKeyMiddleware.cs:42-47` | +| API-H2 | Auth callback `redirectUri` not validated server-side | `AuthController.cs:31-124` | +| API-H3 | `UpdateProposalRequest` exposes `Status` field — over-posting risk | `ProposalDtos.cs:22`, `ProposalService.cs:178-180` | +| API-H4 | No validator for `UpdateProposalRequest` — unlimited string lengths | `Validators/` (missing file) | +| API-H5 | `InvalidOperationException` messages leaked to clients | `GlobalExceptionHandler.cs:60-67` | +| API-H6 | No structured logging in any service or controller | All services | +| API-H7 | No Swagger/OpenAPI configuration at all | `Program.cs`, `.csproj` | +| API-H8 | DevMode controlled solely by config — no `IsDevelopment()` guard | `Program.cs:20` | + +#### Medium + +| ID | Finding | +|----|---------| +| API-M1 | Internal API key always grants `admins` role, never `sysadmins` | +| API-M2 | Silent auth failure when neither Cognito nor DevMode configured | +| API-M3 | Dispatchers can read any proposal's line items (no ownership check) | +| API-M4 | Dispatchers can access PDF endpoints for any proposal | +| API-M5 | Missing validators for VendorProposal, GeneratedPdf, SimilarReference DTOs | +| API-M6 | No file size validation on presigned upload URLs | +| API-M7 | No `.AsNoTracking()` on read-only queries | +| API-M8 | BulkUpdate uses delete-all/insert-all without explicit transaction | +| API-M9 | Dev PDF generation leaks stderr to client | +| API-M10 | Auth callback reveals config state in error responses | +| API-M11 | Silent exception swallowing on audit logging (`catch { }`) | +| API-M12 | Audit trail does not capture before/after values | +| API-M13 | User role change audit does not log previous role | +| API-M14 | Dev signing key hardcoded in committed config | + +--- + +### 2. Web Frontend (1 Critical, 6 High, 13 Medium, 8 Low) + +#### Critical + +| ID | Finding | File | +|----|---------|------| +| WEB-C1 | JWT token stored in localStorage — XSS token theft risk | `authSlice.ts:51,58`, `client.ts:14` | + +#### High + +| ID | Finding | File | +|----|---------|------| +| WEB-H1 | No token refresh mechanism — expired tokens cause abrupt redirect | `authSlice.ts:21-30` | +| WEB-H2 | ProtectedRoute doesn't check loading state — flash-redirect on hydration | `ProtectedRoute.tsx:4-12` | +| WEB-H3 | Dispatcher can view any proposal via direct URL | `App.tsx:40` | +| WEB-H4 | "View Access Roles" button does nothing | `App.tsx:57-59` | +| WEB-H5 | saveMutation has no onError; partial failure leaves inconsistent state | `AdminWorkspace.tsx:114-138` | +| WEB-H6 | approveMutation chains save→approve with no error recovery | `AdminWorkspace.tsx:140-154` | +| WEB-H7 | No React error boundary — blank white screen on crash | `main.tsx` | + +#### Medium + +| ID | Finding | +|----|---------| +| WEB-M1 | Dev login shown when client ID absent — verify API gate | +| WEB-M2 | 401 interceptor clears token but not Redux state | +| WEB-M3 | Proposal form accepts 1-char scope (no minimum) | +| WEB-M4 | ServiceCategory `Other` not in shared contract | +| WEB-M5 | `CreateProposalRequest` type diverges from shared contract | +| WEB-M6 | No file size/type validation on vendor PDF upload | +| WEB-M7 | AdminWorkspace shows no error state for failed fetch | +| WEB-M8 | Dashboard stats show zeros on fetch error | +| WEB-M9 | Missing loading state for line items | +| WEB-M10 | Proposal state transitions not guarded on client | +| WEB-M11 | `returnToReview` API method wired but never called from UI | +| WEB-M12 | Table rows not keyboard accessible | +| WEB-M13 | ToastContainer rendered outside RouterProvider | + +--- + +### 3. Mobile (0 Critical, 4 High, 12 Medium, 11 Low) + +#### High + +| ID | Finding | File | +|----|---------|------| +| MOB-H1 | Offline queue race condition — no mutex, duplicate proposals | `useOfflineDraft.ts:63-94` | +| MOB-H2 | Conditional screen registration — push/deep links may crash | `RootNavigator.tsx:33-78` | +| MOB-H3 | Offline queue sync errors silently swallowed | `App.tsx:80` | +| MOB-H4 | Bulk line item update has no optimistic concurrency | `LineItemEditScreen.tsx:55-101` | + +#### Medium + +| ID | Finding | +|----|---------| +| MOB-M1-M5 | Token refresh gaps, queue processing blocks on first failure, no queue UI, processes on every network event | +| MOB-M6-M8 | Shared contract mismatches (poNumber, id field) | +| MOB-M9-M12 | Navigation UX, loading states, unhandled promise rejections, atob encoding | + +--- + +### 4. Lambda Pipeline (1 Critical, 5 High, 14 Medium, 8 Low) + +#### Critical + +| ID | Finding | File | +|----|---------|------| +| LAM-C1 | Function URL `authType: NONE` — publicly accessible | `compute-stack.ts:241-243` | + +#### High + +| ID | Finding | File | +|----|---------|------| +| LAM-H1 | pdf-generate: `register_pdf` failure doesn't raise — PDF in S3 but not in DB | `pdf-generate/app.py:96-98` | +| LAM-H2 | pdf-extract: exception swallowed, SQS considers it success, no retry | `pdf-extract/app.py:79-83` | +| LAM-H3 | pdf-extract: missing `s3Key` silently skips without batch failure | `pdf-extract/app.py:54-55` | +| LAM-H4 | suggestions: duplicate SQS delivery overwrites admin-edited line items | `suggestions/app.py:232-289` | +| LAM-H5 | `_retry_request` can return undefined `resp` | All 4 Lambda files | + +#### Medium + +| ID | Finding | +|----|---------| +| LAM-M1-M4 | Event validation, prompt injection risk, PDF size limits, Bedrock timeout | +| LAM-M5-M8 | Missing stack traces, numeric validation, tight Lambda timeout, S3 key sanitization | +| LAM-M9-M14 | Stale API key cache, empty env var defaults, KB sync flooding, CDK bundling gaps | + +--- + +### 5. Infrastructure & CI/CD (0 Critical, 5 High, 9 Medium, 10 Low) + +#### High + +| ID | Finding | File | +|----|---------|------| +| INF-H1 | Function URL `authType: NONE` | `compute-stack.ts:241-243` | +| INF-H2 | SQS queues have no encryption at rest | `foundation-stack.ts:162-174` | +| INF-H3 | OpenSearch Serverless allows public network access | `compute-stack.ts:61-70` | +| INF-H4 | No MFA configured on Cognito user pool | `foundation-stack.ts:177-194` | +| INF-H5 | No access logging on HTTP API Gateway | `compute-stack.ts:246-263` | + +#### Medium + +| ID | Finding | +|----|---------| +| INF-M1-M2 | Bedrock wildcard model ARN, AOSS `aoss:*` permissions | +| INF-M3-M4 | No Cognito advanced security, Google OAuth not in CDK | +| INF-M5-M7 | No S3 enforceSSL, no custom domain on CF, no WAF | +| INF-M8-M9 | Workflows pinned to @main, --require-approval never locally | + +--- + +### 6. QA & Testing (6 Critical, 16 High) + +**ZERO application-level test coverage across the entire monorepo.** + +#### Critical Gaps + +| ID | What's Untested | Risk | +|----|-----------------|------| +| QA-C1 | No test project in .NET solution | CI `dotnet test` is a no-op | +| QA-C2 | Proposal state machine | Invalid transitions undetectable | +| QA-C3 | Authorization enforcement | Privilege escalation undetectable | +| QA-C4 | InternalApiKeyMiddleware | Auth bypass regression risk | +| QA-C5 | ProtectedRoute and RoleGuard | Client-side auth unverified | +| QA-C6 | Mobile offline draft and queue | Data loss risk | + +#### High Gaps + +Validators, ProposalNumberGenerator, AuthController, LineItemService state guards, all frontend components, API client interceptors, all Lambda handlers, SQS event parsing, PDF parsers, suggestions logic, CI pipeline executes zero tests. + +--- + +## Prioritized Remediation Plan + +### Phase 1 — Critical Security Fixes +1. Scope internal API key middleware to internal-only routes +2. Guard JWT validation — reject when Authority not configured in non-dev +3. Add DevMode environment guard (`IsDevelopment()` required) +4. Make invalid API key reject request immediately (401) +5. Add React error boundary to web app +6. Fix ProtectedRoute loading state + +### Phase 2 — High Security & Reliability Fixes +7. Remove `Status` from `UpdateProposalRequest` +8. Add `UpdateProposalValidator` +9. Sanitize error messages in GlobalExceptionHandler +10. Fix Lambda error propagation (pdf-extract, pdf-generate) +11. Add suggestions idempotency check +12. Fix `_retry_request` undefined variable +13. Fix AdminWorkspace mutation error handling +14. Fix dead "View Access Roles" button +15. Add mobile offline queue mutex and error handling + +### Phase 3 — Swagger/OpenAPI +16. Add Swashbuckle and configure +17. Add JWT security definition +18. Add `[ProducesResponseType]` to all controllers +19. Gate Swagger UI to dev/staging + +### Phase 4 — Infrastructure Hardening (CDK) +20. SQS encryption, API Gateway logging, Cognito MFA, S3 enforceSSL + +### Phase 5 — Test Infrastructure (future) +25. .NET test project, vitest for web, pytest for Lambdas, CI integration + +--- + +## Positive Findings + +- RDS: private subnets, not publicly accessible, encrypted, deletion protection, 7-day backups +- Cognito: self-signup disabled (admin-created accounts only) +- CORS: properly scoped to production origin +- Secrets: production connection string uses Secrets Manager +- S3: all buckets have `BlockPublicAccess.BLOCK_ALL` +- CloudFront: OAC, HTTPS redirect, security headers, TLS 1.2 minimum +- GitHub Actions: OIDC (no long-lived credentials), minimal permissions +- Monitoring: alarms for DLQ depth, RDS metrics, Lambda errors, API 5xx +- Mobile: tokens in iOS Keychain, no secrets in Fastlane config +- SQS: visibility timeout properly sized for Lambda consumers diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..405fc39 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,57 @@ +# Proposal System - Claude Code Project Memory + +## Project Overview + +Proposal management platform for Sea Haven Industries. Dispatchers submit service requests, AI generates draft line items via Bedrock RAG, admins review/approve in a pricing workspace, system produces branded PDFs. + +## Architecture + +- **api/**: .NET 8 API, EF Core, PostgreSQL, Cognito JWT auth +- **web/**: React 19 + MUI v7 SPA, Vite, CloudFront + S3 +- **mobile/**: React Native 0.85 iOS app, offline-capable, Hermes +- **lambdas/**: Python 3.12 Lambdas (ARM64): pdf-extract, pdf-generate, library-ingest, suggestions, oss-index-creator +- **infra/**: CDK TypeScript (foundation-stack, compute-stack, frontend-stack) +- **shared/**: Shared TypeScript API contracts +- **scripts/**: Local dev helpers + +## Auth Model + +External: Cognito JWT via API Gateway (web + mobile client IDs, groups: dispatchers/admins/sysadmins) +Internal: Lambdas call .NET Function URL with Secrets Manager API key via custom middleware + +## Request Flow + +1. Dispatcher submits proposal (web/mobile) → InReview (no Draft stage) +2. Bedrock RAG suggests line items from pricing library +3. Admin reviews in workspace, edits line items, approves +4. PDF generation queued via SQS → Python Lambda → branded PDF → S3 +5. State machine: InReview → Approved → Sent → Revised + +## Infrastructure + +AWS us-east-1, RDS PostgreSQL 15, S3, SQS+DLQ, Cognito+Google OAuth, OpenSearch Serverless, Bedrock KB, GitHub Actions OIDC, CloudFront+S3 OAC + +## Agent Delegation Rules + +### For audit and hardening work, use the Explore-Plan-Execute pipeline: + +1. Spawn specialist subagents for parallel investigation (api-security, web-audit, mobile-audit, lambda-pipeline, infra-cicd, qa-testing) +2. Consolidate findings into AUDIT-REPORT.md before implementing +3. Prioritize: Critical > High > Medium > Low +4. Implement fixes in logical phases, commit after each phase +5. Use separate git worktrees/branches for parallel implementation where safe + +### Working Rules + +- Never commit secrets, credentials, .env files, or local artifacts +- If secrets found in code: document, remove safely, ensure proper config mechanism +- Preserve existing business logic unless broken, insecure, or contradicted +- Run lint/typecheck/build/test after each phase +- If context reaches 65%, pause, commit, update AUDIT-REPORT.md with HANDOFF ADDENDUM + +### Severity Levels + +- **Critical**: security/data exposure/auth bypass/data corruption +- **High**: broken core workflow, deployment blocker, missing authz, invalid infra +- **Medium**: reliability, validation, logging, test gaps +- **Low**: cleanup, DX, docs, polish \ No newline at end of file diff --git a/api/src/ProposalSystem.Api/Controllers/AuthController.cs b/api/src/ProposalSystem.Api/Controllers/AuthController.cs index 37463ee..a3a1512 100644 --- a/api/src/ProposalSystem.Api/Controllers/AuthController.cs +++ b/api/src/ProposalSystem.Api/Controllers/AuthController.cs @@ -44,33 +44,28 @@ public class AuthController : ControllerBase var handler = new JwtSecurityTokenHandler(); var authority = _config["Auth:Authority"]; - JwtSecurityToken idToken; - if (!string.IsNullOrEmpty(authority)) - { - var configManager = new ConfigurationManager( - $"{authority}/.well-known/openid-configuration", - new OpenIdConnectConfigurationRetriever(), - new HttpDocumentRetriever()); - var oidcConfig = await configManager.GetConfigurationAsync(ct); + if (string.IsNullOrEmpty(authority)) + return StatusCode(500, new { message = "Token validation is not configured" }); - var validationParams = new TokenValidationParameters - { - ValidateIssuerSigningKey = true, - IssuerSigningKeys = oidcConfig.SigningKeys, - ValidateIssuer = true, - ValidIssuer = authority, - ValidateAudience = true, - ValidAudience = clientId, - ValidateLifetime = true, - }; + var configManager = new ConfigurationManager( + $"{authority}/.well-known/openid-configuration", + new OpenIdConnectConfigurationRetriever(), + new HttpDocumentRetriever()); + var oidcConfig = await configManager.GetConfigurationAsync(ct); - handler.ValidateToken(tokenResponse.IdToken, validationParams, out var validatedToken); - idToken = (JwtSecurityToken)validatedToken; - } - else + var validationParams = new TokenValidationParameters { - idToken = handler.ReadJwtToken(tokenResponse.IdToken); - } + ValidateIssuerSigningKey = true, + IssuerSigningKeys = oidcConfig.SigningKeys, + ValidateIssuer = true, + ValidIssuer = authority, + ValidateAudience = true, + ValidAudience = clientId, + ValidateLifetime = true, + }; + + handler.ValidateToken(tokenResponse.IdToken, validationParams, out var validatedToken); + var idToken = (JwtSecurityToken)validatedToken; var sub = idToken.Claims.FirstOrDefault(c => c.Type == "sub")?.Value ?? throw new InvalidOperationException("No sub claim in ID token"); diff --git a/api/src/ProposalSystem.Api/Controllers/ProposalsController.cs b/api/src/ProposalSystem.Api/Controllers/ProposalsController.cs index 274a4be..dcac93f 100644 --- a/api/src/ProposalSystem.Api/Controllers/ProposalsController.cs +++ b/api/src/ProposalSystem.Api/Controllers/ProposalsController.cs @@ -25,6 +25,8 @@ public class ProposalsController : ControllerBase } [HttpPost] + [ProducesResponseType(typeof(ProposalResponse), 201)] + [ProducesResponseType(400)] public async Task> Create( [FromBody] CreateProposalRequest request, CancellationToken ct) @@ -34,6 +36,7 @@ public class ProposalsController : ControllerBase } [HttpGet] + [ProducesResponseType(typeof(PagedResponse), 200)] public async Task>> GetAll( [FromQuery] ProposalFilterRequest filter, CancellationToken ct) @@ -43,6 +46,8 @@ public class ProposalsController : ControllerBase } [HttpGet("{id:guid}")] + [ProducesResponseType(typeof(ProposalResponse), 200)] + [ProducesResponseType(404)] public async Task> GetById(Guid id, CancellationToken ct) { var result = await _proposalService.GetByIdAsync(id, ct); @@ -52,6 +57,9 @@ public class ProposalsController : ControllerBase [HttpPut("{id:guid}")] [Authorize(Roles = "admins,sysadmins")] + [ProducesResponseType(typeof(ProposalResponse), 200)] + [ProducesResponseType(400)] + [ProducesResponseType(404)] public async Task> Update( Guid id, [FromBody] UpdateProposalRequest request, @@ -63,6 +71,9 @@ public class ProposalsController : ControllerBase [HttpPost("{id:guid}/approve")] [Authorize(Roles = "admins,sysadmins")] + [ProducesResponseType(typeof(ProposalResponse), 200)] + [ProducesResponseType(400)] + [ProducesResponseType(404)] public async Task> Approve(Guid id, CancellationToken ct) { var result = await _proposalService.ApproveAsync(id, ct); diff --git a/api/src/ProposalSystem.Api/Middleware/GlobalExceptionHandler.cs b/api/src/ProposalSystem.Api/Middleware/GlobalExceptionHandler.cs index 322b66a..08ee88b 100644 --- a/api/src/ProposalSystem.Api/Middleware/GlobalExceptionHandler.cs +++ b/api/src/ProposalSystem.Api/Middleware/GlobalExceptionHandler.cs @@ -45,7 +45,7 @@ public class GlobalExceptionHandler : IMiddleware { Status = 404, Title = "Not Found", - Detail = exception.Message, + Detail = "The requested resource was not found", } ), UnauthorizedAccessException => ( @@ -63,7 +63,7 @@ public class GlobalExceptionHandler : IMiddleware { Status = 400, Title = "Invalid Operation", - Detail = exception.Message, + Detail = "The requested operation is not valid for the current state", } ), _ => ( @@ -77,10 +77,8 @@ public class GlobalExceptionHandler : IMiddleware ), }; - if (statusCode == HttpStatusCode.InternalServerError) - { - _logger.LogError(exception, "Unhandled exception"); - } + _logger.LogError(exception, "Exception on {Method} {Path}: {Status}", + context.Request.Method, context.Request.Path, (int)statusCode); context.Response.StatusCode = (int)statusCode; context.Response.ContentType = "application/problem+json"; diff --git a/api/src/ProposalSystem.Api/Middleware/InternalApiKeyMiddleware.cs b/api/src/ProposalSystem.Api/Middleware/InternalApiKeyMiddleware.cs index 86ce1a5..c5b599d 100644 --- a/api/src/ProposalSystem.Api/Middleware/InternalApiKeyMiddleware.cs +++ b/api/src/ProposalSystem.Api/Middleware/InternalApiKeyMiddleware.cs @@ -6,6 +6,14 @@ namespace ProposalSystem.Api.Middleware; public class InternalApiKeyMiddleware { + private static readonly string[] AllowedPathPrefixes = + [ + "/api/proposals", + "/api/vendor-proposals", + "/api/generated-pdfs", + "/api/files", + ]; + private readonly RequestDelegate _next; private readonly byte[] _apiKeyBytes; private readonly ILogger _logger; @@ -25,26 +33,35 @@ public class InternalApiKeyMiddleware !string.IsNullOrEmpty(providedKey.ToString())) { var providedBytes = Encoding.UTF8.GetBytes(providedKey.ToString()); - if (CryptographicOperations.FixedTimeEquals(providedBytes, _apiKeyBytes)) - { - var claims = new[] - { - new Claim(ClaimTypes.NameIdentifier, "system"), - new Claim("sub", "system-lambda-caller"), - new Claim(ClaimTypes.Email, "system@proposal-system.internal"), - new Claim("email", "system@proposal-system.internal"), - new Claim("name", "System"), - new Claim(ClaimTypes.Role, "admins"), - new Claim("cognito:groups", "admins"), - }; - var identity = new ClaimsIdentity(claims, "InternalApiKey"); - context.User = new ClaimsPrincipal(identity); - } - else + if (!CryptographicOperations.FixedTimeEquals(providedBytes, _apiKeyBytes)) { _logger.LogWarning("Invalid internal API key from {RemoteIp} on {Path}", context.Connection.RemoteIpAddress, context.Request.Path); + context.Response.StatusCode = 401; + return; } + + var path = context.Request.Path.Value ?? ""; + if (!AllowedPathPrefixes.Any(prefix => path.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))) + { + _logger.LogWarning("Internal API key used on disallowed path {Path} from {RemoteIp}", + context.Request.Path, context.Connection.RemoteIpAddress); + context.Response.StatusCode = 403; + return; + } + + var claims = new[] + { + new Claim(ClaimTypes.NameIdentifier, "system"), + new Claim("sub", "system-lambda-caller"), + new Claim(ClaimTypes.Email, "system@proposal-system.internal"), + new Claim("email", "system@proposal-system.internal"), + new Claim("name", "System"), + new Claim(ClaimTypes.Role, "admins"), + new Claim("cognito:groups", "admins"), + }; + var identity = new ClaimsIdentity(claims, "InternalApiKey"); + context.User = new ClaimsPrincipal(identity); } await _next(context); diff --git a/api/src/ProposalSystem.Api/Program.cs b/api/src/ProposalSystem.Api/Program.cs index 6cc2679..8a12ba8 100644 --- a/api/src/ProposalSystem.Api/Program.cs +++ b/api/src/ProposalSystem.Api/Program.cs @@ -7,6 +7,7 @@ using FluentValidation; using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.EntityFrameworkCore; using Microsoft.IdentityModel.Tokens; +using Microsoft.OpenApi.Models; using ProposalSystem.Api.Middleware; using ProposalSystem.Api.Services; using ProposalSystem.Application.Interfaces; @@ -17,7 +18,7 @@ using ProposalSystem.Infrastructure.Services; var builder = WebApplication.CreateBuilder(args); // Dev mode flag (read early for conditional setup) -var devMode = builder.Configuration.GetValue("Auth:DevMode"); +var devMode = builder.Configuration.GetValue("Auth:DevMode") && builder.Environment.IsDevelopment(); // AWS SDK clients (skip in dev mode — no real AWS credentials needed) if (!devMode) @@ -99,8 +100,8 @@ else if (devMode) } else { - builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) - .AddJwtBearer(); + throw new InvalidOperationException( + "Authentication is not configured. Set Auth:Authority for Cognito or Auth:DevMode=true (Development only)."); } builder.Services.AddAuthorization(); @@ -153,6 +154,37 @@ builder.Services.AddControllers(options => options.JsonSerializerOptions.Converters.Add(new System.Text.Json.Serialization.JsonStringEnumConverter()); }); +// OpenAPI / Swagger +builder.Services.AddEndpointsApiExplorer(); +builder.Services.AddSwaggerGen(options => +{ + options.SwaggerDoc("v1", new OpenApiInfo + { + Title = "Proposal System API", + Version = "v1", + Description = "Sea Haven Industries proposal management API", + }); + options.AddSecurityDefinition("Bearer", new OpenApiSecurityScheme + { + Name = "Authorization", + Type = SecuritySchemeType.Http, + Scheme = "bearer", + BearerFormat = "JWT", + In = ParameterLocation.Header, + Description = "Cognito JWT access token", + }); + options.AddSecurityRequirement(new OpenApiSecurityRequirement + { + { + new OpenApiSecurityScheme + { + Reference = new OpenApiReference { Type = ReferenceType.SecurityScheme, Id = "Bearer" }, + }, + Array.Empty() + }, + }); +}); + // Middleware builder.Services.AddTransient(); @@ -180,6 +212,13 @@ builder.Services.AddAWSLambdaHosting(LambdaEventSource.HttpApi); var app = builder.Build(); app.UseMiddleware(); + +if (!app.Environment.IsProduction()) +{ + app.UseSwagger(); + app.UseSwaggerUI(c => c.SwaggerEndpoint("/swagger/v1/swagger.json", "Proposal System API v1")); +} + app.UseCors(); app.UseMiddleware(); app.UseAuthentication(); diff --git a/api/src/ProposalSystem.Api/ProposalSystem.Api.csproj b/api/src/ProposalSystem.Api/ProposalSystem.Api.csproj index 49ace4e..a0a24e7 100644 --- a/api/src/ProposalSystem.Api/ProposalSystem.Api.csproj +++ b/api/src/ProposalSystem.Api/ProposalSystem.Api.csproj @@ -26,6 +26,7 @@ + diff --git a/api/src/ProposalSystem.Application/DTOs/ProposalDtos.cs b/api/src/ProposalSystem.Application/DTOs/ProposalDtos.cs index cf77689..ff72e71 100644 --- a/api/src/ProposalSystem.Application/DTOs/ProposalDtos.cs +++ b/api/src/ProposalSystem.Application/DTOs/ProposalDtos.cs @@ -18,8 +18,7 @@ public record UpdateProposalRequest( string? Notes, string? PoNumber, string? WorkOrderNumber, - Guid? AssignedAdminId, - ProposalStatus? Status + Guid? AssignedAdminId ); public record ProposalResponse( diff --git a/api/src/ProposalSystem.Application/Validators/UpdateProposalValidator.cs b/api/src/ProposalSystem.Application/Validators/UpdateProposalValidator.cs new file mode 100644 index 0000000..ae6e315 --- /dev/null +++ b/api/src/ProposalSystem.Application/Validators/UpdateProposalValidator.cs @@ -0,0 +1,15 @@ +using FluentValidation; +using ProposalSystem.Application.DTOs; + +namespace ProposalSystem.Application.Validators; + +public class UpdateProposalValidator : AbstractValidator +{ + public UpdateProposalValidator() + { + RuleFor(x => x.RefinedScope).MaximumLength(10000); + RuleFor(x => x.Notes).MaximumLength(5000); + RuleFor(x => x.PoNumber).MaximumLength(50); + RuleFor(x => x.WorkOrderNumber).MaximumLength(50); + } +} diff --git a/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs b/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs index 9af22d1..d1ed846 100644 --- a/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs +++ b/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs @@ -1,4 +1,5 @@ using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; using ProposalSystem.Application.DTOs; using ProposalSystem.Application.Interfaces; using ProposalSystem.Domain.Entities; @@ -13,19 +14,22 @@ public class ProposalService : IProposalService private readonly IProposalNumberGenerator _numberGenerator; private readonly IAuditService _audit; private readonly IJobPublisher _jobPublisher; + private readonly ILogger _logger; public ProposalService( ProposalDbContext db, ICurrentUserService currentUser, IProposalNumberGenerator numberGenerator, IAuditService audit, - IJobPublisher jobPublisher) + IJobPublisher jobPublisher, + ILogger logger) { _db = db; _currentUser = currentUser; _numberGenerator = numberGenerator; _audit = audit; _jobPublisher = jobPublisher; + _logger = logger; } public async Task CreateAsync(CreateProposalRequest request, CancellationToken ct = default) @@ -62,13 +66,13 @@ public class ProposalService : IProposalService { await _audit.LogAsync(AuditAction.Submit, proposal.Id, null, ct); } - catch { } + catch (Exception ex) { _logger.LogWarning(ex, "Non-critical post-save operation failed for proposal {ProposalId}", proposal.Id); } try { await _jobPublisher.PublishAsync("suggestions", new { proposalId = proposal.Id, trigger = "generate" }, ct); } - catch { } + catch (Exception ex) { _logger.LogWarning(ex, "Non-critical post-save operation failed for proposal {ProposalId}", proposal.Id); } return MapToResponse(proposal); } @@ -175,10 +179,6 @@ public class ProposalService : IProposalService 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); diff --git a/infra/lib/compute-stack.ts b/infra/lib/compute-stack.ts index 301817a..4cca936 100644 --- a/infra/lib/compute-stack.ts +++ b/infra/lib/compute-stack.ts @@ -262,11 +262,29 @@ export class ComputeStack extends cdk.Stack { }, }); + const apiAccessLogGroup = new logs.LogGroup(this, 'ApiAccessLogs', { + logGroupName: '/aws/apigateway/proposal-system', + retention: logs.RetentionDays.TWO_MONTHS, + removalPolicy: cdk.RemovalPolicy.DESTROY, + }); + const defaultStage = httpApi.defaultStage!.node.defaultChild as apigatewayv2.CfnStage; defaultStage.defaultRouteSettings = { throttlingBurstLimit: 50, throttlingRateLimit: 100, }; + defaultStage.accessLogSettings = { + destinationArn: apiAccessLogGroup.logGroupArn, + format: JSON.stringify({ + requestId: '$context.requestId', + ip: '$context.identity.sourceIp', + method: '$context.httpMethod', + path: '$context.path', + status: '$context.status', + latency: '$context.responseLatency', + userAgent: '$context.identity.userAgent', + }), + }; const apiIntegration = new apigatewayv2Integrations.HttpLambdaIntegration( 'ApiIntegration', diff --git a/infra/lib/foundation-stack.ts b/infra/lib/foundation-stack.ts index 1a28fc8..b4317ae 100644 --- a/infra/lib/foundation-stack.ts +++ b/infra/lib/foundation-stack.ts @@ -110,6 +110,7 @@ export class FoundationStack extends cdk.Stack { this.uploadsBucket = new s3.Bucket(this, 'UploadsBucket', { bucketName: `proposal-system-uploads-${this.account}`, encryption: s3.BucketEncryption.S3_MANAGED, + enforceSSL: true, versioned: true, blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL, lifecycleRules: [ @@ -141,6 +142,7 @@ export class FoundationStack extends cdk.Stack { this.generatedBucket = new s3.Bucket(this, 'GeneratedBucket', { bucketName: `proposal-system-generated-${this.account}`, encryption: s3.BucketEncryption.S3_MANAGED, + enforceSSL: true, versioned: true, blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL, removalPolicy: cdk.RemovalPolicy.RETAIN, @@ -151,6 +153,7 @@ export class FoundationStack extends cdk.Stack { this.libraryBucket = new s3.Bucket(this, 'LibraryBucket', { bucketName: `proposal-system-library-${this.account}`, encryption: s3.BucketEncryption.S3_MANAGED, + enforceSSL: true, versioned: true, blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL, removalPolicy: cdk.RemovalPolicy.RETAIN, @@ -162,11 +165,13 @@ export class FoundationStack extends cdk.Stack { const dlq = new sqs.Queue(this, 'JobsDlq', { queueName: 'proposal-system-jobs-dlq', retentionPeriod: cdk.Duration.days(14), + encryption: sqs.QueueEncryption.SQS_MANAGED, }); this.jobsQueue = new sqs.Queue(this, 'JobsQueue', { queueName: 'proposal-system-jobs', visibilityTimeout: cdk.Duration.seconds(720), + encryption: sqs.QueueEncryption.SQS_MANAGED, deadLetterQueue: { queue: dlq, maxReceiveCount: 3, @@ -182,6 +187,11 @@ export class FoundationStack extends cdk.Stack { email: { required: true, mutable: true }, fullname: { required: true, mutable: true }, }, + mfa: cognito.Mfa.OPTIONAL, + mfaSecondFactor: { + sms: false, + otp: true, + }, passwordPolicy: { minLength: 12, requireUppercase: true, diff --git a/lambdas/library-ingest/app.py b/lambdas/library-ingest/app.py index ef9a4e8..0d9a9de 100644 --- a/lambdas/library-ingest/app.py +++ b/lambdas/library-ingest/app.py @@ -210,11 +210,13 @@ def _retry_request( method: str, url: str, *, max_retries: int = 3, **kwargs ) -> httpx.Response: kwargs.setdefault("timeout", 10) + last_resp = None for attempt in range(max_retries): try: resp = httpx.request(method, url, **kwargs) if resp.status_code < 500: return resp + last_resp = resp except (httpx.ConnectError, httpx.ReadTimeout, httpx.WriteTimeout) as exc: if attempt == max_retries - 1: raise @@ -222,4 +224,6 @@ def _retry_request( "Retryable error (attempt %d/%d): %s", attempt + 1, max_retries, exc ) time.sleep(min(2**attempt, 4)) - return resp # type: ignore[possibly-undefined] + if last_resp is not None: + return last_resp + raise RuntimeError(f"All {max_retries} retries failed for {method} {url}") diff --git a/lambdas/pdf-extract/app.py b/lambdas/pdf-extract/app.py index 6c0864c..a14df8e 100644 --- a/lambdas/pdf-extract/app.py +++ b/lambdas/pdf-extract/app.py @@ -52,7 +52,8 @@ def handler(event, context): vendor_proposal_id = payload.get("vendorProposalId", "") if not s3_key: - logger.warning("No s3Key in payload for proposal %s", proposal_id) + logger.error("No s3Key in payload for proposal %s, message %s", proposal_id, record.get("messageId")) + batch_item_failures.append({"itemIdentifier": record["messageId"]}) continue process_pdf(proposal_id, s3_key, vendor_proposal_id) @@ -79,8 +80,9 @@ def process_pdf(proposal_id: str, s3_key: str, vendor_proposal_id: str): save_extraction(vendor_proposal_id, extracted) except Exception as e: - logger.error("Error processing PDF: %s", e) + logger.error("Error processing PDF: %s", e, exc_info=True) update_processing_status(vendor_proposal_id, "Failed") + raise finally: if pdf_path: try: @@ -350,11 +352,13 @@ def _retry_request( method: str, url: str, *, max_retries: int = 3, **kwargs ) -> httpx.Response: kwargs.setdefault("timeout", 10) + last_resp = None for attempt in range(max_retries): try: resp = httpx.request(method, url, **kwargs) if resp.status_code < 500: return resp + last_resp = resp except (httpx.ConnectError, httpx.ReadTimeout, httpx.WriteTimeout) as exc: if attempt == max_retries - 1: raise @@ -362,4 +366,6 @@ def _retry_request( "Retryable error (attempt %d/%d): %s", attempt + 1, max_retries, exc ) time.sleep(min(2**attempt, 4)) - return resp # type: ignore[possibly-undefined] + if last_resp is not None: + return last_resp + raise RuntimeError(f"All {max_retries} retries failed for {method} {url}") diff --git a/lambdas/pdf-generate/app.py b/lambdas/pdf-generate/app.py index e8b056a..4a0754e 100644 --- a/lambdas/pdf-generate/app.py +++ b/lambdas/pdf-generate/app.py @@ -552,9 +552,10 @@ def register_pdf(proposal_id: str, s3_key: str): headers=_api_headers(), ) if resp.status_code not in (200, 201): - logger.error("Failed to register PDF: %s %s", resp.status_code, resp.text) + raise RuntimeError(f"Failed to register PDF: {resp.status_code} {resp.text}") except Exception as e: - logger.error("Error registering PDF: %s", e) + logger.error("Error registering PDF: %s", e, exc_info=True) + raise def _api_headers() -> dict: @@ -569,11 +570,13 @@ def _retry_request( method: str, url: str, *, max_retries: int = 3, **kwargs ) -> httpx.Response: kwargs.setdefault("timeout", 10) + last_resp = None for attempt in range(max_retries): try: resp = httpx.request(method, url, **kwargs) if resp.status_code < 500: return resp + last_resp = resp except (httpx.ConnectError, httpx.ReadTimeout, httpx.WriteTimeout) as exc: if attempt == max_retries - 1: raise @@ -581,4 +584,6 @@ def _retry_request( "Retryable error (attempt %d/%d): %s", attempt + 1, max_retries, exc ) time.sleep(min(2**attempt, 4)) - return resp # type: ignore[possibly-undefined] + if last_resp is not None: + return last_resp + raise RuntimeError(f"All {max_retries} retries failed for {method} {url}") diff --git a/lambdas/suggestions/app.py b/lambdas/suggestions/app.py index 17cabf9..655f4a4 100644 --- a/lambdas/suggestions/app.py +++ b/lambdas/suggestions/app.py @@ -65,6 +65,16 @@ def process_suggestion(proposal_id: str, trigger: str): existing_items = fetch_line_items(proposal_id) + has_ai_items = any(li.get("source") == "AI" for li in existing_items) + if has_ai_items: + logger.info("AI items already exist for %s, skipping regeneration", proposal_id) + return + + status = proposal.get("status", "") + if status not in ("InReview", "Revised"): + logger.info("Proposal %s is in status %s, skipping suggestions", proposal_id, status) + return + similar_proposals = retrieve_similar(scope, category) suggested_items = generate_line_items(scope, category, priority, similar_proposals) @@ -325,11 +335,13 @@ def _retry_request( method: str, url: str, *, max_retries: int = 3, **kwargs ) -> httpx.Response: kwargs.setdefault("timeout", 10) + last_resp = None for attempt in range(max_retries): try: resp = httpx.request(method, url, **kwargs) if resp.status_code < 500: return resp + last_resp = resp except (httpx.ConnectError, httpx.ReadTimeout, httpx.WriteTimeout) as exc: if attempt == max_retries - 1: raise @@ -337,4 +349,6 @@ def _retry_request( "Retryable error (attempt %d/%d): %s", attempt + 1, max_retries, exc ) time.sleep(min(2**attempt, 4)) - return resp # type: ignore[possibly-undefined] + if last_resp is not None: + return last_resp + raise RuntimeError(f"All {max_retries} retries failed for {method} {url}") diff --git a/mobile/src/App.tsx b/mobile/src/App.tsx index 3645956..295a5db 100644 --- a/mobile/src/App.tsx +++ b/mobile/src/App.tsx @@ -77,7 +77,7 @@ function AuthBootstrap({ children }: { children: React.ReactNode }) { ); } }) - .catch(() => {}); + .catch((err) => console.error('Offline queue sync failed:', err)); } }); return unsubscribe; diff --git a/mobile/src/hooks/useOfflineDraft.ts b/mobile/src/hooks/useOfflineDraft.ts index 51998b3..8cb2aeb 100644 --- a/mobile/src/hooks/useOfflineDraft.ts +++ b/mobile/src/hooks/useOfflineDraft.ts @@ -60,37 +60,52 @@ export async function queueOfflineSubmission(data: unknown): Promise { await AsyncStorage.setItem(OFFLINE_QUEUE_KEY, JSON.stringify(queue)); } +let _isProcessing = false; + export async function processOfflineQueue( submitFn: (data: unknown) => Promise, ): Promise { - const state = await NetInfo.fetch(); - if (!state.isConnected) return 0; + if (_isProcessing) return 0; + _isProcessing = true; - const stored = await AsyncStorage.getItem(OFFLINE_QUEUE_KEY); - if (!stored) return 0; + try { + const state = await NetInfo.fetch(); + if (!state.isConnected) return 0; - const queue: QueuedSubmission[] = JSON.parse(stored); - let processed = 0; + const stored = await AsyncStorage.getItem(OFFLINE_QUEUE_KEY); + if (!stored) return 0; - for (const item of queue) { - try { - await submitFn(item.data); - processed++; - } catch { - break; + const queue: QueuedSubmission[] = JSON.parse(stored); + let processed = 0; + + for (const item of queue) { + try { + await submitFn(item.data); + processed++; + } catch (error) { + const status = (error as { response?: { status?: number } })?.response?.status; + if (status && status >= 400 && status < 500) { + console.error('Permanent failure for queued submission, skipping:', error); + processed++; + continue; + } + break; + } } - } - if (processed === queue.length) { - await AsyncStorage.removeItem(OFFLINE_QUEUE_KEY); - } else if (processed > 0) { - await AsyncStorage.setItem( - OFFLINE_QUEUE_KEY, - JSON.stringify(queue.slice(processed)), - ); - } + if (processed === queue.length) { + await AsyncStorage.removeItem(OFFLINE_QUEUE_KEY); + } else if (processed > 0) { + await AsyncStorage.setItem( + OFFLINE_QUEUE_KEY, + JSON.stringify(queue.slice(processed)), + ); + } - return processed; + return processed; + } finally { + _isProcessing = false; + } } export async function getOfflineQueueCount(): Promise { diff --git a/mobile/src/navigation/RootNavigator.tsx b/mobile/src/navigation/RootNavigator.tsx index 646f5f5..ad22586 100644 --- a/mobile/src/navigation/RootNavigator.tsx +++ b/mobile/src/navigation/RootNavigator.tsx @@ -36,43 +36,31 @@ function ProposalsNavigator() { return ( - {isAdmin ? ( - <> - - - - - ) : ( - <> - - - - - )} + + + + + ); } diff --git a/web/src/App.tsx b/web/src/App.tsx index 40c0635..59ea093 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -63,8 +63,14 @@ export default function App() { Users and roles are currently managed in AWS Cognito. Contact the system administrator to update access. - diff --git a/web/src/components/ErrorBoundary.tsx b/web/src/components/ErrorBoundary.tsx new file mode 100644 index 0000000..c5b674a --- /dev/null +++ b/web/src/components/ErrorBoundary.tsx @@ -0,0 +1,41 @@ +import React from 'react'; +import { Box, Button, Typography } from '@mui/material'; + +interface Props { + children: React.ReactNode; +} + +interface State { + hasError: boolean; + error: Error | null; +} + +export default class ErrorBoundary extends React.Component { + state: State = { hasError: false, error: null }; + + static getDerivedStateFromError(error: Error): State { + return { hasError: true, error }; + } + + componentDidCatch(error: Error, info: React.ErrorInfo) { + console.error('ErrorBoundary caught:', error, info.componentStack); + } + + render() { + if (this.state.hasError) { + return ( + + Something went wrong + + An unexpected error occurred. Please try refreshing the page. + + + + ); + } + + return this.props.children; + } +} diff --git a/web/src/components/ProtectedRoute.tsx b/web/src/components/ProtectedRoute.tsx index 8e77202..940f732 100644 --- a/web/src/components/ProtectedRoute.tsx +++ b/web/src/components/ProtectedRoute.tsx @@ -1,8 +1,17 @@ import { Navigate } from 'react-router-dom'; +import { Box, CircularProgress } from '@mui/material'; import { useAuth } from '../hooks/useAuth'; export default function ProtectedRoute({ children }: { children: React.ReactNode }) { - const { isAuthenticated } = useAuth(); + const { isAuthenticated, loading } = useAuth(); + + if (loading) { + return ( + + + + ); + } if (!isAuthenticated) { return ; diff --git a/web/src/main.tsx b/web/src/main.tsx index d284c5d..bdd5cfc 100644 --- a/web/src/main.tsx +++ b/web/src/main.tsx @@ -1,6 +1,6 @@ import React from 'react'; import ReactDOM from 'react-dom/client'; -import { BrowserRouter } from 'react-router-dom'; +import { createBrowserRouter, RouterProvider } from 'react-router-dom'; import { Provider } from 'react-redux'; import { QueryClientProvider } from '@tanstack/react-query'; import { ThemeProvider, CssBaseline } from '@mui/material'; @@ -10,17 +10,22 @@ import './index.css'; import { store } from './app/store'; import { queryClient } from './lib/queryClient'; import { theme } from './theme'; +import ErrorBoundary from './components/ErrorBoundary'; import App from './App'; +const router = createBrowserRouter([ + { path: '*', Component: App }, +]); + ReactDOM.createRoot(document.getElementById('root')!).render( - - - + + + diff --git a/web/src/pages/admin/workspace/AdminWorkspace.tsx b/web/src/pages/admin/workspace/AdminWorkspace.tsx index 9a398bf..d8eb7ff 100644 --- a/web/src/pages/admin/workspace/AdminWorkspace.tsx +++ b/web/src/pages/admin/workspace/AdminWorkspace.tsx @@ -135,6 +135,9 @@ export default function AdminWorkspace() { setDirty(false); toast.success('Changes saved'); }, + onError: (error: Error) => { + toast.error(`Save failed: ${error.message}`); + }, }); const approveMutation = useMutation({ @@ -151,6 +154,9 @@ export default function AdminWorkspace() { setDirty(false); toast.success('Proposal approved'); }, + onError: (error: Error) => { + toast.error(`Approval failed: ${error.message}`); + }, }); const sendMutation = useMutation({