Merge pull request #203 from daveharmswebdev/feature/14-2-security-headers-middleware

daveharmswebdev · web-flow · commit 373cbcc8ace4 · 2026-02-14T17:52:50.000-06:00
feat: Add security headers middleware (Story 14.2)
diff --git a/_bmad-output/implementation-artifacts/14-1-cors-configuration.md b/_bmad-output/implementation-artifacts/14-1-cors-configuration.md
@@ -1,6 +1,6 @@
 # Story 14.1: CORS Configuration
 
-Status: dev-complete
+Status: done
 
 ## Story
 
diff --git a/_bmad-output/implementation-artifacts/14-2-security-headers-middleware.md b/_bmad-output/implementation-artifacts/14-2-security-headers-middleware.md
@@ -0,0 +1,181 @@
+# Story 14.2: Security Headers Middleware
+
+Status: review
+
+## Story
+
+As a **developer**,
+I want **standard security headers added to every HTTP response**,
+So that **the application is protected against common browser-based attacks**.
+
+## Acceptance Criteria
+
+1. **Given** any API response is returned **When** I inspect the response headers **Then** the following headers are present:
+   - `X-Frame-Options: DENY`
+   - `X-Content-Type-Options: nosniff`
+   - `Referrer-Policy: strict-origin-when-cross-origin`
+   - `Permissions-Policy: camera=(), microphone=(), geolocation=(), payment=()`
+
+2. **Given** the application is running in production or staging **When** a response is returned **Then** the `Strict-Transport-Security` (HSTS) header is present **And** HSTS is NOT present in development (auto-handled by `app.UseHsts()`)
+
+3. **Given** the Content Security Policy header **When** I inspect the CSP value **Then** it allows:
+   - `'self'` for default sources
+   - Google Fonts (`fonts.googleapis.com`, `fonts.gstatic.com`) for styles and fonts
+   - `blob:` for PDF preview rendering
+   - `*.ingest.sentry.io` for Sentry error reporting
+   - `'unsafe-inline'` for styles (required by Angular Material)
+
+4. **Given** the security headers middleware is registered **When** checking the middleware pipeline order **Then** it is placed after `UseHttpsRedirection()` and `UseHsts()`
+
+## Tasks / Subtasks
+
+- [x] Task 1: Create SecurityHeadersMiddleware class (AC: #1, #3)
+  - [x] 1.1 Create `backend/src/PropertyManager.Api/Middleware/SecurityHeadersMiddleware.cs`
+  - [x] 1.2 Implement `InvokeAsync` that adds all security headers before calling `_next(context)`
+  - [x] 1.3 Add headers: `X-Frame-Options: DENY`, `X-Content-Type-Options: nosniff`, `Referrer-Policy: strict-origin-when-cross-origin`, `Permissions-Policy: camera=(), microphone=(), geolocation=(), payment=()`
+  - [x] 1.4 Add CSP header: `Content-Security-Policy: default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com; img-src 'self' data: blob:; connect-src 'self' https://*.ingest.sentry.io; frame-ancestors 'none'; object-src 'none'`
+
+- [x] Task 2: Create extension method for clean pipeline registration (AC: #4)
+  - [x] 2.1 Add a static `UseSecurityHeaders()` extension method on `IApplicationBuilder` in the middleware file (or a separate `MiddlewareExtensions.cs` if preferred, but co-locating is fine)
+
+- [x] Task 3: Register middleware in Program.cs pipeline (AC: #2, #4)
+  - [x] 3.1 Add `app.UseHsts()` after `app.UseHttpsRedirection()` (built-in, auto-skips in Development)
+  - [x] 3.2 Add `app.UseSecurityHeaders()` after `app.UseHsts()`
+  - [x] 3.3 Resulting pipeline order:
+    ```
+    app.UseMiddleware<GlobalExceptionHandlerMiddleware>();
+    // Swagger (dev only)
+    app.UseHttpsRedirection();
+    app.UseHsts();                    // NEW - Story 14.2
+    app.UseSecurityHeaders();         // NEW - Story 14.2
+    app.UseSerilogRequestLogging();
+    app.UseCors("AllowedOrigins");
+    app.UseAuthentication();
+    app.UseAuthorization();
+    app.MapControllers();
+    app.MapHub<ReceiptHub>("/hubs/receipts");
+    ```
+
+- [x] Task 4: Write unit tests for SecurityHeadersMiddleware (AC: #1, #3)
+  - [x] 4.1 Create `backend/tests/PropertyManager.Api.Tests/Middleware/SecurityHeadersMiddlewareTests.cs`
+  - [x] 4.2 Test: Response includes `X-Frame-Options: DENY`
+  - [x] 4.3 Test: Response includes `X-Content-Type-Options: nosniff`
+  - [x] 4.4 Test: Response includes `Referrer-Policy: strict-origin-when-cross-origin`
+  - [x] 4.5 Test: Response includes `Permissions-Policy: camera=(), microphone=(), geolocation=(), payment=()`
+  - [x] 4.6 Test: Response includes `Content-Security-Policy` with correct value
+  - [x] 4.7 Test: Next middleware is called (pass-through behavior)
+
+## Dev Notes
+
+### Middleware Pattern — Follow Existing Convention
+
+Follow the `GlobalExceptionHandlerMiddleware` pattern at `backend/src/PropertyManager.Api/Middleware/GlobalExceptionHandlerMiddleware.cs`:
+- Same namespace: `PropertyManager.Api.Middleware`
+- Constructor takes `RequestDelegate next` (no logger/environment needed — this middleware just adds headers)
+- `InvokeAsync(HttpContext context)` method
+- Add headers to `context.Response.Headers` **before** calling `await _next(context)`
+
+### CSP Header Value (Exact)
+
+```
+default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com; img-src 'self' data: blob:; connect-src 'self' https://*.ingest.sentry.io; frame-ancestors 'none'; object-src 'none'
+```
+
+Rationale for each directive:
+- `style-src 'unsafe-inline'` — Angular Material injects inline styles
+- `fonts.googleapis.com` / `fonts.gstatic.com` — Google Fonts used in the app
+- `img-src blob:` — PDF preview rendering via blob URLs
+- `connect-src https://*.ingest.sentry.io` — Story 14.4 will add Sentry; pre-allow the domain now
+- `frame-ancestors 'none'` — equivalent to X-Frame-Options DENY, belt-and-suspenders
+
+### HSTS — Use Built-in
+
+Do **not** manually add the `Strict-Transport-Security` header. Use `app.UseHsts()` which:
+- Automatically adds `Strict-Transport-Security: max-age=2592000` (30 days)
+- Automatically **skips** in Development environment (no HSTS on localhost)
+- This satisfies AC #2 with zero custom code
+
+### Pipeline Placement
+
+Headers must be added **early** in the pipeline so every response (including error responses, CORS preflight responses, etc.) gets the security headers. Place after `UseHttpsRedirection()` and `UseHsts()` but before `UseCors()`.
+
+Current pipeline in `Program.cs` (lines 225-249):
+```csharp
+app.UseMiddleware<GlobalExceptionHandlerMiddleware>();
+// Swagger (dev only)
+app.UseHttpsRedirection();
+app.UseSerilogRequestLogging();
+app.UseCors(corsPolicyName);
+app.UseAuthentication();
+app.UseAuthorization();
+```
+
+Target after this story:
+```csharp
+app.UseMiddleware<GlobalExceptionHandlerMiddleware>();
+// Swagger (dev only)
+app.UseHttpsRedirection();
+app.UseHsts();                    // NEW
+app.UseSecurityHeaders();         // NEW
+app.UseSerilogRequestLogging();
+app.UseCors(corsPolicyName);
+app.UseAuthentication();
+app.UseAuthorization();
+```
+
+### Test Pattern — Follow GlobalExceptionHandlerMiddlewareTests
+
+Test file: `backend/tests/PropertyManager.Api.Tests/Middleware/GlobalExceptionHandlerMiddlewareTests.cs`
+
+Key patterns to reuse:
+- `DefaultHttpContext` with `MemoryStream` for response body
+- Simple `RequestDelegate` that returns `Task.CompletedTask` (no exception — just pass through)
+- Assert against `_httpContext.Response.Headers["HeaderName"]`
+- Use `FluentAssertions` (already referenced in test project)
+- Use `[Fact]` attributes (xUnit, already in use)
+
+### Previous Story (14.1) Learnings
+
+- CORS implementation in Story 14.1 was clean — followed the same Program.cs modification pattern
+- Code review feedback (commit `31d29c0`) addressed minor issues — keep code tight and focused
+- The test project already has the `Middleware/` folder structure ready
+
+### Project Structure Notes
+
+- New file: `backend/src/PropertyManager.Api/Middleware/SecurityHeadersMiddleware.cs`
+- Modified file: `backend/src/PropertyManager.Api/Program.cs`
+- New test file: `backend/tests/PropertyManager.Api.Tests/Middleware/SecurityHeadersMiddlewareTests.cs`
+- All paths align with existing project conventions — `Middleware/` folder already exists
+
+### References
+
+- [Source: _bmad-output/planning-artifacts/epic-14-security-hardening-observability.md#Story 14.2]
+- [Source: backend/src/PropertyManager.Api/Middleware/GlobalExceptionHandlerMiddleware.cs] — middleware pattern reference
+- [Source: backend/tests/PropertyManager.Api.Tests/Middleware/GlobalExceptionHandlerMiddlewareTests.cs] — test pattern reference
+- [Source: backend/src/PropertyManager.Api/Program.cs] — pipeline registration
+- [Source: _bmad-output/implementation-artifacts/14-1-cors-configuration.md] — previous story context
+
+## Dev Agent Record
+
+### Agent Model Used
+Claude Opus 4.6
+
+### Debug Log References
+None — clean implementation with no issues.
+
+### Completion Notes List
+- Created `SecurityHeadersMiddleware` with all 5 security headers (X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy, CSP) added before `_next(context)`
+- Co-located `UseSecurityHeaders()` extension method in same file for clean pipeline registration
+- Registered `app.UseHsts()` (built-in HSTS, auto-skips in Development) and `app.UseSecurityHeaders()` in Program.cs after `UseHttpsRedirection()` and before `UseSerilogRequestLogging()`
+- 6 unit tests: one per header + pass-through behavior — all passing
+- Full regression suite: 1,447 tests passing (913 Application + 85 Infrastructure + 449 Api)
+
+### File List
+- `backend/src/PropertyManager.Api/Middleware/SecurityHeadersMiddleware.cs` (NEW)
+- `backend/src/PropertyManager.Api/Program.cs` (MODIFIED)
+- `backend/tests/PropertyManager.Api.Tests/Middleware/SecurityHeadersMiddlewareTests.cs` (NEW)
+- `_bmad-output/implementation-artifacts/14-2-security-headers-middleware.md` (MODIFIED)
+- `_bmad-output/implementation-artifacts/sprint-status.yaml` (MODIFIED)
+
+## Change Log
+- 2026-02-14: Implemented security headers middleware (Story 14.2) — all 4 tasks complete, 6 tests added, 1,447 total tests passing
diff --git a/_bmad-output/implementation-artifacts/sprint-status.yaml b/_bmad-output/implementation-artifacts/sprint-status.yaml
@@ -195,9 +195,9 @@ development_status:
 
   # Epic 14: Security Hardening & Observability
   # User Outcome: "The app is hardened and observable — ready for beta users"
-  epic-14: pending
-  14-1-cors-configuration: pending
-  14-2-security-headers-middleware: pending
+  epic-14: in-progress
+  14-1-cors-configuration: done
+  14-2-security-headers-middleware: review
   14-3-api-rate-limiting: pending
   14-4-backend-sentry-integration: pending
   14-5-frontend-sentry-integration: pending
diff --git a/backend/src/PropertyManager.Api/Middleware/SecurityHeadersMiddleware.cs b/backend/src/PropertyManager.Api/Middleware/SecurityHeadersMiddleware.cs
@@ -0,0 +1,40 @@
+namespace PropertyManager.Api.Middleware;
+
+public class SecurityHeadersMiddleware
+{
+    private const string XFrameOptions = "DENY";
+    private const string XContentTypeOptions = "nosniff";
+    private const string ReferrerPolicy = "strict-origin-when-cross-origin";
+    private const string PermissionsPolicy = "camera=(), microphone=(), geolocation=(), payment=()";
+    private const string ContentSecurityPolicy =
+        "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; " +
+        "font-src 'self' https://fonts.gstatic.com; img-src 'self' data: blob:; " +
+        "connect-src 'self' https://*.ingest.sentry.io; frame-ancestors 'none'; " +
+        "object-src 'none'; base-uri 'self'; form-action 'self'";
+
+    private readonly RequestDelegate _next;
+
+    public SecurityHeadersMiddleware(RequestDelegate next)
+    {
+        _next = next;
+    }
+
+    public async Task InvokeAsync(HttpContext context)
+    {
+        context.Response.Headers["X-Frame-Options"] = XFrameOptions;
+        context.Response.Headers["X-Content-Type-Options"] = XContentTypeOptions;
+        context.Response.Headers["Referrer-Policy"] = ReferrerPolicy;
+        context.Response.Headers["Permissions-Policy"] = PermissionsPolicy;
+        context.Response.Headers["Content-Security-Policy"] = ContentSecurityPolicy;
+
+        await _next(context);
+    }
+}
+
+public static class SecurityHeadersMiddlewareExtensions
+{
+    public static IApplicationBuilder UseSecurityHeaders(this IApplicationBuilder app)
+    {
+        return app.UseMiddleware<SecurityHeadersMiddleware>();
+    }
+}
diff --git a/backend/src/PropertyManager.Api/Program.cs b/backend/src/PropertyManager.Api/Program.cs
@@ -236,6 +236,8 @@
 }
 
 app.UseHttpsRedirection();
+app.UseHsts();
+app.UseSecurityHeaders();
 app.UseSerilogRequestLogging();
 app.UseCors(corsPolicyName);
 
diff --git a/backend/tests/PropertyManager.Api.Tests/Middleware/SecurityHeadersMiddlewareTests.cs b/backend/tests/PropertyManager.Api.Tests/Middleware/SecurityHeadersMiddlewareTests.cs
@@ -0,0 +1,122 @@
+using FluentAssertions;
+using Microsoft.AspNetCore.Http;
+using PropertyManager.Api.Middleware;
+
+namespace PropertyManager.Api.Tests.Middleware;
+
+public class SecurityHeadersMiddlewareTests
+{
+    private readonly DefaultHttpContext _httpContext;
+    private readonly SecurityHeadersMiddleware _middleware;
+    private bool _nextCalled;
+
+    public SecurityHeadersMiddlewareTests()
+    {
+        _httpContext = new DefaultHttpContext();
+        _httpContext.Response.Body = new MemoryStream();
+        _nextCalled = false;
+
+        RequestDelegate next = _ =>
+        {
+            _nextCalled = true;
+            return Task.CompletedTask;
+        };
+
+        _middleware = new SecurityHeadersMiddleware(next);
+    }
+
+    [Fact]
+    public async Task InvokeAsync_AddsXFrameOptionsDeny()
+    {
+        await _middleware.InvokeAsync(_httpContext);
+
+        _httpContext.Response.Headers["X-Frame-Options"].ToString().Should().Be("DENY");
+    }
+
+    [Fact]
+    public async Task InvokeAsync_AddsXContentTypeOptionsNosniff()
+    {
+        await _middleware.InvokeAsync(_httpContext);
+
+        _httpContext.Response.Headers["X-Content-Type-Options"].ToString().Should().Be("nosniff");
+    }
+
+    [Fact]
+    public async Task InvokeAsync_AddsReferrerPolicy()
+    {
+        await _middleware.InvokeAsync(_httpContext);
+
+        _httpContext.Response.Headers["Referrer-Policy"].ToString().Should().Be("strict-origin-when-cross-origin");
+    }
+
+    [Fact]
+    public async Task InvokeAsync_AddsPermissionsPolicy()
+    {
+        await _middleware.InvokeAsync(_httpContext);
+
+        _httpContext.Response.Headers["Permissions-Policy"].ToString()
+            .Should().Be("camera=(), microphone=(), geolocation=(), payment=()");
+    }
+
+    [Fact]
+    public async Task InvokeAsync_AddsContentSecurityPolicy()
+    {
+        await _middleware.InvokeAsync(_httpContext);
+
+        var csp = _httpContext.Response.Headers["Content-Security-Policy"].ToString();
+        csp.Should().Contain("default-src 'self'");
+        csp.Should().Contain("script-src 'self'");
+        csp.Should().Contain("style-src 'self' 'unsafe-inline' https://fonts.googleapis.com");
+        csp.Should().Contain("font-src 'self' https://fonts.gstatic.com");
+        csp.Should().Contain("img-src 'self' data: blob:");
+        csp.Should().Contain("connect-src 'self' https://*.ingest.sentry.io");
+        csp.Should().Contain("frame-ancestors 'none'");
+        csp.Should().Contain("object-src 'none'");
+        csp.Should().Contain("base-uri 'self'");
+        csp.Should().Contain("form-action 'self'");
+    }
+
+    [Fact]
+    public async Task InvokeAsync_ContentSecurityPolicy_HasExactExpectedDirectiveCount()
+    {
+        await _middleware.InvokeAsync(_httpContext);
+
+        var csp = _httpContext.Response.Headers["Content-Security-Policy"].ToString();
+        var directives = csp.Split(';', StringSplitOptions.TrimEntries);
+        directives.Should().HaveCount(10);
+    }
+
+    [Fact]
+    public async Task InvokeAsync_CallsNextMiddleware()
+    {
+        await _middleware.InvokeAsync(_httpContext);
+
+        _nextCalled.Should().BeTrue();
+    }
+
+    [Fact]
+    public async Task InvokeAsync_HeadersSetEvenWhenNextThrows()
+    {
+        var context = new DefaultHttpContext();
+        context.Response.Body = new MemoryStream();
+
+        RequestDelegate throwingNext = _ => throw new InvalidOperationException("downstream failure");
+        var middleware = new SecurityHeadersMiddleware(throwingNext);
+
+        var act = () => middleware.InvokeAsync(context);
+
+        await act.Should().ThrowAsync<InvalidOperationException>();
+        context.Response.Headers["X-Frame-Options"].ToString().Should().Be("DENY");
+        context.Response.Headers["Content-Security-Policy"].ToString().Should().NotBeEmpty();
+    }
+
+    [Fact]
+    public async Task InvokeAsync_OverwritesPreExistingHeaders()
+    {
+        _httpContext.Response.Headers["X-Frame-Options"] = "SAMEORIGIN";
+
+        await _middleware.InvokeAsync(_httpContext);
+
+        _httpContext.Response.Headers["X-Frame-Options"].ToString().Should().Be("DENY");
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -236,6 +236,8 @@`
`236`	`236`	`}`
`237`	`237`
`238`	`238`	`app.UseHttpsRedirection();`
	`239`	`+app.UseHsts();`
	`240`	`+app.UseSecurityHeaders();`
`239`	`241`	`app.UseSerilogRequestLogging();`
`240`	`242`	`app.UseCors(corsPolicyName);`
`241`	`243`