From ac3dcc2f31fbd643fbd1465d19f28fe2dbae53e5 Mon Sep 17 00:00:00 2001 From: Spencer Twaddle <7374698+stwaddle@users.noreply.github.com> Date: Wed, 6 May 2026 22:17:18 -0500 Subject: [PATCH] Security & resource hardening: eliminate CPU/disk attack surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses production CPU spike incident. Key changes: - Guard OTel exporter behind OTEL_EXPORTER_OTLP_ENDPOINT env var; filter tracing to /api paths only — unconditional export was primary suspect - Remove /healthz endpoint entirely (unauthenticated, hit DB on every call) - Replace KnownUserMiddleware with POST /api/users/me called once on login from TokenSync — eliminates unconditional DB write on every request - Add DB indexes: (BudgetId, IsDeleted) on Incomes/Outgos, OwnerUserId on Budgets, SharedWithUserId and (IsPending, SharedWithEmail) on BudgetShares - Move UseRateLimiter() before UseStaticFiles() so all requests are throttled - Replace full-array reorder with move-by-position (id + newIndex) — bounded input, fewer DB writes, better API design - Lock ForwardedHeaders to 172.20.0.0/16 subnet; fixes KnownNetworks deprecation warning (0 warnings in build now) - Add AsNoTracking() to all read-only queries in Summary/Incomes/OutgosController - FrequencyCalculator returns 0 for unknown enum values instead of throwing - Thread.Sleep → await Task.Delay in OIDC startup loop - AllowedHosts locked to budget.stwaddle.com Co-Authored-By: Claude Sonnet 4.6 --- .plans/hardening_050626.md | 138 +++++++++ .../Controllers/IncomesController.cs | 21 +- .../Controllers/OutgosController.cs | 24 +- .../Controllers/SummaryController.cs | 6 +- src/Budget.Api/Controllers/UsersController.cs | 49 ++++ src/Budget.Api/Program.cs | 54 ++-- .../Services/KnownUserMiddleware.cs | 48 ---- src/Budget.Api/appsettings.json | 2 +- src/Budget.Client/src/api/client.ts | 2 +- src/Budget.Client/src/api/incomes.ts | 4 +- src/Budget.Client/src/api/outgos.ts | 4 +- src/Budget.Client/src/auth/TokenSync.tsx | 7 +- src/Budget.Client/src/pages/IncomePage.tsx | 5 +- src/Budget.Client/src/pages/OutgoPage.tsx | 5 +- src/Budget.Core/DTOs/IncomeDtos.cs | 2 +- src/Budget.Core/DTOs/OutgoDtos.cs | 2 +- .../Services/FrequencyCalculator.cs | 2 +- .../Data/AppDbContext.cs | 5 + ...0507024323_AddHardeningIndexes.Designer.cs | 271 ++++++++++++++++++ .../20260507024323_AddHardeningIndexes.cs | 81 ++++++ .../Migrations/AppDbContextModelSnapshot.cs | 10 +- 21 files changed, 623 insertions(+), 119 deletions(-) create mode 100644 .plans/hardening_050626.md create mode 100644 src/Budget.Api/Controllers/UsersController.cs delete mode 100644 src/Budget.Api/Services/KnownUserMiddleware.cs create mode 100644 src/Budget.Infrastructure/Data/Migrations/20260507024323_AddHardeningIndexes.Designer.cs create mode 100644 src/Budget.Infrastructure/Data/Migrations/20260507024323_AddHardeningIndexes.cs diff --git a/.plans/hardening_050626.md b/.plans/hardening_050626.md new file mode 100644 index 0000000..2402fe9 --- /dev/null +++ b/.plans/hardening_050626.md @@ -0,0 +1,138 @@ +# Security & Resource Hardening Plan +**Date:** 2026-05-06 +**Trigger:** Production host CPU pegged >100% for several hours; Disk I/O high; network normal. +**Root cause hypothesis:** C-1 (unconditional OTel exporter) + H-1 (unconditional DB write per request). + +--- + +## Critical + +### ~~C-1 — Guard `UseOtlpExporter()` Behind Env Var Check~~ ✅ DONE +**Resolved 2026-05-06:** Entire OTel block (logging + tracing + exporter) is now gated on `OTEL_EXPORTER_OTLP_ENDPOINT` being set. AspNetCore instrumentation also filtered to `/api` paths only so static file requests never generate spans. The duplicate `builder.Logging.AddOpenTelemetry()` is inside the same guard, eliminating the double-pipeline issue. Build verified clean. + +--- + +### ~~C-2 — Require Auth on `/healthz` (or Remove DB Check)~~ ✅ DONE +**Resolved 2026-05-06:** Endpoint removed entirely. Not wired to any orchestrator health check; budget app is lower priority than recipes/journal. `AddHealthChecks()`, `AddDbContextCheck()`, `MapHealthChecks()`, and both health-check `using` statements removed from `Program.cs`. Build verified clean. + +--- + +### ~~C-3 — Move `UseRateLimiter()` Before `UseStaticFiles()`~~ ✅ DONE +**File:** `src/Budget.Api/Program.cs` +**Risk:** `UseStaticFiles` short-circuits the pipeline before `UseRateLimiter`, so all SPA asset requests (JS bundles, CSS, `index.html` fallback) bypass rate limiting. Combined with C-1, every unthrottled static file request still generates a telemetry span. + +**Fix:** Reorder middleware so `app.UseRateLimiter()` appears before `app.UseDefaultFiles()` and `app.UseStaticFiles()`. + +--- + +## High + +### ~~H-1 — Throttle `KnownUserMiddleware` DB Writes~~ ✅ DONE +**Resolved 2026-05-06:** `KnownUserMiddleware` removed entirely. Replaced with `POST /api/users/me` endpoint in new `UsersController` — same upsert + pending share resolution logic, but now called once on login from `TokenSync.tsx` rather than on every request. `api.post` body param made optional to support the no-body call. Backend and frontend builds verified clean. + +--- + +### H-2 — Reduce `BudgetAuthorizationService` to a Single Query +**File:** `src/Budget.Infrastructure/Services/BudgetAuthorizationService.cs` +**Risk:** `GetAccessAsync` always issues two sequential queries (budget row, then share row). Combined with controller entity loads, a single write endpoint makes 4+ DB round-trips; `GET /summary` makes 5. + +**Fix:** Combine into one projected query: +```csharp +var result = await db.Budgets + .AsNoTracking() + .Where(b => b.Id == budgetId) + .Select(b => new { + b.OwnerUserId, + Share = b.Shares.FirstOrDefault(s => s.SharedWithUserId == userId && !s.IsPending) + }) + .FirstOrDefaultAsync(); +``` + +--- + +### ~~H-3 — Add Missing Database Indexes~~ ✅ DONE +**Resolved 2026-05-06:** Added 5 indexes to `AppDbContext.OnModelCreating`. Migration `20260507024323_AddHardeningIndexes` generated and verified. Replaces single-column `BudgetId` indexes on Incomes/Outgos with composite `(BudgetId, IsDeleted)` versions; adds `BudgetShares.(IsPending, SharedWithEmail)`, `BudgetShares.SharedWithUserId`, and `Budgets.OwnerUserId`. Will be applied automatically at next startup via `MigrateAsync()`. + +--- + +### ~~H-4 — Cap `Reorder` Endpoint Input Size~~ ✅ DONE +**Resolved 2026-05-06:** Replaced full-array reorder with a move-by-position design. `ReorderIncomesRequest`/`ReorderOutgosRequest` removed and replaced with `MoveIncomeRequest(Guid Id, int NewIndex)` / `MoveOutgoRequest(Guid Id, int NewIndex)`. Server loads items sorted by SortOrder, finds the item, shifts only the affected range, sets the new position. Frontend sends `{ id, newIndex }` from dnd-kit's existing `oldIdx`/`newIdx` — `arrayMove` still used for optimistic local state. Input is inherently bounded to one GUID and one integer. + +--- + +### ~~H-5 — Lock Down `ForwardedHeaders` to Known Proxy IP~~ ✅ DONE +**Resolved 2026-05-06:** Replaced clear-all with explicit subnet trust. `KnownIPNetworks` (the .NET 10 replacement for deprecated `KnownNetworks`) is cleared and populated with `172.20.0.0/16` — the Docker network subnet for the nginx-proxy stack. Using the subnet rather than a specific container IP avoids breakage if nginx-proxy gets a different IP on restart. Deprecation warning eliminated; build clean with 0 warnings. + +--- + +## Medium + +### M-1 — OIDC JWKS Fetched Over Plain HTTP +**File:** `src/Budget.Api/Program.cs` +**Risk:** `MetadataAddress` points to `http://auth:8080/...`. JWKS (token signing keys) arrive over plain HTTP. On a compromised Docker network, an attacker could substitute keys and forge valid JWTs. Low severity on a single-host deployment; higher risk in any multi-host topology. + +**Fix:** Enable internal TLS on the auth service, or pin the signing key in configuration instead of relying on discovery. + +--- + +### ~~M-2 — Unknown `Frequency` Enum Crashes Entire Budget~~ ✅ DONE +**File:** `src/Budget.Core/Services/FrequencyCalculator.cs` +**Risk:** An unrecognised enum integer (data corruption or future migration) throws `ArgumentOutOfRangeException`, making all income/outgo/summary endpoints permanently broken for that budget and generating error spans that amplify OTel export volume. + +**Fix:** Validate enum values at the controller layer with `[EnumDataType(typeof(Frequency))]` on write DTOs. Return a graceful default or 422 in the calculator rather than throwing. + +--- + +### ~~M-3 — Missing `AsNoTracking()` on Read Queries~~ ✅ DONE +**Files:** `src/Budget.Api/Controllers/SummaryController.cs`, `IncomesController.cs`, `OutgosController.cs` +**Risk:** EF change tracking runs on every loaded entity even for pure-read endpoints, wasting CPU/memory. With OTel EF instrumentation active, every tracked entity event adds overhead. + +**Fix:** Add `.AsNoTracking()` to all `List` and `Get` queries that do not write. + +--- + +### ~~M-4 — `Thread.Sleep` in Async Startup Loop~~ ✅ DONE +**File:** `src/Budget.Api/Program.cs` +**Risk:** Synchronous `Thread.Sleep` inside the async OIDC discovery retry loop blocks a thread pool thread for up to 2 minutes during container startup. + +**Fix:** +```csharp +await Task.Delay(TimeSpan.FromSeconds(5)); +``` + +--- + +### ~~M-5 — `AllowedHosts: "*"` Disables Host Header Validation~~ ✅ DONE +**File:** `src/Budget.Api/appsettings.json` +**Risk:** Allows HTTP host header injection if the app is ever behind a misconfigured proxy. + +**Fix:** Set to the actual public hostname: +```json +"AllowedHosts": "budget.stwaddle.com" +``` + +--- + +## Low / Informational + +| ID | File | Issue | Fix | +|----|------|-------|-----| +| L-1 | `appsettings.json` | Docker-internal `MetadataAddress` in base config — if the Docker service name `auth` is not resolvable, OIDC discovery silently fails | Move `MetadataAddress` to a Docker Compose env override; base config default should be empty | +| L-2 | `Program.cs` | No timeout on startup `MigrateAsync()` — hangs indefinitely if Postgres is unresponsive | Wrap with a `CancellationTokenSource` with a deadline | +| L-3 | `ErrorHandlingMiddleware.cs` | Full exception stack traces exported via OTel on every unhandled error — exception storms spike collector disk I/O | Consider `LogWarning` for expected exceptions; reserve `LogError` for truly unexpected ones | + +--- + +## Implementation Order + +1. ~~**C-1** — Guard `UseOtlpExporter()` (likely root cause; deploy ASAP)~~ ✅ Done (2026-05-06) +2. ~~**H-1** — Throttle `KnownUserMiddleware` writes (amplifies Disk I/O on every request)~~ ✅ Done (2026-05-06) +3. ~~**C-2** — Auth-gate or simplify `/healthz`~~ ✅ Removed entirely (2026-05-06) +4. ~~**H-3** — Add missing indexes (single migration, low risk)~~ ✅ Done (2026-05-06) +5. ~~**C-3** — Move rate limiter before static files~~ ✅ Done (2026-05-06) +6. ~~**H-4** — Cap `Reorder` input~~ ✅ Done — redesigned as move-by-position (2026-05-06) +7. ~~**H-5** — Lock down `ForwardedHeaders` to proxy IP~~ ✅ Done (2026-05-06) +8. ~~**M-3** — Add `AsNoTracking()` across read controllers~~ ✅ Done (2026-05-06) +9. ~~**M-2** — Harden `FrequencyCalculator` enum handling~~ ✅ Done (2026-05-06) +10. ~~**M-4** — `Thread.Sleep` → `await Task.Delay` in startup loop~~ ✅ Done (2026-05-06) +11. ~~**M-5** — `AllowedHosts: "*"` → `budget.stwaddle.com`~~ ✅ Done (2026-05-06) diff --git a/src/Budget.Api/Controllers/IncomesController.cs b/src/Budget.Api/Controllers/IncomesController.cs index 698aa17..2a2879c 100644 --- a/src/Budget.Api/Controllers/IncomesController.cs +++ b/src/Budget.Api/Controllers/IncomesController.cs @@ -36,6 +36,7 @@ public class IncomesController(AppDbContext db, BudgetAuthorizationService authz if (TryGetUserId(out var userId) is { } err) return err; if (!await authz.CanReadAsync(budgetId, userId)) return Forbid(); var incomes = await db.Incomes + .AsNoTracking() .Where(i => i.BudgetId == budgetId) .OrderBy(i => i.SortOrder) .ToListAsync(); @@ -94,17 +95,21 @@ public class IncomesController(AppDbContext db, BudgetAuthorizationService authz [HttpPut("order")] [EnableRateLimiting("writes")] - public async Task Reorder(Guid budgetId, [FromBody] ReorderIncomesRequest req) + public async Task Reorder(Guid budgetId, [FromBody] MoveIncomeRequest req) { if (TryGetUserId(out var userId) is { } err) return err; if (!await authz.CanWriteAsync(budgetId, userId)) return Forbid(); - var incomes = await db.Incomes.Where(i => i.BudgetId == budgetId).ToListAsync(); - var lookup = incomes.ToDictionary(i => i.Id); - for (int idx = 0; idx < req.OrderedIds.Count; idx++) - { - if (lookup.TryGetValue(req.OrderedIds[idx], out var income)) - income.SortOrder = idx; - } + var incomes = await db.Incomes.Where(i => i.BudgetId == budgetId).OrderBy(i => i.SortOrder).ToListAsync(); + var item = incomes.FirstOrDefault(i => i.Id == req.Id); + if (item is null) return NotFound(); + var oldIdx = incomes.IndexOf(item); + var newIdx = Math.Clamp(req.NewIndex, 0, incomes.Count - 1); + if (oldIdx == newIdx) return NoContent(); + if (newIdx < oldIdx) + for (int i = newIdx; i < oldIdx; i++) incomes[i].SortOrder++; + else + for (int i = oldIdx + 1; i <= newIdx; i++) incomes[i].SortOrder--; + item.SortOrder = newIdx; await db.SaveChangesAsync(); return NoContent(); } diff --git a/src/Budget.Api/Controllers/OutgosController.cs b/src/Budget.Api/Controllers/OutgosController.cs index 2813af4..53512a3 100644 --- a/src/Budget.Api/Controllers/OutgosController.cs +++ b/src/Budget.Api/Controllers/OutgosController.cs @@ -36,7 +36,7 @@ public class OutgosController(AppDbContext db, BudgetAuthorizationService authz) private async Task GetMonthlyIncomeAsync(Guid budgetId) { - var incomes = await db.Incomes.Where(i => i.BudgetId == budgetId).ToListAsync(); + var incomes = await db.Incomes.AsNoTracking().Where(i => i.BudgetId == budgetId).ToListAsync(); return incomes.Sum(i => FrequencyCalculator.ToMonthly(i.Amount, i.Frequency)); } @@ -45,7 +45,7 @@ public class OutgosController(AppDbContext db, BudgetAuthorizationService authz) { if (TryGetUserId(out var userId) is { } err) return err; if (!await authz.CanReadAsync(budgetId, userId)) return Forbid(); - var outgos = await db.Outgos.Where(o => o.BudgetId == budgetId).OrderBy(o => o.SortOrder).ToListAsync(); + var outgos = await db.Outgos.AsNoTracking().Where(o => o.BudgetId == budgetId).OrderBy(o => o.SortOrder).ToListAsync(); var monthlyIncome = await GetMonthlyIncomeAsync(budgetId); return Ok(outgos.Select(o => ToDto(o, monthlyIncome))); } @@ -112,17 +112,21 @@ public class OutgosController(AppDbContext db, BudgetAuthorizationService authz) [HttpPut("order")] [EnableRateLimiting("writes")] - public async Task Reorder(Guid budgetId, [FromBody] ReorderOutgosRequest req) + public async Task Reorder(Guid budgetId, [FromBody] MoveOutgoRequest req) { if (TryGetUserId(out var userId) is { } err) return err; if (!await authz.CanWriteAsync(budgetId, userId)) return Forbid(); - var outgos = await db.Outgos.Where(o => o.BudgetId == budgetId).ToListAsync(); - var lookup = outgos.ToDictionary(o => o.Id); - for (int idx = 0; idx < req.OrderedIds.Count; idx++) - { - if (lookup.TryGetValue(req.OrderedIds[idx], out var outgo)) - outgo.SortOrder = idx; - } + var outgos = await db.Outgos.AsNoTracking().Where(o => o.BudgetId == budgetId).OrderBy(o => o.SortOrder).ToListAsync(); + var item = outgos.FirstOrDefault(o => o.Id == req.Id); + if (item is null) return NotFound(); + var oldIdx = outgos.IndexOf(item); + var newIdx = Math.Clamp(req.NewIndex, 0, outgos.Count - 1); + if (oldIdx == newIdx) return NoContent(); + if (newIdx < oldIdx) + for (int i = newIdx; i < oldIdx; i++) outgos[i].SortOrder++; + else + for (int i = oldIdx + 1; i <= newIdx; i++) outgos[i].SortOrder--; + item.SortOrder = newIdx; await db.SaveChangesAsync(); return NoContent(); } diff --git a/src/Budget.Api/Controllers/SummaryController.cs b/src/Budget.Api/Controllers/SummaryController.cs index d749213..71ce58f 100644 --- a/src/Budget.Api/Controllers/SummaryController.cs +++ b/src/Budget.Api/Controllers/SummaryController.cs @@ -29,11 +29,11 @@ public class SummaryController(AppDbContext db, BudgetAuthorizationService authz if (TryGetUserId(out var userId) is { } err) return err; if (!await authz.CanReadAsync(budgetId, userId)) return Forbid(); - var budget = await db.Budgets.FindAsync(budgetId); + var budget = await db.Budgets.AsNoTracking().FirstOrDefaultAsync(b => b.Id == budgetId); if (budget is null) return NotFound(); - var incomes = await db.Incomes.Where(i => i.BudgetId == budgetId).ToListAsync(); - var outgos = await db.Outgos.Where(o => o.BudgetId == budgetId).ToListAsync(); + var incomes = await db.Incomes.AsNoTracking().Where(i => i.BudgetId == budgetId).ToListAsync(); + var outgos = await db.Outgos.AsNoTracking().Where(o => o.BudgetId == budgetId).ToListAsync(); var monthlyIncome = incomes.Sum(i => FrequencyCalculator.ToMonthly(i.Amount, i.Frequency)); var annualIncome = monthlyIncome * 12m; diff --git a/src/Budget.Api/Controllers/UsersController.cs b/src/Budget.Api/Controllers/UsersController.cs new file mode 100644 index 0000000..ad123c0 --- /dev/null +++ b/src/Budget.Api/Controllers/UsersController.cs @@ -0,0 +1,49 @@ +using Budget.Core.Models; +using Budget.Infrastructure.Data; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc; +using Microsoft.EntityFrameworkCore; + +namespace Budget.Api.Controllers; + +[ApiController] +[Route("api/users")] +[Authorize(Roles = "admin,user")] +public class UsersController(AppDbContext db) : ControllerBase +{ + [HttpPost("me")] + public async Task RegisterMe() + { + var sub = User.FindFirst("sub")?.Value; + var email = User.FindFirst("email")?.Value; + var name = User.FindFirst("name")?.Value; + + if (sub is null || email is null || name is null) + return Unauthorized(); + + var known = await db.KnownUsers.FindAsync(sub); + if (known is null) + { + db.KnownUsers.Add(new KnownUser { Id = sub, Email = email, Name = name, LastSeenAt = DateTimeOffset.UtcNow }); + } + else + { + known.Email = email; + known.Name = name; + known.LastSeenAt = DateTimeOffset.UtcNow; + } + + var pending = await db.BudgetShares + .Where(s => s.IsPending && s.SharedWithEmail == email) + .ToListAsync(); + + foreach (var share in pending) + { + share.SharedWithUserId = sub; + share.IsPending = false; + } + + await db.SaveChangesAsync(); + return NoContent(); + } +} diff --git a/src/Budget.Api/Program.cs b/src/Budget.Api/Program.cs index f7cc3f0..ffb69ca 100644 --- a/src/Budget.Api/Program.cs +++ b/src/Budget.Api/Program.cs @@ -3,11 +3,9 @@ using Budget.Api.Services; using Budget.Infrastructure.Data; using Budget.Infrastructure.Services; using Microsoft.AspNetCore.Authentication.JwtBearer; -using Microsoft.AspNetCore.Diagnostics.HealthChecks; using Microsoft.AspNetCore.HttpOverrides; using Microsoft.AspNetCore.RateLimiting; using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.Diagnostics.HealthChecks; using Microsoft.Extensions.Options; using Microsoft.IdentityModel.Tokens; using OpenTelemetry; @@ -16,20 +14,25 @@ using OpenTelemetry.Trace; var builder = WebApplication.CreateBuilder(args); -builder.Logging.AddOpenTelemetry(logging => +var otlpEndpoint = builder.Configuration["OTEL_EXPORTER_OTLP_ENDPOINT"]; +if (!string.IsNullOrEmpty(otlpEndpoint)) { - logging.IncludeFormattedMessage = true; - logging.IncludeScopes = true; -}); + builder.Logging.AddOpenTelemetry(logging => + { + logging.IncludeFormattedMessage = true; + logging.IncludeScopes = true; + }); -builder.Services.AddOpenTelemetry() - .ConfigureResource(resource => resource - .AddService(serviceName: "budget", serviceVersion: "1.0.0")) - .WithTracing(tracing => tracing - .AddAspNetCoreInstrumentation() - .AddHttpClientInstrumentation() - .AddEntityFrameworkCoreInstrumentation()) - .UseOtlpExporter(); + builder.Services.AddOpenTelemetry() + .ConfigureResource(resource => resource + .AddService(serviceName: "budget", serviceVersion: "1.0.0")) + .WithTracing(tracing => tracing + .AddAspNetCoreInstrumentation(opts => + opts.Filter = ctx => ctx.Request.Path.StartsWithSegments("/api")) + .AddHttpClientInstrumentation() + .AddEntityFrameworkCoreInstrumentation()) + .UseOtlpExporter(); +} var connStr = builder.Configuration.GetConnectionString("DefaultConnection") ?? $"Host={builder.Configuration["POSTGRES_HOST"] ?? "localhost"};" + @@ -43,8 +46,9 @@ builder.Services.AddDbContext(opt => opt.UseNpgsql(connStr)); builder.Services.Configure(options => { options.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto; - options.KnownNetworks.Clear(); options.KnownProxies.Clear(); + options.KnownIPNetworks.Clear(); + options.KnownIPNetworks.Add(System.Net.IPNetwork.Parse("172.20.0.0/16")); }); var oidc = builder.Configuration.GetSection("Oidc"); @@ -109,9 +113,6 @@ builder.Services.AddControllers() .AddJsonOptions(opts => opts.JsonSerializerOptions.Converters.Add( new System.Text.Json.Serialization.JsonStringEnumConverter())); -builder.Services.AddHealthChecks() - .AddDbContextCheck(); - var app = builder.Build(); // Block startup until OIDC discovery succeeds (handles auth/app container race). @@ -136,7 +137,7 @@ while (true) break; } startupLogger.LogWarning(ex, "OIDC discovery failed; retrying in 5 s"); - Thread.Sleep(TimeSpan.FromSeconds(5)); + await Task.Delay(TimeSpan.FromSeconds(5)); } } @@ -150,26 +151,15 @@ using (var scope = app.Services.CreateScope()) app.UseForwardedHeaders(); app.UseMiddleware(); +app.UseRateLimiter(); + app.UseDefaultFiles(); app.UseStaticFiles(); app.UseAuthentication(); -app.UseMiddleware(); app.UseAuthorization(); -app.UseRateLimiter(); - app.MapControllers(); -app.MapHealthChecks("/healthz", new HealthCheckOptions -{ - ResultStatusCodes = - { - [HealthStatus.Healthy] = StatusCodes.Status200OK, - [HealthStatus.Degraded] = StatusCodes.Status200OK, - [HealthStatus.Unhealthy] = StatusCodes.Status503ServiceUnavailable, - } -}); - app.MapFallbackToFile("index.html"); app.Run(); diff --git a/src/Budget.Api/Services/KnownUserMiddleware.cs b/src/Budget.Api/Services/KnownUserMiddleware.cs deleted file mode 100644 index c41ac76..0000000 --- a/src/Budget.Api/Services/KnownUserMiddleware.cs +++ /dev/null @@ -1,48 +0,0 @@ -using Budget.Core.Models; -using Budget.Infrastructure.Data; -using Microsoft.EntityFrameworkCore; - -namespace Budget.Api.Services; - -public class KnownUserMiddleware(RequestDelegate next) -{ - public async Task InvokeAsync(HttpContext context, AppDbContext db) - { - if (context.User.Identity?.IsAuthenticated == true) - { - var sub = context.User.FindFirst("sub")?.Value; - var email = context.User.FindFirst("email")?.Value; - var name = context.User.FindFirst("name")?.Value; - - if (sub != null && email != null && name != null) - { - var known = await db.KnownUsers.FindAsync(sub); - if (known is null) - { - db.KnownUsers.Add(new KnownUser { Id = sub, Email = email, Name = name, LastSeenAt = DateTimeOffset.UtcNow }); - } - else - { - known.Email = email; - known.Name = name; - known.LastSeenAt = DateTimeOffset.UtcNow; - } - - // Resolve pending shares for this user's email - var pending = await db.BudgetShares - .Where(s => s.IsPending && s.SharedWithEmail == email) - .ToListAsync(); - - foreach (var share in pending) - { - share.SharedWithUserId = sub; - share.IsPending = false; - } - - await db.SaveChangesAsync(); - } - } - - await next(context); - } -} diff --git a/src/Budget.Api/appsettings.json b/src/Budget.Api/appsettings.json index 2932264..5ba93c8 100644 --- a/src/Budget.Api/appsettings.json +++ b/src/Budget.Api/appsettings.json @@ -5,7 +5,7 @@ "Microsoft.AspNetCore": "Warning" } }, - "AllowedHosts": "*", + "AllowedHosts": "budget.stwaddle.com", "ConnectionStrings": { "DefaultConnection": "" }, diff --git a/src/Budget.Client/src/api/client.ts b/src/Budget.Client/src/api/client.ts index 450f635..28cad81 100644 --- a/src/Budget.Client/src/api/client.ts +++ b/src/Budget.Client/src/api/client.ts @@ -24,7 +24,7 @@ async function request(method: string, path: string, body?: unknown): Promise export const api = { get: (path: string) => request('GET', path), - post: (path: string, body: unknown) => request('POST', path, body), + post: (path: string, body?: unknown) => request('POST', path, body), put: (path: string, body: unknown) => request('PUT', path, body), delete: (path: string) => request('DELETE', path), }; diff --git a/src/Budget.Client/src/api/incomes.ts b/src/Budget.Client/src/api/incomes.ts index 0442c8d..324a6cf 100644 --- a/src/Budget.Client/src/api/incomes.ts +++ b/src/Budget.Client/src/api/incomes.ts @@ -4,7 +4,7 @@ import type { IncomeDto, Frequency } from '../types/index'; interface CreateIncomeRequest { name: string; frequency: Frequency; amount: number; } interface UpdateIncomeRequest { name: string; frequency: Frequency; amount: number; } -interface ReorderRequest { orderedIds: string[]; } +interface MoveRequest { id: string; newIndex: number; } export function useIncomes(budgetId: string) { return useQuery({ @@ -44,7 +44,7 @@ export function useDeleteIncome(budgetId: string) { export function useReorderIncomes(budgetId: string) { const qc = useQueryClient(); return useMutation({ - mutationFn: (req: ReorderRequest) => api.put(`/api/budgets/${budgetId}/incomes/order`, req), + mutationFn: (req: MoveRequest) => api.put(`/api/budgets/${budgetId}/incomes/order`, req), onSuccess: () => qc.invalidateQueries({ queryKey: ['budgets', budgetId, 'incomes'] }), meta: { errorMessage: 'Failed to save order' }, }); diff --git a/src/Budget.Client/src/api/outgos.ts b/src/Budget.Client/src/api/outgos.ts index 1219276..839e33b 100644 --- a/src/Budget.Client/src/api/outgos.ts +++ b/src/Budget.Client/src/api/outgos.ts @@ -7,7 +7,7 @@ interface CreateOutgoRequest { frequency: Frequency; amount: number; paymentSource: string | null; notes: string | null; } interface UpdateOutgoRequest extends CreateOutgoRequest { id: string; } -interface ReorderRequest { orderedIds: string[]; } +interface MoveRequest { id: string; newIndex: number; } export function useOutgos(budgetId: string) { return useQuery({ @@ -61,7 +61,7 @@ export function useDeleteOutgo(budgetId: string) { export function useReorderOutgos(budgetId: string) { const qc = useQueryClient(); return useMutation({ - mutationFn: (req: ReorderRequest) => api.put(`/api/budgets/${budgetId}/outgos/order`, req), + mutationFn: (req: MoveRequest) => api.put(`/api/budgets/${budgetId}/outgos/order`, req), onSuccess: () => qc.invalidateQueries({ queryKey: ['budgets', budgetId, 'outgos'] }), meta: { errorMessage: 'Failed to save order' }, }); diff --git a/src/Budget.Client/src/auth/TokenSync.tsx b/src/Budget.Client/src/auth/TokenSync.tsx index e924975..e3161f0 100644 --- a/src/Budget.Client/src/auth/TokenSync.tsx +++ b/src/Budget.Client/src/auth/TokenSync.tsx @@ -1,11 +1,16 @@ import { useEffect } from 'react'; import { useAuth } from 'react-oidc-context'; -import { setTokenProvider } from '../api/client'; +import { setTokenProvider, api } from '../api/client'; export function TokenSync() { const auth = useAuth(); + useEffect(() => { setTokenProvider(() => auth.user?.access_token ?? null); + if (auth.user?.access_token) { + api.post('/api/users/me', undefined).catch(() => {}); + } }, [auth.user]); + return null; } diff --git a/src/Budget.Client/src/pages/IncomePage.tsx b/src/Budget.Client/src/pages/IncomePage.tsx index 05c67fc..73cd6dd 100644 --- a/src/Budget.Client/src/pages/IncomePage.tsx +++ b/src/Budget.Client/src/pages/IncomePage.tsx @@ -238,9 +238,8 @@ export function IncomePage() { if (!over || active.id === over.id) return; const oldIdx = displayItems.findIndex(i => i.id === active.id); const newIdx = displayItems.findIndex(i => i.id === over.id); - const reordered = arrayMove(displayItems, oldIdx, newIdx); - setDisplayItems(reordered); - reorderIncomes.mutate({ orderedIds: reordered.map(i => i.id) }); + setDisplayItems(items => arrayMove(items, oldIdx, newIdx)); + reorderIncomes.mutate({ id: active.id as string, newIndex: newIdx }); }; if (isLoading) return <>; diff --git a/src/Budget.Client/src/pages/OutgoPage.tsx b/src/Budget.Client/src/pages/OutgoPage.tsx index 7c38b2e..a813219 100644 --- a/src/Budget.Client/src/pages/OutgoPage.tsx +++ b/src/Budget.Client/src/pages/OutgoPage.tsx @@ -384,9 +384,8 @@ export function OutgoPage() { if (!over || active.id === over.id) return; const oldIdx = displayItems.findIndex(o => o.id === active.id); const newIdx = displayItems.findIndex(o => o.id === over.id); - const reordered = arrayMove(displayItems, oldIdx, newIdx); - setDisplayItems(reordered); - reorderOutgos.mutate({ orderedIds: reordered.map(o => o.id) }); + setDisplayItems(items => arrayMove(items, oldIdx, newIdx)); + reorderOutgos.mutate({ id: active.id as string, newIndex: newIdx }); }; if (isLoading) return <>; diff --git a/src/Budget.Core/DTOs/IncomeDtos.cs b/src/Budget.Core/DTOs/IncomeDtos.cs index 35397bc..e286582 100644 --- a/src/Budget.Core/DTOs/IncomeDtos.cs +++ b/src/Budget.Core/DTOs/IncomeDtos.cs @@ -23,4 +23,4 @@ public record UpdateIncomeRequest( Frequency Frequency, [Range(0, 9_999_999_999_999_999.99)] decimal Amount); -public record ReorderIncomesRequest([Required] List OrderedIds); +public record MoveIncomeRequest([Required] Guid Id, [Range(0, int.MaxValue)] int NewIndex); diff --git a/src/Budget.Core/DTOs/OutgoDtos.cs b/src/Budget.Core/DTOs/OutgoDtos.cs index 4d5f2d5..c29701b 100644 --- a/src/Budget.Core/DTOs/OutgoDtos.cs +++ b/src/Budget.Core/DTOs/OutgoDtos.cs @@ -36,4 +36,4 @@ public record UpdateOutgoRequest( [MaxLength(100)] string? PaymentSource, [MaxLength(1000)] string? Notes); -public record ReorderOutgosRequest([Required] List OrderedIds); +public record MoveOutgoRequest([Required] Guid Id, [Range(0, int.MaxValue)] int NewIndex); diff --git a/src/Budget.Core/Services/FrequencyCalculator.cs b/src/Budget.Core/Services/FrequencyCalculator.cs index cf1a8a3..7fdea75 100644 --- a/src/Budget.Core/Services/FrequencyCalculator.cs +++ b/src/Budget.Core/Services/FrequencyCalculator.cs @@ -15,7 +15,7 @@ public static class FrequencyCalculator Frequency.SemiMonthly => amount * 2m, Frequency.Biweekly => amount * 26m / 12m, Frequency.Weekly => amount * 52m / 12m, - _ => throw new ArgumentOutOfRangeException(nameof(frequency)), + _ => 0m, }; public static decimal ToAnnually(decimal amount, Frequency frequency) => diff --git a/src/Budget.Infrastructure/Data/AppDbContext.cs b/src/Budget.Infrastructure/Data/AppDbContext.cs index 6ee45df..f6be698 100644 --- a/src/Budget.Infrastructure/Data/AppDbContext.cs +++ b/src/Budget.Infrastructure/Data/AppDbContext.cs @@ -26,6 +26,7 @@ public class AppDbContext(DbContextOptions options) : DbContext(op .HasColumnType("xid") .ValueGeneratedOnAddOrUpdate() .IsConcurrencyToken(); + b.HasIndex(x => x.OwnerUserId); }); modelBuilder.Entity(b => @@ -34,6 +35,7 @@ public class AppDbContext(DbContextOptions options) : DbContext(op b.Property(x => x.Name).IsRequired().HasMaxLength(200); b.Property(x => x.Amount).HasPrecision(18, 2); b.HasQueryFilter(x => !x.IsDeleted); + b.HasIndex(x => new { x.BudgetId, x.IsDeleted }); }); modelBuilder.Entity(b => @@ -45,6 +47,7 @@ public class AppDbContext(DbContextOptions options) : DbContext(op b.Property(x => x.Notes).HasMaxLength(1000); b.Property(x => x.Amount).HasPrecision(18, 2); b.HasQueryFilter(x => !x.IsDeleted); + b.HasIndex(x => new { x.BudgetId, x.IsDeleted }); }); modelBuilder.Entity(b => @@ -61,6 +64,8 @@ public class AppDbContext(DbContextOptions options) : DbContext(op b.Property(x => x.SharedWithUserId).HasMaxLength(200); b.Property(x => x.SharedWithEmail).IsRequired().HasMaxLength(200); b.HasIndex(x => new { x.BudgetId, x.SharedWithEmail }).IsUnique(); + b.HasIndex(x => x.SharedWithUserId); + b.HasIndex(x => new { x.IsPending, x.SharedWithEmail }); b.HasQueryFilter(x => !x.IsDeleted); }); } diff --git a/src/Budget.Infrastructure/Data/Migrations/20260507024323_AddHardeningIndexes.Designer.cs b/src/Budget.Infrastructure/Data/Migrations/20260507024323_AddHardeningIndexes.Designer.cs new file mode 100644 index 0000000..d274861 --- /dev/null +++ b/src/Budget.Infrastructure/Data/Migrations/20260507024323_AddHardeningIndexes.Designer.cs @@ -0,0 +1,271 @@ +// +using System; +using Budget.Infrastructure.Data; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; +using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata; + +#nullable disable + +namespace Budget.Infrastructure.Data.Migrations +{ + [DbContext(typeof(AppDbContext))] + [Migration("20260507024323_AddHardeningIndexes")] + partial class AddHardeningIndexes + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasAnnotation("ProductVersion", "10.0.7") + .HasAnnotation("Relational:MaxIdentifierLength", 63); + + NpgsqlModelBuilderExtensions.UseIdentityByDefaultColumns(modelBuilder); + + modelBuilder.Entity("Budget.Core.Models.Budget", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("DeletedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("IsDeleted") + .HasColumnType("boolean"); + + b.Property("Name") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("OwnerUserId") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("UpdatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("xmin") + .IsConcurrencyToken() + .ValueGeneratedOnAddOrUpdate() + .HasColumnType("xid") + .HasColumnName("xmin"); + + b.HasKey("Id"); + + b.HasIndex("OwnerUserId"); + + b.ToTable("Budgets"); + }); + + modelBuilder.Entity("Budget.Core.Models.BudgetShare", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("BudgetId") + .HasColumnType("uuid"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("DeletedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("IsDeleted") + .HasColumnType("boolean"); + + b.Property("IsPending") + .HasColumnType("boolean"); + + b.Property("Permission") + .HasColumnType("integer"); + + b.Property("SharedWithEmail") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("SharedWithUserId") + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.HasKey("Id"); + + b.HasIndex("SharedWithUserId"); + + b.HasIndex("BudgetId", "SharedWithEmail") + .IsUnique(); + + b.HasIndex("IsPending", "SharedWithEmail"); + + b.ToTable("BudgetShares"); + }); + + modelBuilder.Entity("Budget.Core.Models.Income", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("Amount") + .HasPrecision(18, 2) + .HasColumnType("numeric(18,2)"); + + b.Property("BudgetId") + .HasColumnType("uuid"); + + b.Property("DeletedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("Frequency") + .HasColumnType("integer"); + + b.Property("IsDeleted") + .HasColumnType("boolean"); + + b.Property("Name") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("SortOrder") + .HasColumnType("integer"); + + b.HasKey("Id"); + + b.HasIndex("BudgetId", "IsDeleted"); + + b.ToTable("Incomes"); + }); + + modelBuilder.Entity("Budget.Core.Models.KnownUser", b => + { + b.Property("Id") + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("Email") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("LastSeenAt") + .HasColumnType("timestamp with time zone"); + + b.Property("Name") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.HasKey("Id"); + + b.ToTable("KnownUsers"); + }); + + modelBuilder.Entity("Budget.Core.Models.Outgo", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("Amount") + .HasPrecision(18, 2) + .HasColumnType("numeric(18,2)"); + + b.Property("BudgetId") + .HasColumnType("uuid"); + + b.Property("Category") + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("DeletedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("Frequency") + .HasColumnType("integer"); + + b.Property("IsDeleted") + .HasColumnType("boolean"); + + b.Property("Name") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("Notes") + .HasMaxLength(1000) + .HasColumnType("character varying(1000)"); + + b.Property("PaymentSource") + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("SortOrder") + .HasColumnType("integer"); + + b.Property("Type") + .HasColumnType("integer"); + + b.HasKey("Id"); + + b.HasIndex("BudgetId", "IsDeleted"); + + b.ToTable("Outgos"); + }); + + modelBuilder.Entity("Budget.Core.Models.BudgetShare", b => + { + b.HasOne("Budget.Core.Models.Budget", "Budget") + .WithMany("Shares") + .HasForeignKey("BudgetId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Budget"); + }); + + modelBuilder.Entity("Budget.Core.Models.Income", b => + { + b.HasOne("Budget.Core.Models.Budget", "Budget") + .WithMany("Incomes") + .HasForeignKey("BudgetId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Budget"); + }); + + modelBuilder.Entity("Budget.Core.Models.Outgo", b => + { + b.HasOne("Budget.Core.Models.Budget", "Budget") + .WithMany("Outgos") + .HasForeignKey("BudgetId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Budget"); + }); + + modelBuilder.Entity("Budget.Core.Models.Budget", b => + { + b.Navigation("Incomes"); + + b.Navigation("Outgos"); + + b.Navigation("Shares"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/src/Budget.Infrastructure/Data/Migrations/20260507024323_AddHardeningIndexes.cs b/src/Budget.Infrastructure/Data/Migrations/20260507024323_AddHardeningIndexes.cs new file mode 100644 index 0000000..0438162 --- /dev/null +++ b/src/Budget.Infrastructure/Data/Migrations/20260507024323_AddHardeningIndexes.cs @@ -0,0 +1,81 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace Budget.Infrastructure.Data.Migrations +{ + /// + public partial class AddHardeningIndexes : Migration + { + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_Outgos_BudgetId", + table: "Outgos"); + + migrationBuilder.DropIndex( + name: "IX_Incomes_BudgetId", + table: "Incomes"); + + migrationBuilder.CreateIndex( + name: "IX_Outgos_BudgetId_IsDeleted", + table: "Outgos", + columns: new[] { "BudgetId", "IsDeleted" }); + + migrationBuilder.CreateIndex( + name: "IX_Incomes_BudgetId_IsDeleted", + table: "Incomes", + columns: new[] { "BudgetId", "IsDeleted" }); + + migrationBuilder.CreateIndex( + name: "IX_BudgetShares_IsPending_SharedWithEmail", + table: "BudgetShares", + columns: new[] { "IsPending", "SharedWithEmail" }); + + migrationBuilder.CreateIndex( + name: "IX_BudgetShares_SharedWithUserId", + table: "BudgetShares", + column: "SharedWithUserId"); + + migrationBuilder.CreateIndex( + name: "IX_Budgets_OwnerUserId", + table: "Budgets", + column: "OwnerUserId"); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropIndex( + name: "IX_Outgos_BudgetId_IsDeleted", + table: "Outgos"); + + migrationBuilder.DropIndex( + name: "IX_Incomes_BudgetId_IsDeleted", + table: "Incomes"); + + migrationBuilder.DropIndex( + name: "IX_BudgetShares_IsPending_SharedWithEmail", + table: "BudgetShares"); + + migrationBuilder.DropIndex( + name: "IX_BudgetShares_SharedWithUserId", + table: "BudgetShares"); + + migrationBuilder.DropIndex( + name: "IX_Budgets_OwnerUserId", + table: "Budgets"); + + migrationBuilder.CreateIndex( + name: "IX_Outgos_BudgetId", + table: "Outgos", + column: "BudgetId"); + + migrationBuilder.CreateIndex( + name: "IX_Incomes_BudgetId", + table: "Incomes", + column: "BudgetId"); + } + } +} diff --git a/src/Budget.Infrastructure/Data/Migrations/AppDbContextModelSnapshot.cs b/src/Budget.Infrastructure/Data/Migrations/AppDbContextModelSnapshot.cs index f6b280c..969b5b7 100644 --- a/src/Budget.Infrastructure/Data/Migrations/AppDbContextModelSnapshot.cs +++ b/src/Budget.Infrastructure/Data/Migrations/AppDbContextModelSnapshot.cs @@ -58,6 +58,8 @@ namespace Budget.Infrastructure.Data.Migrations b.HasKey("Id"); + b.HasIndex("OwnerUserId"); + b.ToTable("Budgets"); }); @@ -96,9 +98,13 @@ namespace Budget.Infrastructure.Data.Migrations b.HasKey("Id"); + b.HasIndex("SharedWithUserId"); + b.HasIndex("BudgetId", "SharedWithEmail") .IsUnique(); + b.HasIndex("IsPending", "SharedWithEmail"); + b.ToTable("BudgetShares"); }); @@ -134,7 +140,7 @@ namespace Budget.Infrastructure.Data.Migrations b.HasKey("Id"); - b.HasIndex("BudgetId"); + b.HasIndex("BudgetId", "IsDeleted"); b.ToTable("Incomes"); }); @@ -210,7 +216,7 @@ namespace Budget.Infrastructure.Data.Migrations b.HasKey("Id"); - b.HasIndex("BudgetId"); + b.HasIndex("BudgetId", "IsDeleted"); b.ToTable("Outgos"); });