Introduce a filter to send bad request results with details when Mode…

…lState is invalid

Fixes #6789

src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/FilterDescriptor.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Diagnostics;
 
 namespace Microsoft.AspNetCore.Mvc.Filters
 {
@@ -21,6 +22,7 @@ namespace Microsoft.AspNetCore.Mvc.Filters
     /// For <see cref="IExceptionFilter"/> implementations, the filter runs only after an exception has occurred,
     /// and so the observed order of execution will be opposite that of other filters.
     /// </remarks>
+    [DebuggerDisplay("Filter = {Filter.ToString(),nq}, Order = {Order}")]
     public class FilterDescriptor
     {
         /// <summary>
@@ -43,9 +45,8 @@ public FilterDescriptor(IFilterMetadata filter, int filterScope)
             Filter = filter;
             Scope = filterScope;
 
-            var orderedFilter = Filter as IOrderedFilter;
 
-            if (orderedFilter != null)
+            if (Filter is IOrderedFilter orderedFilter)
             {
                 Order = orderedFilter.Order;
             }

src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs

@@ -0,0 +1,24 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using Microsoft.AspNetCore.Mvc.ModelBinding;
+
+namespace Microsoft.AspNetCore.Mvc
+{
+    /// <summary>
+    /// Options used to configure behavior for types annotated with <see cref="ApiControllerAttribute"/>.
+    /// </summary>
+    public class ApiBehaviorOptions
+    {
+        /// <summary>
+        /// The delegate invoked on actions annotated with <see cref="ApiControllerAttribute"/> to convert invalid
+        /// <see cref="ModelStateDictionary"/> into an <see cref="IActionResult"/>
+        /// <para>
+        /// By default, the delegate produces a <see cref="BadRequestObjectResult"/> using <see cref="ProblemDetails"/>
+        /// as the problem format. To disable this feature, set the value of the delegate to <c>null</c>.
+        /// </para>
+        /// </summary>
+        public Func<ActionContext, IActionResult> InvalidModelStateResponseFactory { get; set; }
+    }
+}

src/Microsoft.AspNetCore.Mvc.Core/ApiControllerAttribute.cs

@@ -11,8 +11,8 @@ namespace Microsoft.AspNetCore.Mvc
     /// this attribute can be used to target conventions, filters and other behaviors based on the purpose
     /// of the controller.
     /// </summary>
-    [AttributeUsage(AttributeTargets.Class)]
-    public class ApiControllerAttribute : ControllerAttribute , IApiBehaviorMetadata
+    [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
+    public class ApiControllerAttribute : ControllerAttribute, IApiBehaviorMetadata
     {
     }
 }

src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs

@@ -147,6 +147,8 @@ internal static void AddMvcCoreServices(IServiceCollection services)
             //
             services.TryAddEnumerable(
                 ServiceDescriptor.Transient<IConfigureOptions<MvcOptions>, MvcCoreMvcOptionsSetup>());
+            services.TryAddEnumerable(
+                ServiceDescriptor.Transient<IConfigureOptions<MvcOptions>, ApiConventionsMvcOptionsSetup>());
             services.TryAddEnumerable(
                 ServiceDescriptor.Transient<IConfigureOptions<RouteOptions>, MvcCoreRouteOptionsSetup>());
 
@@ -157,8 +159,11 @@ internal static void AddMvcCoreServices(IServiceCollection services)
 
             services.TryAddEnumerable(
                 ServiceDescriptor.Transient<IApplicationModelProvider, DefaultApplicationModelProvider>());
+            services.TryAddEnumerable(
+                ServiceDescriptor.Transient<IApplicationModelProvider, ApiControllerApplicationModelProvider>());
             services.TryAddEnumerable(
                 ServiceDescriptor.Transient<IActionDescriptorProvider, ControllerActionDescriptorProvider>());
+
             services.TryAddSingleton<IActionDescriptorCollectionProvider, ActionDescriptorCollectionProvider>();
 
             //

src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ModelStateInvalidFilter.cs

@@ -0,0 +1,60 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using Microsoft.AspNetCore.Mvc.Filters;
+using Microsoft.AspNetCore.Mvc.Internal;
+using Microsoft.Extensions.Logging;
+
+namespace Microsoft.AspNetCore.Mvc.Infrastructure
+{
+    /// <summary>
+    /// A <see cref="IActionFilter"/> that responds to invalid <see cref="ActionContext.ModelState"/>. This filter is
+    /// added to all types and actions annotated with <see cref="ApiControllerAttribute"/>.
+    /// See <see cref="MvcOptions.ApiBehavior"/> for ways to configure this filter.
+    /// </summary>
+    public class ModelStateInvalidFilter : IActionFilter, IOrderedFilter
+    {
+        private readonly ApiBehaviorOptions _apiConventions;
+        private readonly ILogger _logger;
+
+        public ModelStateInvalidFilter(MvcOptions mvcOptions, ILogger logger)
+        {
+            _apiConventions = mvcOptions?.ApiBehavior ?? throw new ArgumentNullException(nameof(mvcOptions));
+            _logger = logger ?? throw new ArgumentNullException(nameof(logger));
+        }
+
+        /// <summary>
+        /// Gets the order value for determining the order of execution of filters. Filters execute in
+        /// ascending numeric value of the <see cref="Order"/> property.
+        /// </summary>
+        /// <remarks>
+        /// <para>
+        /// Filters are executed in a sequence determined by an ascending sort of the <see cref="Order"/> property.
+        /// </para>
+        /// <para>
+        /// The default Order for this attribute is -2000 so that it runs early in the pipeline.
+        /// </para>
+        /// <para>
+        /// Look at <see cref="IOrderedFilter.Order"/> for more detailed info.
+        /// </para>
+        /// </remarks>
+        public int Order => -2000;
+
+        /// <inheritdoc />
+        public bool IsReusable => true;
+
+        public void OnActionExecuted(ActionExecutedContext context)
+        {
+        }
+
+        public void OnActionExecuting(ActionExecutingContext context)
+        {
+            if (context.Result == null && !context.ModelState.IsValid)
+            {
+                _logger.AutoValidateModelFilterExecuting();
+                context.Result = _apiConventions.InvalidModelStateResponseFactory(context);
+            }
+        }
+    }
+}

src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiControllerApplicationModelProvider.cs

@@ -0,0 +1,61 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Linq;
+using Microsoft.AspNetCore.Mvc.ApplicationModels;
+using Microsoft.AspNetCore.Mvc.Infrastructure;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
+
+namespace Microsoft.AspNetCore.Mvc.Internal
+{
+    public class ApiControllerApplicationModelProvider : IApplicationModelProvider
+    {
+        private readonly ApiBehaviorOptions _apiConventions;
+        private readonly ModelStateInvalidFilter _modelStateInvalidFilter;
+
+        public ApiControllerApplicationModelProvider(IOptions<MvcOptions> mvcOptions, ILoggerFactory loggerFactory)
+        {
+            _apiConventions = mvcOptions.Value.ApiBehavior;
+            _modelStateInvalidFilter = new ModelStateInvalidFilter(
+                mvcOptions.Value,
+                loggerFactory.CreateLogger<ModelStateInvalidFilter>());
+        }
+
+        /// <remarks>
+        /// Order is set to execute after the <see cref="DefaultApplicationModelProvider"/>.
+        /// </remarks>
+        public int Order => -1000 + 10;
+
+        public void OnProvidersExecuted(ApplicationModelProviderContext context)
+        {
+        }
+
+        public void OnProvidersExecuting(ApplicationModelProviderContext context)
+        {
+            foreach (var controllerModel in context.Result.Controllers)
+            {
+                if (controllerModel.Attributes.OfType<IApiBehaviorMetadata>().Any())
+                {
+                    // Skip adding the filter if the feature is disabled.
+                    if (_apiConventions.InvalidModelStateResponseFactory != null)
+                    {
+                        controllerModel.Filters.Add(_modelStateInvalidFilter);
+                    }
+                }
+
+                foreach (var actionModel in controllerModel.Actions)
+                {
+                    if (actionModel.Attributes.OfType<IApiBehaviorMetadata>().Any())
+                    {
+                        // Skip adding the filter if the feature is disabled.
+                        if (_apiConventions.InvalidModelStateResponseFactory != null)
+                        {
+                            actionModel.Filters.Add(_modelStateInvalidFilter);
+                        }
+                    }
+                }
+            }
+        }
+    }
+}

src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiConventionsMvcOptionsSetup.cs

@@ -0,0 +1,39 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using Microsoft.AspNetCore.Mvc.Infrastructure;
+using Microsoft.Extensions.Options;
+
+namespace Microsoft.AspNetCore.Mvc.Internal
+{
+    public class ApiConventionsMvcOptionsSetup : IConfigureOptions<MvcOptions>
+    {
+        private readonly IErrorDescriptionFactory _errorDescriptionFactory;
+
+        public ApiConventionsMvcOptionsSetup(IErrorDescriptionFactory errorDescriptionFactory)
+        {
+            _errorDescriptionFactory = errorDescriptionFactory;
+        }
+
+        public void Configure(MvcOptions options)
+        {
+            options.ApiBehavior.InvalidModelStateResponseFactory = GetInvalidModelStateResponse;
+
+            IActionResult GetInvalidModelStateResponse(ActionContext context)
+            {
+                var errorDetails = _errorDescriptionFactory.CreateErrorDescription(
+                    context.ActionDescriptor, 
+                    new ValidationProblemDetails(context.ModelState));
+
+                return new BadRequestObjectResult(errorDetails)
+                {
+                    ContentTypes =
+                    {
+                        "application/problem+json",
+                        "application/problem+xml",
+                    },
+                };
+            }
+        }
+    }
+}

src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs

@@ -79,7 +79,8 @@ internal static class MvcCoreLoggerExtensions
 
         private static readonly Action<ILogger, Exception> _cannotApplyRequestFormLimits;
         private static readonly Action<ILogger, Exception> _appliedRequestFormLimits;
-
+
+        private static readonly Action<ILogger, Exception> _autoValidateModelFilterExecuting;
 
         static MvcCoreLoggerExtensions()
         {
@@ -282,6 +283,12 @@ static MvcCoreLoggerExtensions()
                 LogLevel.Debug,
                 2,
                 "Applied the configured form options on the current request.");
+
+            _autoValidateModelFilterExecuting = LoggerMessage.Define(
+                LogLevel.Debug,
+                1,
+                "AutoValidateModeFilter returned a BadRequestObjectResult since the ModelState is invalid.");
+
         }
 
         public static IDisposable ActionScope(this ILogger logger, ActionDescriptor action)
@@ -592,6 +599,11 @@ public static void AppliedRequestFormLimits(this ILogger logger)
             _appliedRequestFormLimits(logger, null);
         }
 
+        public static void AutoValidateModelFilterExecuting(this ILogger logger)
+        {
+            _autoValidateModelFilterExecuting(logger, null);
+        }
+
         private class ActionLogScope : IReadOnlyList<KeyValuePair<string, object>>
         {
             private readonly ActionDescriptor _action;

src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs

@@ -168,5 +168,10 @@ public int MaxModelValidationErrors
         /// <see langword="false"/> by default.
         /// </summary>
         public bool AllowBindingUndefinedValueToEnumType { get; set; }
+
+        /// <summary>
+        /// Gets <see cref="ApiBehaviorOptions"/> used to configure behavior on types annotated with <see cref="ApiControllerAttribute"/>.
+        /// </summary>
+        public ApiBehaviorOptions ApiBehavior { get; } = new ApiBehaviorOptions();
     }
 }

test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ModelStateInvalidFilterTest.cs

@@ -0,0 +1,90 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc.Abstractions;
+using Microsoft.AspNetCore.Mvc.Filters;
+using Microsoft.AspNetCore.Routing;
+using Microsoft.Extensions.Logging.Abstractions;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Mvc.Infrastructure
+{
+    public class ModelStateInvalidFilterTest
+    {
+        [Fact]
+        public void OnActionExecuting_NoOpsIfResultIsAlreadySet()
+        {
+            // Arrange
+            var filter = new ModelStateInvalidFilter(new MvcOptions
+            {
+                ApiBehavior =
+                {
+                   InvalidModelStateResponseFactory = _ => new BadRequestResult(),
+                },
+            }, NullLogger.Instance);
+            var context = GetActionExecutingContext();
+            var expected = new OkResult();
+            context.Result = expected;
+
+            // Act
+            filter.OnActionExecuting(context);
+
+            // Assert
+            Assert.Same(expected, context.Result);
+        }
+
+        [Fact]
+        public void OnActionExecuting_NoOpsIfModelStateIsValid()
+        {
+            // Arrange
+            var filter = new ModelStateInvalidFilter(new MvcOptions
+            {
+                ApiBehavior =
+                {
+                   InvalidModelStateResponseFactory = _ => new BadRequestResult(),
+                },
+            }, NullLogger.Instance);
+            var context = GetActionExecutingContext();
+
+            // Act
+            filter.OnActionExecuting(context);
+
+            // Assert
+            Assert.Null(context.Result);
+        }
+
+        [Fact]
+        public void OnActionExecuting_InvokesResponseFactoryIfModelStateIsInvalid()
+        {
+            // Arrange
+            var expected = new BadRequestResult();
+            var filter = new ModelStateInvalidFilter(new MvcOptions
+            {
+                ApiBehavior =
+                {
+                   InvalidModelStateResponseFactory = _ => expected,
+                },
+            }, NullLogger.Instance);
+            var context = GetActionExecutingContext();
+            context.ModelState.AddModelError("some-key", "some-error");
+
+            // Act
+            filter.OnActionExecuting(context);
+
+            // Assert
+            Assert.Same(expected, context.Result);
+        }
+
+        private static ActionExecutingContext GetActionExecutingContext()
+        {
+            return new ActionExecutingContext(
+                new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()),
+                Array.Empty<IFilterMetadata>(),
+                new Dictionary<string, object>(),
+                new object());
+        }
+    }
+}