From 091c5fcb444970222d2ad5ff2d10bd7bed711240 Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Wed, 20 May 2026 18:51:31 -0400 Subject: [PATCH 1/2] Fix Phase 1 security and data integrity audit findings (#49) BLOCK-01: Add API Gateway JWT authorizer with Cognito, route internal Lambda calls through Function URL to bypass gateway auth BLOCK-02/03: Prevent proposal number race condition with pg_advisory_xact_lock and filter revision numbers from max-number query BLOCK-04: Restrict VendorProposals and GeneratedPdfs to admins/sysadmins BLOCK-05: Sum all vendor costs instead of overwriting with single vendor BLOCK-06: Enable ValidateAudience on JWT, add Auth env vars to API Lambda BLOCK-07: Validate ID token signature in AuthController via OIDC discovery BLOCK-08: Use batchItemFailures in all Lambda SQS handlers BLOCK-09: Increase SQS visibility timeout from 180s to 720s FIX-10: Scope dispatcher queries to own proposals (IDOR fix) --- README.md | 25 ++++++++-- .../Controllers/AuthController.cs | 47 ++++++++++++++++--- .../Controllers/GeneratedPdfsController.cs | 2 +- .../Controllers/VendorProposalsController.cs | 15 +++--- api/src/ProposalSystem.Api/Program.cs | 5 +- .../Services/ProposalNumberGenerator.cs | 5 ++ .../Services/ProposalService.cs | 10 +++- infra/bin/app.ts | 2 + infra/lib/compute-stack.ts | 44 +++++++++++++++-- infra/lib/foundation-stack.ts | 8 +++- lambdas/library-ingest/app.py | 15 ++++-- lambdas/pdf-extract/app.py | 25 ++++++---- lambdas/pdf-generate/app.py | 15 ++++-- lambdas/suggestions/app.py | 17 ++++--- 14 files changed, 182 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 11bd291..de193c5 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Internal proposal management platform for Sea Haven Industries. Dispatchers subm Monorepo with five primary services: -- **.NET 8 API** -- Clean Architecture REST API hosted on Lambda behind API Gateway +- **.NET 8 API** -- Clean Architecture REST API hosted on Lambda behind API Gateway (JWT-authorized) with Function URL for internal access - **React 19 Web** -- MUI v7 admin/dispatcher workspace served via CloudFront + S3 - **React Native Mobile** -- iOS-first field app for dispatchers (offline-capable) - **Python Lambdas** -- PDF extraction, PDF generation, library ingestion, AI suggestions, AOSS index provisioning @@ -46,13 +46,13 @@ All resources are in **us-east-1** (account 328440206208). | CDK Stack | Key Resources | |---|---| | `proposal-system-foundation` | RDS PostgreSQL 15 (t4g.small), S3 buckets, SQS queue + DLQ, Cognito user pool, Secrets Manager | -| `proposal-system-compute` | API Gateway HTTP API, .NET 8 API Lambda, Python Lambdas (pdf-extract, pdf-generate, library-ingest, suggestions, oss-index-creator), OpenSearch Serverless collection, Bedrock KB | +| `proposal-system-compute` | API Gateway HTTP API (JWT authorizer), .NET 8 API Lambda + Function URL, Python Lambdas (pdf-extract, pdf-generate, library-ingest, suggestions, oss-index-creator), OpenSearch Serverless collection, Bedrock KB | | `proposal-system-frontend` | CloudFront distribution (S3 OAC) | | Resource Type | Names | |---|---| | S3 Buckets | `proposal-system-uploads`, `proposal-system-generated`, `proposal-system-library`, `seahaven-ios-certificates` | -| SQS | `proposal-system-jobs` + `proposal-system-jobs-dlq` (message body filtering by jobType) | +| SQS | `proposal-system-jobs` (720s visibility, reportBatchItemFailures) + `proposal-system-jobs-dlq` (message body filtering by jobType) | | Secrets | `proposal-system/db-credentials`, `proposal-system/internal-api-key` | ## Local Development @@ -149,12 +149,29 @@ Build and upload to TestFlight is handled by the `cd-mobile-ios.yaml` reusable w | `ASC_ISSUER_ID` | App Store Connect issuer | | `ASC_KEY_CONTENT` | App Store Connect API key (base64) | +## Authentication & Authorization + +Two-layer auth architecture with defense-in-depth: + +| Path | Authorizer | Authentication | +|---|---|---| +| External clients → API Gateway `/{proxy+}` | Cognito JWT authorizer (web + mobile client IDs) | .NET JWT middleware (ValidateAudience=true) | +| `/api/health` | None (public) | None | +| `/api/auth/callback`, `/api/auth/dev-login` | None (unauthenticated) | None (pre-auth endpoints) | +| Internal Lambdas → Function URL | None (NONE auth type) | Internal API key (`X-Internal-Api-Key` header, value from Secrets Manager) | + +**Role-based access:** Cognito groups (`dispatchers`, `admins`, `sysadmins`) map to API roles via `cognito:groups` claim. Dispatchers can only see their own proposals (ownership enforced in service layer). VendorProposals and GeneratedPdfs endpoints restricted to admins/sysadmins. + +**Internal API key:** Python Lambdas call the .NET API via a Lambda Function URL (bypasses API Gateway JWT check). The `InternalApiKeyMiddleware` validates the key and assigns the `admins` role to the synthetic identity. + ## Data Flow 1. Dispatcher submits proposal request (web or mobile) -2. API creates proposal record, publishes SQS message +2. API creates proposal record (with advisory-locked number generation), publishes SQS message 3. If vendor PDF attached: `pdf-extract` Lambda parses and structures data 4. Suggestions Lambda queries Bedrock KB for similar proposals, generates line items via Claude 5. Admin reviews/edits line items in pricing workspace 6. On approval: `pdf-generate` Lambda creates branded PDF 7. On send: `library-ingest` Lambda adds approved proposal to KB for future matching + +Failed SQS messages are reported via `batchItemFailures` and retried up to 3 times before moving to the DLQ. diff --git a/api/src/ProposalSystem.Api/Controllers/AuthController.cs b/api/src/ProposalSystem.Api/Controllers/AuthController.cs index 19dcf12..37463ee 100644 --- a/api/src/ProposalSystem.Api/Controllers/AuthController.cs +++ b/api/src/ProposalSystem.Api/Controllers/AuthController.cs @@ -5,6 +5,8 @@ using System.Text; using System.Text.Json; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; +using Microsoft.IdentityModel.Protocols; +using Microsoft.IdentityModel.Protocols.OpenIdConnect; using Microsoft.IdentityModel.Tokens; using ProposalSystem.Domain.Entities; using ProposalSystem.Infrastructure.Data; @@ -40,7 +42,35 @@ public class AuthController : ControllerBase return BadRequest(new { message = "Failed to exchange authorization code" }); var handler = new JwtSecurityTokenHandler(); - var idToken = handler.ReadJwtToken(tokenResponse.IdToken); + + 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); + + var validationParams = new TokenValidationParameters + { + ValidateIssuerSigningKey = true, + IssuerSigningKeys = oidcConfig.SigningKeys, + ValidateIssuer = true, + ValidIssuer = authority, + ValidateAudience = true, + ValidAudience = clientId, + ValidateLifetime = true, + }; + + handler.ValidateToken(tokenResponse.IdToken, validationParams, out var validatedToken); + idToken = (JwtSecurityToken)validatedToken; + } + else + { + idToken = handler.ReadJwtToken(tokenResponse.IdToken); + } var sub = idToken.Claims.FirstOrDefault(c => c.Type == "sub")?.Value ?? throw new InvalidOperationException("No sub claim in ID token"); @@ -71,12 +101,17 @@ public class AuthController : ControllerBase _db.Users.Add(user); await _db.SaveChangesAsync(ct); } - else if (user.Email != email || user.DisplayName != name) + else { - user.Email = email; - user.DisplayName = name; - user.UpdatedAt = DateTime.UtcNow; - await _db.SaveChangesAsync(ct); + var changed = false; + if (user.Email != email) { user.Email = email; changed = true; } + if (user.DisplayName != name) { user.DisplayName = name; changed = true; } + if (user.Role != role) { user.Role = role; changed = true; } + if (changed) + { + user.UpdatedAt = DateTime.UtcNow; + await _db.SaveChangesAsync(ct); + } } return Ok(new AuthResponse( diff --git a/api/src/ProposalSystem.Api/Controllers/GeneratedPdfsController.cs b/api/src/ProposalSystem.Api/Controllers/GeneratedPdfsController.cs index 8f3d181..e06c409 100644 --- a/api/src/ProposalSystem.Api/Controllers/GeneratedPdfsController.cs +++ b/api/src/ProposalSystem.Api/Controllers/GeneratedPdfsController.cs @@ -9,7 +9,7 @@ namespace ProposalSystem.Api.Controllers; [ApiController] [Route("api/generated-pdfs")] -[Authorize] +[Authorize(Roles = "admins,sysadmins")] public class GeneratedPdfsController : ControllerBase { private readonly ProposalDbContext _db; diff --git a/api/src/ProposalSystem.Api/Controllers/VendorProposalsController.cs b/api/src/ProposalSystem.Api/Controllers/VendorProposalsController.cs index f311a99..9b3b455 100644 --- a/api/src/ProposalSystem.Api/Controllers/VendorProposalsController.cs +++ b/api/src/ProposalSystem.Api/Controllers/VendorProposalsController.cs @@ -8,7 +8,7 @@ namespace ProposalSystem.Api.Controllers; [ApiController] [Route("api/vendor-proposals")] -[Authorize] +[Authorize(Roles = "admins,sysadmins")] public class VendorProposalsController : ControllerBase { private readonly ProposalDbContext _db; @@ -38,14 +38,13 @@ public class VendorProposalsController : ControllerBase await _db.SaveChangesAsync(ct); - if (vendor.TotalVendorCost > 0) + var proposal = await _db.Proposals.FindAsync(new object[] { vendor.ProposalId }, ct); + if (proposal != null) { - var proposal = await _db.Proposals.FindAsync(new object[] { vendor.ProposalId }, ct); - if (proposal != null) - { - proposal.VendorTotalCost = vendor.TotalVendorCost; - await _db.SaveChangesAsync(ct); - } + proposal.VendorTotalCost = await _db.VendorProposals + .Where(v => v.ProposalId == vendor.ProposalId) + .SumAsync(v => v.TotalVendorCost, ct); + await _db.SaveChangesAsync(ct); } return NoContent(); diff --git a/api/src/ProposalSystem.Api/Program.cs b/api/src/ProposalSystem.Api/Program.cs index 088f998..2024dda 100644 --- a/api/src/ProposalSystem.Api/Program.cs +++ b/api/src/ProposalSystem.Api/Program.cs @@ -63,11 +63,14 @@ if (!string.IsNullOrEmpty(cognitoAuthority)) .AddJwtBearer(options => { options.Authority = cognitoAuthority; + var webClientId = builder.Configuration["COGNITO_WEB_CLIENT_ID"] ?? ""; + var mobileClientId = builder.Configuration["COGNITO_MOBILE_CLIENT_ID"] ?? ""; options.TokenValidationParameters = new TokenValidationParameters { ValidateIssuerSigningKey = true, ValidateIssuer = true, - ValidateAudience = false, + ValidateAudience = true, + ValidAudiences = new[] { webClientId, mobileClientId }.Where(s => !string.IsNullOrEmpty(s)).ToList(), ValidateLifetime = true, RoleClaimType = "cognito:groups", }; diff --git a/api/src/ProposalSystem.Infrastructure/Services/ProposalNumberGenerator.cs b/api/src/ProposalSystem.Infrastructure/Services/ProposalNumberGenerator.cs index 2b14af2..ab7b1b7 100644 --- a/api/src/ProposalSystem.Infrastructure/Services/ProposalNumberGenerator.cs +++ b/api/src/ProposalSystem.Infrastructure/Services/ProposalNumberGenerator.cs @@ -18,8 +18,13 @@ public class ProposalNumberGenerator : IProposalNumberGenerator var year = DateTime.UtcNow.Year; var prefix = $"SHI-{year}-"; + // Advisory lock prevents concurrent number generation within the same transaction + await _db.Database.ExecuteSqlRawAsync( + "SELECT pg_advisory_xact_lock(hashtext('proposal_number_gen'))", ct); + var lastNumber = await _db.Proposals .Where(p => p.ProposalNumber.StartsWith(prefix)) + .Where(p => !p.ProposalNumber.Contains("-R")) .OrderByDescending(p => p.ProposalNumber) .Select(p => p.ProposalNumber) .FirstOrDefaultAsync(ct); diff --git a/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs b/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs index e807934..e41273c 100644 --- a/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs +++ b/api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs @@ -30,6 +30,8 @@ public class ProposalService : IProposalService public async Task CreateAsync(CreateProposalRequest request, CancellationToken ct = default) { + await using var transaction = await _db.Database.BeginTransactionAsync(ct); + var proposalNumber = await _numberGenerator.GenerateAsync(ct); var now = DateTime.UtcNow; @@ -53,6 +55,7 @@ public class ProposalService : IProposalService _db.Proposals.Add(proposal); await _db.SaveChangesAsync(ct); + await transaction.CommitAsync(ct); await _audit.LogAsync(AuditAction.Submit, proposal.Id, null, ct); @@ -69,7 +72,10 @@ public class ProposalService : IProposalService .Include(p => p.ApprovedBy) .FirstOrDefaultAsync(p => p.Id == id, ct); - return proposal == null ? null : MapToResponse(proposal); + if (proposal == null) return null; + if (_currentUser.Role == UserRole.Dispatcher && proposal.SubmittedById != _currentUser.UserId) + return null; + return MapToResponse(proposal); } public async Task> GetAllAsync(ProposalFilterRequest filter, CancellationToken ct = default) @@ -79,7 +85,7 @@ public class ProposalService : IProposalService .Include(p => p.AssignedAdmin) .AsQueryable(); - if (filter.Mine) + if (_currentUser.Role == UserRole.Dispatcher || filter.Mine) { query = query.Where(p => p.SubmittedById == _currentUser.UserId); } diff --git a/infra/bin/app.ts b/infra/bin/app.ts index 759b825..ce76967 100644 --- a/infra/bin/app.ts +++ b/infra/bin/app.ts @@ -28,6 +28,8 @@ const compute = new ComputeStack(app, 'proposal-system-compute', { libraryBucket: foundation.libraryBucket, jobsQueue: foundation.jobsQueue, userPool: foundation.userPool, + webClientId: foundation.webClientId, + mobileClientId: foundation.mobileClientId, }); new FrontendStack(app, 'proposal-system-frontend', { diff --git a/infra/lib/compute-stack.ts b/infra/lib/compute-stack.ts index 98e624e..f0b46bd 100644 --- a/infra/lib/compute-stack.ts +++ b/infra/lib/compute-stack.ts @@ -2,6 +2,7 @@ import * as cdk from 'aws-cdk-lib'; import * as ec2 from 'aws-cdk-lib/aws-ec2'; import * as lambda from 'aws-cdk-lib/aws-lambda'; import * as apigatewayv2 from 'aws-cdk-lib/aws-apigatewayv2'; +import * as apigatewayv2Authorizers from 'aws-cdk-lib/aws-apigatewayv2-authorizers'; import * as apigatewayv2Integrations from 'aws-cdk-lib/aws-apigatewayv2-integrations'; import * as iam from 'aws-cdk-lib/aws-iam'; import * as s3 from 'aws-cdk-lib/aws-s3'; @@ -24,6 +25,8 @@ export interface ComputeStackProps extends cdk.StackProps { libraryBucket: s3.IBucket; jobsQueue: sqs.IQueue; userPool: cognito.IUserPool; + webClientId: string; + mobileClientId: string; } export class ComputeStack extends cdk.Stack { @@ -209,6 +212,11 @@ export class ComputeStack extends cdk.Stack { LIBRARY_BUCKET: props.libraryBucket.bucketName, JOBS_QUEUE_URL: props.jobsQueue.queueUrl, INTERNAL_API_KEY_SECRET_ARN: internalApiKeySecret.secretArn, + Auth__Authority: `https://cognito-idp.${this.region}.amazonaws.com/${props.userPool.userPoolId}`, + Auth__ClientId: props.webClientId, + Auth__CognitoDomain: `proposal-system-seahaven.auth.${this.region}.amazoncognito.com`, + COGNITO_WEB_CLIENT_ID: props.webClientId, + COGNITO_MOBILE_CLIENT_ID: props.mobileClientId, }, tracing: lambda.Tracing.ACTIVE, logRetention: logs.RetentionDays.TWO_MONTHS, @@ -226,6 +234,11 @@ export class ComputeStack extends cdk.Stack { resources: [props.userPool.userPoolArn], })); + // Function URL for internal Lambda-to-API calls (bypasses API Gateway JWT authorizer) + const apiFunctionUrl = apiFunction.addFunctionUrl({ + authType: lambda.FunctionUrlAuthType.NONE, + }); + // API Gateway HTTP API const httpApi = new apigatewayv2.HttpApi(this, 'HttpApi', { apiName: 'proposal-system-gateway', @@ -251,10 +264,29 @@ export class ComputeStack extends cdk.Stack { apiFunction ); + const jwtAuthorizer = new apigatewayv2Authorizers.HttpJwtAuthorizer( + 'CognitoAuthorizer', + `https://cognito-idp.${this.region}.amazonaws.com/${props.userPool.userPoolId}`, + { jwtAudience: [props.webClientId, props.mobileClientId] }, + ); + + httpApi.addRoutes({ + path: '/api/health', + methods: [apigatewayv2.HttpMethod.GET], + integration: apiIntegration, + }); + + httpApi.addRoutes({ + path: '/api/auth/{proxy+}', + methods: [apigatewayv2.HttpMethod.POST], + integration: apiIntegration, + }); + httpApi.addRoutes({ path: '/{proxy+}', methods: [apigatewayv2.HttpMethod.ANY], integration: apiIntegration, + authorizer: jwtAuthorizer, }); // Python Lambda: Suggestions Engine @@ -272,7 +304,7 @@ export class ComputeStack extends cdk.Stack { environment: { KNOWLEDGE_BASE_ID: knowledgeBase.attrKnowledgeBaseId, MODEL_ID: 'us.anthropic.claude-sonnet-4-5-20250929-v1:0', - API_BASE_URL: httpApi.apiEndpoint, + API_BASE_URL: apiFunctionUrl.url, INTERNAL_API_KEY_SECRET_ARN: internalApiKeySecret.secretArn, }, logRetention: logs.RetentionDays.TWO_MONTHS, @@ -303,7 +335,7 @@ export class ComputeStack extends cdk.Stack { environment: { UPLOADS_BUCKET: props.uploadsBucket.bucketName, MODEL_ID: 'us.anthropic.claude-sonnet-4-5-20250929-v1:0', - API_BASE_URL: httpApi.apiEndpoint, + API_BASE_URL: apiFunctionUrl.url, INTERNAL_API_KEY_SECRET_ARN: internalApiKeySecret.secretArn, }, logRetention: logs.RetentionDays.TWO_MONTHS, @@ -330,7 +362,7 @@ export class ComputeStack extends cdk.Stack { securityGroups: [props.lambdaSecurityGroup], environment: { GENERATED_BUCKET: props.generatedBucket.bucketName, - API_BASE_URL: httpApi.apiEndpoint, + API_BASE_URL: apiFunctionUrl.url, INTERNAL_API_KEY_SECRET_ARN: internalApiKeySecret.secretArn, }, logRetention: logs.RetentionDays.TWO_MONTHS, @@ -355,7 +387,7 @@ export class ComputeStack extends cdk.Stack { LIBRARY_BUCKET: props.libraryBucket.bucketName, KNOWLEDGE_BASE_ID: knowledgeBase.attrKnowledgeBaseId, DATA_SOURCE_ID: dataSource.attrDataSourceId, - API_BASE_URL: httpApi.apiEndpoint, + API_BASE_URL: apiFunctionUrl.url, INTERNAL_API_KEY_SECRET_ARN: internalApiKeySecret.secretArn, }, logRetention: logs.RetentionDays.TWO_MONTHS, @@ -371,6 +403,7 @@ export class ComputeStack extends cdk.Stack { // SQS Event Sources with message filtering suggestionsFunction.addEventSource(new lambdaEventSources.SqsEventSource(props.jobsQueue, { batchSize: 1, + reportBatchItemFailures: true, filters: [ lambda.FilterCriteria.filter({ body: { jobType: lambda.FilterRule.isEqual('suggestions') }, @@ -380,6 +413,7 @@ export class ComputeStack extends cdk.Stack { pdfExtractFunction.addEventSource(new lambdaEventSources.SqsEventSource(props.jobsQueue, { batchSize: 1, + reportBatchItemFailures: true, filters: [ lambda.FilterCriteria.filter({ body: { jobType: lambda.FilterRule.isEqual('pdf-extract') }, @@ -389,6 +423,7 @@ export class ComputeStack extends cdk.Stack { pdfGenerateFunction.addEventSource(new lambdaEventSources.SqsEventSource(props.jobsQueue, { batchSize: 1, + reportBatchItemFailures: true, filters: [ lambda.FilterCriteria.filter({ body: { jobType: lambda.FilterRule.isEqual('pdf-generate') }, @@ -398,6 +433,7 @@ export class ComputeStack extends cdk.Stack { libraryIngestFunction.addEventSource(new lambdaEventSources.SqsEventSource(props.jobsQueue, { batchSize: 1, + reportBatchItemFailures: true, filters: [ lambda.FilterCriteria.filter({ body: { jobType: lambda.FilterRule.isEqual('library-ingest') }, diff --git a/infra/lib/foundation-stack.ts b/infra/lib/foundation-stack.ts index 0f2ad3e..f04e40a 100644 --- a/infra/lib/foundation-stack.ts +++ b/infra/lib/foundation-stack.ts @@ -17,6 +17,8 @@ export class FoundationStack extends cdk.Stack { public readonly libraryBucket: s3.IBucket; public readonly jobsQueue: sqs.IQueue; public readonly userPool: cognito.IUserPool; + public readonly webClientId: string; + public readonly mobileClientId: string; constructor(scope: Construct, id: string, props?: cdk.StackProps) { super(scope, id, props); @@ -156,7 +158,7 @@ export class FoundationStack extends cdk.Stack { this.jobsQueue = new sqs.Queue(this, 'JobsQueue', { queueName: 'proposal-system-jobs', - visibilityTimeout: cdk.Duration.seconds(180), + visibilityTimeout: cdk.Duration.seconds(720), deadLetterQueue: { queue: dlq, maxReceiveCount: 3, @@ -234,6 +236,8 @@ export class FoundationStack extends cdk.Stack { }, }); + this.webClientId = webClient.userPoolClientId; + // Mobile App Client (PKCE) const mobileClient = userPool.addClient('MobileClient', { userPoolClientName: 'proposal-system-mobile', @@ -253,6 +257,8 @@ export class FoundationStack extends cdk.Stack { }, }); + this.mobileClientId = mobileClient.userPoolClientId; + // CloudWatch Log Groups const logGroupNames = [ 'proposal-system-api', diff --git a/lambdas/library-ingest/app.py b/lambdas/library-ingest/app.py index eca3195..0533df8 100644 --- a/lambdas/library-ingest/app.py +++ b/lambdas/library-ingest/app.py @@ -41,12 +41,17 @@ def _get_api_key() -> str: def handler(event, context): + batch_item_failures = [] for record in event.get("Records", []): - body = json.loads(record["body"]) - payload = body.get("payload", body) - proposal_id = payload["proposalId"] - process_ingestion(proposal_id) - return {"statusCode": 200} + try: + body = json.loads(record["body"]) + payload = body.get("payload", body) + proposal_id = payload["proposalId"] + process_ingestion(proposal_id) + except Exception as e: + logger.error("Failed to process record %s: %s", record.get("messageId"), e) + batch_item_failures.append({"itemIdentifier": record["messageId"]}) + return {"batchItemFailures": batch_item_failures} def process_ingestion(proposal_id: str): diff --git a/lambdas/pdf-extract/app.py b/lambdas/pdf-extract/app.py index f1bc293..26a0ec5 100644 --- a/lambdas/pdf-extract/app.py +++ b/lambdas/pdf-extract/app.py @@ -41,19 +41,24 @@ def _get_api_key() -> str: def handler(event, context): + batch_item_failures = [] for record in event.get("Records", []): - body = json.loads(record["body"]) - payload = body.get("payload", body) - proposal_id = payload["proposalId"] - s3_key = payload.get("s3Key", "") - vendor_proposal_id = payload.get("vendorProposalId", "") + try: + body = json.loads(record["body"]) + payload = body.get("payload", body) + proposal_id = payload["proposalId"] + s3_key = payload.get("s3Key", "") + vendor_proposal_id = payload.get("vendorProposalId", "") - if not s3_key: - logger.warning("No s3Key in payload for proposal %s", proposal_id) - continue + if not s3_key: + logger.warning("No s3Key in payload for proposal %s", proposal_id) + continue - process_pdf(proposal_id, s3_key, vendor_proposal_id) - return {"statusCode": 200} + process_pdf(proposal_id, s3_key, vendor_proposal_id) + except Exception as e: + logger.error("Failed to process record %s: %s", record.get("messageId"), e) + batch_item_failures.append({"itemIdentifier": record["messageId"]}) + return {"batchItemFailures": batch_item_failures} def process_pdf(proposal_id: str, s3_key: str, vendor_proposal_id: str): diff --git a/lambdas/pdf-generate/app.py b/lambdas/pdf-generate/app.py index 37e75db..62ce665 100644 --- a/lambdas/pdf-generate/app.py +++ b/lambdas/pdf-generate/app.py @@ -65,12 +65,17 @@ def _get_api_key() -> str: def handler(event, context): + batch_item_failures = [] for record in event.get("Records", []): - body = json.loads(record["body"]) - payload = body.get("payload", body) - proposal_id = payload["proposalId"] - generate_pdf(proposal_id) - return {"statusCode": 200} + try: + body = json.loads(record["body"]) + payload = body.get("payload", body) + proposal_id = payload["proposalId"] + generate_pdf(proposal_id) + except Exception as e: + logger.error("Failed to process record %s: %s", record.get("messageId"), e) + batch_item_failures.append({"itemIdentifier": record["messageId"]}) + return {"batchItemFailures": batch_item_failures} def generate_pdf(proposal_id: str): diff --git a/lambdas/suggestions/app.py b/lambdas/suggestions/app.py index 9e68f83..9d7ede2 100644 --- a/lambdas/suggestions/app.py +++ b/lambdas/suggestions/app.py @@ -38,13 +38,18 @@ def _get_api_key() -> str: def handler(event, context): + batch_item_failures = [] for record in event.get("Records", []): - body = json.loads(record["body"]) - payload = body.get("payload", body) - proposal_id = payload["proposalId"] - trigger = payload.get("trigger", "generate") - process_suggestion(proposal_id, trigger) - return {"statusCode": 200} + try: + body = json.loads(record["body"]) + payload = body.get("payload", body) + proposal_id = payload["proposalId"] + trigger = payload.get("trigger", "generate") + process_suggestion(proposal_id, trigger) + except Exception as e: + logger.error("Failed to process record %s: %s", record.get("messageId"), e) + batch_item_failures.append({"itemIdentifier": record["messageId"]}) + return {"batchItemFailures": batch_item_failures} def process_suggestion(proposal_id: str, trigger: str): From 184bc1da7e5dc36a2a04b9d40d0ba0916ff7c5ff Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Wed, 20 May 2026 19:07:49 -0400 Subject: [PATCH 2/2] Fix Phase 2 audit findings: reliability, UX, and operational monitoring BLOCK-10: Add CloudWatch alarms (DLQ, Lambda errors, RDS, API 5xx) with SNS email BLOCK-11: Remove sync-over-async deadlock in CurrentUserService BLOCK-12: Add AppDelegate OAuth URL callback handler for mobile BLOCK-13: Wire mobile 401 interceptor to dispatch Redux logout BLOCK-14: Fix JWT base64 padding crash and SysAdmin role detection BLOCK-15: Reset pagination to page 1 on filter change BLOCK-16: Add unsaved-changes guard (beforeunload + useBlocker) to AdminWorkspace FIX-08: Add BulkUpdateLineItems FluentValidation validator FIX-13: Display auth errors on LoginPage FIX-25: Add token refresh with retry queue to mobile API client FIX-44: Add httpx retry logic to all Lambda handlers FIX-42/43: Align docker-compose PG version (15) and DB name (proposals) with RDS --- .../Services/CurrentUserService.cs | 7 +- .../BulkUpdateLineItemsValidator.cs | 38 ++++++++++ docker-compose.yml | 4 +- infra/bin/app.ts | 1 + infra/lib/compute-stack.ts | 43 +++++++++++ infra/lib/foundation-stack.ts | 64 ++++++++++++++++ lambdas/library-ingest/app.py | 28 ++++++- lambdas/pdf-extract/app.py | 28 ++++++- lambdas/pdf-generate/app.py | 32 ++++++-- lambdas/suggestions/app.py | 39 +++++++--- mobile/ios/ProposalSystem/AppDelegate.swift | 9 +++ mobile/src/App.tsx | 9 ++- mobile/src/lib/api/auth.ts | 15 ++-- mobile/src/lib/api/client.ts | 74 +++++++++++++++++-- web/src/hooks/usePaginatedList.ts | 8 ++ .../pages/admin/workspace/AdminWorkspace.tsx | 35 ++++++++- web/src/pages/auth/LoginPage.tsx | 9 ++- 17 files changed, 396 insertions(+), 47 deletions(-) create mode 100644 api/src/ProposalSystem.Application/Validators/BulkUpdateLineItemsValidator.cs diff --git a/api/src/ProposalSystem.Api/Services/CurrentUserService.cs b/api/src/ProposalSystem.Api/Services/CurrentUserService.cs index 393b11e..ed98bae 100644 --- a/api/src/ProposalSystem.Api/Services/CurrentUserService.cs +++ b/api/src/ProposalSystem.Api/Services/CurrentUserService.cs @@ -94,11 +94,8 @@ public class CurrentUserService : ICurrentUserService private User GetOrThrow() { - if (_cachedUser != null) return _cachedUser; - - ResolveAsync().GetAwaiter().GetResult(); - return _cachedUser - ?? throw new UnauthorizedAccessException("Could not resolve current user"); + ?? throw new InvalidOperationException( + "CurrentUserService.ResolveAsync() was not called. Ensure the authentication middleware runs before accessing user properties."); } } diff --git a/api/src/ProposalSystem.Application/Validators/BulkUpdateLineItemsValidator.cs b/api/src/ProposalSystem.Application/Validators/BulkUpdateLineItemsValidator.cs new file mode 100644 index 0000000..d1d2018 --- /dev/null +++ b/api/src/ProposalSystem.Application/Validators/BulkUpdateLineItemsValidator.cs @@ -0,0 +1,38 @@ +using FluentValidation; +using ProposalSystem.Application.DTOs; + +namespace ProposalSystem.Application.Validators; + +public class BulkUpdateLineItemsValidator : AbstractValidator +{ + public BulkUpdateLineItemsValidator() + { + RuleFor(x => x.LineItems).NotNull(); + + RuleForEach(x => x.LineItems).ChildRules(entry => + { + entry.RuleFor(x => x.Description) + .NotEmpty().WithMessage("Description is required") + .MaximumLength(1000); + + entry.RuleFor(x => x.Quantity) + .GreaterThan(0).WithMessage("Quantity must be positive"); + + entry.RuleFor(x => x.Unit) + .NotEmpty().WithMessage("Unit is required") + .MaximumLength(50); + + entry.RuleFor(x => x.TotalPrice) + .GreaterThanOrEqualTo(0).WithMessage("Total price cannot be negative"); + + entry.RuleFor(x => x.UnitPrice) + .GreaterThanOrEqualTo(0) + .When(x => x.UnitPrice.HasValue) + .WithMessage("Unit price cannot be negative"); + + entry.RuleFor(x => x.PricingMode).IsInEnum(); + entry.RuleFor(x => x.Source).IsInEnum(); + entry.RuleFor(x => x.SortOrder).GreaterThanOrEqualTo(0); + }); + } +} diff --git a/docker-compose.yml b/docker-compose.yml index 5031858..d337060 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,10 +1,10 @@ services: postgres: - image: postgres:16-alpine + image: postgres:15-alpine ports: - "5432:5432" environment: - POSTGRES_DB: proposalsystem + POSTGRES_DB: proposals POSTGRES_USER: postgres POSTGRES_PASSWORD: localdev volumes: diff --git a/infra/bin/app.ts b/infra/bin/app.ts index 759b825..e28961c 100644 --- a/infra/bin/app.ts +++ b/infra/bin/app.ts @@ -28,6 +28,7 @@ const compute = new ComputeStack(app, 'proposal-system-compute', { libraryBucket: foundation.libraryBucket, jobsQueue: foundation.jobsQueue, userPool: foundation.userPool, + alarmTopic: foundation.alarmTopic, }); new FrontendStack(app, 'proposal-system-frontend', { diff --git a/infra/lib/compute-stack.ts b/infra/lib/compute-stack.ts index 98e624e..082d170 100644 --- a/infra/lib/compute-stack.ts +++ b/infra/lib/compute-stack.ts @@ -6,12 +6,15 @@ import * as apigatewayv2Integrations from 'aws-cdk-lib/aws-apigatewayv2-integrat import * as iam from 'aws-cdk-lib/aws-iam'; import * as s3 from 'aws-cdk-lib/aws-s3'; import * as sqs from 'aws-cdk-lib/aws-sqs'; +import * as sns from 'aws-cdk-lib/aws-sns'; import * as cognito from 'aws-cdk-lib/aws-cognito'; import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager'; import * as lambdaEventSources from 'aws-cdk-lib/aws-lambda-event-sources'; import * as bedrock from 'aws-cdk-lib/aws-bedrock'; import * as opensearchserverless from 'aws-cdk-lib/aws-opensearchserverless'; import * as logs from 'aws-cdk-lib/aws-logs'; +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as cloudwatchActions from 'aws-cdk-lib/aws-cloudwatch-actions'; import * as cr from 'aws-cdk-lib/custom-resources'; import { Construct } from 'constructs'; @@ -24,6 +27,7 @@ export interface ComputeStackProps extends cdk.StackProps { libraryBucket: s3.IBucket; jobsQueue: sqs.IQueue; userPool: cognito.IUserPool; + alarmTopic: sns.ITopic; } export class ComputeStack extends cdk.Stack { @@ -405,6 +409,45 @@ export class ComputeStack extends cdk.Stack { ], })); + // CloudWatch Alarms + const alarmAction = new cloudwatchActions.SnsAction(props.alarmTopic); + + const lambdaFunctions = [ + { fn: apiFunction, name: 'api' }, + { fn: suggestionsFunction, name: 'suggestions' }, + { fn: pdfExtractFunction, name: 'pdf-extract' }, + { fn: pdfGenerateFunction, name: 'pdf-generate' }, + { fn: libraryIngestFunction, name: 'library-ingest' }, + ]; + + for (const { fn, name } of lambdaFunctions) { + const alarm = new cloudwatch.Alarm(this, `LambdaErrors-${name}`, { + alarmName: `proposal-system-${name}-errors`, + alarmDescription: `Lambda errors for ${name}`, + metric: fn.metricErrors({ period: cdk.Duration.minutes(5) }), + threshold: 1, + evaluationPeriods: 1, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + alarm.addAlarmAction(alarmAction); + } + + const api5xxAlarm = new cloudwatch.Alarm(this, 'Api5xxAlarm', { + alarmName: 'proposal-system-api-5xx', + alarmDescription: 'API Gateway 5xx errors', + metric: new cloudwatch.Metric({ + namespace: 'AWS/ApiGateway', + metricName: '5xx', + dimensionsMap: { ApiId: httpApi.httpApiId }, + statistic: 'Sum', + period: cdk.Duration.minutes(5), + }), + threshold: 5, + evaluationPeriods: 1, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + api5xxAlarm.addAlarmAction(alarmAction); + // Outputs new cdk.CfnOutput(this, 'ApiEndpoint', { value: httpApi.apiEndpoint }); new cdk.CfnOutput(this, 'ApiFunctionArn', { value: apiFunction.functionArn }); diff --git a/infra/lib/foundation-stack.ts b/infra/lib/foundation-stack.ts index 0f2ad3e..25565b0 100644 --- a/infra/lib/foundation-stack.ts +++ b/infra/lib/foundation-stack.ts @@ -6,6 +6,10 @@ import * as sqs from 'aws-cdk-lib/aws-sqs'; import * as cognito from 'aws-cdk-lib/aws-cognito'; import * as logs from 'aws-cdk-lib/aws-logs'; import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager'; +import * as sns from 'aws-cdk-lib/aws-sns'; +import * as snsSubscriptions from 'aws-cdk-lib/aws-sns-subscriptions'; +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as cloudwatchActions from 'aws-cdk-lib/aws-cloudwatch-actions'; import { Construct } from 'constructs'; export class FoundationStack extends cdk.Stack { @@ -17,6 +21,7 @@ export class FoundationStack extends cdk.Stack { public readonly libraryBucket: s3.IBucket; public readonly jobsQueue: sqs.IQueue; public readonly userPool: cognito.IUserPool; + public readonly alarmTopic: sns.ITopic; constructor(scope: Construct, id: string, props?: cdk.StackProps) { super(scope, id, props); @@ -253,6 +258,64 @@ export class FoundationStack extends cdk.Stack { }, }); + // SNS Alarm Topic + const alarmTopic = new sns.Topic(this, 'AlarmTopic', { + topicName: 'proposal-system-alarms', + displayName: 'Proposal System Alarms', + }); + alarmTopic.addSubscription( + new snsSubscriptions.EmailSubscription('adam@seahavenind.com'), + ); + this.alarmTopic = alarmTopic; + + const alarmAction = new cloudwatchActions.SnsAction(alarmTopic); + + // DLQ Alarm: any message landing in DLQ indicates a processing failure + const dlqAlarm = new cloudwatch.Alarm(this, 'DlqDepthAlarm', { + alarmName: 'proposal-system-dlq-depth', + alarmDescription: 'Messages in DLQ — SQS processing failures', + metric: dlq.metricApproximateNumberOfMessagesVisible({ + period: cdk.Duration.minutes(1), + }), + threshold: 0, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD, + evaluationPeriods: 1, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }); + dlqAlarm.addAlarmAction(alarmAction); + + // RDS Alarms + const rdsAlarms = [ + new cloudwatch.Alarm(this, 'RdsCpuAlarm', { + alarmName: 'proposal-system-rds-cpu', + alarmDescription: 'RDS CPU utilization above 80%', + metric: dbInstance.metricCPUUtilization({ period: cdk.Duration.minutes(5) }), + threshold: 80, + evaluationPeriods: 3, + treatMissingData: cloudwatch.TreatMissingData.BREACHING, + }), + new cloudwatch.Alarm(this, 'RdsConnectionsAlarm', { + alarmName: 'proposal-system-rds-connections', + alarmDescription: 'RDS database connections above 80', + metric: dbInstance.metricDatabaseConnections({ period: cdk.Duration.minutes(5) }), + threshold: 80, + evaluationPeriods: 2, + treatMissingData: cloudwatch.TreatMissingData.NOT_BREACHING, + }), + new cloudwatch.Alarm(this, 'RdsFreeStorageAlarm', { + alarmName: 'proposal-system-rds-free-storage', + alarmDescription: 'RDS free storage below 2 GB', + metric: dbInstance.metricFreeStorageSpace({ period: cdk.Duration.minutes(5) }), + threshold: 2_000_000_000, + comparisonOperator: cloudwatch.ComparisonOperator.LESS_THAN_THRESHOLD, + evaluationPeriods: 1, + treatMissingData: cloudwatch.TreatMissingData.BREACHING, + }), + ]; + for (const alarm of rdsAlarms) { + alarm.addAlarmAction(alarmAction); + } + // CloudWatch Log Groups const logGroupNames = [ 'proposal-system-api', @@ -280,5 +343,6 @@ export class FoundationStack extends cdk.Stack { new cdk.CfnOutput(this, 'DbSecretArn', { value: this.dbSecret.secretArn }); new cdk.CfnOutput(this, 'WebClientId', { value: webClient.userPoolClientId }); new cdk.CfnOutput(this, 'MobileClientId', { value: mobileClient.userPoolClientId }); + new cdk.CfnOutput(this, 'AlarmTopicArn', { value: alarmTopic.topicArn }); } } diff --git a/lambdas/library-ingest/app.py b/lambdas/library-ingest/app.py index eca3195..ccb5040 100644 --- a/lambdas/library-ingest/app.py +++ b/lambdas/library-ingest/app.py @@ -8,6 +8,7 @@ then triggers a KB sync. import json import logging import os +import time from datetime import datetime import boto3 @@ -67,10 +68,10 @@ def process_ingestion(proposal_id: str): def fetch_proposal(proposal_id: str) -> dict | None: try: - resp = httpx.get( + resp = _retry_request( + "GET", f"{API_BASE_URL}/api/proposals/{proposal_id}", headers=_api_headers(), - timeout=10, ) if resp.status_code == 200: return resp.json() @@ -81,10 +82,10 @@ def fetch_proposal(proposal_id: str) -> dict | None: def fetch_line_items(proposal_id: str) -> list[dict]: try: - resp = httpx.get( + resp = _retry_request( + "GET", f"{API_BASE_URL}/api/proposals/{proposal_id}/line-items", headers=_api_headers(), - timeout=10, ) if resp.status_code == 200: return resp.json() @@ -198,3 +199,22 @@ def _api_headers() -> dict: if api_key: headers["X-Internal-Api-Key"] = api_key return headers + + +def _retry_request( + method: str, url: str, *, max_retries: int = 3, **kwargs +) -> httpx.Response: + kwargs.setdefault("timeout", 10) + for attempt in range(max_retries): + try: + resp = httpx.request(method, url, **kwargs) + if resp.status_code < 500: + return resp + except (httpx.ConnectError, httpx.ReadTimeout, httpx.WriteTimeout) as exc: + if attempt == max_retries - 1: + raise + logger.warning( + "Retryable error (attempt %d/%d): %s", attempt + 1, max_retries, exc + ) + time.sleep(min(2**attempt, 4)) + return resp # type: ignore[possibly-undefined] diff --git a/lambdas/pdf-extract/app.py b/lambdas/pdf-extract/app.py index f1bc293..29f5e19 100644 --- a/lambdas/pdf-extract/app.py +++ b/lambdas/pdf-extract/app.py @@ -9,6 +9,7 @@ import json import logging import os import tempfile +import time import boto3 import httpx @@ -299,7 +300,8 @@ def save_extraction(vendor_proposal_id: str, extracted: dict): } try: - resp = httpx.put( + resp = _retry_request( + "PUT", f"{API_BASE_URL}/api/vendor-proposals/{vendor_proposal_id}", json={ "vendorName": extracted["vendorName"], @@ -308,7 +310,6 @@ def save_extraction(vendor_proposal_id: str, extracted: dict): "processingStatus": "Complete", }, headers=_api_headers(), - timeout=10, ) if resp.status_code not in (200, 204): logger.error( @@ -322,11 +323,11 @@ def update_processing_status(vendor_proposal_id: str, status: str): if not vendor_proposal_id: return try: - httpx.put( + _retry_request( + "PUT", f"{API_BASE_URL}/api/vendor-proposals/{vendor_proposal_id}/status", json={"processingStatus": status}, headers=_api_headers(), - timeout=10, ) except Exception as e: logger.error("Error updating status: %s", e) @@ -338,3 +339,22 @@ def _api_headers() -> dict: if api_key: headers["X-Internal-Api-Key"] = api_key return headers + + +def _retry_request( + method: str, url: str, *, max_retries: int = 3, **kwargs +) -> httpx.Response: + kwargs.setdefault("timeout", 10) + for attempt in range(max_retries): + try: + resp = httpx.request(method, url, **kwargs) + if resp.status_code < 500: + return resp + except (httpx.ConnectError, httpx.ReadTimeout, httpx.WriteTimeout) as exc: + if attempt == max_retries - 1: + raise + logger.warning( + "Retryable error (attempt %d/%d): %s", attempt + 1, max_retries, exc + ) + time.sleep(min(2**attempt, 4)) + return resp # type: ignore[possibly-undefined] diff --git a/lambdas/pdf-generate/app.py b/lambdas/pdf-generate/app.py index 37e75db..4b80250 100644 --- a/lambdas/pdf-generate/app.py +++ b/lambdas/pdf-generate/app.py @@ -7,6 +7,7 @@ Triggered via SQS when an admin requests PDF generation. import json import logging import os +import time from datetime import datetime from io import BytesIO @@ -96,10 +97,10 @@ def generate_pdf(proposal_id: str): def fetch_proposal(proposal_id: str) -> dict | None: try: - resp = httpx.get( + resp = _retry_request( + "GET", f"{API_BASE_URL}/api/proposals/{proposal_id}", headers=_api_headers(), - timeout=10, ) if resp.status_code == 200: return resp.json() @@ -110,10 +111,10 @@ def fetch_proposal(proposal_id: str) -> dict | None: def fetch_line_items(proposal_id: str) -> list[dict]: try: - resp = httpx.get( + resp = _retry_request( + "GET", f"{API_BASE_URL}/api/proposals/{proposal_id}/line-items", headers=_api_headers(), - timeout=10, ) if resp.status_code == 200: return resp.json() @@ -524,11 +525,11 @@ def upload_pdf(s3_key: str, pdf_bytes: bytes): def register_pdf(proposal_id: str, s3_key: str): try: - resp = httpx.post( + resp = _retry_request( + "POST", f"{API_BASE_URL}/api/generated-pdfs", json={"proposalId": proposal_id, "s3Key": s3_key}, headers=_api_headers(), - timeout=10, ) if resp.status_code not in (200, 201): logger.error("Failed to register PDF: %s %s", resp.status_code, resp.text) @@ -542,3 +543,22 @@ def _api_headers() -> dict: if api_key: headers["X-Internal-Api-Key"] = api_key return headers + + +def _retry_request( + method: str, url: str, *, max_retries: int = 3, **kwargs +) -> httpx.Response: + kwargs.setdefault("timeout", 10) + for attempt in range(max_retries): + try: + resp = httpx.request(method, url, **kwargs) + if resp.status_code < 500: + return resp + except (httpx.ConnectError, httpx.ReadTimeout, httpx.WriteTimeout) as exc: + if attempt == max_retries - 1: + raise + logger.warning( + "Retryable error (attempt %d/%d): %s", attempt + 1, max_retries, exc + ) + time.sleep(min(2**attempt, 4)) + return resp # type: ignore[possibly-undefined] diff --git a/lambdas/suggestions/app.py b/lambdas/suggestions/app.py index 9e68f83..1c4ecb3 100644 --- a/lambdas/suggestions/app.py +++ b/lambdas/suggestions/app.py @@ -7,6 +7,7 @@ to generate line item suggestions for new proposals. import json import logging import os +import time import boto3 import httpx @@ -72,10 +73,10 @@ def process_suggestion(proposal_id: str, trigger: str): def fetch_line_items(proposal_id: str) -> list[dict]: try: - resp = httpx.get( + resp = _retry_request( + "GET", f"{API_BASE_URL}/api/proposals/{proposal_id}/line-items", headers=_api_headers(), - timeout=10, ) if resp.status_code == 200: return resp.json() @@ -86,10 +87,10 @@ def fetch_line_items(proposal_id: str) -> list[dict]: def fetch_proposal(proposal_id: str) -> dict | None: try: - resp = httpx.get( + resp = _retry_request( + "GET", f"{API_BASE_URL}/api/proposals/{proposal_id}", headers=_api_headers(), - timeout=10, ) if resp.status_code == 200: return resp.json() @@ -263,7 +264,8 @@ def post_line_items(proposal_id: str, items: list[dict], existing_items: list[di ) try: - resp = httpx.put( + resp = _retry_request( + "PUT", f"{API_BASE_URL}/api/proposals/{proposal_id}/line-items", json={"lineItems": line_items_payload}, headers=_api_headers(), @@ -288,14 +290,14 @@ def store_similar_references(proposal_id: str, similar_proposals: list[dict]): continue try: - httpx.post( + _retry_request( + "POST", f"{API_BASE_URL}/api/proposals/{proposal_id}/similar-references", json={ "referencedLibraryItemId": library_item_id, "similarityScore": sp["score"], }, headers=_api_headers(), - timeout=10, ) except Exception as e: logger.error("Error storing similar reference: %s", e) @@ -303,11 +305,11 @@ def store_similar_references(proposal_id: str, similar_proposals: list[dict]): def update_status_to_in_review(proposal_id: str): try: - httpx.put( + _retry_request( + "PUT", f"{API_BASE_URL}/api/proposals/{proposal_id}", json={"status": "InReview"}, headers=_api_headers(), - timeout=10, ) except Exception as e: logger.error("Error updating status: %s", e) @@ -319,3 +321,22 @@ def _api_headers() -> dict: if api_key: headers["X-Internal-Api-Key"] = api_key return headers + + +def _retry_request( + method: str, url: str, *, max_retries: int = 3, **kwargs +) -> httpx.Response: + kwargs.setdefault("timeout", 10) + for attempt in range(max_retries): + try: + resp = httpx.request(method, url, **kwargs) + if resp.status_code < 500: + return resp + except (httpx.ConnectError, httpx.ReadTimeout, httpx.WriteTimeout) as exc: + if attempt == max_retries - 1: + raise + logger.warning( + "Retryable error (attempt %d/%d): %s", attempt + 1, max_retries, exc + ) + time.sleep(min(2**attempt, 4)) + return resp # type: ignore[possibly-undefined] diff --git a/mobile/ios/ProposalSystem/AppDelegate.swift b/mobile/ios/ProposalSystem/AppDelegate.swift index 8553e43..f165713 100644 --- a/mobile/ios/ProposalSystem/AppDelegate.swift +++ b/mobile/ios/ProposalSystem/AppDelegate.swift @@ -2,6 +2,7 @@ import UIKit import React import React_RCTAppDelegate import ReactAppDependencyProvider +import RCTLinking @main class AppDelegate: UIResponder, UIApplicationDelegate { @@ -31,6 +32,14 @@ class AppDelegate: UIResponder, UIApplicationDelegate { return true } + + func application( + _ app: UIApplication, + open url: URL, + options: [UIApplication.OpenURLOptionsKey: Any] = [:] + ) -> Bool { + return RCTLinkingManager.application(app, open: url, options: options) + } } class ReactNativeDelegate: RCTDefaultReactNativeFactoryDelegate { diff --git a/mobile/src/App.tsx b/mobile/src/App.tsx index 8f8a506..3645956 100644 --- a/mobile/src/App.tsx +++ b/mobile/src/App.tsx @@ -11,9 +11,10 @@ import { theme } from './theme'; import { queryClient } from './lib/queryClient'; import { RootNavigator } from './navigation/RootNavigator'; import { tokenStorage, userStorage } from './lib/storage'; -import { setUser, setLoading } from './store/slices/authSlice'; +import { setUser, setLoading, logout as logoutAction } from './store/slices/authSlice'; import { processOfflineQueue } from './hooks/useOfflineDraft'; import { proposalsApi, CreateProposalRequest } from './lib/api/proposals'; +import { onSessionExpired } from './lib/api/client'; class ErrorBoundary extends Component<{ children: ReactNode }, { error: Error | null }> { state = { error: null as Error | null }; @@ -56,6 +57,12 @@ function AuthBootstrap({ children }: { children: React.ReactNode }) { })(); }, [dispatch]); + useEffect(() => { + return onSessionExpired(() => { + dispatch(logoutAction()); + }); + }, [dispatch]); + useEffect(() => { const unsubscribe = NetInfo.addEventListener((state) => { if (state.isConnected) { diff --git a/mobile/src/lib/api/auth.ts b/mobile/src/lib/api/auth.ts index ac94406..8ceb230 100644 --- a/mobile/src/lib/api/auth.ts +++ b/mobile/src/lib/api/auth.ts @@ -1,6 +1,6 @@ import { authorize, refresh, revoke } from 'react-native-app-auth'; import Config from '../../config'; -import apiClient from './client'; +import apiClient, { registerTokenRefresh } from './client'; import { authenticateWithCredentials } from './cognito-auth'; import { tokenStorage, @@ -12,11 +12,14 @@ import { function parseUserFromIdToken(idToken: string): StoredUser { const base64Url = idToken.split('.')[1]; const base64 = base64Url.replace(/-/g, '+').replace(/_/g, '/'); - const payload = JSON.parse(atob(base64)); + const padded = base64.padEnd(base64.length + ((4 - (base64.length % 4)) % 4), '='); + const payload = JSON.parse(atob(padded)); const groups: string[] = payload['cognito:groups'] || []; - const role = groups.includes('admins') || groups.includes('sysadmins') - ? 'Admin' - : 'Dispatcher'; + const role = groups.includes('sysadmins') + ? 'SysAdmin' + : groups.includes('admins') + ? 'Admin' + : 'Dispatcher'; return { id: payload.sub, email: payload.email, @@ -117,3 +120,5 @@ export const authApi = { return res.data; }, }; + +registerTokenRefresh(() => authApi.refreshTokens()); diff --git a/mobile/src/lib/api/client.ts b/mobile/src/lib/api/client.ts index 22aa326..82c5868 100644 --- a/mobile/src/lib/api/client.ts +++ b/mobile/src/lib/api/client.ts @@ -2,6 +2,41 @@ import axios from 'axios'; import Config from '../../config'; import { tokenStorage } from '../storage'; +type RefreshFn = () => Promise; +let _refreshTokens: RefreshFn | null = null; + +export function registerTokenRefresh(fn: RefreshFn) { + _refreshTokens = fn; +} + +type SessionExpiredListener = () => void; + +const sessionExpiredListeners: SessionExpiredListener[] = []; + +export function onSessionExpired(listener: SessionExpiredListener): () => void { + sessionExpiredListeners.push(listener); + return () => { + const idx = sessionExpiredListeners.indexOf(listener); + if (idx >= 0) sessionExpiredListeners.splice(idx, 1); + }; +} + +function emitSessionExpired() { + sessionExpiredListeners.forEach((fn) => fn()); +} + +let isRefreshing = false; +let refreshSubscribers: Array<(token: string) => void> = []; + +function subscribeToRefresh(cb: (token: string) => void) { + refreshSubscribers.push(cb); +} + +function onRefreshComplete(token: string) { + refreshSubscribers.forEach((cb) => cb(token)); + refreshSubscribers = []; +} + const apiClient = axios.create({ baseURL: Config.API_URL, headers: { @@ -23,15 +58,40 @@ apiClient.interceptors.request.use( apiClient.interceptors.response.use( (response) => response, - (error) => { + async (error) => { if (error.response) { - const { status, data } = error.response; + const { status, data, config: originalRequest } = error.response; - if (status === 401) { - tokenStorage.clear(); - return Promise.reject( - new Error('Session expired. Please log in again.'), - ); + if (status === 401 && !originalRequest._retry) { + originalRequest._retry = true; + + if (isRefreshing) { + return new Promise((resolve) => { + subscribeToRefresh((token) => { + originalRequest.headers.Authorization = `Bearer ${token}`; + resolve(apiClient(originalRequest)); + }); + }); + } + + isRefreshing = true; + try { + if (!_refreshTokens) throw new Error('No refresh handler'); + await _refreshTokens(); + const tokens = await tokenStorage.get(); + const newToken = tokens?.accessToken ?? ''; + onRefreshComplete(newToken); + originalRequest.headers.Authorization = `Bearer ${newToken}`; + return apiClient(originalRequest); + } catch { + await tokenStorage.clear(); + emitSessionExpired(); + return Promise.reject( + new Error('Session expired. Please log in again.'), + ); + } finally { + isRefreshing = false; + } } if (status === 403) { diff --git a/web/src/hooks/usePaginatedList.ts b/web/src/hooks/usePaginatedList.ts index a3fc19c..f560c0c 100644 --- a/web/src/hooks/usePaginatedList.ts +++ b/web/src/hooks/usePaginatedList.ts @@ -32,6 +32,14 @@ export function usePaginatedList(fetchFn: FetchFn, extraParams: Record { + if (prevExtraKey.current !== extraKey) { + prevExtraKey.current = extraKey; + setPage(1); + } + }, [extraKey]); + const reload = useCallback(() => { setLoading(true); setErr(''); diff --git a/web/src/pages/admin/workspace/AdminWorkspace.tsx b/web/src/pages/admin/workspace/AdminWorkspace.tsx index fc6e4eb..5156b5e 100644 --- a/web/src/pages/admin/workspace/AdminWorkspace.tsx +++ b/web/src/pages/admin/workspace/AdminWorkspace.tsx @@ -1,5 +1,5 @@ -import { useState, useEffect } from 'react'; -import { useParams, useNavigate } from 'react-router-dom'; +import { useState, useEffect, useCallback } from 'react'; +import { useParams, useNavigate, useBlocker } from 'react-router-dom'; import { useQuery, useMutation } from '@tanstack/react-query'; import { Box, @@ -81,6 +81,23 @@ export default function AdminWorkspace() { } }, [proposal]); + useEffect(() => { + if (!dirty) return; + const handler = (e: BeforeUnloadEvent) => { + e.preventDefault(); + }; + window.addEventListener('beforeunload', handler); + return () => window.removeEventListener('beforeunload', handler); + }, [dirty]); + + const blocker = useBlocker( + useCallback( + ({ currentLocation, nextLocation }: { currentLocation: { pathname: string }; nextLocation: { pathname: string } }) => + dirty && currentLocation.pathname !== nextLocation.pathname, + [dirty], + ), + ); + const saveMutation = useMutation({ mutationFn: async () => { await adminApi.updateProposal(id!, { refinedScope }); @@ -387,6 +404,20 @@ export default function AdminWorkspace() { + {/* Unsaved Changes Navigation Guard */} + blocker.reset?.()}> + Unsaved Changes + + You have unsaved changes. Are you sure you want to leave? + + + + + + + {/* Approve Confirmation Dialog */} setApproveDialogOpen(false)}> Approve Proposal diff --git a/web/src/pages/auth/LoginPage.tsx b/web/src/pages/auth/LoginPage.tsx index 6866e2e..ad9ff68 100644 --- a/web/src/pages/auth/LoginPage.tsx +++ b/web/src/pages/auth/LoginPage.tsx @@ -1,7 +1,7 @@ import { useEffect, useState } from 'react'; import { useNavigate } from 'react-router-dom'; import { useDispatch } from 'react-redux'; -import { Box, Button, Card, CardContent, Typography, Divider, Stack } from '@mui/material'; +import { Alert, Box, Button, Card, CardContent, Typography, Divider, Stack } from '@mui/material'; import GoogleIcon from '@mui/icons-material/Google'; import AdminPanelSettingsIcon from '@mui/icons-material/AdminPanelSettings'; import EngineeringIcon from '@mui/icons-material/Engineering'; @@ -28,7 +28,7 @@ function buildLoginUrl(): string { export default function LoginPage() { const navigate = useNavigate(); const dispatch = useDispatch(); - const { isAuthenticated } = useAuth(); + const { isAuthenticated, error } = useAuth(); const [loading, setLoading] = useState(false); const isDevMode = !CLIENT_ID; @@ -103,6 +103,11 @@ export default function LoginPage() { + {error && ( + + {error} + + )} {isDevMode ? ( <>