Don't apply the CSRF verdict to remote authentication callbacks

src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/RemoteAuthenticationCsrfTests.cs

@@ -0,0 +1,167 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#nullable enable
+
+using System.Net;
+using System.Net.Http;
+using System.Text.Encodings.Web;
+using Microsoft.AspNetCore.Antiforgery;
+using Microsoft.AspNetCore.Authentication;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Hosting;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.TestHost;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
+
+namespace Microsoft.AspNetCore.Tests;
+
+// A remote provider's callback (e.g. OpenID Connect response_mode=form_post) is a cross-site form POST by
+// protocol design. When the callback path also matches a routed endpoint that requires antiforgery
+// validation, the auto-injected CSRF middleware records an invalid verdict for it, and the handler used to
+// fail while reading its own callback body - before any of its events could run.
+public class RemoteAuthenticationCsrfTests
+{
+    [Fact]
+    public async Task RemoteCallback_CrossSiteFormPost_CanReadCallbackForm()
+    {
+        using var app = await CreateAppWithTokenAntiforgery();
+
+        var response = await app.GetTestClient().SendAsync(CreateCallbackRequest());
+
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+        Assert.Equal("handled:2", await response.Content.ReadAsStringAsync());
+    }
+
+    [Fact]
+    public async Task RemoteCallback_WhenHandlerSkipsRequest_WithTokenAntiforgery_ProtectsDownstreamEndpoint()
+    {
+        using var app = await CreateAppWithTokenAntiforgery(skipRequest: true);
+
+        var response = await app.GetTestClient().SendAsync(CreateCallbackRequest());
+
+        Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
+        Assert.Equal("protected", await response.Content.ReadAsStringAsync());
+    }
+
+    [Fact]
+    public async Task RemoteCallback_WhenHandlerSkipsRequest_RestoresAutoCsrfVerdict()
+    {
+        using var app = await CreateAppWithAutoCsrfOnly(skipRequest: true);
+
+        var response = await app.GetTestClient().SendAsync(CreateCallbackRequest());
+
+        Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
+        Assert.Equal("protected", await response.Content.ReadAsStringAsync());
+    }
+
+    private static HttpRequestMessage CreateCallbackRequest()
+    {
+        var request = new HttpRequestMessage(HttpMethod.Post, "/signin-oidc")
+        {
+            Content = new FormUrlEncodedContent(new Dictionary<string, string>
+            {
+                ["state"] = "fakestate",
+                ["code"] = "fakecode",
+            })
+        };
+        request.Headers.Add("Sec-Fetch-Site", "cross-site");
+        return request;
+    }
+
+    private static Task<WebApplication> CreateAppWithTokenAntiforgery(bool skipRequest = false)
+        => CreateApp(skipRequest, useTokenAntiforgery: true);
+
+    private static Task<WebApplication> CreateAppWithAutoCsrfOnly(bool skipRequest = false)
+        => CreateApp(skipRequest, useTokenAntiforgery: false);
+
+    private static async Task<WebApplication> CreateApp(bool skipRequest, bool useTokenAntiforgery)
+    {
+        var builder = WebApplication.CreateBuilder();
+        builder.WebHost.UseTestServer();
+        builder.Services.AddAuthentication("signin")
+            .AddScheme<AuthenticationSchemeOptions, NoOpHandler>("signin", _ => { })
+            .AddScheme<FakeRemoteOptions, FakeRemoteHandler>("remote", o =>
+            {
+                o.CallbackPath = "/signin-oidc";
+                o.SignInScheme = "signin";
+                o.SkipRequest = skipRequest;
+            });
+        builder.Services.AddAuthorization();
+        if (useTokenAntiforgery)
+        {
+            builder.Services.AddAntiforgery();
+        }
+
+        var app = builder.Build();
+        app.UseAuthentication();
+        app.UseAuthorization();
+        if (useTokenAntiforgery)
+        {
+            app.UseAntiforgery();
+        }
+
+        // Stands in for a catch-all server-rendered page: it makes routing match the remote callback path,
+        // which is what causes the CSRF middleware to record a verdict for the callback request.
+        app.MapPost("/{**slug}", EnforceCsrf).WithMetadata(new RequiresValidationMetadata());
+
+        await app.StartAsync();
+        return app;
+    }
+
+    private static string EnforceCsrf(HttpContext context)
+    {
+        var feature = context.Features.Get<IAntiforgeryValidationFeature>();
+        if (feature is null)
+        {
+            return "passthrough";
+        }
+
+        if (!feature.IsValid)
+        {
+            context.Response.StatusCode = StatusCodes.Status400BadRequest;
+            return "protected";
+        }
+
+        return "allowed";
+    }
+
+    private sealed class RequiresValidationMetadata : IAntiforgeryMetadata
+    {
+        public bool RequiresValidation => true;
+    }
+
+    private sealed class NoOpHandler(IOptionsMonitor<AuthenticationSchemeOptions> options, ILoggerFactory logger, UrlEncoder encoder)
+        : AuthenticationHandler<AuthenticationSchemeOptions>(options, logger, encoder)
+    {
+        protected override Task<AuthenticateResult> HandleAuthenticateAsync()
+            => Task.FromResult(AuthenticateResult.NoResult());
+    }
+
+    private sealed class FakeRemoteOptions : RemoteAuthenticationOptions
+    {
+        public bool SkipRequest { get; set; }
+    }
+
+    private sealed class FakeRemoteHandler(IOptionsMonitor<FakeRemoteOptions> options, ILoggerFactory logger, UrlEncoder encoder)
+        : RemoteAuthenticationHandler<FakeRemoteOptions>(options, logger, encoder)
+    {
+        protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync()
+        {
+            // Mirrors OpenIdConnectHandler.HandleRemoteAuthenticateAsync: the form_post callback body is read
+            // before the handler raises any of its events.
+            var form = await Request.ReadFormAsync(Context.RequestAborted);
+
+            if (Options.SkipRequest)
+            {
+                return HandleRequestResult.SkipHandler();
+            }
+
+            Response.StatusCode = StatusCodes.Status200OK;
+            await Response.WriteAsync($"handled:{form.Count}");
+            return HandleRequestResult.Handle();
+        }
+    }
+}

src/Security/Authentication/Core/src/Microsoft.AspNetCore.Authentication.csproj

@@ -12,6 +12,7 @@
 
   <ItemGroup>
     <Compile Include="$(SharedSourceRoot)SecurityHelper\**\*.cs" />
+    <Compile Include="$(SharedSourceRoot)RemoteAuthenticationAntiforgery.cs" LinkBase="Shared" />
   </ItemGroup>
 
   <ItemGroup>

src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs

@@ -4,6 +4,7 @@
 using System.Diagnostics;
 using System.Security.Cryptography;
 using System.Text.Encodings.Web;
+using Microsoft.AspNetCore.Antiforgery;
 using Microsoft.AspNetCore.WebUtilities;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
@@ -79,6 +80,11 @@ public virtual async Task<bool> HandleRequestAsync()
             return false;
         }
 
+        return await RemoteAuthenticationAntiforgery.HandleWithoutAntiforgeryVerdictAsync(Context, HandleRequestCoreAsync);
+    }
+
+    private async Task<bool> HandleRequestCoreAsync()
+    {
         AuthenticationTicket? ticket = null;
         Exception? exception = null;
         AuthenticationProperties? properties = null;

src/Security/Authentication/OpenIdConnect/src/Microsoft.AspNetCore.Authentication.OpenIdConnect.csproj

@@ -19,6 +19,7 @@
 
   <ItemGroup>
     <Compile Include="$(SharedSourceRoot)StringHelpers.cs" LinkBase="Shared" />
+    <Compile Include="$(SharedSourceRoot)RemoteAuthenticationAntiforgery.cs" LinkBase="Shared" />
   </ItemGroup>
 
 </Project>

src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs

@@ -11,6 +11,7 @@
 using System.Text;
 using System.Text.Encodings.Web;
 using System.Text.Json;
+using Microsoft.AspNetCore.Antiforgery;
 using Microsoft.AspNetCore.Authentication.OAuth;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.WebUtilities;
@@ -88,13 +89,16 @@ public OpenIdConnectHandler(IOptionsMonitor<OpenIdConnectOptions> options, ILogg
     /// <inheritdoc />
     public override Task<bool> HandleRequestAsync()
     {
+        // Both paths below are, like the sign-in callback, cross-site requests owned by this handler, and
+        // HandleRemoteSignOutAsync reads a form_post body, so they need the same antiforgery verdict handling
+        // that RemoteAuthenticationHandler applies to CallbackPath.
         if (Options.RemoteSignOutPath.HasValue && Options.RemoteSignOutPath == Request.Path)
         {
-            return HandleRemoteSignOutAsync();
+            return RemoteAuthenticationAntiforgery.HandleWithoutAntiforgeryVerdictAsync(Context, HandleRemoteSignOutAsync);
         }
         else if (Options.SignedOutCallbackPath.HasValue && Options.SignedOutCallbackPath == Request.Path)
         {
-            return HandleSignOutCallbackAsync();
+            return RemoteAuthenticationAntiforgery.HandleWithoutAntiforgeryVerdictAsync(Context, HandleSignOutCallbackAsync);
         }
 
         return base.HandleRequestAsync();

src/Shared/RemoteAuthenticationAntiforgery.cs

@@ -0,0 +1,45 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.AspNetCore.Http;
+
+namespace Microsoft.AspNetCore.Antiforgery;
+
+// Shared between Microsoft.AspNetCore.Authentication (RemoteAuthenticationHandler) and the remote handlers
+// that own additional callback paths of their own (e.g. Microsoft.AspNetCore.Authentication.OpenIdConnect).
+//
+// A remote provider's callback is a cross-site request by protocol design: OpenID Connect
+// response_mode=form_post and WS-Federation both deliver the response as a top-level form POST from the
+// identity provider's origin. Cross-origin CSRF protection therefore records an invalid
+// IAntiforgeryValidationFeature verdict for it, and the handler then fails on its very first action - reading
+// the callback body - before any of its events can run, so an application has no way to opt out.
+//
+// These callbacks carry their own forgery protection: the state parameter round-trips a protected
+// AuthenticationProperties payload whose correlation id must match the correlation cookie, which the handler
+// validates. The verdict is therefore suppressed while the handler owns the request, and restored when the
+// handler declines it so the rest of the pipeline still sees the original verdict.
+internal static class RemoteAuthenticationAntiforgery
+{
+    public static async Task<bool> HandleWithoutAntiforgeryVerdictAsync(HttpContext context, Func<Task<bool>> handler)
+    {
+        var suppressedVerdict = context.Features.Get<IAntiforgeryValidationFeature>();
+        if (suppressedVerdict is { IsValid: false })
+        {
+            context.Features.Set<IAntiforgeryValidationFeature?>(null);
+        }
+
+        var handled = false;
+        try
+        {
+            handled = await handler();
+            return handled;
+        }
+        finally
+        {
+            if (!handled && suppressedVerdict is { IsValid: false })
+            {
+                context.Features.Set(suppressedVerdict);
+            }
+        }
+    }
+}