Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Commit 0626a72

Browse files
pranavkmpranavkm
committed
Introduce a filter to send bad request results with details when ModelState is invalid
Fixes #6789
1 parent 47287c5 commit 0626a72

File tree

19 files changed

+723
-6
lines changed

19 files changed

+723
-6
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using System.Diagnostics;
56

67
namespace Microsoft.AspNetCore.Mvc.Filters
78
{
@@ -21,6 +22,7 @@ namespace Microsoft.AspNetCore.Mvc.Filters
2122
/// For <see cref="IExceptionFilter"/> implementations, the filter runs only after an exception has occurred,
2223
/// and so the observed order of execution will be opposite that of other filters.
2324
/// </remarks>
25+
[DebuggerDisplay("Filter = {Filter.ToString(),nq}, Order = {Order}")]
2426
public class FilterDescriptor
2527
{
2628
/// <summary>
@@ -43,9 +45,8 @@ public FilterDescriptor(IFilterMetadata filter, int filterScope)
4345
Filter = filter;
4446
Scope = filterScope;
4547

46-
var orderedFilter = Filter as IOrderedFilter;
4748

48-
if (orderedFilter != null)
49+
if (Filter is IOrderedFilter orderedFilter)
4950
{
5051
Order = orderedFilter.Order;
5152
}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using Microsoft.AspNetCore.Mvc.ModelBinding;
6+
7+
namespace Microsoft.AspNetCore.Mvc
8+
{
9+
/// <summary>
10+
/// Options used to configure behavior for types annotated with <see cref="ApiControllerAttribute"/>.
11+
/// </summary>
12+
public class ApiBehaviorOptions
13+
{
14+
/// <summary>
15+
/// The delegate invoked on actions annotated with <see cref="ApiControllerAttribute"/> to convert invalid
16+
/// <see cref="ModelStateDictionary"/> into an <see cref="IActionResult"/>
17+
/// <para>
18+
/// By default, the delegate produces a <see cref="BadRequestObjectResult"/> using <see cref="ProblemDetails"/>
19+
/// as the problem format. To disable this feature, set the value of the delegate to <c>null</c>.
20+
/// </para>
21+
/// </summary>
22+
public Func<ActionContext, IActionResult> InvalidModelStateResponseFactory { get; set; }
23+
}
24+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ namespace Microsoft.AspNetCore.Mvc
1111
/// this attribute can be used to target conventions, filters and other behaviors based on the purpose
1212
/// of the controller.
1313
/// </summary>
14-
[AttributeUsage(AttributeTargets.Class)]
15-
public class ApiControllerAttribute : ControllerAttribute , IApiBehaviorMetadata
14+
[AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
15+
public class ApiControllerAttribute : ControllerAttribute, IApiBehaviorMetadata
1616
{
1717
}
1818
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ internal static void AddMvcCoreServices(IServiceCollection services)
147147
//
148148
services.TryAddEnumerable(
149149
ServiceDescriptor.Transient<IConfigureOptions<MvcOptions>, MvcCoreMvcOptionsSetup>());
150+
services.TryAddEnumerable(
151+
ServiceDescriptor.Transient<IConfigureOptions<MvcOptions>, ApiConventionsMvcOptionsSetup>());
150152
services.TryAddEnumerable(
151153
ServiceDescriptor.Transient<IConfigureOptions<RouteOptions>, MvcCoreRouteOptionsSetup>());
152154

@@ -157,8 +159,11 @@ internal static void AddMvcCoreServices(IServiceCollection services)
157159

158160
services.TryAddEnumerable(
159161
ServiceDescriptor.Transient<IApplicationModelProvider, DefaultApplicationModelProvider>());
162+
services.TryAddEnumerable(
163+
ServiceDescriptor.Transient<IApplicationModelProvider, ApiControllerApplicationModelProvider>());
160164
services.TryAddEnumerable(
161165
ServiceDescriptor.Transient<IActionDescriptorProvider, ControllerActionDescriptorProvider>());
166+
162167
services.TryAddSingleton<IActionDescriptorCollectionProvider, ActionDescriptorCollectionProvider>();
163168

164169
//

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using Microsoft.AspNetCore.Mvc.Filters;
6+
using Microsoft.AspNetCore.Mvc.Internal;
7+
using Microsoft.Extensions.Logging;
8+
9+
namespace Microsoft.AspNetCore.Mvc.Infrastructure
10+
{
11+
/// <summary>
12+
/// A <see cref="IActionFilter"/> that responds to invalid <see cref="ActionContext.ModelState"/>. This filter is
13+
/// added to all types and actions annotated with <see cref="ApiControllerAttribute"/>.
14+
/// See <see cref="MvcOptions.ApiBehavior"/> for ways to configure this filter.
15+
/// </summary>
16+
public class ModelStateInvalidFilter : IActionFilter, IOrderedFilter
17+
{
18+
private readonly ApiBehaviorOptions _apiConventions;
19+
private readonly ILogger _logger;
20+
21+
public ModelStateInvalidFilter(MvcOptions mvcOptions, ILogger logger)
22+
{
23+
_apiConventions = mvcOptions?.ApiBehavior ?? throw new ArgumentNullException(nameof(mvcOptions));
24+
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
25+
}
26+
27+
/// <summary>
28+
/// Gets the order value for determining the order of execution of filters. Filters execute in
29+
/// ascending numeric value of the <see cref="Order"/> property.
30+
/// </summary>
31+
/// <remarks>
32+
/// <para>
33+
/// Filters are executed in a sequence determined by an ascending sort of the <see cref="Order"/> property.
34+
/// </para>
35+
/// <para>
36+
/// The default Order for this attribute is -2000 so that it runs early in the pipeline.
37+
/// </para>
38+
/// <para>
39+
/// Look at <see cref="IOrderedFilter.Order"/> for more detailed info.
40+
/// </para>
41+
/// </remarks>
42+
public int Order => -2000;
43+
44+
/// <inheritdoc />
45+
public bool IsReusable => true;
46+
47+
public void OnActionExecuted(ActionExecutedContext context)
48+
{
49+
}
50+
51+
public void OnActionExecuting(ActionExecutingContext context)
52+
{
53+
if (context.Result == null && !context.ModelState.IsValid)
54+
{
55+
_logger.AutoValidateModelFilterExecuting();
56+
context.Result = _apiConventions.InvalidModelStateResponseFactory(context);
57+
}
58+
}
59+
}
60+
}

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System.Linq;
5+
using Microsoft.AspNetCore.Mvc.ApplicationModels;
6+
using Microsoft.AspNetCore.Mvc.Infrastructure;
7+
using Microsoft.Extensions.Logging;
8+
using Microsoft.Extensions.Options;
9+
10+
namespace Microsoft.AspNetCore.Mvc.Internal
11+
{
12+
public class ApiControllerApplicationModelProvider : IApplicationModelProvider
13+
{
14+
private readonly ApiBehaviorOptions _apiConventions;
15+
private readonly ModelStateInvalidFilter _modelStateInvalidFilter;
16+
17+
public ApiControllerApplicationModelProvider(IOptions<MvcOptions> mvcOptions, ILoggerFactory loggerFactory)
18+
{
19+
_apiConventions = mvcOptions.Value.ApiBehavior;
20+
_modelStateInvalidFilter = new ModelStateInvalidFilter(
21+
mvcOptions.Value,
22+
loggerFactory.CreateLogger<ModelStateInvalidFilter>());
23+
}
24+
25+
/// <remarks>
26+
/// Order is set to execute after the <see cref="DefaultApplicationModelProvider"/>.
27+
/// </remarks>
28+
public int Order => -1000 + 10;
29+
30+
public void OnProvidersExecuted(ApplicationModelProviderContext context)
31+
{
32+
}
33+
34+
public void OnProvidersExecuting(ApplicationModelProviderContext context)
35+
{
36+
foreach (var controllerModel in context.Result.Controllers)
37+
{
38+
if (controllerModel.Attributes.OfType<IApiBehaviorMetadata>().Any())
39+
{
40+
// Skip adding the filter if the feature is disabled.
41+
if (_apiConventions.InvalidModelStateResponseFactory != null)
42+
{
43+
controllerModel.Filters.Add(_modelStateInvalidFilter);
44+
}
45+
}
46+
47+
foreach (var actionModel in controllerModel.Actions)
48+
{
49+
if (actionModel.Attributes.OfType<IApiBehaviorMetadata>().Any())
50+
{
51+
// Skip adding the filter if the feature is disabled.
52+
if (_apiConventions.InvalidModelStateResponseFactory != null)
53+
{
54+
actionModel.Filters.Add(_modelStateInvalidFilter);
55+
}
56+
}
57+
}
58+
}
59+
}
60+
}
61+
}

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using Microsoft.AspNetCore.Mvc.Infrastructure;
5+
using Microsoft.Extensions.Options;
6+
7+
namespace Microsoft.AspNetCore.Mvc.Internal
8+
{
9+
public class ApiConventionsMvcOptionsSetup : IConfigureOptions<MvcOptions>
10+
{
11+
private readonly IErrorDescriptionFactory _errorDescriptionFactory;
12+
13+
public ApiConventionsMvcOptionsSetup(IErrorDescriptionFactory errorDescriptionFactory)
14+
{
15+
_errorDescriptionFactory = errorDescriptionFactory;
16+
}
17+
18+
public void Configure(MvcOptions options)
19+
{
20+
options.ApiBehavior.InvalidModelStateResponseFactory = GetInvalidModelStateResponse;
21+
22+
IActionResult GetInvalidModelStateResponse(ActionContext context)
23+
{
24+
var errorDetails = _errorDescriptionFactory.CreateErrorDescription(
25+
context.ActionDescriptor,
26+
new ValidationProblemDetails(context.ModelState));
27+
28+
return new BadRequestObjectResult(errorDetails)
29+
{
30+
ContentTypes =
31+
{
32+
"application/problem+json",
33+
"application/problem+xml",
34+
},
35+
};
36+
}
37+
}
38+
}
39+
}

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,8 @@ internal static class MvcCoreLoggerExtensions
7979

8080
private static readonly Action<ILogger, Exception> _cannotApplyRequestFormLimits;
8181
private static readonly Action<ILogger, Exception> _appliedRequestFormLimits;
82-
82+
83+
private static readonly Action<ILogger, Exception> _autoValidateModelFilterExecuting;
8384

8485
static MvcCoreLoggerExtensions()
8586
{
@@ -282,6 +283,12 @@ static MvcCoreLoggerExtensions()
282283
LogLevel.Debug,
283284
2,
284285
"Applied the configured form options on the current request.");
286+
287+
_autoValidateModelFilterExecuting = LoggerMessage.Define(
288+
LogLevel.Debug,
289+
1,
290+
"AutoValidateModeFilter returned a BadRequestObjectResult since the ModelState is invalid.");
291+
285292
}
286293

287294
public static IDisposable ActionScope(this ILogger logger, ActionDescriptor action)
@@ -592,6 +599,11 @@ public static void AppliedRequestFormLimits(this ILogger logger)
592599
_appliedRequestFormLimits(logger, null);
593600
}
594601

602+
public static void AutoValidateModelFilterExecuting(this ILogger logger)
603+
{
604+
_autoValidateModelFilterExecuting(logger, null);
605+
}
606+
595607
private class ActionLogScope : IReadOnlyList<KeyValuePair<string, object>>
596608
{
597609
private readonly ActionDescriptor _action;

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,5 +168,10 @@ public int MaxModelValidationErrors
168168
/// <see langword="false"/> by default.
169169
/// </summary>
170170
public bool AllowBindingUndefinedValueToEnumType { get; set; }
171+
172+
/// <summary>
173+
/// Gets <see cref="ApiBehaviorOptions"/> used to configure behavior on types annotated with <see cref="ApiControllerAttribute"/>.
174+
/// </summary>
175+
public ApiBehaviorOptions ApiBehavior { get; } = new ApiBehaviorOptions();
171176
}
172177
}

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using Microsoft.AspNetCore.Http;
7+
using Microsoft.AspNetCore.Mvc.Abstractions;
8+
using Microsoft.AspNetCore.Mvc.Filters;
9+
using Microsoft.AspNetCore.Routing;
10+
using Microsoft.Extensions.Logging.Abstractions;
11+
using Xunit;
12+
13+
namespace Microsoft.AspNetCore.Mvc.Infrastructure
14+
{
15+
public class ModelStateInvalidFilterTest
16+
{
17+
[Fact]
18+
public void OnActionExecuting_NoOpsIfResultIsAlreadySet()
19+
{
20+
// Arrange
21+
var filter = new ModelStateInvalidFilter(new MvcOptions
22+
{
23+
ApiBehavior =
24+
{
25+
InvalidModelStateResponseFactory = _ => new BadRequestResult(),
26+
},
27+
}, NullLogger.Instance);
28+
var context = GetActionExecutingContext();
29+
var expected = new OkResult();
30+
context.Result = expected;
31+
32+
// Act
33+
filter.OnActionExecuting(context);
34+
35+
// Assert
36+
Assert.Same(expected, context.Result);
37+
}
38+
39+
[Fact]
40+
public void OnActionExecuting_NoOpsIfModelStateIsValid()
41+
{
42+
// Arrange
43+
var filter = new ModelStateInvalidFilter(new MvcOptions
44+
{
45+
ApiBehavior =
46+
{
47+
InvalidModelStateResponseFactory = _ => new BadRequestResult(),
48+
},
49+
}, NullLogger.Instance);
50+
var context = GetActionExecutingContext();
51+
52+
// Act
53+
filter.OnActionExecuting(context);
54+
55+
// Assert
56+
Assert.Null(context.Result);
57+
}
58+
59+
[Fact]
60+
public void OnActionExecuting_InvokesResponseFactoryIfModelStateIsInvalid()
61+
{
62+
// Arrange
63+
var expected = new BadRequestResult();
64+
var filter = new ModelStateInvalidFilter(new MvcOptions
65+
{
66+
ApiBehavior =
67+
{
68+
InvalidModelStateResponseFactory = _ => expected,
69+
},
70+
}, NullLogger.Instance);
71+
var context = GetActionExecutingContext();
72+
context.ModelState.AddModelError("some-key", "some-error");
73+
74+
// Act
75+
filter.OnActionExecuting(context);
76+
77+
// Assert
78+
Assert.Same(expected, context.Result);
79+
}
80+
81+
private static ActionExecutingContext GetActionExecutingContext()
82+
{
83+
return new ActionExecutingContext(
84+
new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()),
85+
Array.Empty<IFilterMetadata>(),
86+
new Dictionary<string, object>(),
87+
new object());
88+
}
89+
}
90+
}

0 commit comments

Comments
 (0)