Rework how we add metadata to skip antiforgery checks

samples/JS.Yarp/Startup.cs

@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using Duende.Bff;
 using Duende.Bff.Yarp;
 using Microsoft.AspNetCore.Builder;

src/Duende.Bff.Yarp/BffYarpEndpointRouteBuilderExtensions.cs

@@ -19,20 +19,17 @@ public static class BffYarpEndpointRouteBuilderExtensions
     /// <param name="endpoints"></param>
     /// <param name="localPath"></param>
     /// <param name="apiAddress"></param>
-    /// <param name="requireAntiForgeryCheck"></param>
     /// <returns></returns>
     public static IEndpointConventionBuilder MapRemoteBffApiEndpoint(
         this IEndpointRouteBuilder endpoints,
         PathString localPath, 
-        string apiAddress, 
-        bool requireAntiForgeryCheck = true)
+        string apiAddress)
     {
         endpoints.CheckLicense();
 
         return endpoints.Map(
                 localPath.Add("/{**catch-all}").Value!,
                 RemoteApiEndpoint.Map(localPath, apiAddress))
-            .WithMetadata(new BffRemoteApiEndpointMetadata { RequireAntiForgeryHeader =  requireAntiForgeryCheck });
-
+            .WithMetadata(new BffRemoteApiEndpointMetadata());
     }
 }

src/Duende.Bff.Yarp/ReverseProxyEndpointConventionBuilderExtensions.cs

@@ -17,38 +17,36 @@ public static class ReverseProxyEndpointConventionBuilderExtensions
     /// </summary>
     /// <param name="endpoints"></param>
     /// <param name="configureAction"></param>
-    /// <param name="requireAntiforgeryCheck"></param>
     /// <returns></returns>
     public static ReverseProxyConventionBuilder MapBffReverseProxy(this IEndpointRouteBuilder endpoints,
-        Action<IReverseProxyApplicationBuilder> configureAction,
-        bool requireAntiforgeryCheck = true)
+        Action<IReverseProxyApplicationBuilder> configureAction)
     {
         return endpoints.MapReverseProxy(configureAction)
-            .AsBffApiEndpoint(requireAntiforgeryCheck);
+            .AsBffApiEndpoint();
     }
 
     /// <summary>
     /// Adds YARP with anti-forgery protection 
     /// </summary>
     /// <param name="endpoints"></param>
-    /// <param name="requireAntiforgeryCheck"></param>
     /// <returns></returns>
-    public static ReverseProxyConventionBuilder MapBffReverseProxy(this IEndpointRouteBuilder endpoints,
-        bool requireAntiforgeryCheck = true)
+    public static ReverseProxyConventionBuilder MapBffReverseProxy(this IEndpointRouteBuilder endpoints)
     {
         return endpoints.MapReverseProxy()
-            .AsBffApiEndpoint(requireAntiforgeryCheck);
+            .AsBffApiEndpoint();
     }
-
+
+    // TODO: do we also need a SkipAntiforgery API?
+    // TODO: review the API comment below
+
     /// <summary>
     /// Adds anti-forgery protection to YARP
     /// </summary>
     /// <param name="builder"></param>
-    /// <param name="requireAntiforgeryCheck"></param>
     /// <returns></returns>
-    public static ReverseProxyConventionBuilder AsBffApiEndpoint(this ReverseProxyConventionBuilder builder,
-        bool requireAntiforgeryCheck = true)
+    public static ReverseProxyConventionBuilder AsBffApiEndpoint(this ReverseProxyConventionBuilder builder)
     {
-        return builder.WithMetadata(new BffApiAttribute(requireAntiforgeryCheck));
+        return builder.WithMetadata(new BffApiAttribute());
     }
+
 }

src/Duende.Bff/BffMiddleware.cs

@@ -1,7 +1,6 @@
 // Copyright (c) Duende Software. All rights reserved.
 // See LICENSE in the project root for license information.
 
-using System.Linq;
 using System.Threading.Tasks;
 using Duende.Bff.Logging;
 using Microsoft.AspNetCore.Http;
@@ -51,25 +50,11 @@ public async Task Invoke(HttpContext context)
             return;
         }
 
-        var localEndpointMetadata = endpoint.Metadata.GetOrderedMetadata<BffApiAttribute>();
-        if (localEndpointMetadata.Any())
+        var isBffEndpoint = endpoint.Metadata.GetMetadata<IBffApiEndpoint>() != null;
+        if (isBffEndpoint)
         {
-            var requireLocalAntiForgeryCheck = localEndpointMetadata.First().RequireAntiForgeryCheck;
-            if (requireLocalAntiForgeryCheck)
-            {
-                if (!context.CheckAntiForgeryHeader(_options))
-                {
-                    _logger.AntiForgeryValidationFailed(context.Request.Path);
-
-                    context.Response.StatusCode = 401;
-                    return;
-                }
-            }
-        }
-        else
-        {
-            var remoteEndpoint = endpoint.Metadata.GetMetadata<BffRemoteApiEndpointMetadata>();
-            if (remoteEndpoint is { RequireAntiForgeryHeader: true })
+            var requireAntiForgeryCheck = endpoint.Metadata.GetMetadata<IBffApiSkipAntiforgry>() == null;
+            if (requireAntiForgeryCheck)
             {
                 if (!context.CheckAntiForgeryHeader(_options))
                 {

src/Duende.Bff/Endpoints/BffApiAttribute.cs

@@ -6,23 +6,10 @@
 namespace Duende.Bff;
 
 /// <summary>
-/// Decorates a controller as a local BFF API endpoint
-/// This allows the BFF midleware to automatically add the antiforgery header checks as well as 302 to 401 conversion
+/// Decorates a controller or action as a local BFF API endpoint
+/// By default, this provides anti-forgery protection and response handling.
 /// </summary>
 [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)]
-public class BffApiAttribute : Attribute
+public class BffApiAttribute : Attribute, IBffApiEndpoint
 {
-    /// <summary>
-    /// specifies if anti-forgery check is required
-    /// </summary>
-    public bool RequireAntiForgeryCheck { get; }
-
-    /// <summary>
-    /// ctor
-    /// </summary>
-    /// <param name="requireAntiForgeryCheck">Specifies if the antiforgery header gets checked</param>
-    public BffApiAttribute(bool requireAntiForgeryCheck = true)
-    {
-        RequireAntiForgeryCheck = requireAntiForgeryCheck;
-    }
-}
+}

src/Duende.Bff/Endpoints/BffApiSkipAntiforgeryAttribute.cs

@@ -0,0 +1,14 @@
+// Copyright (c) Duende Software. All rights reserved.
+// See LICENSE in the project root for license information.
+
+using System;
+
+namespace Duende.Bff;
+
+/// <summary>
+/// This attribute indicates that the BFF midleware will ignore the antiforgery header checks.
+/// </summary>
+[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)]
+public class BffApiSkipAntiforgeryAttribute : Attribute, IBffApiSkipAntiforgry
+{
+}

src/Duende.Bff/Endpoints/BffAuthorizationMiddlewareResultHandler.cs

@@ -32,25 +32,21 @@ public BffAuthorizationMiddlewareResultHandler()
             var endpoint = context.GetEndpoint();
 
             if (endpoint == null) throw new ArgumentNullException(nameof(endpoint));
-
-            var remoteApi = endpoint.Metadata.GetMetadata<BffRemoteApiEndpointMetadata>();
-            var localApi = endpoint.Metadata.GetMetadata<BffApiAttribute>();
 
-            if (remoteApi == null && localApi == null)
+            var isBffEndpoint = endpoint.Metadata.GetMetadata<IBffApiEndpoint>() != null;
+            if (isBffEndpoint)
             {
-                return _handler.HandleAsync(next, context, policy, authorizeResult);
-            }
-
-            if (authorizeResult.Challenged)
-            {
-                context.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
-                return Task.CompletedTask;
-            }
+                if (authorizeResult.Challenged)
+                {
+                    context.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
+                    return Task.CompletedTask;
+                }
 
-            if (authorizeResult.Forbidden)
-            {
-                context.Response.StatusCode = (int)HttpStatusCode.Forbidden;
-                return Task.CompletedTask;
+                if (authorizeResult.Forbidden)
+                {
+                    context.Response.StatusCode = (int)HttpStatusCode.Forbidden;
+                    return Task.CompletedTask;
+                }
             }
 
             return _handler.HandleAsync(next, context, policy, authorizeResult);

src/Duende.Bff/Endpoints/BffRemoteApiEndpointMetadata.cs

@@ -6,7 +6,7 @@ namespace Duende.Bff;
 /// <summary>
 /// Endpoint metadata for a remote BFF API endpoint
 /// </summary>
-public class BffRemoteApiEndpointMetadata
+public class BffRemoteApiEndpointMetadata : IBffApiEndpoint
 {
     /// <summary>
     /// Required token type (if any)
@@ -18,11 +18,6 @@ public class BffRemoteApiEndpointMetadata
     /// </summary>
     public bool OptionalUserToken { get; set; }
 
-    /// <summary>
-    /// Require presence of anti-forgery header
-    /// </summary>
-    public bool RequireAntiForgeryHeader { get; set; } = true;
-
     /// <summary>
     /// Maps to UserAccessTokenParameters and included if set
     /// </summary>

src/Duende.Bff/Endpoints/EndpointConventionBuilderExtensions.cs

@@ -15,10 +15,19 @@ public static class EndpointConventionBuilderExtensions
     /// This metadata is used by the BFF middleware.
     /// </summary>
     /// <param name="builder"></param>
-    /// <param name="requireAntiForgeryCheck">Specifies if the antiforgery header gets checked</param>
     /// <returns></returns>
-    public static IEndpointConventionBuilder AsBffApiEndpoint(this IEndpointConventionBuilder builder, bool requireAntiForgeryCheck = true)
+    public static IEndpointConventionBuilder AsBffApiEndpoint(this IEndpointConventionBuilder builder)
     {
-        return builder.WithMetadata(new BffApiAttribute(requireAntiForgeryCheck));
+        return builder.WithMetadata(new BffApiAttribute());
+    }
+
+    /// <summary>
+    /// Adds marker that will cause the BFF framework to skip all antiforgery for this endpoint.
+    /// </summary>
+    /// <param name="builder"></param>
+    /// <returns></returns>
+    public static IEndpointConventionBuilder SkipAntiforgery(this IEndpointConventionBuilder builder)
+    {
+        return builder.WithMetadata(new BffApiSkipAntiforgeryAttribute());
     }
 }

src/Duende.Bff/Endpoints/IBffApiEndpoint.cs

@@ -0,0 +1,12 @@
+// Copyright (c) Duende Software. All rights reserved.
+// See LICENSE in the project root for license information.
+
+namespace Duende.Bff;
+
+/// <summary>
+/// Marks an endpoint as BFF API endpoint.
+/// By default, this provides anti-forgery protection and response handling.
+/// </summary>
+public interface IBffApiEndpoint
+{
+}

src/Duende.Bff/Endpoints/IBffApiSkipAntiforgry.cs

@@ -0,0 +1,11 @@
+// Copyright (c) Duende Software. All rights reserved.
+// See LICENSE in the project root for license information.
+
+namespace Duende.Bff;
+
+/// <summary>
+/// Indicates that the BFF midleware will ignore the antiforgery header checks.
+/// </summary>
+public interface IBffApiSkipAntiforgry
+{
+}

test/Duende.Bff.Tests/Endpoints/LocalEndpointTests.cs

@@ -34,7 +34,7 @@ public async Task calls_to_authorized_local_endpoint_should_succeed()
         }
 
         [Fact]
-        public async Task calls_to_authorized_local_endpoint_without_csrf_should_succeed()
+        public async Task calls_to_authorized_local_endpoint_without_csrf_should_succeed_without_antiforgery_header()
         {
             await BffHost.BffLoginAsync("alice");
 
@@ -61,7 +61,7 @@ public async Task unauthenticated_calls_to_authorized_local_endpoint_should_fail
         }
 
         [Fact]
-        public async Task calls_to_local_endpoint_should_require_csrf()
+        public async Task calls_to_local_endpoint_should_require_antiforgery_header()
         {
             var req = new HttpRequestMessage(HttpMethod.Get, BffHost.Url("/local_anon"));
             var response = await BffHost.BrowserClient.SendAsync(req);
@@ -70,7 +70,7 @@ public async Task calls_to_local_endpoint_should_require_csrf()
         }
 
         [Fact]
-        public async Task calls_to_local_endpoint_without_antiforgery_should_not_require_csrf()
+        public async Task calls_to_local_endpoint_without_csrf_should_not_require_antiforgery_header()
         {
             var req = new HttpRequestMessage(HttpMethod.Get, BffHost.Url("/local_anon_no_csrf"));
             var response = await BffHost.BrowserClient.SendAsync(req);

test/Duende.Bff.Tests/TestHosts/BffHost.cs

@@ -205,7 +205,8 @@ private void Configure(IApplicationBuilder app)
                         context.Response.ContentType = "application/json";
                         await context.Response.WriteAsync(JsonSerializer.Serialize(response));
                     })
-                    .AsBffApiEndpoint(requireAntiForgeryCheck: false);
+                    .AsBffApiEndpoint()
+                    .SkipAntiforgery();
 
                 endpoints.Map("/local_authz", async context =>
                     {
@@ -271,7 +272,8 @@ private void Configure(IApplicationBuilder app)
                         await context.Response.WriteAsync(JsonSerializer.Serialize(response));
                     })
                     .RequireAuthorization()
-                    .AsBffApiEndpoint(requireAntiForgeryCheck: false);
+                    .AsBffApiEndpoint()
+                    .SkipAntiforgery();
 
 
                 endpoints.Map("/always_fail_authz_non_bff_endpoint", context => { return Task.CompletedTask; })
@@ -286,7 +288,8 @@ private void Configure(IApplicationBuilder app)
                     .RequireAccessToken();
 
                 endpoints.MapRemoteBffApiEndpoint(
-                        "/api_user_no_csrf", _apiHost.Url(), requireAntiForgeryCheck: false)
+                        "/api_user_no_csrf", _apiHost.Url())
+                    .SkipAntiforgery()
                     .RequireAccessToken();
 
                 endpoints.MapRemoteBffApiEndpoint(