Fix MSRC c805e950: challenge open-redirect + logout CSRF validation

F1 (Microsoft.Identity.Web.UI / AccountController.Challenge):
- Reject redirectUri values that pass Url.IsLocalUrl() but decode to
  a protocol-relative form (leading %2F%2F...), which Cookie auth would
  emit verbatim in Location and some clients decode to //host before
  following. New IsPercentEncodedSlashBypass helper runs after both raw
  input and Uri-coerced candidate.

F3 (Microsoft.Identity.Web / MapLoginAndLogout):
- Remove .DisableAntiforgery() from /logout.
- Add .RequireAuthorization() as primary CSRF gate.
- Validate IAntiforgery.IsRequestValidAsync(context) in the handler
  when IAntiforgery is registered, independent of whether UseAntiforgery()
  middleware is in the pipeline. Makes /logout pipeline-shape-agnostic
  (Blazor, MVC, minimal-API hosts all get equivalent protection).
- When IAntiforgery is not registered, log a one-time warning at
  MapLoginAndLogout time (EventId=1, AntiforgeryNotRegistered) so
  operators can see the graceful-degradation state; RequireAuthorization
  + SameSite=Lax cookies remain as the gate.

Tests:
- F1: 44/44 on net8/net9 (16 unit + 28 integration).
- F3: 22/22 on net8/net9/net10 (16 unit + 6 integration including
  MVC-shape graceful-degradation coverage).

Reviewed independently by two models (both passes sign-off).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: 2 people

changelog.md

@@ -8,6 +8,25 @@
 - Fix race condition in `MergedOptions` causing sporadic "No ClientId was specified" errors under concurrent `GraphServiceClient` usage. See [#3760](https://github.com/AzureAD/microsoft-identity-web/pull/3760).
 - Fix `CredentialsProvider` DI lifetime mismatch causing startup crash in Development mode when using `AddMicrosoftIdentityWebApi()`. See [#3783](https://github.com/AzureAD/microsoft-identity-web/pull/3783).
 
+### Behavior changes
+- **`/MicrosoftIdentity/Account/Challenge` — redirect URI validation.** The `redirectUri` query-string parameter is now validated. Accepted values:
+  - Local paths (e.g. `/home`, `/counter?tab=1`) — unchanged behavior.
+  - Same-origin absolute URLs (matching scheme, host, and effective port of the current request). These are coerced to their path-and-query before being stored in `AuthenticationProperties.RedirectUri`. This preserves the canonical `[AuthorizeForScopes]` / `MsalUiRequiredException` step-up consent flow, which goes through `MicrosoftIdentityConsentAndConditionalAccessHandler.ChallengeUser()` and passes `NavigationManager.Uri` (always absolute) for Blazor Server, or an absolute request URL for Razor Pages / MVC.
+  - Any other value (external host, different scheme, different port, protocol-relative `//host`, empty, or `null`) falls back to `~/`.
+  - **UX note:** URL fragments (`#section`) are dropped when a same-origin absolute URL is coerced. If a Blazor Server page depends on a fragment being preserved across step-up consent, pass a relative path explicitly rather than relying on `NavigationManager.Uri`.
+  - **Reverse-proxy deployments:** apps behind a reverse proxy (Azure App Service, Container Apps, AKS ingress, nginx, etc.) should configure `app.UseForwardedHeaders(new ForwardedHeadersOptions { ForwardedHeaders = ForwardedHeaders.XForwardedProto | ForwardedHeaders.XForwardedHost })` before `UseAuthentication()`. Without it, `Request.Scheme` / `Request.Host` reflect the internal container/pod hostname, the same-origin check fails for the external `NavigationManager.Uri`, and step-up lands the user on `/` rather than the original page.
+
+- **Blazor `MapGroup(...).MapLoginAndLogout()` — `/logout` endpoint.** The generated `POST /logout` endpoint now (a) requires authentication (`RequireAuthorization()`) and (b) requires an antiforgery token (the previous `DisableAntiforgery()` opt-out has been removed). UX and integration implications:
+  - Forms rendered by `SignOutLink` / `LogInOrOut` already include the antiforgery token and continue to work without code changes.
+  - Custom clients that `fetch`/`XMLHttpRequest` to `/logout` must now include the antiforgery token. Obtain it via `IAntiforgery.GetAndStoreTokens(context).RequestToken` and send it in the request header configured by `AddAntiforgery(options => options.HeaderName = "...")` (default `RequestVerificationToken`) or as the configured form field.
+  - The `ReturnUrl` form value is now treated as strictly local: any non-local value (absolute URL, protocol-relative, `/\host`, etc.) is coerced to `/`. Apps that previously passed an absolute URL should switch to a relative path.
+  - **Edge case:** a user whose authentication cookie has already expired and who then clicks the logout button will be redirected to the login page (because of `RequireAuthorization()`) rather than seeing a silent no-op. This is a minor change from previous behavior; the happy path (authenticated user clicking logout) is unchanged.
+  - **Blazor WebAssembly clients are unaffected.** WASM apps sign out through `Microsoft.Authentication.WebAssembly.Msal` / the `/authentication/logout` JS interop path, not by POSTing to the server-side `/logout` endpoint, so no client code changes are needed.
+  - **Server-side Blazor using `AuthenticationStateProvider` / `SignOutAsync` is unaffected.** The new gate only applies to direct HTTP POSTs to `/logout`. Components that call `AuthenticationStateProvider.GetAuthenticationStateAsync()` or sign out through the scheme handler continue to work unchanged.
+  - **Graceful degradation when antiforgery is not configured.** The check is performed inside the endpoint handler by explicitly resolving `IAntiforgery` from request services (not via endpoint metadata / middleware coupling). This means the `/logout` endpoint works correctly on every pipeline shape: (a) minimal API with both `AddAntiforgery()` and `UseAntiforgery()` wired — token is validated by middleware and re-checked by the handler (idempotent); (b) MVC / Razor Pages hosts that call `AddControllersWithViews()` or `AddRazorPages()` (which transitively register `IAntiforgery`) but do not call `UseAntiforgery()` — the handler validates the token directly; (c) hosts that reuse `MapLoginAndLogout` without any antiforgery configuration — the handler skips validation and `RequireAuthorization()` + cookie `SameSite=Lax` remain the CSRF gate, matching pre-4.8.0 behavior. For scenario (c), a single warning is logged at endpoint map time recommending that `AddAntiforgery()` be configured.
+
+- **`/MicrosoftIdentity/Account/Challenge` — `%2f` / `%5c` defense-in-depth.** In addition to the path-and-query re-check for protocol-relative shapes (`//host`, `/\host`), `redirectUri` values whose path begins with `/%2f`, `/%5c`, `/%2F`, or `/%5C` are now rejected and coerced to `~/`. Browsers per RFC 3986 treat these as literal path characters (a direct hit yields a 404), so this change does not affect legitimate deep-links. It guards against misconfigured reverse proxies (NGINX, IIS ARR, F5) that can decode `%2f` → `/` while rewriting `Location` headers, which would otherwise reopen the protocol-relative bypass after the proxy pass.
+
 ### Dependencies updates
 - Upgrade Microsoft Application Insights packages. See [#3763](https://github.com/AzureAD/microsoft-identity-web/pull/3763).
 - Bump net8/net9/net10 runtime package baselines to patched crypto servicing versions. See [#3779](https://github.com/AzureAD/microsoft-identity-web/pull/3779).

src/Microsoft.Identity.Web.UI/Areas/MicrosoftIdentity/Controllers/AccountController.cs

@@ -1,13 +1,15 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+using System;
 using System.Collections.Generic;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authentication.Cookies;
 using Microsoft.AspNetCore.Authentication.OAuth;
 using Microsoft.AspNetCore.Authentication.OpenIdConnect;
 using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.Extensions.Options;
 
@@ -112,7 +114,42 @@ public IActionResult Challenge(
             {
                 oAuthChallengeProperties.Scope = scope.Split(" ");
             }
-            oAuthChallengeProperties.RedirectUri = redirectUri;
+
+            // Validate the redirect URI. Accept:
+            //   * Local URLs (e.g. "/path") — the common MVC pattern.
+            //   * Same-origin absolute URLs (coerced to PathAndQuery) — required because
+            //     Microsoft.Identity.Web's own MicrosoftIdentityConsentAndConditionalAccessHandler
+            //     passes NavigationManager.Uri (always absolute) for Blazor Server step-up
+            //     consent and for Razor Pages / MVC step-up. Rejecting those would break the
+            //     canonical [AuthorizeForScopes] / MsalUiRequiredException flow.
+            // Reject everything else. The post-sign-in 302 honors AuthenticationProperties.RedirectUri
+            // as-is (CookieAuthenticationHandler does not enforce IsLocalUrl), so the check must
+            // happen here. This closes the open-redirect class of bug matching the SignIn action's
+            // own IsLocalUrl gate added in PR #1219.
+            string? safeRedirect = null;
+            if (!string.IsNullOrEmpty(redirectUri))
+            {
+                if (Url.IsLocalUrl(redirectUri) && !IsPercentEncodedSlashBypass(redirectUri))
+                {
+                    safeRedirect = redirectUri;
+                }
+                else if (Uri.TryCreate(redirectUri, UriKind.Absolute, out var absolute)
+                         && IsSameOrigin(absolute, HttpContext.Request))
+                {
+                    // PathAndQuery of a same-origin absolute URL can still begin with "//" or "/\"
+                    // for inputs like "http://victim.app//evil.com/x" (Uri.Host="victim.app",
+                    // PathAndQuery="//evil.com/x") — a protocol-relative URL that CookieAuthenticationHandler
+                    // would emit verbatim in its Location header. Re-run IsLocalUrl on the coerced value
+                    // to reject those shapes.
+                    var candidate = absolute.PathAndQuery;
+                    if (Url.IsLocalUrl(candidate) && !IsPercentEncodedSlashBypass(candidate))
+                    {
+                        safeRedirect = candidate;
+                    }
+                }
+            }
+
+            oAuthChallengeProperties.RedirectUri = safeRedirect ?? Url.Content("~/")!;
 
             return Challenge(
                 oAuthChallengeProperties,
@@ -186,5 +223,39 @@ public async Task<IActionResult> EditProfile([FromRoute] string scheme)
             properties.Items[Constants.Policy] = _optionsMonitor.Get(scheme).EditProfilePolicyId;
             return Challenge(properties, scheme);
         }
+
+        /// <summary>
+        /// Returns <c>true</c> when <paramref name="absolute"/> has the same origin (scheme + host + port)
+        /// as <paramref name="request"/>. Used by <c>Challenge</c> to accept same-origin absolute redirect URIs
+        /// without opening an open-redirect sink.
+        /// </summary>
+        private static bool IsSameOrigin(Uri absolute, HttpRequest request)
+        {
+            if (!string.Equals(absolute.Scheme, request.Scheme, StringComparison.OrdinalIgnoreCase))
+            {
+                return false;
+            }
+
+            if (!string.Equals(absolute.Host, request.Host.Host, StringComparison.OrdinalIgnoreCase))
+            {
+                return false;
+            }
+
+            int requestPort = request.Host.Port ?? (request.IsHttps ? 443 : 80);
+            return absolute.Port == requestPort;
+        }
+
+        /// <summary>
+        /// Defense-in-depth: reject paths whose first segment starts with a percent-encoded
+        /// forward or backward slash (<c>%2f</c>/<c>%5c</c>). Browsers per RFC 3986 treat these
+        /// as literal path characters, but misconfigured reverse proxies (NGINX, IIS ARR, F5)
+        /// can decode them into <c>//</c> or <c>/\</c> when rewriting the <c>Location</c>
+        /// header, reopening the protocol-relative bypass that this controller otherwise
+        /// closes. Comparison is case-insensitive because the RFC 3986 encoding is
+        /// hex-case-insensitive.
+        /// </summary>
+        private static bool IsPercentEncodedSlashBypass(string path) =>
+            path.StartsWith("/%2f", StringComparison.OrdinalIgnoreCase)
+            || path.StartsWith("/%5c", StringComparison.OrdinalIgnoreCase);
     }
 }

src/Microsoft.Identity.Web/Blazor/LoginLogoutEndpointRouteBuilderExtensions.cs

@@ -4,12 +4,15 @@
 using System;
 using System.Diagnostics.CodeAnalysis;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Antiforgery;
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authentication.Cookies;
 using Microsoft.AspNetCore.Authentication.OpenIdConnect;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Routing;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 using Microsoft.IdentityModel.Protocols.OpenIdConnect;
 
 namespace Microsoft.Identity.Web;
@@ -40,6 +43,8 @@ public static IEndpointConventionBuilder MapLoginAndLogout(this IEndpointRouteBu
     {
         var group = endpoints.MapGroup("");
 
+        WarnIfAntiforgeryMissing(endpoints.ServiceProvider);
+
         // Enhanced login endpoint that supports incremental consent and Conditional Access
         group.MapGet("/login", (
             string? returnUrl,
@@ -81,28 +86,90 @@ public static IEndpointConventionBuilder MapLoginAndLogout(this IEndpointRouteBu
 
         group.MapPost("/logout", async (HttpContext context) =>
         {
+            // Defense-in-depth CSRF validation (MSRC hardening). When the host has registered
+            // IAntiforgery via AddAntiforgery() (or indirectly via AddControllersWithViews,
+            // AddRazorPages, AddMvc, etc.), we explicitly validate the request token here —
+            // independently of whether UseAntiforgery() middleware is in the pipeline. This
+            // avoids coupling our endpoint to pipeline shape: MVC hosts that rely on filter-
+            // time validation, minimal-API hosts that wire UseAntiforgery(), and Blazor hosts
+            // that wire both all receive equivalent protection at this endpoint. Hosts that
+            // do not register IAntiforgery at all fall back to RequireAuthorization() +
+            // SameSite=Lax cookie semantics as the primary CSRF gate — matching pre-MSRC
+            // behavior and logged once at map time (see WarnIfAntiforgeryMissing).
+            // IsRequestValidAsync is safe to call after UseAntiforgery() middleware has already
+            // validated: the form is buffered (ReadFormAsync is cached on HttpRequest), tokens
+            // are not single-use in the default flow, and re-validation is a cheap hash verify —
+            // not a no-op, but inexpensive.
+            var antiforgery = context.RequestServices.GetService<IAntiforgery>();
+            if (antiforgery is not null && !await antiforgery.IsRequestValidAsync(context))
+            {
+                return Results.BadRequest();
+            }
+
             string? returnUrl = null;
             if (context.Request.HasFormContentType)
             {
                 var form = await context.Request.ReadFormAsync();
                 returnUrl = form["ReturnUrl"];
             }
 
-            return TypedResults.SignOut(GetAuthProperties(returnUrl),
+            return (IResult)TypedResults.SignOut(GetAuthProperties(returnUrl),
                 [CookieAuthenticationDefaults.AuthenticationScheme, OpenIdConnectDefaults.AuthenticationScheme]);
         })
-        .DisableAntiforgery();
+        .RequireAuthorization();
 
         return group;
     }
 
-    private static AuthenticationProperties GetAuthProperties(string? returnUrl)
+    // Emits a single warning at endpoint-build time when IAntiforgery isn't registered in DI.
+    // This surfaces the graceful-degradation state to operators so it's not silently invisible
+    // that CSRF protection at /logout relies solely on RequireAuthorization + SameSite=Lax
+    // (rather than token validation). Called once per MapLoginAndLogout invocation.
+    private static void WarnIfAntiforgeryMissing(IServiceProvider? serviceProvider)
+    {
+        var isService = serviceProvider?.GetService<IServiceProviderIsService>();
+        var antiforgeryRegistered = isService?.IsService(typeof(IAntiforgery)) ?? false;
+        if (antiforgeryRegistered)
+        {
+            return;
+        }
+
+        var loggerFactory = serviceProvider?.GetService<ILoggerFactory>();
+        var logger = loggerFactory?.CreateLogger(typeof(LoginLogoutEndpointRouteBuilderExtensions).FullName!);
+        logger?.LogWarning(
+            new EventId(1, "AntiforgeryNotRegistered"),
+            "MapLoginAndLogout was called but IAntiforgery is not registered in DI. The /logout " +
+            "endpoint will rely on RequireAuthorization and SameSite=Lax cookies as its CSRF gate. " +
+            "To enable antiforgery token validation, call services.AddAntiforgery() (and, for " +
+            "minimal APIs, app.UseAntiforgery()).");
+    }
+
+    /// <summary>
+    /// Builds <see cref="AuthenticationProperties"/> with a strictly-local <c>RedirectUri</c>.
+    /// Any non-local input (absolute URL, protocol-relative "//host", slash-backslash "/\host",
+    /// or anything not starting with a single '/') is coerced to "/". This matches the
+    /// semantics of <see cref="Microsoft.AspNetCore.Mvc.IUrlHelper.IsLocalUrl"/> and prevents
+    /// open-redirect attacks via the ReturnUrl query/form parameter.
+    /// </summary>
+    internal static AuthenticationProperties GetAuthProperties(string? returnUrl)
     {
         const string pathBase = "/";
-        if (string.IsNullOrEmpty(returnUrl)) returnUrl = pathBase;
-        else if (returnUrl.StartsWith("//", StringComparison.Ordinal)) returnUrl = pathBase; // Prevent protocol-relative redirects
-        else if (!Uri.IsWellFormedUriString(returnUrl, UriKind.Relative)) returnUrl = new Uri(returnUrl, UriKind.Absolute).PathAndQuery;
-        else if (returnUrl[0] != '/') returnUrl = $"{pathBase}{returnUrl}";
-        return new AuthenticationProperties { RedirectUri = returnUrl };
+        return new AuthenticationProperties { RedirectUri = IsLocalUrl(returnUrl) ? returnUrl! : pathBase };
+    }
+
+    private static bool IsLocalUrl(string? url)
+    {
+        if (string.IsNullOrEmpty(url))
+        {
+            return false;
+        }
+
+        // "/foo" is local, but not "//foo" (protocol-relative) and not "/\foo" (slash-backslash).
+        if (url[0] == '/')
+        {
+            return url.Length == 1 || (url[1] != '/' && url[1] != '\\');
+        }
+
+        return false;
     }
 }

src/Microsoft.Identity.Web/PublicAPI/net10.0/InternalAPI.Unshipped.txt

@@ -1 +1,2 @@
 #nullable enable
+static Microsoft.Identity.Web.LoginLogoutEndpointRouteBuilderExtensions.GetAuthProperties(string? returnUrl) -> Microsoft.AspNetCore.Authentication.AuthenticationProperties!

src/Microsoft.Identity.Web/PublicAPI/net8.0/InternalAPI.Unshipped.txt

@@ -1 +1,2 @@
 #nullable enable
+static Microsoft.Identity.Web.LoginLogoutEndpointRouteBuilderExtensions.GetAuthProperties(string? returnUrl) -> Microsoft.AspNetCore.Authentication.AuthenticationProperties!

src/Microsoft.Identity.Web/PublicAPI/net9.0/InternalAPI.Unshipped.txt

@@ -1 +1,2 @@
 #nullable enable
+static Microsoft.Identity.Web.LoginLogoutEndpointRouteBuilderExtensions.GetAuthProperties(string? returnUrl) -> Microsoft.AspNetCore.Authentication.AuthenticationProperties!