From 159d0256f828b6c2b032a82c3052f0d221a6b5ce Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Mon, 14 Sep 2020 14:45:05 +0200 Subject: [PATCH 1/2] Break up AttrCapabilities.AllowChange Before, `AttrCapabilities.AllowChange` was used for both POST and PATCH requests (although documentation said it was only for PATCH). This commit adds a new flag and changes the meaning of the old one: - AttrCapabilities.AllowCreate affects only POST - AttrCapabilities.AllowChange affects only PATCH --- docs/usage/resources/attributes.md | 16 ++++++- .../Models/Passport.cs | 2 +- .../Models/TodoItem.cs | 6 +-- .../JsonApiDotNetCoreExample/Models/User.cs | 2 +- .../Resources/Annotations/AttrCapabilities.cs | 20 +++++--- .../Serialization/RequestDeserializer.cs | 13 ++++-- .../Acceptance/Spec/CreatingDataTests.cs | 33 +++++++++++++ .../Acceptance/Spec/UpdatingDataTests.cs | 46 +++++++++++++++++++ .../Client/RequestSerializerTests.cs | 3 +- .../Server/RequestDeserializerTests.cs | 29 ------------ .../Server/ResponseSerializerTests.cs | 6 +-- test/UnitTests/TestModels.cs | 1 - 12 files changed, 124 insertions(+), 53 deletions(-) diff --git a/docs/usage/resources/attributes.md b/docs/usage/resources/attributes.md index fab38aa4d5..2394e977cf 100644 --- a/docs/usage/resources/attributes.md +++ b/docs/usage/resources/attributes.md @@ -50,9 +50,21 @@ public class User : Identifiable } ``` -### Mutability +### Createability -Attributes can be marked as mutable, which will allow `PATCH` requests to update them. When immutable, an HTTP 422 response is returned. +Attributes can be marked as creatable, which will allow `POST` requests to assign a value to them. When sent but not allowed, an HTTP 422 response is returned. + +```c# +public class Person : Identifiable +{ + [Attr(Capabilities = AttrCapabilities.AllowCreate)] + public string CreatorName { get; set; } +} +``` + +### Changeability + +Attributes can be marked as changeable, which will allow `PATCH` requests to update them. When sent but not allowed, an HTTP 422 response is returned. ```c# public class Person : Identifiable diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs b/src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs index c42367aaff..483ecceeda 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs @@ -62,7 +62,7 @@ public string BirthCountryName [EagerLoad] public Country BirthCountry { get; set; } - [Attr(Capabilities = AttrCapabilities.All & ~AttrCapabilities.AllowChange)] + [Attr(Capabilities = AttrCapabilities.All & ~(AttrCapabilities.AllowCreate | AttrCapabilities.AllowChange))] [NotMapped] public string GrantedVisaCountries => GrantedVisas == null || !GrantedVisas.Any() ? null diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs b/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs index c87ea2e5bb..782b2521be 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs @@ -23,7 +23,7 @@ public TodoItem() [Attr] public Guid GuidProperty { get; set; } - [Attr] + [Attr(Capabilities = AttrCapabilities.All & ~AttrCapabilities.AllowCreate)] public string AlwaysChangingValue { get => Guid.NewGuid().ToString(); @@ -39,10 +39,10 @@ public string AlwaysChangingValue [Attr] public DateTime? UpdatedDate { get; set; } - [Attr(Capabilities = AttrCapabilities.All & ~AttrCapabilities.AllowChange)] + [Attr(Capabilities = AttrCapabilities.All & ~(AttrCapabilities.AllowCreate | AttrCapabilities.AllowChange))] public string CalculatedValue => "calculated"; - [Attr] + [Attr(Capabilities = AttrCapabilities.All & ~AttrCapabilities.AllowChange)] public DateTimeOffset? OffsetDate { get; set; } public int? OwnerId { get; set; } diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/User.cs b/src/Examples/JsonApiDotNetCoreExample/Models/User.cs index 2760278844..fde27f2922 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/User.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/User.cs @@ -13,7 +13,7 @@ public class User : Identifiable [Attr] public string UserName { get; set; } - [Attr(Capabilities = AttrCapabilities.AllowChange)] + [Attr(Capabilities = AttrCapabilities.AllowCreate | AttrCapabilities.AllowChange)] public string Password { get => _password; diff --git a/src/JsonApiDotNetCore/Resources/Annotations/AttrCapabilities.cs b/src/JsonApiDotNetCore/Resources/Annotations/AttrCapabilities.cs index 7205f01241..af1448ce43 100644 --- a/src/JsonApiDotNetCore/Resources/Annotations/AttrCapabilities.cs +++ b/src/JsonApiDotNetCore/Resources/Annotations/AttrCapabilities.cs @@ -11,29 +11,35 @@ public enum AttrCapabilities None = 0, /// - /// Whether or not GET requests can return the attribute. - /// Attempts to retrieve when disabled will return an HTTP 422 response. + /// Whether or not GET requests can retrieve the attribute. + /// Attempts to retrieve when disabled will return an HTTP 400 response. /// AllowView = 1, + /// + /// Whether or not POST requests can assign the attribute value. + /// Attempts to assign when disabled will return an HTTP 422 response. + /// + AllowCreate = 2, + /// /// Whether or not PATCH requests can update the attribute value. /// Attempts to update when disabled will return an HTTP 422 response. /// - AllowChange = 2, + AllowChange = 4, /// /// Whether or not an attribute can be filtered on via a query string parameter. - /// Attempts to sort when disabled will return an HTTP 400 response. + /// Attempts to filter when disabled will return an HTTP 400 response. /// - AllowFilter = 4, + AllowFilter = 8, /// /// Whether or not an attribute can be sorted on via a query string parameter. /// Attempts to sort when disabled will return an HTTP 400 response. /// - AllowSort = 8, + AllowSort = 16, - All = AllowView | AllowChange | AllowFilter | AllowSort + All = AllowView | AllowCreate | AllowChange | AllowFilter | AllowSort } } diff --git a/src/JsonApiDotNetCore/Serialization/RequestDeserializer.cs b/src/JsonApiDotNetCore/Serialization/RequestDeserializer.cs index e7b3734b2f..cb96508306 100644 --- a/src/JsonApiDotNetCore/Serialization/RequestDeserializer.cs +++ b/src/JsonApiDotNetCore/Serialization/RequestDeserializer.cs @@ -46,16 +46,23 @@ protected override void AfterProcessField(IIdentifiable resource, ResourceFieldA { if (field is AttrAttribute attr) { - if (attr.Capabilities.HasFlag(AttrCapabilities.AllowChange)) + if (_httpContextAccessor.HttpContext.Request.Method == HttpMethod.Post.Method && + !attr.Capabilities.HasFlag(AttrCapabilities.AllowCreate)) { - _targetedFields.Attributes.Add(attr); + throw new InvalidRequestBodyException( + "Assigning to the requested attribute is not allowed.", + $"Assigning to '{attr.PublicName}' is not allowed.", null); } - else + + if (_httpContextAccessor.HttpContext.Request.Method == HttpMethod.Patch.Method && + !attr.Capabilities.HasFlag(AttrCapabilities.AllowChange)) { throw new InvalidRequestBodyException( "Changing the value of the requested attribute is not allowed.", $"Changing the value of '{attr.PublicName}' is not allowed.", null); } + + _targetedFields.Attributes.Add(attr); } else if (field is RelationshipAttribute relationship) _targetedFields.Relationships.Add(relationship); diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs index 5815d6ec90..58eb06c4a2 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs @@ -325,6 +325,39 @@ public async Task CreateResource_UnknownResourceType_Fails() Assert.Contains("Request body: <<", errorDocument.Errors[0].Detail); } + [Fact] + public async Task CreateResource_Blocked_Fails() + { + // Arrange + var content = new + { + data = new + { + type = "todoItems", + attributes = new Dictionary + { + { "alwaysChangingValue", "X" } + } + } + }; + + var requestBody = JsonConvert.SerializeObject(content); + + // Act + var (body, response) = await Post("/api/v1/todoItems", requestBody); + + // Assert + AssertEqualStatusCode(HttpStatusCode.UnprocessableEntity, response); + + var errorDocument = JsonConvert.DeserializeObject(body); + Assert.Single(errorDocument.Errors); + + var error = errorDocument.Errors.Single(); + Assert.Equal(HttpStatusCode.UnprocessableEntity, errorDocument.Errors[0].StatusCode); + Assert.Equal("Failed to deserialize request body: Assigning to the requested attribute is not allowed.", error.Title); + Assert.StartsWith("Assigning to 'alwaysChangingValue' is not allowed. - Request body:", error.Detail); + } + [Fact] public async Task CreateRelationship_ToOneWithImplicitRemove_IsCreated() { diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingDataTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingDataTests.cs index 117c4cb464..b575c4a890 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingDataTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingDataTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Net; using System.Net.Http; @@ -252,6 +253,51 @@ public async Task Respond_422_If_Broken_JSON_Payload() Assert.StartsWith("Invalid character after parsing", error.Detail); } + [Fact] + public async Task Respond_422_If_Blocked_For_Update() + { + // Arrange + var todoItem = _todoItemFaker.Generate(); + _context.TodoItems.Add(todoItem); + await _context.SaveChangesAsync(); + + var content = new + { + data = new + { + type = "todoItems", + id = todoItem.StringId, + attributes = new Dictionary + { + { "offsetDate", "2000-01-01" } + } + } + }; + + var builder = WebHost.CreateDefaultBuilder() + .UseStartup(); + var server = new TestServer(builder); + var client = server.CreateClient(); + + var requestBody = JsonConvert.SerializeObject(content); + var request = PrepareRequest(HttpMethod.Patch.Method, "/api/v1/todoItems/" + todoItem.StringId, requestBody); + + // Act + var response = await client.SendAsync(request); + + // Assert + AssertEqualStatusCode(HttpStatusCode.UnprocessableEntity, response); + + var responseBody = await response.Content.ReadAsStringAsync(); + var errorDocument = JsonConvert.DeserializeObject(responseBody); + Assert.Single(errorDocument.Errors); + + var error = errorDocument.Errors.Single(); + Assert.Equal(HttpStatusCode.UnprocessableEntity, errorDocument.Errors[0].StatusCode); + Assert.Equal("Failed to deserialize request body: Changing the value of the requested attribute is not allowed.", error.Title); + Assert.StartsWith("Changing the value of 'offsetDate' is not allowed. - Request body:", error.Detail); + } + [Fact] public async Task Can_Patch_Resource() { diff --git a/test/UnitTests/Serialization/Client/RequestSerializerTests.cs b/test/UnitTests/Serialization/Client/RequestSerializerTests.cs index a001b2c1f9..e5a2d696ce 100644 --- a/test/UnitTests/Serialization/Client/RequestSerializerTests.cs +++ b/test/UnitTests/Serialization/Client/RequestSerializerTests.cs @@ -40,8 +40,7 @@ public void SerializeSingle_ResourceWithDefaultTargetFields_CanBuild() ""intField"":0, ""nullableIntField"":123, ""guidField"":""00000000-0000-0000-0000-000000000000"", - ""complexField"":null, - ""immutable"":null + ""complexField"":null } } }"; diff --git a/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs b/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs index e2ca796928..c01f2f9b7e 100644 --- a/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs +++ b/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs @@ -1,7 +1,5 @@ using System.Collections.Generic; using System.ComponentModel.Design; -using System.Net; -using JsonApiDotNetCore.Errors; using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Resources.Annotations; using JsonApiDotNetCore.Serialization; @@ -37,33 +35,6 @@ public void DeserializeAttributes_VariousUpdatedMembers_RegistersTargetedFields( Assert.Empty(relationshipsToUpdate); } - [Fact] - public void DeserializeAttributes_UpdatedImmutableMember_ThrowsInvalidOperationException() - { - // Arrange - SetupFieldsManager(out _, out _); - var content = new Document - { - Data = new ResourceObject - { - Type = "testResource", - Id = "1", - Attributes = new Dictionary - { - { "immutable", "some string" }, - } - } - }; - var body = JsonConvert.SerializeObject(content); - - // Act, assert - var exception = Assert.Throws(() => _deserializer.Deserialize(body)); - - Assert.Equal(HttpStatusCode.UnprocessableEntity, exception.Error.StatusCode); - Assert.Equal("Failed to deserialize request body: Changing the value of the requested attribute is not allowed.", exception.Error.Title); - Assert.Equal("Changing the value of 'immutable' is not allowed.", exception.Error.Detail); - } - [Fact] public void DeserializeRelationships_MultipleDependentRelationships_RegistersUpdatedRelationships() { diff --git a/test/UnitTests/Serialization/Server/ResponseSerializerTests.cs b/test/UnitTests/Serialization/Server/ResponseSerializerTests.cs index 665b4b9647..46eeb5fb27 100644 --- a/test/UnitTests/Serialization/Server/ResponseSerializerTests.cs +++ b/test/UnitTests/Serialization/Server/ResponseSerializerTests.cs @@ -34,8 +34,7 @@ public void SerializeSingle_ResourceWithDefaultTargetFields_CanSerialize() ""intField"":0, ""nullableIntField"":123, ""guidField"":""00000000-0000-0000-0000-000000000000"", - ""complexField"":null, - ""immutable"":null + ""complexField"":null } } }"; @@ -67,8 +66,7 @@ public void SerializeMany_ResourceWithDefaultTargetFields_CanSerialize() ""intField"":0, ""nullableIntField"":123, ""guidField"":""00000000-0000-0000-0000-000000000000"", - ""complexField"":null, - ""immutable"":null + ""complexField"":null } }] }"; diff --git a/test/UnitTests/TestModels.cs b/test/UnitTests/TestModels.cs index 28d36773b9..efde2004b9 100644 --- a/test/UnitTests/TestModels.cs +++ b/test/UnitTests/TestModels.cs @@ -14,7 +14,6 @@ public sealed class TestResource : Identifiable [Attr] public int? NullableIntField { get; set; } [Attr] public Guid GuidField { get; set; } [Attr] public ComplexType ComplexField { get; set; } - [Attr(Capabilities = AttrCapabilities.All & ~AttrCapabilities.AllowChange)] public string Immutable { get; set; } } public class TestResourceWithList : Identifiable From ef06cf0a13d461a96f040fe879227497dcc7f96a Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Thu, 17 Sep 2020 10:35:20 +0200 Subject: [PATCH 2/2] Review feedback --- docs/usage/resources/attributes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/usage/resources/attributes.md b/docs/usage/resources/attributes.md index 2394e977cf..7742624b15 100644 --- a/docs/usage/resources/attributes.md +++ b/docs/usage/resources/attributes.md @@ -50,7 +50,7 @@ public class User : Identifiable } ``` -### Createability +### Creatability Attributes can be marked as creatable, which will allow `POST` requests to assign a value to them. When sent but not allowed, an HTTP 422 response is returned.