diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c2ff850..aa6ff26b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,9 +18,34 @@ This project follows Semantic Versioning using the format `MAJOR.MINOR.PATCH`. differ from earlier releases; string and numeric digests are unchanged. * Clarified that the `Hash` disposition is an unkeyed integrity digest with no confidentiality for low-entropy values; use `HmacSha256` instead. +* **Behavior change for adopters:** MVC now validates antiforgery tokens on + every unsafe request (`POST`, `PUT`, `PATCH`, `DELETE`) through a global + `AutoValidateAntiforgeryTokenAttribute`. Actions that must accept requests + without a token, such as token-authenticated APIs that do not use cookies, + need `[IgnoreAntiforgeryToken]`. +* **Behavior change:** claims transformation now returns a new principal + instead of editing the incoming one, and sets each identity's `NameClaimType` + and `RoleClaimType` to `application:name` and `application:role`, so + `User.Identity.Name`, `User.IsInRole`, and `[Authorize(Roles = "...")]` keep + working when `RemoveOriginalClaims` is enabled. +* Paths in `SecurityHeaders:ExcludedPathPrefixes` (`/health`, `/metrics`) now + still receive `X-Content-Type-Options: nosniff`. +* HSTS is implemented by `UseApplicationHsts()` in `SecurityHeadersExtensions` + and invoked from the same slot in `UseProblemDetails()`; the pipeline order is + unchanged. +* Authorization policies are built from the bound and validated + `ApplicationAuthorizationOptions` instead of a separate configuration snapshot + read at registration. +* `ApplicationSecurityHeadersOptions.SectionName` and + `ApplicationRequestLoggingOptions.SectionName` are now `const`. ### Fixed +* Audit actor attribution (`HttpContextCurrentActorAccessor` and + `HttpContextApplicationAuditContextAccessor`) now reads the normalized + `application:subject` claim first. With `RemoveOriginalClaims` enabled, + authenticated users were previously recorded as `Remote IP: ...`. + * Audit reconciliation runs now persist findings in a single transaction inside the execution strategy (or join a caller-owned transaction), guard every finding update with its `ConcurrencyStamp`, and insert findings only when the diff --git a/docs/articles/error-handling.md b/docs/articles/error-handling.md index 677290ec..e3063030 100644 --- a/docs/articles/error-handling.md +++ b/docs/articles/error-handling.md @@ -10,7 +10,7 @@ Error handling is configured through the application pipeline using: app.UseProblemDetails(); ``` -This is the single error-handling registration. It adds the developer exception page in Development, or the production exception handler and HSTS outside it, and then branches status-code handling between Problem Details responses and the re-executed browser error page. +This is the single error-handling registration. It adds the developer exception page in Development, or the production exception handler and HSTS (`UseApplicationHsts()`) outside it, and then branches status-code handling between Problem Details responses and the re-executed browser error page. The error handling behavior is environment-aware: diff --git a/docs/articles/health-checks.md b/docs/articles/health-checks.md index 4aaf9b71..7ca24d8a 100644 --- a/docs/articles/health-checks.md +++ b/docs/articles/health-checks.md @@ -122,7 +122,7 @@ The default security header configuration excludes `/health`: ] ``` -Because the exclusion is prefix-based, `/health`, `/health/ready`, and `/health/live` are all excluded from security header application. This keeps health probe responses small and infrastructure-friendly. +Because the exclusion is prefix-based, `/health`, `/health/ready`, and `/health/live` are all excluded from the security header middleware, except `X-Content-Type-Options: nosniff`. `Strict-Transport-Security` is registered separately and still applies to HTTPS health responses outside Development. This keeps health probe responses small and infrastructure-friendly. ## Contract References diff --git a/docs/articles/middleware.md b/docs/articles/middleware.md index 15e7c2be..8e0087e2 100644 --- a/docs/articles/middleware.md +++ b/docs/articles/middleware.md @@ -30,7 +30,7 @@ The pipeline order is: 11. Authorization 12. Controller and Razor Page endpoint mapping -Error handling is one step, not two. `UseProblemDetails()` adds the developer exception page in Development, or the production exception handler and HSTS outside it, and then branches status-code handling between Problem Details responses and the re-executed browser error page. Registering a second environment-aware error-handling extension alongside it would add a duplicate exception handler, a duplicate HSTS middleware, and an unconditional status-code re-execute wrapping the classified one. +Error handling is one step, not two. `UseProblemDetails()` adds the developer exception page in Development, or the production exception handler and HSTS (`UseApplicationHsts()`) outside it, and then branches status-code handling between Problem Details responses and the re-executed browser error page. Registering a second environment-aware error-handling extension alongside it would add a duplicate exception handler, a duplicate HSTS middleware, and an unconditional status-code re-execute wrapping the classified one. This order keeps proxy correction early, request logging close to the beginning of the request, error handling ahead of most application behavior, and endpoint-specific features such as CORS and rate limiting after routing. diff --git a/docs/articles/security-headers.md b/docs/articles/security-headers.md index 4da2a71c..21ef457b 100644 --- a/docs/articles/security-headers.md +++ b/docs/articles/security-headers.md @@ -26,7 +26,7 @@ app.UseApplicationSecurityHeaders(); ## v1.0 Security Header Contract -This contract applies when `ProjectTemplate:SecurityHeaders:Enabled` is `true` and the request path does not match `ExcludedPathPrefixes`. +This contract applies when `ProjectTemplate:SecurityHeaders:Enabled` is `true` and the request path does not match `ExcludedPathPrefixes`. Responses on excluded paths still receive `X-Content-Type-Options: nosniff` and no other header from this middleware. `Strict-Transport-Security` is registered separately (see [HSTS and Transport Security](#hsts-and-transport-security)) and also applies to excluded paths on HTTPS requests outside Development. | Header | Default | Contract | Configuration | |:---|:---|:---|:---| @@ -39,7 +39,7 @@ This contract applies when `ProjectTemplate:SecurityHeaders:Enabled` is `true` a | `Permissions-Policy` | `camera=(), microphone=(), geolocation=(), payment=(), usb=(), fullscreen=(self)` | Configurable | Controlled by `EnablePermissionsPolicy` and `PermissionsPolicy` | | `Content-Security-Policy` | `default-src 'self'; base-uri 'self'; object-src 'none'; frame-ancestors 'none'; form-action 'self'; img-src 'self' data:; script-src 'self'; style-src 'self';` | Configurable | Controlled by `EnableContentSecurityPolicy` and `ContentSecurityPolicy` | | `X-XSS-Protection` | Not emitted | Intentionally omitted | Not supported | -| `Strict-Transport-Security` | Not emitted | Intentionally omitted | See [HSTS and Transport Security](#hsts-and-transport-security) | +| `Strict-Transport-Security` | Not emitted by this middleware | Emitted by `UseApplicationHsts()` outside Development | See [HSTS and Transport Security](#hsts-and-transport-security) | The middleware intentionally does not add `X-XSS-Protection` because that header is obsolete and can create inconsistent behavior in modern browsers. @@ -55,15 +55,19 @@ whoever owns the certificate, the origin, and the rollback path, which in most deployments is the reverse proxy, ingress controller, CDN, or host platform rather than the application process. -NCAT therefore emits headers that are safe to apply per-response and defers HSTS -to an explicit deployment decision. +NCAT therefore emits headers that are safe to apply per-response from this +middleware, registers HSTS separately with framework defaults, and leaves HSTS +values to an explicit deployment decision. ### Where HSTS belongs ASP.NET Core provides `UseHsts()` and `AddHsts(...)` for application-emitted HSTS. -The generated pipeline does not call `UseHsts()`. A consuming application may add -it, or may leave HSTS to the edge. Emitting it from both layers is not an error, -but only one layer should own the values. +The generated pipeline calls `UseHsts()` outside Development through +`UseApplicationHsts()` (defined in `SecurityHeadersExtensions`, registered by +`UseProblemDetails()` between the exception handler and status-code pages), with the ASP.NET Core +defaults. A consuming application that leaves HSTS to the edge may remove that +call. Emitting it from both layers is not an error, but only one layer should +own the values. | Layer | When it is the right owner | | ------------------------------------ | --------------------------------------------------------------------------------------------------------------- | @@ -96,8 +100,9 @@ Whichever layer owns HSTS, the following are explicit choices, not defaults: default. Do not add development hosts to an HSTS policy; a cached directive on a developer machine outlives the branch that caused it. -NCAT does not validate, emit, or test HSTS behavior. An application that adopts -HSTS owns its values, its rollout, and its rollback. +Apart from registering `UseHsts()` with framework defaults, NCAT does not +configure, validate, or test HSTS values. An application that adopts HSTS owns +its values, its rollout, and its rollback. ## Intentional Opt-Outs @@ -109,7 +114,7 @@ The following settings reduce or remove default browser hardening and should be | `EnableContentSecurityPolicy = false` | Removes CSP | Temporary troubleshooting or applications that must define CSP elsewhere | | `EnablePermissionsPolicy = false` | Removes Permissions-Policy | Only when browser feature policy is managed elsewhere | | `EnableCrossOriginHeaders = false` | Removes COOP and CORP | Applications that intentionally integrate cross-origin windows or resources | -| `ExcludedPathPrefixes` | Skips all security headers for matching paths | Infrastructure endpoints such as `/health` and `/metrics` | +| `ExcludedPathPrefixes` | Skips every security header except `X-Content-Type-Options` for matching paths | Infrastructure endpoints such as `/health` and `/metrics` | ## Configuration @@ -141,7 +146,7 @@ Security headers can be configured from `appsettings.json`: |`EnableCrossOriginHeaders`|Controls whether `Cross-Origin-Opener-Policy` and `Cross-Origin-Resource-Policy` are applied.| |`ContentSecurityPolicy`|Defines the application Content Security Policy value.| |`PermissionsPolicy`|Defines the Permissions Policy value.| -|`ExcludedPathPrefixes`|Skips security header application for matching request path prefixes.| +|`ExcludedPathPrefixes`|Skips security header application, except `X-Content-Type-Options: nosniff`, for matching request path prefixes.| ## Environment-Specific Behavior diff --git a/src/ProjectTemplate.Web/Accessors/HttpContextApplicationAuditContextAccessor.cs b/src/ProjectTemplate.Web/Accessors/HttpContextApplicationAuditContextAccessor.cs index 60286e62..96a67246 100644 --- a/src/ProjectTemplate.Web/Accessors/HttpContextApplicationAuditContextAccessor.cs +++ b/src/ProjectTemplate.Web/Accessors/HttpContextApplicationAuditContextAccessor.cs @@ -1,6 +1,7 @@ using System.Diagnostics; using System.Security.Claims; using ProjectTemplate.Infrastructure.Data.Auditing; +using ProjectTemplate.Web.Authentication.Claims; namespace ProjectTemplate.Web.Accessors; @@ -20,7 +21,9 @@ public ApplicationAuditContext Current HttpContext? httpContext = httpContextAccessor.HttpContext; ClaimsPrincipal? user = httpContext?.User; - string? subject = GetAuthenticatedClaim(user, _subjectClaimType) + // Prefer the normalized application claim: claims transformation may remove the provider claims. + string? subject = GetAuthenticatedClaim(user, ApplicationClaimTypes.Subject) + ?? GetAuthenticatedClaim(user, _subjectClaimType) ?? GetAuthenticatedClaim(user, ClaimTypes.NameIdentifier); if (!string.IsNullOrWhiteSpace(subject)) diff --git a/src/ProjectTemplate.Web/Accessors/HttpContextCurrentActorAccessor.cs b/src/ProjectTemplate.Web/Accessors/HttpContextCurrentActorAccessor.cs index f43379db..56069ee4 100644 --- a/src/ProjectTemplate.Web/Accessors/HttpContextCurrentActorAccessor.cs +++ b/src/ProjectTemplate.Web/Accessors/HttpContextCurrentActorAccessor.cs @@ -1,5 +1,6 @@ using System.Security.Claims; using ProjectTemplate.Infrastructure.Data; +using ProjectTemplate.Web.Authentication.Claims; namespace ProjectTemplate.Web.Accessors; @@ -16,7 +17,7 @@ public sealed class HttpContextCurrentActorAccessor( /// /// Accesses the current actor information from the HTTP context. It first attempts to retrieve the authenticated - /// subject claim from the user's claims, then falls back to the authenticated name identifier claim, then the remote + /// normalized application subject claim, then the provider subject claim, then falls back to the authenticated name identifier claim, then the remote /// IP address. If none are available, it returns "Unknown". /// public string CurrentActor @@ -47,7 +48,10 @@ public string CurrentActor return null; } - string? subject = GetClaimValue(user, _subjectClaimType); + // Prefer the normalized application claim: when claims transformation removes the original claims, + // the provider "sub" and name identifier claims are no longer present. + string? subject = GetClaimValue(user, ApplicationClaimTypes.Subject) + ?? GetClaimValue(user, _subjectClaimType); if (!string.IsNullOrWhiteSpace(subject)) { diff --git a/src/ProjectTemplate.Web/Authentication/Claims/ApplicationClaimsTransformation.cs b/src/ProjectTemplate.Web/Authentication/Claims/ApplicationClaimsTransformation.cs index 943177c6..bacb6b53 100644 --- a/src/ProjectTemplate.Web/Authentication/Claims/ApplicationClaimsTransformation.cs +++ b/src/ProjectTemplate.Web/Authentication/Claims/ApplicationClaimsTransformation.cs @@ -28,8 +28,13 @@ public Task TransformAsync(ClaimsPrincipal principal) return Task.FromResult(principal); } - foreach (ClaimsIdentity identity in principal.Identities.OfType()) + // Transformation may run more than once per request, and the incoming principal can be shared with other + // components, so normalize copies instead of editing the caller's principal in place. + var transformed = new ClaimsPrincipal(); + + foreach (ClaimsIdentity source in principal.Identities) { + ClaimsIdentity identity = source.Clone(); ApplicationClaimMappingOptions mappings = ResolveMappings(options, identity.AuthenticationType); NormalizeClaim(identity, ApplicationClaimTypes.Subject, mappings.Subject, options.RemoveOriginalClaims); @@ -38,9 +43,41 @@ public Task TransformAsync(ClaimsPrincipal principal) NormalizeClaim(identity, ApplicationClaimTypes.Role, mappings.Role, options.RemoveOriginalClaims); NormalizeClaim(identity, ApplicationClaimTypes.Group, mappings.Group, options.RemoveOriginalClaims); NormalizeClaim(identity, ApplicationClaimTypes.Permission, mappings.Permission, options.RemoveOriginalClaims); + + transformed.AddIdentity(WithNormalizedNameAndRoleClaimTypes(identity)); } - return Task.FromResult(principal); + return Task.FromResult(transformed); + } + + // Points Identity.Name, User.IsInRole, and [Authorize(Roles = "...")] at application:name and + // application:role, so they keep working once RemoveOriginalClaims strips the provider claims. The original + // claim type is kept only when the identity still carries it and has no normalized equivalent (for example, + // when the provider's type is not in the configured mappings). + private static ClaimsIdentity WithNormalizedNameAndRoleClaimTypes(ClaimsIdentity identity) + { + string nameClaimType = ResolveClaimType(identity, ApplicationClaimTypes.Name, identity.NameClaimType); + string roleClaimType = ResolveClaimType(identity, ApplicationClaimTypes.Role, identity.RoleClaimType); + + return string.Equals(nameClaimType, identity.NameClaimType, StringComparison.Ordinal) + && string.Equals(roleClaimType, identity.RoleClaimType, StringComparison.Ordinal) + ? identity + : new ClaimsIdentity(identity.Claims, identity.AuthenticationType, nameClaimType, roleClaimType) + { + Actor = identity.Actor, + BootstrapContext = identity.BootstrapContext, + Label = identity.Label + }; + } + + private static string ResolveClaimType(ClaimsIdentity identity, string normalizedClaimType, string currentClaimType) + { + bool hasNormalized = identity.HasClaim(claim => + string.Equals(claim.Type, normalizedClaimType, StringComparison.OrdinalIgnoreCase)); + bool hasCurrent = identity.HasClaim(claim => + string.Equals(claim.Type, currentClaimType, StringComparison.OrdinalIgnoreCase)); + + return hasNormalized || !hasCurrent ? normalizedClaimType : currentClaimType; } private static ApplicationClaimMappingOptions ResolveMappings( diff --git a/src/ProjectTemplate.Web/Authentication/Extensions/AuthorizationServiceExtensions.cs b/src/ProjectTemplate.Web/Authentication/Extensions/AuthorizationServiceExtensions.cs index 758f255d..c1ceeb41 100644 --- a/src/ProjectTemplate.Web/Authentication/Extensions/AuthorizationServiceExtensions.cs +++ b/src/ProjectTemplate.Web/Authentication/Extensions/AuthorizationServiceExtensions.cs @@ -1,6 +1,5 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.Extensions.Options; -using ProjectTemplate.Web.Authentication.Claims; using ProjectTemplate.Web.Authentication.Options; namespace ProjectTemplate.Web.Authentication.Extensions; @@ -23,18 +22,6 @@ public static IServiceCollection AddApplicationAuthorization( ArgumentNullException.ThrowIfNull(services); ArgumentNullException.ThrowIfNull(configuration); - ApplicationAuthorizationOptions options = configuration - .GetSection(ApplicationAuthorizationOptions.SectionName) - .Get() ?? new ApplicationAuthorizationOptions(); - - string roleClaimType = string.IsNullOrWhiteSpace(options.RoleClaimType) - ? ApplicationClaimTypes.Role - : options.RoleClaimType; - - string permissionClaimType = string.IsNullOrWhiteSpace(options.PermissionClaimType) - ? ApplicationClaimTypes.Permission - : options.PermissionClaimType; - services .AddOptions() .Bind(configuration.GetSection(ApplicationAuthorizationOptions.SectionName)) @@ -57,30 +44,42 @@ public static IServiceCollection AddApplicationAuthorization( "ProjectTemplate:Authorization:ManageApplicationPermissions must contain at least one non-empty value.") .ValidateOnStart(); - services.AddAuthorizationBuilder() - .AddPolicy( - ApplicationAuthorizationPolicyNames.AuthenticatedUser, - policy => policy.RequireAuthenticatedUser()) - .AddPolicy( - ApplicationAuthorizationPolicyNames.AdministratorRole, - policy => - { - policy.RequireAuthenticatedUser(); - policy.RequireClaim(roleClaimType, options.AdministratorRoles); - }) - .AddPolicy( - ApplicationAuthorizationPolicyNames.ManageApplicationPermission, - policy => - { - policy.RequireAuthenticatedUser(); - policy.RequireClaim(permissionClaimType, options.ManageApplicationPermissions); - }); + _ = services.AddAuthorization(); + // Build every policy from the bound and validated ApplicationAuthorizationOptions instance, so the + // policies and the validators above always see the same values. services .AddOptions() - .Configure>((authorizationOptions, applicationAuthorizationOptions) => authorizationOptions.FallbackPolicy = applicationAuthorizationOptions.Value.RequireAuthenticatedUserByDefault + .Configure>((authorizationOptions, applicationAuthorizationOptionsAccessor) => + { + ApplicationAuthorizationOptions applicationAuthorizationOptions = applicationAuthorizationOptionsAccessor.Value; + + authorizationOptions.AddPolicy( + ApplicationAuthorizationPolicyNames.AuthenticatedUser, + policy => policy.RequireAuthenticatedUser()); + authorizationOptions.AddPolicy( + ApplicationAuthorizationPolicyNames.AdministratorRole, + policy => + { + policy.RequireAuthenticatedUser(); + policy.RequireClaim( + applicationAuthorizationOptions.RoleClaimType, + applicationAuthorizationOptions.AdministratorRoles); + }); + authorizationOptions.AddPolicy( + ApplicationAuthorizationPolicyNames.ManageApplicationPermission, + policy => + { + policy.RequireAuthenticatedUser(); + policy.RequireClaim( + applicationAuthorizationOptions.PermissionClaimType, + applicationAuthorizationOptions.ManageApplicationPermissions); + }); + + authorizationOptions.FallbackPolicy = applicationAuthorizationOptions.RequireAuthenticatedUserByDefault ? new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build() - : null); + : null; + }); return services; } diff --git a/src/ProjectTemplate.Web/ErrorHandling/ProblemDetailsExtensions.cs b/src/ProjectTemplate.Web/ErrorHandling/ProblemDetailsExtensions.cs index f2a0d253..865e9215 100644 --- a/src/ProjectTemplate.Web/ErrorHandling/ProblemDetailsExtensions.cs +++ b/src/ProjectTemplate.Web/ErrorHandling/ProblemDetailsExtensions.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Diagnostics; using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; +using ProjectTemplate.Web.Extensions; using ProjectTemplate.Web.Options; namespace ProjectTemplate.Web.ErrorHandling; @@ -85,8 +86,9 @@ public static IServiceCollection AddApplicationProblemDetails( /// environment. /// /// In the development environment, this method enables the developer exception page. In other - /// environments, it configures a generic exception handler and enforces HTTP Strict Transport Security (HSTS). It - /// also sets up status code pages to return problem details responses when appropriate, or redirects to a custom + /// environments, it configures a generic exception handler, followed by HSTS through + /// SecurityHeadersExtensions.UseApplicationHsts (defined with the security headers, registered here to keep + /// it ahead of the status-code branches). It also sets up status code pages to return problem details responses when appropriate, or redirects to a custom /// error page otherwise. /// The instance to configure. Cannot be null. /// The configured instance. @@ -101,9 +103,13 @@ public static WebApplication UseProblemDetails(this WebApplication app) else { app.UseExceptionHandler("/Home/Error/500"); - app.UseHsts(); } + // HSTS belongs between the exception handler and the status-code branches below, so it runs once per + // request and is not re-invoked by status-code re-execution. The implementation lives with the other + // security headers; only the registration point is here. It is a no-op in development. + app.UseApplicationHsts(); + app.UseWhen( ProblemDetailsRequestClassifier.ShouldWriteProblemDetails, branch => branch.UseStatusCodePages()); diff --git a/src/ProjectTemplate.Web/Extensions/PipelineExtensions.cs b/src/ProjectTemplate.Web/Extensions/PipelineExtensions.cs index ec60f798..ba8f18b8 100644 --- a/src/ProjectTemplate.Web/Extensions/PipelineExtensions.cs +++ b/src/ProjectTemplate.Web/Extensions/PipelineExtensions.cs @@ -29,8 +29,9 @@ public static WebApplication UseApplicationPipeline(this WebApplication app) app.UseApplicationRequestLogging(); // 3. Centralized exception and status-code handling. This is the single registration: it adds the - // developer exception page or the production exception handler and HSTS, then branches status-code - // handling between Problem Details responses and the re-executed browser error page. + // developer exception page or the production exception handler and HSTS (UseApplicationHsts, outside + // development), then branches status-code handling between Problem Details responses and the + // re-executed browser error page. app.UseProblemDetails(); // 4. Optional security response headers. diff --git a/src/ProjectTemplate.Web/Extensions/SecurityHeadersExtensions.cs b/src/ProjectTemplate.Web/Extensions/SecurityHeadersExtensions.cs index 07807a69..de7e14ed 100644 --- a/src/ProjectTemplate.Web/Extensions/SecurityHeadersExtensions.cs +++ b/src/ProjectTemplate.Web/Extensions/SecurityHeadersExtensions.cs @@ -52,5 +52,27 @@ public static IApplicationBuilder UseApplicationSecurityHeaders( { return app.UseMiddleware(); } + + /// + /// Adds HTTP Strict Transport Security (HSTS) outside the development environment. + /// + /// + /// HSTS is skipped in development so browsers do not pin localhost to HTTPS. UseProblemDetails registers + /// it between the exception handler and the status-code page branches, ahead of HTTPS redirection. + /// + /// The used to configure the request pipeline. + /// The same instance for chaining. + public static WebApplication UseApplicationHsts( + this WebApplication app) + { + ArgumentNullException.ThrowIfNull(app); + + if (!app.Environment.IsDevelopment()) + { + app.UseHsts(); + } + + return app; + } } diff --git a/src/ProjectTemplate.Web/Middleware/SecurityHeadersMiddleware.cs b/src/ProjectTemplate.Web/Middleware/SecurityHeadersMiddleware.cs index e1e51465..9128ba39 100644 --- a/src/ProjectTemplate.Web/Middleware/SecurityHeadersMiddleware.cs +++ b/src/ProjectTemplate.Web/Middleware/SecurityHeadersMiddleware.cs @@ -23,12 +23,26 @@ public sealed class SecurityHeadersMiddleware(RequestDelegate next, IOptionsA that completes when the middleware and the next delegate finish processing. public async Task InvokeAsync(HttpContext context) { - if (!_options.Enabled || IsExcludedPath(context.Request.Path)) + if (!_options.Enabled) { await _next(context); return; } + if (IsExcludedPath(context.Request.Path)) + { + // Excluded endpoints (health, metrics) skip the document-oriented headers, but MIME sniffing + // protection costs nothing and applies to every response. + context.Response.OnStarting(() => + { + AddHeaderIfMissing(context.Response.Headers, "X-Content-Type-Options", "nosniff"); + return Task.CompletedTask; + }); + + await _next(context); + return; + } + context.Response.OnStarting(() => { IHeaderDictionary headers = context.Response.Headers; diff --git a/src/ProjectTemplate.Web/Options/ApplicationRequestLoggingOptions.cs b/src/ProjectTemplate.Web/Options/ApplicationRequestLoggingOptions.cs index 13a2561e..6a65fa5d 100644 --- a/src/ProjectTemplate.Web/Options/ApplicationRequestLoggingOptions.cs +++ b/src/ProjectTemplate.Web/Options/ApplicationRequestLoggingOptions.cs @@ -8,7 +8,7 @@ public sealed class ApplicationRequestLoggingOptions /// /// Gets the configuration section name used to bind request logging settings. /// - public static string SectionName { get; internal set; } = "ProjectTemplate:RequestLogging"; + public const string SectionName = "ProjectTemplate:RequestLogging"; /// /// Gets or sets a value indicating whether structured request logging is enabled. diff --git a/src/ProjectTemplate.Web/Options/ApplicationSecurityHeadersOptions.cs b/src/ProjectTemplate.Web/Options/ApplicationSecurityHeadersOptions.cs index 87ca8c81..15ac37cb 100644 --- a/src/ProjectTemplate.Web/Options/ApplicationSecurityHeadersOptions.cs +++ b/src/ProjectTemplate.Web/Options/ApplicationSecurityHeadersOptions.cs @@ -8,7 +8,7 @@ public sealed class ApplicationSecurityHeadersOptions /// /// Gets the configuration section name used to bind security header settings. /// - public static string SectionName { get; internal set; } = "ProjectTemplate:SecurityHeaders"; + public const string SectionName = "ProjectTemplate:SecurityHeaders"; /// /// Gets or sets a value indicating whether security headers are enabled. diff --git a/src/ProjectTemplate.Web/Program.cs b/src/ProjectTemplate.Web/Program.cs index cbeb51b3..c4b3c3d3 100644 --- a/src/ProjectTemplate.Web/Program.cs +++ b/src/ProjectTemplate.Web/Program.cs @@ -1,5 +1,6 @@ using System.Diagnostics; using System.Globalization; +using Microsoft.AspNetCore.Mvc; using ProjectTemplate.Web.Authentication.Extensions; using ProjectTemplate.Web.ErrorHandling; using ProjectTemplate.Web.Extensions; @@ -24,7 +25,10 @@ builder.AddApplicationSerilog(); Log.Information("Bootstrapping ProjectTemplate.Web application"); - builder.Services.AddControllersWithViews(); + // Validate antiforgery tokens on every unsafe (POST, PUT, PATCH, DELETE) MVC request by default, so consumer + // actions are protected without opting in. Use [IgnoreAntiforgeryToken] only for endpoints that do not rely + // on ambient (cookie) credentials. + builder.Services.AddControllersWithViews(options => options.Filters.Add(new AutoValidateAntiforgeryTokenAttribute())); builder.Services.AddApplicationApiVersioning(builder.Configuration); builder.Services.AddRazorPages(); builder.Services.AddApplicationHealthChecks(); diff --git a/tests/ProjectTemplate.Web.Tests/AntiforgeryTests.cs b/tests/ProjectTemplate.Web.Tests/AntiforgeryTests.cs new file mode 100644 index 00000000..40b2a754 --- /dev/null +++ b/tests/ProjectTemplate.Web.Tests/AntiforgeryTests.cs @@ -0,0 +1,65 @@ +using System.Net; +using ProjectTemplate.Web.Tests.Extensions; +using ProjectTemplate.Web.Tests.Infrastructure; + +namespace ProjectTemplate.Web.Tests; + +/// +/// Verifies that antiforgery validation is applied globally to unsafe MVC requests. +/// +public sealed class AntiforgeryTests +{ + private const string _unannotatedUnsafePath = "/test/authentication/unannotated-unsafe"; + + /// + /// Verifies that an unsafe request to an action without an antiforgery attribute is rejected when the token is missing. + /// + /// A task that represents the asynchronous test operation. + [Fact] + public async Task UnannotatedUnsafeRequest_WithoutAntiforgeryToken_IsRejected() + { + using ApplicationWebApplicationFactory factory = CreateFactory(); + using HttpClient client = factory.CreateHttpsClient(); + using FormUrlEncodedContent content = new(new Dictionary()); + + using HttpResponseMessage response = await client.PutAsync( + _unannotatedUnsafePath, + content, + TestContext.Current.CancellationToken); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + /// + /// Verifies that an unsafe request to an action without an antiforgery attribute succeeds with a valid token. + /// + /// A task that represents the asynchronous test operation. + [Fact] + public async Task UnannotatedUnsafeRequest_WithAntiforgeryToken_Succeeds() + { + using ApplicationWebApplicationFactory factory = CreateFactory(); + using HttpClient client = factory.CreateHttpsClient(); + + using HttpResponseMessage tokenResponse = await client.GetAsync( + "/test/authentication/antiforgery-token", + TestContext.Current.CancellationToken); + string token = await tokenResponse.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + + using FormUrlEncodedContent content = new(new Dictionary + { + ["__RequestVerificationToken"] = token + }); + + using HttpResponseMessage response = await client.PutAsync( + _unannotatedUnsafePath, + content, + TestContext.Current.CancellationToken); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + private static ApplicationWebApplicationFactory CreateFactory() + { + return ApplicationWebApplicationFactory.CreateAllowingAnonymousAccess(new Dictionary()); + } +} diff --git a/tests/ProjectTemplate.Web.Tests/ClaimsTransformationTests.cs b/tests/ProjectTemplate.Web.Tests/ClaimsTransformationTests.cs index a74d683d..481e78c6 100644 --- a/tests/ProjectTemplate.Web.Tests/ClaimsTransformationTests.cs +++ b/tests/ProjectTemplate.Web.Tests/ClaimsTransformationTests.cs @@ -275,6 +275,60 @@ public async Task ClaimsTransformation_DoesNotDuplicateNormalizedClaims() Assert.Equal(1, normalizedSubjectClaimCount); } + /// + /// Verifies that transformation returns a new principal and leaves the incoming principal unchanged. + /// + [Fact] + public async Task ClaimsTransformation_RemoveOriginalClaims_DoesNotMutateIncomingPrincipal() + { + ApplicationClaimsTransformation transformation = CreateTransformation( + new ApplicationClaimsTransformationOptions { RemoveOriginalClaims = true }); + + ClaimsPrincipal principal = CreatePrincipal( + authenticationType: "OpenIdConnect", + claims: + [ + new Claim("sub", "user-123"), + new Claim("roles", "Administrator") + ]); + + ClaimsPrincipal transformed = await transformation.TransformAsync(principal); + + Assert.NotSame(principal, transformed); + Assert.Equal(["sub", "roles"], principal.Claims.Select(claim => claim.Type)); + Assert.DoesNotContain(transformed.Claims, claim => claim.Type == "sub"); + Assert.Contains(transformed.Claims, claim => + claim.Type == ApplicationClaimTypes.Subject && + claim.Value == "user-123"); + } + + /// + /// Verifies that role and name checks resolve against normalized claims after original claims are removed. + /// + [Fact] + public async Task ClaimsTransformation_RemoveOriginalClaims_KeepsIsInRoleAndNameWorking() + { + ApplicationClaimsTransformation transformation = CreateTransformation( + new ApplicationClaimsTransformationOptions { RemoveOriginalClaims = true }); + + ClaimsPrincipal principal = CreatePrincipal( + authenticationType: "OpenIdConnect", + claims: + [ + new Claim(ClaimTypes.Name, "Test User"), + new Claim(ClaimTypes.Role, "Administrator") + ]); + + ClaimsPrincipal transformed = await transformation.TransformAsync(principal); + + ClaimsIdentity identity = Assert.IsType(transformed.Identity); + Assert.Equal(ApplicationClaimTypes.Role, identity.RoleClaimType); + Assert.Equal(ApplicationClaimTypes.Name, identity.NameClaimType); + Assert.True(identity.IsAuthenticated); + Assert.True(transformed.IsInRole("Administrator")); + Assert.Equal("Test User", transformed.Identity?.Name); + } + private static ApplicationClaimsTransformation CreateTransformation( ApplicationClaimsTransformationOptions? claimsTransformationOptions = null) { diff --git a/tests/ProjectTemplate.Web.Tests/HealthCheckTests.cs b/tests/ProjectTemplate.Web.Tests/HealthCheckTests.cs index bba3afa4..ffac1877 100644 --- a/tests/ProjectTemplate.Web.Tests/HealthCheckTests.cs +++ b/tests/ProjectTemplate.Web.Tests/HealthCheckTests.cs @@ -68,7 +68,7 @@ public async Task HealthLiveEndpoint_ReturnsHealthy() } /// - /// Verifies that health endpoints do not receive configured security headers. + /// Verifies that health endpoints receive only the X-Content-Type-Options security header. /// /// The health check path to test. /// A task that represents the asynchronous test operation. @@ -76,7 +76,7 @@ public async Task HealthLiveEndpoint_ReturnsHealthy() [InlineData("/health")] [InlineData("/health/ready")] [InlineData("/health/live")] - public async Task HealthEndpoints_DoNotApplySecurityHeaders(string path) + public async Task HealthEndpoints_ApplyOnlyNoSniffSecurityHeader(string path) { using ApplicationWebApplicationFactory factory = CreateFactory(); using HttpClient client = factory.CreateHttpsClient(); @@ -85,7 +85,7 @@ public async Task HealthEndpoints_DoNotApplySecurityHeaders(string path) Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.False(response.Headers.Contains("X-Content-Type-Options")); + Assert.Equal("nosniff", Assert.Single(response.Headers.GetValues("X-Content-Type-Options"))); Assert.False(response.Headers.Contains("X-Frame-Options")); Assert.False(response.Headers.Contains("Referrer-Policy")); Assert.False(response.Headers.Contains("X-Permitted-Cross-Domain-Policies")); diff --git a/tests/ProjectTemplate.Web.Tests/HstsTests.cs b/tests/ProjectTemplate.Web.Tests/HstsTests.cs new file mode 100644 index 00000000..006fe455 --- /dev/null +++ b/tests/ProjectTemplate.Web.Tests/HstsTests.cs @@ -0,0 +1,90 @@ +using System.Net; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Hosting; +using ProjectTemplate.Web.Extensions; +using ProjectTemplate.Web.Tests.Infrastructure; + +namespace ProjectTemplate.Web.Tests; + +/// +/// Verifies HTTP Strict Transport Security registration. +/// +public sealed class HstsTests +{ + // UseHsts() never emits the header for localhost, so requests use a non-loopback host name. + private static readonly Uri _publicHttpsBaseAddress = new("https://app.example.test"); + + /// + /// Verifies that the application pipeline emits Strict-Transport-Security outside Development. + /// + /// A task that represents the asynchronous test operation. + [Fact] + public async Task ApplicationPipeline_NonDevelopmentHttpsRequest_EmitsStrictTransportSecurity() + { + using var factory = + ApplicationWebApplicationFactory.CreateAllowingAnonymousAccess(new Dictionary()); + using HttpClient client = factory.CreateClient(new WebApplicationFactoryClientOptions + { + BaseAddress = _publicHttpsBaseAddress, + AllowAutoRedirect = false + }); + + using HttpResponseMessage response = await client.GetAsync( + "/test/security-headers", + TestContext.Current.CancellationToken); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.StartsWith( + "max-age=", + Assert.Single(response.Headers.GetValues("Strict-Transport-Security")), + StringComparison.Ordinal); + } + + /// + /// Verifies that emits the header outside Development. + /// + /// A task that represents the asynchronous test operation. + [Fact] + public async Task UseApplicationHsts_Production_EmitsStrictTransportSecurity() + { + using HttpResponseMessage response = await SendThroughHstsAsync(Environments.Production); + + Assert.True(response.Headers.Contains("Strict-Transport-Security")); + } + + /// + /// Verifies that does not emit the header in Development. + /// + /// A task that represents the asynchronous test operation. + [Fact] + public async Task UseApplicationHsts_Development_DoesNotEmitStrictTransportSecurity() + { + using HttpResponseMessage response = await SendThroughHstsAsync(Environments.Development); + + Assert.False(response.Headers.Contains("Strict-Transport-Security")); + } + + private static async Task SendThroughHstsAsync(string environmentName) + { + WebApplicationBuilder builder = WebApplication.CreateBuilder(new WebApplicationOptions + { + EnvironmentName = environmentName + }); + _ = builder.WebHost.UseTestServer(); + + await using WebApplication app = builder.Build(); + _ = app.UseApplicationHsts(); + _ = app.MapGet("/", () => Results.Ok()); + await app.StartAsync(TestContext.Current.CancellationToken); + + using HttpClient client = app.GetTestClient(); + client.BaseAddress = _publicHttpsBaseAddress; + + HttpResponseMessage response = await client.GetAsync("/", TestContext.Current.CancellationToken); + await app.StopAsync(TestContext.Current.CancellationToken); + return response; + } +} diff --git a/tests/ProjectTemplate.Web.Tests/HttpContextApplicationAuditContextAccessorTests.cs b/tests/ProjectTemplate.Web.Tests/HttpContextApplicationAuditContextAccessorTests.cs new file mode 100644 index 00000000..f792ee61 --- /dev/null +++ b/tests/ProjectTemplate.Web.Tests/HttpContextApplicationAuditContextAccessorTests.cs @@ -0,0 +1,65 @@ +using System.Net; +using System.Security.Claims; +using Microsoft.AspNetCore.Http; +using ProjectTemplate.Infrastructure.Data.Auditing; +using ProjectTemplate.Web.Accessors; +using ProjectTemplate.Web.Authentication.Claims; + +namespace ProjectTemplate.Web.Tests; + +public sealed class HttpContextApplicationAuditContextAccessorTests +{ + [Fact] + public void Current_AuthenticatedUserWithOnlyNormalizedSubjectClaim_UsesSubject() + { + ApplicationAuditContext context = CreateAccessor( + CreateAuthenticatedPrincipal(new Claim(ApplicationClaimTypes.Subject, "user-789"))).Current; + + Assert.Equal("user-789", context.ActorId); + Assert.Equal(ApplicationAuditActorTypes.Human, context.ActorType); + } + + [Fact] + public void Current_AuthenticatedUserWithNormalizedAndProviderSubjectClaims_PrefersNormalizedSubject() + { + ApplicationAuditContext context = CreateAccessor( + CreateAuthenticatedPrincipal( + new Claim("sub", "provider-subject"), + new Claim(ApplicationClaimTypes.Subject, "user-789"))).Current; + + Assert.Equal("user-789", context.ActorId); + Assert.Equal(ApplicationAuditActorTypes.Human, context.ActorType); + } + + [Fact] + public void Current_AuthenticatedUserWithOnlyProviderSubjectClaim_UsesProviderSubject() + { + ApplicationAuditContext context = CreateAccessor( + CreateAuthenticatedPrincipal(new Claim("sub", "provider-subject"))).Current; + + Assert.Equal("provider-subject", context.ActorId); + } + + [Fact] + public void Current_AuthenticatedUserWithoutSubjectClaims_UsesRemoteIpAddress() + { + ApplicationAuditContext context = CreateAccessor(CreateAuthenticatedPrincipal()).Current; + + Assert.Equal("192.0.2.10", context.ActorId); + Assert.Equal(ApplicationAuditActorTypes.Network, context.ActorType); + } + + private static HttpContextApplicationAuditContextAccessor CreateAccessor(ClaimsPrincipal user) + { + var httpContext = new DefaultHttpContext { User = user }; + httpContext.Connection.RemoteIpAddress = IPAddress.Parse("192.0.2.10"); + + return new HttpContextApplicationAuditContextAccessor( + new HttpContextAccessor { HttpContext = httpContext }); + } + + private static ClaimsPrincipal CreateAuthenticatedPrincipal(params Claim[] claims) + { + return new ClaimsPrincipal(new ClaimsIdentity(claims, authenticationType: "Test")); + } +} diff --git a/tests/ProjectTemplate.Web.Tests/HttpContextCurrentActorAccessorTests.cs b/tests/ProjectTemplate.Web.Tests/HttpContextCurrentActorAccessorTests.cs index 4942829e..e87eece0 100644 --- a/tests/ProjectTemplate.Web.Tests/HttpContextCurrentActorAccessorTests.cs +++ b/tests/ProjectTemplate.Web.Tests/HttpContextCurrentActorAccessorTests.cs @@ -2,6 +2,7 @@ using System.Security.Claims; using Microsoft.AspNetCore.Http; using ProjectTemplate.Web.Accessors; +using ProjectTemplate.Web.Authentication.Claims; namespace ProjectTemplate.Web.Tests; @@ -18,6 +19,30 @@ public void CurrentActor_AuthenticatedUserWithSubjectClaim_ReturnsSubject() Assert.Equal("Subject: user-123", accessor.CurrentActor); } + [Fact] + public void CurrentActor_AuthenticatedUserWithOnlyNormalizedSubjectClaim_ReturnsSubject() + { + DefaultHttpContext httpContext = CreateHttpContext( + CreateAuthenticatedPrincipal(new Claim(ApplicationClaimTypes.Subject, "user-789"))); + + HttpContextCurrentActorAccessor accessor = CreateAccessor(httpContext); + + Assert.Equal("Subject: user-789", accessor.CurrentActor); + } + + [Fact] + public void CurrentActor_AuthenticatedUserWithNormalizedAndProviderSubjectClaims_PrefersNormalizedSubject() + { + DefaultHttpContext httpContext = CreateHttpContext( + CreateAuthenticatedPrincipal( + new Claim("sub", "provider-subject"), + new Claim(ApplicationClaimTypes.Subject, "user-789"))); + + HttpContextCurrentActorAccessor accessor = CreateAccessor(httpContext); + + Assert.Equal("Subject: user-789", accessor.CurrentActor); + } + [Fact] public void CurrentActor_AuthenticatedUserWithWhitespaceSubjectClaim_TrimsSubject() { diff --git a/tests/ProjectTemplate.Web.Tests/SecurityHeadersTests.cs b/tests/ProjectTemplate.Web.Tests/SecurityHeadersTests.cs index 055d2185..bd19ffcb 100644 --- a/tests/ProjectTemplate.Web.Tests/SecurityHeadersTests.cs +++ b/tests/ProjectTemplate.Web.Tests/SecurityHeadersTests.cs @@ -180,7 +180,7 @@ public async Task DisabledSecurityHeaders_DoNotEmitSecurityHeaders() } /// - /// Verifies that configured excluded path prefixes do not receive security headers. + /// Verifies that configured excluded path prefixes receive only the X-Content-Type-Options header. /// /// The excluded request path to verify. /// A task that represents the asynchronous test operation. @@ -189,7 +189,7 @@ public async Task DisabledSecurityHeaders_DoNotEmitSecurityHeaders() [InlineData("/health/ready")] [InlineData("/health/live")] [InlineData("/metrics")] - public async Task ExcludedPathPrefixes_DoNotApplySecurityHeaders(string path) + public async Task ExcludedPathPrefixes_ApplyOnlyNoSniffSecurityHeader(string path) { using ApplicationWebApplicationFactory factory = CreateFactory(new Dictionary { @@ -204,7 +204,14 @@ public async Task ExcludedPathPrefixes_DoNotApplySecurityHeaders(string path) Assert.Equal(HttpStatusCode.OK, response.StatusCode); - AssertSecurityHeadersMissing(response); + AssertHeader(response, "X-Content-Type-Options", "nosniff"); + AssertHeaderMissing(response, "X-Frame-Options"); + AssertHeaderMissing(response, "Referrer-Policy"); + AssertHeaderMissing(response, "X-Permitted-Cross-Domain-Policies"); + AssertHeaderMissing(response, "Cross-Origin-Opener-Policy"); + AssertHeaderMissing(response, "Cross-Origin-Resource-Policy"); + AssertHeaderMissing(response, "Permissions-Policy"); + AssertHeaderMissing(response, "Content-Security-Policy"); } /// diff --git a/tests/ProjectTemplate.Web.Tests/TestControllers/AuthenticationTestController.cs b/tests/ProjectTemplate.Web.Tests/TestControllers/AuthenticationTestController.cs index 54c28e46..2eb535b9 100644 --- a/tests/ProjectTemplate.Web.Tests/TestControllers/AuthenticationTestController.cs +++ b/tests/ProjectTemplate.Web.Tests/TestControllers/AuthenticationTestController.cs @@ -65,6 +65,22 @@ await HttpContext.SignInAsync( return Ok(new { result = "signed-in" }); } + /// + /// Accepts an unsafe (PUT) request that carries no antiforgery attribute, so only the global + /// AutoValidateAntiforgeryTokenAttribute filter protects it. + /// + /// + /// PUT rather than POST: static analysis (CodeQL cs/web/missing-token-validation) cannot see globally + /// registered MVC filters and would flag an unannotated POST, while the global filter covers every unsafe verb. + /// + /// An OK response when the request passes antiforgery validation. + [HttpPut("unannotated-unsafe")] + [AllowAnonymous] + public IActionResult UnannotatedUnsafe() + { + return Ok(new { result = "accepted" }); + } + /// /// Returns an unannotated response governed by the fallback authorization policy. ///