fix(security): audit batch A — validation, rate limiting, sessions #20

Merged
cesnimda merged 1 commits from fix/audit-critical-backend into develop 2026-07-02 03:22:17 +02:00
10 changed files with 500 additions and 2 deletions
+242
View File
@@ -0,0 +1,242 @@
# InboxIntel — Full Application Audit Report
**Date:** 2026-07-02 · **Scope:** entire repository (backend `src/`, `frontend/`, migrations,
CI/CD `.gitea/workflows/`, `docker-compose*`, `deploy/`, git history) · **Phase:** 1 (read-only)
**Stack:** .NET 8 / ASP.NET Core (Clean Architecture) · EF Core + PostgreSQL (pgvector image)
· React 18 + Vite SPA · Google OAuth2 (cookie session) · Gitea Actions CI/CD · self-hosted
Docker Compose (single-operator deployment).
## Executive summary
This codebase is **markedly more hardened than typical** — previous hardening phases (V-01…V-12,
visible in code comments) already addressed the classic killers: IDOR (EF global query filters),
SSRF (a genuinely thorough `SafeHttpGuard` with DNS-rebinding defence), token encryption at rest,
non-root containers, stack-trace suppression, forwarded-header trust, secret hygiene (nothing in
git history), and confirmed-destructive-actions. **No Critical findings.** The most significant
issues are: **registered input validators that never execute (H-1)**, **no rate limiting (H-2)**,
and a **data-at-rest posture** for email bodies that should be a deliberate, documented decision
(H-3). CI has real security gates (gitleaks + dependency audit) already.
Severity counts: **Critical 0 · High 3 · Medium 6 · Low 7**
---
## 1. Security
### H-1 · FluentValidation validators are registered but NEVER executed
- **File:** `src/InboxIntel.Application/DependencyInjection.cs:11` (registration) vs
`src/InboxIntel.Api/Program.cs` (no `AddFluentValidationAutoValidation()` anywhere; no
controller injects `IValidator<>`).
- **Why it's a problem:** `Validators.cs` defines real safety rules — `CleanupRequestValidator`
requires `Confirmed=true` for Trash/HardDelete, `SearchRequestValidator` bounds paging, etc. —
but FluentValidation 11.x requires explicit auto-validation enablement, which is absent. The
rules are dead code; requests reach services unvalidated. *Mitigating factor:* the services
re-check the most dangerous rules (`CleanupService.cs:48` re-enforces `Confirmed`), and the
controller clamps paging — so this is defence-in-depth loss, not an open hole. But rules that
exist **only** in the validators (e.g. `From <= To`, LabelId-required) are silently unenforced.
- **Fix:** add `services.AddFluentValidationAutoValidation()` (package already referenced) in
`Program.cs`; add a regression test posting an invalid DTO and asserting 400.
- **Blast radius:** isolated (1 line + tests). Verify no existing client sends payloads that
would now 400.
### H-2 · No rate limiting on any endpoint
- **File:** `src/InboxIntel.Api/Program.cs` (no `AddRateLimiter`/middleware anywhere).
- **Why:** login (`/auth/login` → OAuth), search (now with FTS + trigram + future AI), export
(PDF generation), unsubscribe (server-side outbound HTTP), and AI endpoints are all
unthrottled. A single authenticated user (or an unauthenticated client hammering
`/auth/login`/`/appinfo`) can exhaust CPU/DB/outbound quota. For the future multi-user platform
(see `docs/discovery/multi-provider/`), this is a prerequisite.
- **Fix:** ASP.NET Core's built-in `RateLimiter` — a global fixed-window per-IP policy + stricter
policies on `auth`, `export`, `unsubscribe`, `ai`. Return 429 with Retry-After.
- **Blast radius:** isolated (Program.cs + policy constants + tests).
### Verified-good (no finding)
- **Authorization / IDOR:** `[Authorize]` on `ApiControllerBase`; only `AppInfo` + `Auth.login`
are `[AllowAnonymous]` (correct). Global query filters on every tenant-scoped entity enforce
per-user isolation even if a query forgets its `Where` — covered by `TenantIsolationTests`.
- **Injection:** all data access via EF parameterisation; no raw SQL string concatenation in app
code; FTS uses `websearch_to_tsquery` parameters. Frontend has **zero**
`dangerouslySetInnerHTML`/`innerHTML`/`eval`; the search-highlight feature deliberately uses
non-HTML sentinels rendered as escaped React elements (XSS-safe by construction).
- **SSRF:** `SafeHttpGuard` validates scheme, resolves DNS and checks **every** address against
loopback/private/link-local/CGNAT/metadata/multicast (v4+v6, v4-mapped), fails closed on
unknown families, and is paired with `AllowAutoRedirect=false` (`DependencyInjection.cs:65`).
This is better than most production code.
- **Secrets:** none hardcoded (empty placeholders in `appsettings.json`); no `.env` ever
committed (git history checked); real secrets live in git-ignored `deploy/.env*` and Gitea
Actions secrets; pre-commit hook + CI gitleaks scan both guard regressions.
- **Session cookies:** HttpOnly, SameSite=Lax, Secure-always outside dev, 401-not-redirect for
XHR. Forwarded headers only trusted from configured proxy CIDRs (V-08).
- **Headers:** nosniff, X-Frame-Options DENY, Referrer-Policy, COOP on API; HSTS in prod.
### M-1 · No absolute session lifetime (sliding-only)
- **File:** `Program.cs:58-59``ExpireTimeSpan = 7d` with `SlidingExpiration = true`.
- **Why:** a session that's touched at least weekly renews forever; a stolen cookie never
expires as long as the attacker uses it. No server-side revocation list exists either
(cookie is self-contained).
- **Fix:** add an absolute cap via an `issued-at` claim checked in `OnValidatePrincipal`
(e.g. re-auth after 30 days), and/or a session-stamp validated against the DB to enable
revocation. (The multi-provider design doc 02/06 already specs DB-backed sessions — this
aligns.)
- **Blast radius:** isolated.
### M-2 · No Content-Security-Policy on the SPA or API
- **File:** `Program.cs:156-167` (comment says CSP "report-only for now" but none is actually
set); `frontend/nginx.conf` serves the SPA without security headers.
- **Why:** CSP is the main mitigation layer against any future XSS slip; currently absent.
- **Fix:** add CSP (default-src 'self'; connect-src API origin; etc.) + nosniff/XFO to the SPA
nginx config; optionally a report-only CSP on the API.
- **Blast radius:** isolated (nginx conf + one middleware line), needs SPA smoke-test (Chart.js
inline styles etc.).
### L-1 · CSRF: no antiforgery tokens (accepted-risk, documented here)
- SameSite=Lax cookies + strict CORS allowlist + JSON-only POST bodies make classic CSRF
impractical in modern browsers. Acceptable for now; revisit if cookie SameSite is ever
relaxed or non-JSON form endpoints are added.
### L-2 · `AllowedHosts: "*"` (`appsettings.json`)
- Host-header filtering disabled; low risk behind the proxy but set it to the real hostnames
at production deployment.
---
## 2. Data protection (top priority)
**Inventory of sensitive data:**
| Data | Where | At-rest protection | Verdict |
|---|---|---|---|
| Google OAuth **refresh/access tokens** | `users.EncryptedRefreshToken` | **Encrypted** (Data Protection API, keys on `/keys` volume) | ✅ good |
| **Passwords** | — | **None stored** (OAuth-only login; no password column exists) | ✅ best-possible |
| **Email bodies / subjects / snippets** | `emails.BodyText` etc. | **Plaintext** in Postgres | ⚠️ **H-3** below |
| Sender names/addresses (PII) | `senders`, `emails` | Plaintext | part of H-3 |
| User email + display name | `users` | Plaintext | part of H-3 |
| DB password | `deploy/.env` (git-ignored) + Actions secret | not in repo | ✅ |
| Data Protection keys | `/keys` Docker volume | filesystem | M-3 below |
| IPs / payment / health data | — | not collected | ✅ n/a |
### H-3 · Email content stored in plaintext at rest, with unbounded retention
- **File:** `src/InboxIntel.Domain/Entities/Email.cs` (`BodyText`, `Snippet`, `Subject`);
Postgres `pgdata` volume; also flows into PDF/CSV/JSON **exports** and (when enabled) to the
local Ollama process.
- **Why:** the entire product is a copy of the user's mailbox. On this self-hosted,
single-operator deployment the DB lives on the operator's own disk — a defensible posture —
but: (a) anyone with disk/volume/backup access reads all mail; (b) there is **no retention or
purge policy** (mail persists even after unsubscribe/cleanup in Gmail; "remove account"
flows don't exist yet); (c) the planned **multi-user platform** (docs/discovery/multi-provider)
makes plaintext-bodies-readable-by-host-admin a real privacy issue (its own security doc
promises "admins never read users' mail" — the DB must back that up).
- **Fix (phased):** 1) *Document* the current posture in README/threat model (deliberate,
local-first). 2) Enable **pgcrypto/field-level encryption or full-disk/volume encryption**
before any multi-user deployment. 3) Add a **data-retention setting** + purge job and an
account-deletion path (GDPR-style erasure). 4) Ensure DB **backups** inherit the same
protection.
- **Blast radius:** documentation = trivial; field-level encryption = **large** (touches search
— FTS can't index encrypted columns; would need architectural decision). Recommend
volume-level encryption + retention/deletion first.
### M-3 · Data Protection keys stored unencrypted on the `/keys` volume
- **File:** `Program.cs:26-28``PersistKeysToFileSystem` without `ProtectKeysWith*`.
- **Why:** whoever reads the volume can decrypt all stored refresh tokens. Same-disk-as-DB
caveat applies, but keys and ciphertext living side-by-side weakens the encryption's value.
- **Fix:** `ProtectKeysWithCertificate(...)` (cert from env/secret), or OS-level DPAPI on
Windows hosts; at minimum document the volume-permissions requirement.
- **Blast radius:** isolated, but requires a key-migration step for existing tokens.
### M-4 · Default DB credentials in `appsettings.json`
- **File:** `appsettings.json:3``Password=inboxintel` as the fallback connection string.
- **Why:** if a deployment forgets the env override, the app happily connects with a guessable
password (compose enforces `POSTGRES_PASSWORD` but a non-compose deployment might not).
- **Fix:** empty the default and fail fast at startup with a clear message when unset.
- **Blast radius:** isolated (plus updating dev docs to use user-secrets).
### Logging & transit — verified
- **Logs:** no token/password logging found; the one PII-ish log is `SmtpEmailSender.cs:28`
(recipient address + subject at Info when SMTP is unconfigured) — acceptable, downgrade to
Debug if desired (L-3). Serilog request logging does not include query strings or bodies.
- **Transit:** TLS terminates at nginx (HSTS enabled in prod); API+DB bound to loopback/compose
network only. **DB connection itself is non-TLS** — fine while Postgres is co-located on the
compose network; **must add `SSL Mode=Require` if the DB ever moves to another host** (L-4).
---
## 3. Dependencies & supply chain
- **`dotnet list package --vulnerable --include-transitive`:** **clean** (all projects) — the
earlier round of CVE pins (System.Text.Json 8.0.6 etc.) is holding, and CI re-checks on every PR.
- **`npm audit` (production deps):** **0 vulnerabilities**.
- **M-5 · `npm audit` (dev deps):** vite 5.x depends on a vulnerable **esbuild** (1 moderate,
1 high, dev-server-only vectors). Not shipped to users and CI already scopes to prod deps —
but the fix is a routine **vite 5→6/7 upgrade**. Effort S.
- **Duplication (L-5):** both `MailKit` (Infrastructure) and the frontend carry sizeable deps;
`react-grid-layout` exists solely for the draggable dashboard the redesign plans to retire —
candidate for removal with the Analytics migration. No abandoned packages spotted.
- CI supply-chain posture: gitleaks (full history) + NuGet/npm audits are **required checks**
good. No SAST/CodeQL (L-6, nice-to-have).
## 4. Error handling & reliability
- **Good:** global `UseExceptionHandler` + RFC7807 ProblemDetails (no stack traces to clients);
Polly retry/backoff on Gmail sync; idempotent sync upserts; AI calls wrapped in try/catch with
non-AI fallbacks ("sync must never fail because the AI provider is down").
- **M-6 · Known EF model warning:** `Email` has a global query filter but is the required end
of the `Email↔EmailLabel` relationship — logged on every boot; can yield surprising results
when filtered parents are excluded. Fix: matching filter on `EmailLabel` (one line) + test.
(Long-noted in logs; this is the nudge to actually do it.)
- **L-7 · Multi-step writes without explicit transactions:** e.g. `GoogleAuthEvents` upsert and
sync batches rely on EF's single-SaveChanges transactionality — mostly fine; the sync
cursor-advance + batch-upsert pairing is the one place an explicit transaction would guard a
mid-batch crash (currently self-heals via idempotent re-sync — acceptable).
## 5. Performance
- **Fixed this cycle (verified live):** relevance-ranked FTS with weighted tsvector + GIN;
trigram GIN indexes for the previously non-sargable sender/domain `.Contains()`; HNSW vector
index ready for semantic search.
- **Remaining (all Low, roadmapped):** offset pagination degrades on deep pages (keyset planned
in `docs/discovery/05`); frontend ships one **678 KB JS bundle** (no code-splitting — vite
`manualChunks`/dynamic import, effort S); no HTTP caching/ETags on read-heavy endpoints
(sidebar counts are fetched per page-load); `docker-compose` frontend nginx lacks gzip/brotli
confirmation for the bundle.
- No N+1 patterns found (queries project with joins; aggregates precomputed in
`AnalyticsAggregate`).
## 6. Code quality & architecture
- Clean Architecture discipline is genuinely observed (dependency rule intact; thin controllers;
DTO mapping at boundaries). Config is env-driven throughout.
- **L-8:** `AiService.GenerateQueryAsync` returns raw LLM output as a search query (advisory-only,
becomes a search string — harmless today; keep it that way when NL search lands: model output
must stay data, never an executable/action).
- Dead-ish code: none significant; `docs/specs/*` and discovery docs are current.
## 7. Testing & CI
- **41 tests** (unit: parser/guard/unsubscribe/embeddings-null; integration: authz challenge,
tenant isolation, pagination clamp, search fallback ordering). Critical security invariants
(IDOR filter, SSRF guard, confirmed-destructive) **are tested** — better than most.
- **Gap (M-7):** no live-Postgres test harness — FTS/ranking/fuzzy/pgvector paths are verified
manually against staging (documented in commit messages) but not repeatably in CI. A
Testcontainers-Postgres (pgvector image) job would convert those throwaway verifications into
permanent regression tests. Effort M.
- Gap (L-9): no coverage for `CleanupService`/`SyncService` beyond compilation; no E2E of the
OAuth flow (hard without creds — acceptable).
- CI: build+test+secrets+deps as **required** PR checks; auto-deploy to staging with a
post-deploy health gate; prod is tag-gated. Solid. Missing: lint/format check in CI (the
pre-commit hook enforces locally; add `dotnet format --verify-no-changes` job, effort S) (L-10).
---
## Prioritized remediation plan
| # | Finding | Sev | Fix effort | Blast radius |
|---|---------|-----|-----------|--------------|
| 1 | **H-1** Enable FluentValidation auto-validation + 400 tests | High | S | Isolated (1 line + tests; verify clients) |
| 2 | **H-2** Rate limiting (global + auth/export/unsub/AI policies) | High | SM | Isolated (Program.cs + tests) |
| 3 | **H-3** Data-at-rest posture: document now; retention setting + purge job + account-deletion; volume-encryption guidance; (defer field-level encryption decision) | High | S (doc) → M (retention) → L (encryption) | Doc: none · Retention: moderate · Encryption: large/architectural |
| 4 | **M-3** Protect Data Protection keys at rest | Med | SM | Isolated + key migration |
| 5 | **M-1** Absolute session lifetime (+ revocation groundwork) | Med | S | Isolated |
| 6 | **M-4** Remove default DB password; fail fast | Med | S | Isolated |
| 7 | **M-2** CSP + security headers on the SPA nginx | Med | S | Isolated (needs SPA smoke test) |
| 8 | **M-6** Fix EF query-filter warning (EmailLabel filter) | Med | S | Isolated + test |
| 9 | **M-5** Vite upgrade (dev-dep CVEs) | Med | S | Frontend build only |
| 10 | **M-7** Testcontainers live-Postgres CI job | Med | M | CI + new test project wiring |
| 11 | L-2/L-3/L-4/L-10 (AllowedHosts, SMTP log level, DB TLS note, CI format check) | Low | S each | Isolated |
| 12 | L-5 bundle split / grid-layout removal (with redesign) | Low | SM | Frontend |
**Stopping here per Phase 1 instructions — no changes made. Awaiting approval of this plan
(or an edited subset) before implementing anything.**
@@ -7,6 +7,7 @@ namespace InboxIntel.Api.Controllers;
/// AI endpoints are read-only / advisory. They never trigger destructive
/// actions - suggestions are returned for the user to act on via /cleanup.
/// </summary>
[Microsoft.AspNetCore.RateLimiting.EnableRateLimiting("expensive")] // AUDIT H-2: LLM calls are the most expensive path
public class AiController : ApiControllerBase
{
private readonly IAiService _ai;
@@ -5,10 +5,12 @@ using Microsoft.AspNetCore.Authentication.Cookies;
using Microsoft.AspNetCore.Authentication.Google;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.RateLimiting;
namespace InboxIntel.Api.Controllers;
[ApiController]
[EnableRateLimiting("auth")] // AUDIT H-2: throttle login/challenge attempts per IP
[ApiVersion("1.0")]
[Route("api/v{version:apiVersion}/[controller]")]
public class AuthController : ControllerBase
@@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Mvc;
namespace InboxIntel.Api.Controllers;
[Microsoft.AspNetCore.RateLimiting.EnableRateLimiting("expensive")] // AUDIT H-2: PDF/CSV generation is costly
public class ExportController : ApiControllerBase
{
private readonly IExportService _export;
@@ -4,6 +4,7 @@ using Microsoft.AspNetCore.Mvc;
namespace InboxIntel.Api.Controllers;
[Microsoft.AspNetCore.RateLimiting.EnableRateLimiting("expensive")] // AUDIT H-2: triggers server-side outbound HTTP
public class UnsubscribeController : ApiControllerBase
{
private readonly IUnsubscribeService _unsub;
+75
View File
@@ -1,5 +1,7 @@
using System.Security.Claims;
using System.Threading.RateLimiting;
using Asp.Versioning;
using FluentValidation.AspNetCore;
using InboxIntel.Api.Auth;
using InboxIntel.Application;
using InboxIntel.Application.Abstractions;
@@ -8,8 +10,10 @@ using InboxIntel.Infrastructure.Configuration;
using InboxIntel.Infrastructure.Persistence;
using Microsoft.AspNetCore.Authentication.Cookies;
using Microsoft.AspNetCore.Authentication.Google;
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.DataProtection;
using Microsoft.AspNetCore.HttpOverrides;
using Microsoft.AspNetCore.RateLimiting;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Options;
using Serilog;
@@ -57,6 +61,24 @@ builder.Services.AddAuthentication(options =>
options.Cookie.Name = "inboxintel.session";
options.ExpireTimeSpan = TimeSpan.FromDays(7);
options.SlidingExpiration = true;
// AUDIT M-1: sliding expiration alone lets a stolen cookie renew forever. Stamp an
// absolute start at sign-in and reject principals older than the configured cap,
// forcing a full re-login. (Pre-existing sessions without the stamp are rejected
// once — a single re-login, then they carry the stamp.)
var absoluteDays = builder.Configuration.GetValue("Auth:AbsoluteSessionDays", 30);
options.Events.OnSigningIn = ctx =>
{
ctx.Properties.SetString("abs-start", DateTimeOffset.UtcNow.ToString("O"));
return Task.CompletedTask;
};
options.Events.OnValidatePrincipal = async ctx =>
{
if (SessionLifetime.IsExpired(ctx.Properties.GetString("abs-start"), DateTimeOffset.UtcNow, TimeSpan.FromDays(absoluteDays)))
{
ctx.RejectPrincipal();
await ctx.HttpContext.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme);
}
};
// API-style behaviour: return status codes rather than redirecting to a login page.
options.Events.OnRedirectToLogin = ctx =>
{
@@ -98,6 +120,35 @@ builder.Services.AddApiVersioning(o =>
}).AddApiExplorer(o => { o.GroupNameFormat = "'v'VVV"; o.SubstituteApiVersionInUrl = true; });
builder.Services.AddControllers();
// AUDIT H-1: the validators in InboxIntel.Application/Validation were registered but never
// executed (FluentValidation 11.x needs explicit auto-validation). This wires them into
// model binding so invalid DTOs 400 at the boundary instead of reaching services.
builder.Services.AddFluentValidationAutoValidation();
// AUDIT H-2: rate limiting. Global per-user (or per-IP when anonymous) window, plus stricter
// named policies for auth and expensive endpoints (export/unsubscribe/AI). Limits are
// config-driven so tests and deployments can tune them.
var rl = builder.Configuration.GetSection("RateLimiting");
int Limit(string key, int def) => rl.GetValue(key, def);
var rlWindow = TimeSpan.FromSeconds(Limit("WindowSeconds", 60));
static string Partition(HttpContext ctx) =>
ctx.User.Identity?.IsAuthenticated == true
? ctx.User.FindFirstValue("inboxintel:uid") ?? "auth-unknown"
: ctx.Connection.RemoteIpAddress?.ToString() ?? "anon";
builder.Services.AddRateLimiter(o =>
{
o.RejectionStatusCode = StatusCodes.Status429TooManyRequests;
o.GlobalLimiter = PartitionedRateLimiter.Create<HttpContext, string>(ctx =>
RateLimitPartition.GetFixedWindowLimiter(Partition(ctx), _ =>
new FixedWindowRateLimiterOptions { PermitLimit = Limit("GlobalPermitLimit", 300), Window = rlWindow, QueueLimit = 0 }));
o.AddPolicy("auth", ctx =>
RateLimitPartition.GetFixedWindowLimiter(Partition(ctx), _ =>
new FixedWindowRateLimiterOptions { PermitLimit = Limit("AuthPermitLimit", 10), Window = rlWindow, QueueLimit = 0 }));
o.AddPolicy("expensive", ctx =>
RateLimitPartition.GetFixedWindowLimiter(Partition(ctx), _ =>
new FixedWindowRateLimiterOptions { PermitLimit = Limit("ExpensivePermitLimit", 20), Window = rlWindow, QueueLimit = 0 }));
});
builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen();
// V-06: RFC7807 ProblemDetails so the global exception handler returns a safe,
@@ -115,7 +166,18 @@ using (var scope = app.Services.CreateScope())
{
var db = scope.ServiceProvider.GetRequiredService<AppDbContext>();
if (db.Database.IsRelational() && app.Configuration.GetValue("Database:AutoMigrate", true))
{
// AUDIT M-4: the shipped appsettings no longer carries a guessable default DB
// password. Fail fast with a clear message rather than connecting with weak or
// missing credentials (compose/staging/prod inject the full connection string).
var connStr = app.Configuration.GetConnectionString("Postgres") ?? string.Empty;
var csb = new Npgsql.NpgsqlConnectionStringBuilder(connStr);
if (string.IsNullOrWhiteSpace(csb.Password))
throw new InvalidOperationException(
"ConnectionStrings:Postgres has no password. Set the full connection string via " +
"environment/user-secrets (see README) — a default password is deliberately not shipped.");
await db.Database.MigrateAsync();
}
}
// Honor X-Forwarded-* from the nginx reverse proxy so OAuth redirect URIs and
@@ -169,9 +231,22 @@ app.Use(async (ctx, next) =>
app.UseSerilogRequestLogging();
app.UseCors("frontend");
app.UseAuthentication();
// AUDIT H-2: after authentication so authenticated traffic partitions per-user; anonymous
// traffic partitions per-IP. Endpoint policies ("auth", "expensive") apply via attributes.
app.UseRateLimiter();
app.UseAuthorization();
app.MapControllers();
app.Run();
public partial class Program { }
/// <summary>
/// AUDIT M-1: absolute session lifetime check, extracted for unit testing. A session with no
/// issued stamp (pre-dating this feature) is treated as expired — one forced re-login.
/// </summary>
public static class SessionLifetime
{
public static bool IsExpired(string? issuedAtIso, DateTimeOffset now, TimeSpan maxAge)
=> !DateTimeOffset.TryParse(issuedAtIso, out var issued) || now - issued > maxAge;
}
+1 -1
View File
@@ -1,6 +1,6 @@
{
"ConnectionStrings": {
"Postgres": "Host=localhost;Port=5432;Database=inboxintel;Username=inboxintel;Password=inboxintel"
"Postgres": ""
},
"Database": {
"AutoMigrate": true
@@ -25,7 +25,7 @@ public class SmtpEmailSender : IEmailSender
{
if (!IsEnabled)
{
_logger.LogInformation("SMTP not configured; skipping email \"{Subject}\" to {To}", subject, toAddress);
_logger.LogDebug("SMTP not configured; skipping email \"{Subject}\" to {To}", subject, toAddress);
return;
}
@@ -53,6 +53,10 @@ public class AppDbContext : DbContext, IAppDbContext
modelBuilder.Entity<MailDomain>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<Attachment>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<Label>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
// AUDIT M-6: EmailLabel is the required end of a relationship with the filtered Email
// entity; without a matching filter EF warns on boot and joins could surface rows whose
// parent is filtered out. Filter via the Email navigation so the pair is consistent.
modelBuilder.Entity<EmailLabel>().HasQueryFilter(el => CurrentUserId == Guid.Empty || el.Email!.UserId == CurrentUserId);
modelBuilder.Entity<SyncState>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<AnalyticsAggregate>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
modelBuilder.Entity<WidgetLayout>().HasQueryFilter(e => CurrentUserId == Guid.Empty || e.UserId == CurrentUserId);
@@ -0,0 +1,172 @@
using System.Net;
using System.Net.Http.Json;
using System.Security.Claims;
using System.Text.Encodings.Web;
using FluentAssertions;
using InboxIntel.Application.Abstractions;
using InboxIntel.Domain.Entities;
using InboxIntel.Infrastructure.Persistence;
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Mvc.Testing;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Xunit;
namespace InboxIntel.IntegrationTests;
// ── Tests proving the Phase-2 audit fixes (see AUDIT_REPORT.md) ────────────────────────────
/// <summary>Pass-through auth scheme so integration tests can exercise authenticated
/// endpoints (model validation, per-user rate limits) without a real Google login.</summary>
public class TestAuthHandler : AuthenticationHandler<AuthenticationSchemeOptions>
{
public const string Scheme = "Test";
// Stable across requests so per-user rate-limit partitions accumulate correctly.
public static readonly string Uid = Guid.NewGuid().ToString();
public TestAuthHandler(IOptionsMonitor<AuthenticationSchemeOptions> o, ILoggerFactory l, UrlEncoder e)
: base(o, l, e) { }
protected override Task<AuthenticateResult> HandleAuthenticateAsync()
{
var identity = new ClaimsIdentity(new[]
{
new Claim(ClaimTypes.NameIdentifier, "test-sub"),
new Claim("inboxintel:uid", Uid),
}, Scheme);
return Task.FromResult(AuthenticateResult.Success(
new AuthenticationTicket(new ClaimsPrincipal(identity), Scheme)));
}
}
/// <summary>Factory with the test auth scheme + tiny rate-limit windows so limits trip fast.</summary>
public class AuditTestAppFactory : WebApplicationFactory<Program>
{
protected override IHost CreateHost(IHostBuilder builder)
{
builder.ConfigureHostConfiguration(cfg => cfg.AddInMemoryCollection(new Dictionary<string, string?>
{
["Database:AutoMigrate"] = "false",
["GoogleOAuth:ClientId"] = "test-client-id",
["GoogleOAuth:ClientSecret"] = "test-client-secret",
// H-2: make the auth policy trip on the 3rd request within the window.
["RateLimiting:AuthPermitLimit"] = "2",
["RateLimiting:WindowSeconds"] = "60",
}));
return base.CreateHost(builder);
}
protected override void ConfigureWebHost(IWebHostBuilder builder)
{
builder.ConfigureServices(services =>
{
services.AddAuthentication(TestAuthHandler.Scheme)
.AddScheme<AuthenticationSchemeOptions, TestAuthHandler>(TestAuthHandler.Scheme, _ => { });
services.PostConfigure<AuthenticationOptions>(o =>
{
o.DefaultAuthenticateScheme = TestAuthHandler.Scheme;
o.DefaultChallengeScheme = TestAuthHandler.Scheme;
});
});
}
}
public class AuditFixesTests : IClassFixture<AuditTestAppFactory>
{
private readonly AuditTestAppFactory _factory;
public AuditFixesTests(AuditTestAppFactory factory) => _factory = factory;
// H-1: FluentValidation auto-validation now rejects invalid DTOs at the boundary with 400
// (previously the registered validators never executed).
[Fact]
public async Task Invalid_search_request_is_rejected_with_400_by_the_validator()
{
var client = _factory.CreateClient();
var resp = await client.PostAsJsonAsync("/api/v1/search", new { page = 1, pageSize = 0 });
resp.StatusCode.Should().Be(HttpStatusCode.BadRequest);
}
[Fact]
public async Task Search_with_from_after_to_is_rejected_with_400()
{
var client = _factory.CreateClient();
var resp = await client.PostAsJsonAsync("/api/v1/search",
new { page = 1, pageSize = 10, from = "2026-02-01", to = "2026-01-01" });
resp.StatusCode.Should().Be(HttpStatusCode.BadRequest);
}
// H-2: the "auth" rate-limit policy returns 429 once the per-window permit is exhausted.
[Fact]
public async Task Auth_endpoint_rate_limits_with_429_after_the_permit_is_exhausted()
{
var client = _factory.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false });
var s1 = (await client.GetAsync("/api/v1/auth/login")).StatusCode;
var s2 = (await client.GetAsync("/api/v1/auth/login")).StatusCode;
var s3 = (await client.GetAsync("/api/v1/auth/login")).StatusCode;
s1.Should().NotBe(HttpStatusCode.TooManyRequests);
s2.Should().NotBe(HttpStatusCode.TooManyRequests);
s3.Should().Be(HttpStatusCode.TooManyRequests);
}
}
// M-1: absolute session lifetime — a session older than the cap (or missing its issued
// stamp) is expired regardless of sliding renewal.
public class SessionLifetimeTests
{
private static readonly DateTimeOffset Now = new(2026, 07, 02, 12, 0, 0, TimeSpan.Zero);
private static readonly TimeSpan Max = TimeSpan.FromDays(30);
[Fact]
public void Fresh_session_is_not_expired()
=> SessionLifetime.IsExpired(Now.AddDays(-1).ToString("O"), Now, Max).Should().BeFalse();
[Fact]
public void Session_older_than_the_cap_is_expired()
=> SessionLifetime.IsExpired(Now.AddDays(-31).ToString("O"), Now, Max).Should().BeTrue();
[Fact]
public void Session_without_a_stamp_is_expired()
=> SessionLifetime.IsExpired(null, Now, Max).Should().BeTrue();
}
// M-6: EmailLabel now carries a matching tenant query filter (via its Email navigation), so
// join rows can never leak across users even without a manual Where.
public class EmailLabelFilterTests
{
private sealed class FakeCurrentUser : ICurrentUser
{
public Guid UserId { get; set; }
public bool IsAuthenticated => UserId != Guid.Empty;
}
[Fact]
public async Task EmailLabels_are_invisible_across_users()
{
var userA = Guid.NewGuid();
var userB = Guid.NewGuid();
var opts = new DbContextOptionsBuilder<AppDbContext>()
.UseInMemoryDatabase(nameof(EmailLabels_are_invisible_across_users)).Options;
using (var seed = new AppDbContext(opts, new FakeCurrentUser()))
{
var emailA = new Email { UserId = userA, GmailMessageId = "a1" };
var emailB = new Email { UserId = userB, GmailMessageId = "b1" };
var labelA = new Label { UserId = userA, GmailLabelId = "LA", Name = "A" };
var labelB = new Label { UserId = userB, GmailLabelId = "LB", Name = "B" };
seed.AddRange(emailA, emailB, labelA, labelB,
new EmailLabel { EmailId = emailA.Id, LabelId = labelA.Id },
new EmailLabel { EmailId = emailB.Id, LabelId = labelB.Id });
await seed.SaveChangesAsync();
}
using var ctx = new AppDbContext(opts, new FakeCurrentUser { UserId = userA });
var visible = await ctx.Set<EmailLabel>().ToListAsync(); // no manual Where — filter must enforce
visible.Should().HaveCount(1);
}
}