From f6c9fe0494096f8a8fcea8eeb5ecf44a31b37d49 Mon Sep 17 00:00:00 2001 From: Simon Gent Date: Thu, 28 Jun 2018 08:23:22 +0100 Subject: [PATCH 1/4] Check if Model State is valid and throw a 400 --- .../Controllers/BaseJsonApiController.cs | 3 + .../Extensions/ModelStateExtensions.cs | 23 +++++++ .../JsonApiDotNetCore.csproj | 2 +- .../BaseJsonApiController_Tests.cs | 62 +++++++++++++++++++ 4 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs diff --git a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs index 760f8f8d56..45f16e29ec 100644 --- a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs +++ b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs @@ -1,5 +1,6 @@ using System.Collections.Generic; using System.Threading.Tasks; +using JsonApiDotNetCore.Extensions; using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Models; using JsonApiDotNetCore.Services; @@ -152,6 +153,7 @@ public virtual async Task PostAsync([FromBody] T entity) if (!_jsonApiContext.Options.AllowClientGeneratedIds && !string.IsNullOrEmpty(entity.StringId)) return Forbidden(); + if (!ModelState.IsValid) return BadRequest(ModelState.ConvertToErrorCollection()); entity = await _create.CreateAsync(entity); @@ -164,6 +166,7 @@ public virtual async Task PatchAsync(TId id, [FromBody] T entity) if (entity == null) return UnprocessableEntity(); + if (!ModelState.IsValid) return BadRequest(ModelState.ConvertToErrorCollection()); var updatedEntity = await _update.UpdateAsync(id, entity); diff --git a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs new file mode 100644 index 0000000000..0ccdc224eb --- /dev/null +++ b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs @@ -0,0 +1,23 @@ +using JsonApiDotNetCore.Internal; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.EntityFrameworkCore.Internal; + +namespace JsonApiDotNetCore.Extensions +{ + public static class ModelStateExtensions + { + public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary modelState) + { + ErrorCollection errors = new ErrorCollection(); + foreach (var entry in modelState) + { + if (!entry.Value.Errors.Any()) continue; + foreach (var modelError in entry.Value.Errors) + { + errors.Errors.Add(new Error(400, entry.Key, modelError.ErrorMessage, modelError.Exception != null ? ErrorMeta.FromException(modelError.Exception) : null)); + } + } + return errors; + } + } +} diff --git a/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj b/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj index 7019a8e6cc..75fc955402 100755 --- a/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj +++ b/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj @@ -1,6 +1,6 @@  - 2.3.1 + 2.3.2 $(NetStandardVersion) JsonApiDotNetCore JsonApiDotNetCore diff --git a/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs b/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs index 9c59372846..4edfeddbe7 100644 --- a/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs +++ b/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs @@ -5,7 +5,10 @@ using Moq; using Xunit; using System.Threading.Tasks; +using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Internal; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; namespace UnitTests { @@ -153,6 +156,25 @@ public async Task PatchAsync_Calls_Service() VerifyApplyContext(); } + [Fact] + public async Task PatchAsync_ModelStateInvalid() + { + // arrange + const int id = 0; + var resource = new Resource(); + var serviceMock = new Mock>(); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, update: serviceMock.Object); + controller.ModelState.AddModelError("Id", "Failed Validation"); + + // act + var response = await controller.PatchAsync(id, resource); + + // assert + serviceMock.Verify(m => m.UpdateAsync(id, It.IsAny()), Times.Never); + Assert.IsType(response); + Assert.IsType(((BadRequestObjectResult) response).Value); + } + [Fact] public async Task PatchAsync_Throws_405_If_No_Service() { @@ -168,6 +190,46 @@ public async Task PatchAsync_Throws_405_If_No_Service() Assert.Equal(405, exception.GetStatusCode()); } + [Fact] + public async Task PostAsync_Calls_Service() + { + // arrange + var resource = new Resource(); + var serviceMock = new Mock>(); + _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions()); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, create: serviceMock.Object); + serviceMock.Setup(m => m.CreateAsync(It.IsAny())).ReturnsAsync(resource); + controller.ControllerContext = new Microsoft.AspNetCore.Mvc.ControllerContext {HttpContext = new DefaultHttpContext()}; + + // act + await controller.PostAsync(resource); + + // assert + serviceMock.Verify(m => m.CreateAsync(It.IsAny()), Times.Once); + VerifyApplyContext(); + } + + [Fact] + public async Task PostAsync_ModelStateInvalid() + { + // arrange + var resource = new Resource(); + var serviceMock = new Mock>(); + _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions()); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, create: serviceMock.Object); + controller.ModelState.AddModelError("Id", "Failed Validation"); + + // act + var response = await controller.PostAsync(resource); + + // assert + serviceMock.Verify(m => m.CreateAsync(It.IsAny()), Times.Never); + Assert.IsType(response); + Assert.IsType(((BadRequestObjectResult)response).Value); + } + [Fact] public async Task PatchRelationshipsAsync_Calls_Service() { From a9ca2933a7e144bc8c51c5b8266ebb8ffe586b88 Mon Sep 17 00:00:00 2001 From: Simon Gent Date: Thu, 28 Jun 2018 09:03:33 +0100 Subject: [PATCH 2/4] bumped version no --- src/JsonApiDotNetCore/JsonApiDotNetCore.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj b/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj index 75fc955402..cc337c28dc 100755 --- a/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj +++ b/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj @@ -1,6 +1,6 @@  - 2.3.2 + 2.3.3 $(NetStandardVersion) JsonApiDotNetCore JsonApiDotNetCore From 9d1611581e1d80b3514b73b12a3fa1c6242fe29e Mon Sep 17 00:00:00 2001 From: Simon Gent Date: Thu, 28 Jun 2018 14:02:19 +0100 Subject: [PATCH 3/4] styling updates --- src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs | 6 ++++-- src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs index 45f16e29ec..761e3cd957 100644 --- a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs +++ b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs @@ -153,7 +153,8 @@ public virtual async Task PostAsync([FromBody] T entity) if (!_jsonApiContext.Options.AllowClientGeneratedIds && !string.IsNullOrEmpty(entity.StringId)) return Forbidden(); - if (!ModelState.IsValid) return BadRequest(ModelState.ConvertToErrorCollection()); + if (!ModelState.IsValid) + return BadRequest(ModelState.ConvertToErrorCollection()); entity = await _create.CreateAsync(entity); @@ -166,7 +167,8 @@ public virtual async Task PatchAsync(TId id, [FromBody] T entity) if (entity == null) return UnprocessableEntity(); - if (!ModelState.IsValid) return BadRequest(ModelState.ConvertToErrorCollection()); + if (!ModelState.IsValid) + return BadRequest(ModelState.ConvertToErrorCollection()); var updatedEntity = await _update.UpdateAsync(id, entity); diff --git a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs index 0ccdc224eb..8eb0fc95f7 100644 --- a/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/ModelStateExtensions.cs @@ -11,7 +11,8 @@ public static ErrorCollection ConvertToErrorCollection(this ModelStateDictionary ErrorCollection errors = new ErrorCollection(); foreach (var entry in modelState) { - if (!entry.Value.Errors.Any()) continue; + if (!entry.Value.Errors.Any()) + continue; foreach (var modelError in entry.Value.Errors) { errors.Errors.Add(new Error(400, entry.Key, modelError.ErrorMessage, modelError.Exception != null ? ErrorMeta.FromException(modelError.Exception) : null)); From 4d0d422655d09f302c0c8d96a9f1aaa7f7241010 Mon Sep 17 00:00:00 2001 From: Simon Gent Date: Thu, 28 Jun 2018 16:26:20 +0100 Subject: [PATCH 4/4] added ValidateModelState as feature flag in options --- .../Configuration/JsonApiOptions.cs | 10 ++++ .../Controllers/BaseJsonApiController.cs | 4 +- .../BaseJsonApiController_Tests.cs | 51 +++++++++++++++++-- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs b/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs index 502d678f0d..54fcf5afaf 100644 --- a/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs +++ b/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs @@ -125,6 +125,16 @@ public class JsonApiOptions /// public bool EnableOperations { get; set; } + /// + /// Whether or not to validate model state. + /// + /// + /// + /// options.ValidateModelState = true; + /// + /// + public bool ValidateModelState { get; set; } + [Obsolete("JsonContract resolver can now be set on SerializerSettings.")] public IContractResolver JsonContractResolver { diff --git a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs index 761e3cd957..f4041e97d6 100644 --- a/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs +++ b/src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs @@ -153,7 +153,7 @@ public virtual async Task PostAsync([FromBody] T entity) if (!_jsonApiContext.Options.AllowClientGeneratedIds && !string.IsNullOrEmpty(entity.StringId)) return Forbidden(); - if (!ModelState.IsValid) + if (_jsonApiContext.Options.ValidateModelState && !ModelState.IsValid) return BadRequest(ModelState.ConvertToErrorCollection()); entity = await _create.CreateAsync(entity); @@ -167,7 +167,7 @@ public virtual async Task PatchAsync(TId id, [FromBody] T entity) if (entity == null) return UnprocessableEntity(); - if (!ModelState.IsValid) + if (_jsonApiContext.Options.ValidateModelState && !ModelState.IsValid) return BadRequest(ModelState.ConvertToErrorCollection()); var updatedEntity = await _update.UpdateAsync(id, entity); diff --git a/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs b/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs index 4edfeddbe7..d2d3ac8319 100644 --- a/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs +++ b/test/UnitTests/Controllers/BaseJsonApiController_Tests.cs @@ -146,6 +146,8 @@ public async Task PatchAsync_Calls_Service() const int id = 0; var resource = new Resource(); var serviceMock = new Mock>(); + _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions()); var controller = new BaseJsonApiController(_jsonApiContextMock.Object, update: serviceMock.Object); // act @@ -157,12 +159,34 @@ public async Task PatchAsync_Calls_Service() } [Fact] - public async Task PatchAsync_ModelStateInvalid() + public async Task PatchAsync_ModelStateInvalid_ValidateModelStateDisbled() + { + // arrange + const int id = 0; + var resource = new Resource(); + var serviceMock = new Mock>(); + _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions { ValidateModelState = false }); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, update: serviceMock.Object); + + // act + var response = await controller.PatchAsync(id, resource); + + // assert + serviceMock.Verify(m => m.UpdateAsync(id, It.IsAny()), Times.Once); + VerifyApplyContext(); + Assert.IsNotType(response); + } + + [Fact] + public async Task PatchAsync_ModelStateInvalid_ValidateModelStateEnabled() { // arrange const int id = 0; var resource = new Resource(); var serviceMock = new Mock>(); + _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions{ValidateModelState = true}); var controller = new BaseJsonApiController(_jsonApiContextMock.Object, update: serviceMock.Object); controller.ModelState.AddModelError("Id", "Failed Validation"); @@ -211,13 +235,34 @@ public async Task PostAsync_Calls_Service() } [Fact] - public async Task PostAsync_ModelStateInvalid() + public async Task PostAsync_ModelStateInvalid_ValidateModelStateDisabled() { // arrange var resource = new Resource(); var serviceMock = new Mock>(); _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); - _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions()); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions { ValidateModelState = false }); + var controller = new BaseJsonApiController(_jsonApiContextMock.Object, create: serviceMock.Object); + serviceMock.Setup(m => m.CreateAsync(It.IsAny())).ReturnsAsync(resource); + controller.ControllerContext = new Microsoft.AspNetCore.Mvc.ControllerContext { HttpContext = new DefaultHttpContext() }; + + // act + var response = await controller.PostAsync(resource); + + // assert + serviceMock.Verify(m => m.CreateAsync(It.IsAny()), Times.Once); + VerifyApplyContext(); + Assert.IsNotType(response); + } + + [Fact] + public async Task PostAsync_ModelStateInvalid_ValidateModelStateEnabled() + { + // arrange + var resource = new Resource(); + var serviceMock = new Mock>(); + _jsonApiContextMock.Setup(a => a.ApplyContext(It.IsAny>())).Returns(_jsonApiContextMock.Object); + _jsonApiContextMock.SetupGet(a => a.Options).Returns(new JsonApiOptions { ValidateModelState = true }); var controller = new BaseJsonApiController(_jsonApiContextMock.Object, create: serviceMock.Object); controller.ModelState.AddModelError("Id", "Failed Validation");