fix(auth): honor ASPNETCORE_FORWARDEDHEADERS_ENABLED for ACA#1153
Merged
BenjaminMichaelis merged 1 commit intoMay 22, 2026
Merged
Conversation
…EDHEADERS_ENABLED=true When ASPNETCORE_FORWARDEDHEADERS_ENABLED=true (set by Terraform for ACA), ASP.NET Core's built-in startup filter already calls UseForwardedHeaders() with KnownNetworks/KnownProxies cleared (trusting all proxies). The app's manual AddTrustedForwardedHeaders was throwing InvalidOperationException on startup because no CIDRs were configured, and UseForwardedHeaders() was being called a second time redundantly. - AddTrustedForwardedHeaders: return early when ASPNETCORE_FORWARDEDHEADERS_ENABLED=true - Program.cs: skip app.UseForwardedHeaders() when the env var is set - Updated error message to mention the env var as an alternative to CIDRs Fixes GitHub OAuth login on dev.essentialcsharp.com where X-Forwarded-Proto was being silently dropped, causing redirect_uri=http:// instead of https://.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates forwarded-headers configuration to avoid conflicting with ASP.NET Core’s built-in ASPNETCORE_FORWARDEDHEADERS_ENABLED=true startup filter (used by Azure Container Apps), preventing duplicate/throwing forwarded-header setup and fixing OAuth redirect scheme issues behind proxies.
Changes:
- Skip manual
app.UseForwardedHeaders()whenASPNETCORE_FORWARDEDHEADERS_ENABLED=true. - Make
AddTrustedForwardedHeadersno-op when that env var is enabled to avoid startup failures when no trusted CIDRs/proxies are configured. - Expand the forwarded-headers configuration exception message to document the env-var-based option for platform-managed proxies.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| EssentialCSharp.Web/Program.cs | Conditionally disables manual UseForwardedHeaders() to avoid double execution when platform startup filter is enabled. |
| EssentialCSharp.Web/Extensions/IServiceCollectionExtensions.cs | Skips custom trusted-proxy configuration when the platform-managed forwarded-headers path is enabled; updates related error message. |
Comment on lines
+484
to
+489
| // Skip manual UseForwardedHeaders when ASPNETCORE_FORWARDEDHEADERS_ENABLED=true; | ||
| // the built-in startup filter already called it before this pipeline runs. | ||
| if (!string.Equals(app.Configuration["ASPNETCORE_FORWARDEDHEADERS_ENABLED"], "true", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| app.UseForwardedHeaders(); | ||
| } |
| "Forwarded headers are enabled but no trusted proxies are configured. " + | ||
| "Set ForwardedHeaders:TrustedProxyCidrs or ForwardedHeaders:TrustedProxies."); | ||
| "Set ForwardedHeaders:TrustedProxyCidrs or ForwardedHeaders:TrustedProxies, " + | ||
| "or set ASPNETCORE_FORWARDEDHEADERS_ENABLED=true for platform-managed proxies (e.g. Azure Container Apps)."); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
GitHub OAuth login on dev was generating an
http://callback URL and failing withredirect_uri is not associated with this application. The deployment now usesASPNETCORE_FORWARDEDHEADERS_ENABLED=true, so app-level forwarded-header setup must not conflict with ASP.NET Core's built-in handling.What changed
AddTrustedForwardedHeadersnow exits early whenASPNETCORE_FORWARDEDHEADERS_ENABLED=true.Program.csskips manualapp.UseForwardedHeaders()when that env var is enabled to avoid redundant middleware execution.Notes for reviewers
This keeps the existing explicit CIDR path intact for environments that use
ForwardedHeaders:TrustedProxyCidrs/TrustedProxies, while making ACA's env-var path safe and non-conflicting.