Skip to content

fix(auth): honor ASPNETCORE_FORWARDEDHEADERS_ENABLED for ACA#1153

Merged
BenjaminMichaelis merged 1 commit into
mainfrom
benjaminmichaelis/fix-github-oauth-redirect-uri
May 22, 2026
Merged

fix(auth): honor ASPNETCORE_FORWARDEDHEADERS_ENABLED for ACA#1153
BenjaminMichaelis merged 1 commit into
mainfrom
benjaminmichaelis/fix-github-oauth-redirect-uri

Conversation

@BenjaminMichaelis
Copy link
Copy Markdown
Member

Why

GitHub OAuth login on dev was generating an http:// callback URL and failing with redirect_uri is not associated with this application. The deployment now uses ASPNETCORE_FORWARDEDHEADERS_ENABLED=true, so app-level forwarded-header setup must not conflict with ASP.NET Core's built-in handling.

What changed

  • AddTrustedForwardedHeaders now exits early when ASPNETCORE_FORWARDEDHEADERS_ENABLED=true.
  • Program.cs skips manual app.UseForwardedHeaders() when that env var is enabled to avoid redundant middleware execution.
  • Updated the forwarded-header configuration error message to document the env-var-based option for platform-managed proxies like Azure Container Apps.

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.

…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://.
Copilot AI review requested due to automatic review settings May 22, 2026 21:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() when ASPNETCORE_FORWARDEDHEADERS_ENABLED=true.
  • Make AddTrustedForwardedHeaders no-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).");
@BenjaminMichaelis BenjaminMichaelis merged commit a62a63b into main May 22, 2026
9 checks passed
@BenjaminMichaelis BenjaminMichaelis deleted the benjaminmichaelis/fix-github-oauth-redirect-uri branch May 22, 2026 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants