Introduce a filter to send bad request results with details when ModelState is invalid (#6849)

* Introduce a filter to send bad request results with details when ModelState is invalid

Fixes #6789

Author: pranavkm

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,37 @@
+// 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
+    {
+        private Func<ActionContext, IActionResult> _invalidModelStateResponseFactory;
+
+        /// <summary>
+        /// 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.
+        /// </para>
+        /// </summary>
+        public Func<ActionContext, IActionResult> InvalidModelStateResponseFactory
+        {
+            get => _invalidModelStateResponseFactory;
+            set => _invalidModelStateResponseFactory = value ?? throw new ArgumentNullException(nameof(value));
+        }
+
+        /// <summary>
+        /// Disables the filter that returns an <see cref="BadRequestObjectResult"/> when
+        /// <see cref="ActionContext.ModelState"/> is invalid. 
+        /// <seealso cref="InvalidModelStateResponseFactory"/>.
+        /// </summary>
+        public bool EnableModelStateInvalidFilter { get; set; } = true;
+    }
+}

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<ApiBehaviorOptions>, ApiBehaviorOptionsSetup>());
             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="ApiBehaviorOptions"/> for ways to configure this filter.
+    /// </summary>
+    public class ModelStateInvalidFilter : IActionFilter, IOrderedFilter
+    {
+        private readonly ApiBehaviorOptions _apiBehaviorOptions;
+        private readonly ILogger _logger;
+
+        public ModelStateInvalidFilter(ApiBehaviorOptions apiBehaviorOptions, ILogger logger)
+        {
+            _apiBehaviorOptions = apiBehaviorOptions ?? throw new ArgumentNullException(nameof(apiBehaviorOptions));
+            _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.ModelStateInvalidFilterExecuting();
+                context.Result = _apiBehaviorOptions.InvalidModelStateResponseFactory(context);
+            }
+        }
+    }
+}

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

@@ -0,0 +1,45 @@
+// 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.Infrastructure;
+using Microsoft.Extensions.Options;
+
+namespace Microsoft.AspNetCore.Mvc.Internal
+{
+    public class ApiBehaviorOptionsSetup : IConfigureOptions<ApiBehaviorOptions>
+    {
+        private readonly IErrorDescriptionFactory _errorDescriptionFactory;
+
+        public ApiBehaviorOptionsSetup(IErrorDescriptionFactory errorDescriptionFactory)
+        {
+            _errorDescriptionFactory = errorDescriptionFactory;
+        }
+
+        public void Configure(ApiBehaviorOptions options)
+        {
+            if (options == null)
+            {
+                throw new ArgumentNullException(nameof(options));
+            }
+
+            options.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/ApiControllerApplicationModelProvider.cs

@@ -0,0 +1,73 @@
+// 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.Diagnostics;
+using System.Linq;
+using Microsoft.AspNetCore.Mvc.ApplicationModels;
+using Microsoft.AspNetCore.Mvc.Core;
+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 _apiBehaviorOptions;
+        private readonly ModelStateInvalidFilter _modelStateInvalidFilter;
+
+        public ApiControllerApplicationModelProvider(IOptions<ApiBehaviorOptions> apiBehaviorOptions, ILoggerFactory loggerFactory)
+        {
+            _apiBehaviorOptions = apiBehaviorOptions.Value;
+            if (_apiBehaviorOptions.EnableModelStateInvalidFilter && _apiBehaviorOptions.InvalidModelStateResponseFactory == null)
+            {
+                throw new ArgumentException(Resources.FormatPropertyOfTypeCannotBeNull(
+                    typeof(ApiBehaviorOptions),
+                    nameof(ApiBehaviorOptions.InvalidModelStateResponseFactory)));
+            }
+
+            _modelStateInvalidFilter = new ModelStateInvalidFilter(
+                apiBehaviorOptions.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())
+                {
+                    if (_apiBehaviorOptions.EnableModelStateInvalidFilter)
+                    {
+                        Debug.Assert(_apiBehaviorOptions.InvalidModelStateResponseFactory != null);
+                        controllerModel.Filters.Add(_modelStateInvalidFilter);
+                    }
+
+                    continue;
+                }
+
+                foreach (var actionModel in controllerModel.Actions)
+                {
+                    if (actionModel.Attributes.OfType<IApiBehaviorMetadata>().Any())
+                    {
+                        if (_apiBehaviorOptions.EnableModelStateInvalidFilter)
+                        {
+                            Debug.Assert(_apiBehaviorOptions.InvalidModelStateResponseFactory != null);
+                            actionModel.Filters.Add(_modelStateInvalidFilter);
+                        }
+                    }
+                }
+            }
+        }
+    }
+}

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> _modelStateInvalidFilterExecuting;
 
         static MvcCoreLoggerExtensions()
         {
@@ -282,6 +283,12 @@ static MvcCoreLoggerExtensions()
                 LogLevel.Debug,
                 2,
                 "Applied the configured form options on the current request.");
+
+            _modelStateInvalidFilterExecuting = LoggerMessage.Define(
+                LogLevel.Debug,
+                1,
+                "The request has model state errors, returning an error response.");
+
         }
 
         public static IDisposable ActionScope(this ILogger logger, ActionDescriptor action)
@@ -592,6 +599,8 @@ public static void AppliedRequestFormLimits(this ILogger logger)
             _appliedRequestFormLimits(logger, null);
         }
 
+        public static void ModelStateInvalidFilterExecuting(this ILogger logger) => _modelStateInvalidFilterExecuting(logger, null);
+
         private class ActionLogScope : IReadOnlyList<KeyValuePair<string, object>>
         {
             private readonly ActionDescriptor _action;

test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs

@@ -248,6 +248,13 @@ private Dictionary<Type, Type[]> MutliRegistrationServiceTypes
                             typeof(MvcCoreRouteOptionsSetup),
                         }
                     },
+                    {
+                        typeof(IConfigureOptions<ApiBehaviorOptions>),
+                        new Type[]
+                        {
+                            typeof(ApiBehaviorOptionsSetup),
+                        }
+                    },
                     {
                         typeof(IActionConstraintProvider),
                         new Type[]
@@ -288,6 +295,7 @@ private Dictionary<Type, Type[]> MutliRegistrationServiceTypes
                         new Type[]
                         {
                             typeof(DefaultApplicationModelProvider),
+                            typeof(ApiControllerApplicationModelProvider),
                         }
                     },
                 };