Fix demo auth privilege escalation and approval attribution

docker-compose.yml

@@ -4,10 +4,13 @@ services:
       context: .
       dockerfile: Dockerfile
     ports:
-      - "8888:5000"
+      - "127.0.0.1:8888:5000"
     environment:
       ASPNETCORE_ENVIRONMENT: ${ASPNETCORE_ENVIRONMENT:-Development}
       ASPNETCORE_URLS: ${ASPNETCORE_URLS:-http://+:5000}
+      AUTH_MODE: ${AUTH_MODE:-demo}
+      SECUREFIX_DEMO_TOKEN: ${SECUREFIX_DEMO_TOKEN:-}
+      SECUREFIX_DEMO_REVIEWER_TOKEN: ${SECUREFIX_DEMO_REVIEWER_TOKEN:-}
       DATABASE_URL: ${DATABASE_URL:-sqlite:securefix.db}
       DATABASE_PROVIDER: ${DATABASE_PROVIDER:-sqlite}
       AI_PROVIDER: ${AI_PROVIDER:-mock}

docs/runbook.md

@@ -21,8 +21,10 @@
 - **Rotate/verify configuration**: confirm `AUTH_MODE=entra` and `AzureAd__TenantId`,
   `AzureAd__ClientId`, `AzureAd__Audience` are set in the deployment environment. The API
   fails to start in `Production` if `AUTH_MODE` is not `entra` (fail closed).
-- **Local/dev only**: `AUTH_MODE=demo` uses a static bearer token (`SECUREFIX_DEMO_TOKEN`)
-  and a self-asserted `X-User-Role` header — never use this mode outside local development.
+- **Local/dev only**: `AUTH_MODE=demo` uses `SECUREFIX_DEMO_TOKEN` for the fixed Developer
+  identity and, optionally, `SECUREFIX_DEMO_REVIEWER_TOKEN` for the fixed SecurityReviewer
+  identity. Set distinct, non-empty, high-entropy values; role and identity headers are ignored.
+  The Compose port is bound to `127.0.0.1`. Never use this mode outside local development.
 
 ## Failure handling
 

docs/security-model.md

@@ -21,9 +21,12 @@ The API supports two authentication modes, selected via `AUTH_MODE`:
   using Microsoft.Identity.Web. Tokens must have the correct issuer, audience, signature,
   and expiration. Roles are asserted by Entra ID in the token's `roles` claim, which
   Microsoft.Identity.Web maps to `ClaimTypes.Role` for use by `[Authorize(Roles = ...)]`.
-- `demo` (local/dev only): a static bearer token plus a self-asserted `X-User-Role` header.
-  This mode is intentionally weak — it exists only for offline demos — and the API fails
-  to start if `ASPNETCORE_ENVIRONMENT=Production` and `AUTH_MODE` is not `entra` (fail closed).
+- `demo` (local/dev only): explicitly configured `SECUREFIX_DEMO_TOKEN` and optional
+  `SECUREFIX_DEMO_REVIEWER_TOKEN` bearer tokens map to fixed Developer and SecurityReviewer
+  identities. Client-supplied identity and role headers are ignored. Configure distinct,
+  non-empty, high-entropy tokens for local testing; requests are unauthenticated when a token
+  is unset. Compose publishes only on loopback by default. The API fails to start if
+  `ASPNETCORE_ENVIRONMENT=Production` and `AUTH_MODE` is not `entra` (fail closed).
 
 App roles are defined once on the Entra ID app registration (see
 `infra/entra-app-registration.sh`) and assigned to users/groups from the Enterprise
@@ -40,4 +43,3 @@ Application's "Users and groups" blade — never granted by the application itse
 
 Secrets must come from environment variables or managed secret storage in production.
 No credentials are committed to source control.
-

src/SecureFix.Api/AuthenticationConfigurator.cs

@@ -10,8 +10,8 @@ namespace SecureFix.Api;
 /// - "entra" (recommended, required in Production): validates Microsoft Entra ID issued
 ///   JWTs via Microsoft.Identity.Web. App roles (Admin, SecurityReviewer, Developer, Viewer)
 ///   are read from the token's "roles" claim and mapped to ClaimTypes.Role automatically.
-/// - "demo" (local/dev only): uses a static bearer token + X-User-Role header. This mode
-///   is intentionally weak and must never be enabled in Production.
+/// - "demo" (local/dev only): uses explicitly configured developer and reviewer bearer
+///   tokens with fixed identities and roles. This mode must never be enabled in Production.
 /// </summary>
 public static class AuthenticationConfigurator
 {

src/SecureFix.Api/Controllers/WorkflowsController.cs

@@ -88,7 +88,7 @@
     /// Only SecurityReviewer role is authorized.
     /// </summary>
     /// <param name="id">Alert/Workflow ID.</param>
-    /// <param name="request">Approval details (reviewer, optional reason).</param>
+    /// <param name="request">Approval decision and optional reason.</param>
     /// <returns>Updated workflow status.</returns>
     /// <response code="200">Workflow approved successfully.</response>
     /// <response code="400">Invalid request (validation failed, already decided, workflow not found).</response>
@@ -116,7 +116,7 @@
             });
         }
 
         if (request == null || request.Decision?.ToLower() != "approved")
         {
             return BadRequest(new ProblemDetails
             {
@@ -126,22 +126,16 @@
             });
         }
 
-        var reviewerRole = User.FindFirstValue(ClaimTypes.Role) ?? request.ReviewerRole ?? "SecurityReviewer";
-        if (string.IsNullOrWhiteSpace(request.Reviewer))
+        if (!TryGetAuthenticatedReviewer(out var reviewer, out var reviewerRole))
         {
-            return BadRequest(new ProblemDetails
-            {
-                Title = "Invalid Reviewer",
-                Status = StatusCodes.Status400BadRequest,
-                Detail = "Reviewer identity is required."
-            });
+            return Forbid();
         }
 
         try
         {
-            _logger.LogInformation("Approving workflow {WorkflowId} by {Reviewer} as {Role}", id, request.Reviewer, reviewerRole);
+            _logger.LogInformation("Approving workflow {WorkflowId} by {Reviewer} as {Role}", id, reviewer, reviewerRole);
 
-            var updatedStatus = await _approvalService.ApproveAlertAsync(id, request.Reviewer, request.Reason, reviewerRole);
+            var updatedStatus = await _approvalService.ApproveAlertAsync(id, reviewer, request.Reason, reviewerRole);
 
             return Ok(updatedStatus);
         }
@@ -203,7 +197,7 @@
     /// Only SecurityReviewer role is authorized.
     /// </summary>
     /// <param name="id">Alert/Workflow ID.</param>
-    /// <param name="request">Rejection details (reviewer, optional reason).</param>
+    /// <param name="request">Rejection decision and optional reason.</param>
     /// <returns>Updated workflow status (Rejected).</returns>
     /// <response code="200">Workflow rejected successfully.</response>
     /// <response code="400">Invalid request (validation failed, already decided, workflow not found).</response>
@@ -231,7 +225,7 @@
             });
         }
 
         if (request == null || request.Decision?.ToLower() != "rejected")
         {
             return BadRequest(new ProblemDetails
             {
@@ -241,22 +235,16 @@
             });
         }
 
-        var reviewerRole = User.FindFirstValue(ClaimTypes.Role) ?? request.ReviewerRole ?? "SecurityReviewer";
-        if (string.IsNullOrWhiteSpace(request.Reviewer))
+        if (!TryGetAuthenticatedReviewer(out var reviewer, out var reviewerRole))
         {
-            return BadRequest(new ProblemDetails
-            {
-                Title = "Invalid Reviewer",
-                Status = StatusCodes.Status400BadRequest,
-                Detail = "Reviewer identity is required."
-            });
+            return Forbid();
         }
 
         try
         {
-            _logger.LogInformation("Rejecting workflow {WorkflowId} by {Reviewer} as {Role}", id, request.Reviewer, reviewerRole);
+            _logger.LogInformation("Rejecting workflow {WorkflowId} by {Reviewer} as {Role}", id, reviewer, reviewerRole);
 
-            var updatedStatus = await _approvalService.RejectAlertAsync(id, request.Reviewer, request.Reason, reviewerRole);
+            var updatedStatus = await _approvalService.RejectAlertAsync(id, reviewer, request.Reason, reviewerRole);
 
             return Ok(updatedStatus);
         }
@@ -312,4 +300,13 @@
             });
         }
     }
+
+    private bool TryGetAuthenticatedReviewer(out string reviewer, out string reviewerRole)
+    {
+        reviewer = User.FindFirstValue(ClaimTypes.NameIdentifier) ?? User.Identity?.Name ?? string.Empty;
+        reviewerRole = User.FindFirstValue(ClaimTypes.Role) ?? string.Empty;
+
+        return !string.IsNullOrWhiteSpace(reviewer)
+            && (reviewerRole is "SecurityReviewer" or "Admin");
+    }
 }

src/SecureFix.Api/DemoAuthenticationHandler.cs

@@ -26,14 +26,20 @@ protected override Task<AuthenticateResult> HandleAuthenticateAsync()
         }
 
         var token = authorization["Bearer ".Length..].Trim();
-        var expectedToken = Environment.GetEnvironmentVariable("SECUREFIX_DEMO_TOKEN") ?? "securefix-demo-token";
-        if (!string.Equals(token, expectedToken, StringComparison.Ordinal))
+        var developerToken = Environment.GetEnvironmentVariable("SECUREFIX_DEMO_TOKEN");
+        var reviewerToken = Environment.GetEnvironmentVariable("SECUREFIX_DEMO_REVIEWER_TOKEN");
+        var isDeveloper = !string.IsNullOrWhiteSpace(developerToken)
+            && string.Equals(token, developerToken, StringComparison.Ordinal);
+        var isReviewer = !string.IsNullOrWhiteSpace(reviewerToken)
+            && string.Equals(token, reviewerToken, StringComparison.Ordinal);
+
+        if ((!isDeveloper && !isReviewer) || (isDeveloper && isReviewer))
         {
-            return Task.FromResult(AuthenticateResult.Fail("Invalid demo token."));
+            return Task.FromResult(AuthenticateResult.Fail("Invalid or misconfigured demo credentials."));
         }
 
-        var userId = Request.Headers["X-User-Id"].FirstOrDefault() ?? "demo-user";
-        var role = Request.Headers["X-User-Role"].FirstOrDefault() ?? "Developer";
+        var userId = isReviewer ? "demo-security-reviewer" : "demo-developer";
+        var role = isReviewer ? "SecurityReviewer" : "Developer";
 
         var claims = new[]
         {

src/SecureFix.Api/SecureFix.Api.csproj

@@ -9,7 +9,7 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="10.0.3" />
     <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="10.0.11" />
-    <PackageReference Include="Microsoft.Identity.Web" Version="3.8.2" />
+    <PackageReference Include="Microsoft.Identity.Web" Version="4.16.0" />
     <PackageReference Include="Microsoft.OpenApi" Version="2.7.5" />
     <PackageReference Include="Serilog.AspNetCore" Version="10.0.0" />
     <PackageReference Include="Serilog.Sinks.Console" Version="6.1.1" />

src/SecureFix.Core/Models/ApprovalRequestDto.cs

@@ -8,15 +8,13 @@ namespace SecureFix.Core.Models;
 public class ApprovalRequestDto
 {
     /// <summary>
-    /// Reviewer identity (user ID or email).
+    /// Legacy client field; ignored. Reviewer identity comes from authenticated claims.
     /// </summary>
-    [Required]
     [StringLength(255)]
-    public string Reviewer { get; set; } = null!;
+    public string? Reviewer { get; set; }
 
     /// <summary>
-    /// Role of the reviewer submitting the decision.
-    /// Only SecurityReviewer and Admin are allowed to approve or reject workflow actions.
+    /// Legacy client field; ignored. Reviewer role comes from authenticated claims.
     /// </summary>
     [StringLength(100)]
     public string? ReviewerRole { get; set; }

tests/SecureFix.Tests/WorkflowsControllerSecurityTests.cs

@@ -0,0 +1,129 @@
+namespace SecureFix.Tests;
+
+using System.Security.Claims;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using SecureFix.Api.Controllers;
+using SecureFix.Core.Models;
+using SecureFix.Core.Services;
+
+public class WorkflowsControllerSecurityTests
+{
+    [Fact]
+    public async Task ApproveWorkflow_UsesAuthenticatedReviewerInsteadOfRequestIdentity()
+    {
+        var response = new WorkflowStatusResponse();
+        var approvalService = new Mock<IApprovalService>();
+        approvalService
+            .Setup(service => service.ApproveAlertAsync(
+                "workflow-1",
+                "authenticated-reviewer",
+                "reviewed",
+                "SecurityReviewer"))
+            .ReturnsAsync(response);
+        var controller = CreateController(approvalService, "authenticated-reviewer", "SecurityReviewer");
+
+        var result = await controller.ApproveWorkflow("workflow-1", new ApprovalRequestDto
+        {
+            Decision = "approved",
+            Reviewer = "forged-reviewer",
+            ReviewerRole = "Admin",
+            Reason = "reviewed"
+        });
+
+        Assert.Same(response, Assert.IsType<OkObjectResult>(result).Value);
+        approvalService.VerifyAll();
+    }
+
+    [Fact]
+    public async Task RejectWorkflow_UsesAuthenticatedReviewerInsteadOfRequestIdentity()
+    {
+        var response = new WorkflowStatusResponse();
+        var approvalService = new Mock<IApprovalService>();
+        approvalService
+            .Setup(service => service.RejectAlertAsync(
+                "workflow-1",
+                "authenticated-reviewer",
+                "needs more testing",
+                "SecurityReviewer"))
+            .ReturnsAsync(response);
+        var controller = CreateController(approvalService, "authenticated-reviewer", "SecurityReviewer");
+
+        var result = await controller.RejectWorkflow("workflow-1", new ApprovalRequestDto
+        {
+            Decision = "rejected",
+            Reviewer = "forged-reviewer",
+            ReviewerRole = "Admin",
+            Reason = "needs more testing"
+        });
+
+        Assert.Same(response, Assert.IsType<OkObjectResult>(result).Value);
+        approvalService.VerifyAll();
+    }
+
+    [Fact]
+    public async Task ApproveWorkflow_RejectsCallerWithoutAuthenticatedReviewerIdentity()
+    {
+        var approvalService = new Mock<IApprovalService>();
+        var controller = CreateController(approvalService, null, "SecurityReviewer");
+
+        var result = await controller.ApproveWorkflow("workflow-1", new ApprovalRequestDto
+        {
+            Decision = "approved",
+            Reviewer = "forged-reviewer",
+            ReviewerRole = "SecurityReviewer"
+        });
+
+        Assert.IsType<ForbidResult>(result);
+        approvalService.VerifyNoOtherCalls();
+    }
+
+    [Fact]
+    public async Task ApproveWorkflow_RejectsDeveloperDespiteReviewerRoleInRequest()
+    {
+        var approvalService = new Mock<IApprovalService>();
+        var controller = CreateController(approvalService, "authenticated-developer", "Developer");
+
+        var result = await controller.ApproveWorkflow("workflow-1", new ApprovalRequestDto
+        {
+            Decision = "approved",
+            Reviewer = "authenticated-developer",
+            ReviewerRole = "SecurityReviewer"
+        });
+
+        Assert.IsType<ForbidResult>(result);
+        approvalService.VerifyNoOtherCalls();
+    }
+
+    private static WorkflowsController CreateController(
+        Mock<IApprovalService> approvalService,
+        string? reviewerIdentity,
+        string role)
+    {
+        var claims = new List<Claim>
+        {
+            new(ClaimTypes.Role, role)
+        };
+        if (reviewerIdentity is not null)
+        {
+            claims.Add(new Claim(ClaimTypes.NameIdentifier, reviewerIdentity));
+        }
+
+        var controller = new WorkflowsController(
+            approvalService.Object,
+            NullLogger<WorkflowsController>.Instance)
+        {
+            ControllerContext = new ControllerContext
+            {
+                HttpContext = new DefaultHttpContext
+                {
+                    User = new ClaimsPrincipal(new ClaimsIdentity(claims, "test"))
+                }
+            }
+        };
+
+        return controller;
+    }
+}