Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POST instance: don't attempt to parse instance from body when it's empty or not json #888

Merged
merged 5 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions src/Altinn.App.Api/Controllers/InstancesController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@
[ProducesResponseType(typeof(Instance), StatusCodes.Status201Created)]
[ProducesResponseType(StatusCodes.Status400BadRequest)]
[RequestSizeLimit(RequestSizeLimit)]
public async Task<ActionResult<Instance>> Post(

Check warning on line 191 in src/Altinn.App.Api/Controllers/InstancesController.cs

View workflow job for this annotation

GitHub Actions / Static code analysis

Refactor this method to reduce its Cognitive Complexity from 31 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
[FromRoute] string org,
[FromRoute] string app,
[FromQuery] int? instanceOwnerPartyId,
Expand Down Expand Up @@ -1147,7 +1147,7 @@
// NOTE: part.Name is nullable on the type here, but `RequestPartValidator.ValidatePart` which is called
// further up the stack will error out if it actually null, so we just sanity-check here
// and throw if it is null.
// TODO: improve the modelling of this type.

Check warning on line 1150 in src/Altinn.App.Api/Controllers/InstancesController.cs

View workflow job for this annotation

GitHub Actions / Static code analysis

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
if (part.Name is null)
{
throw new InvalidOperationException("Unexpected state - part name is null");
Expand Down Expand Up @@ -1228,14 +1228,8 @@
{
RequestPart? instancePart = parts.Find(part => part.Name == "instance");

// assume that first part with no name is an instanceTemplate
if (
instancePart == null
&& parts.Count == 1
&& parts[0].ContentType.Contains("application/json")
&& parts[0].Name == null
&& parts[0].Bytes.Length > 0
)
// If the request has a single part with no name, assume it is the instance template
if (instancePart == null && parts.Count == 1 && parts[0].Name == null)
{
instancePart = parts[0];
}
Expand All @@ -1244,7 +1238,17 @@
{
parts.Remove(instancePart);

return System.Text.Json.JsonSerializer.Deserialize<Instance>(instancePart.Bytes, _jsonSerializerOptionsWeb);
// Some clients might set contentType to application/json even if the body is empty
if (
instancePart is { Bytes.Length: > 0 }
&& instancePart.ContentType.Contains("application/json", StringComparison.Ordinal)
)
{
return System.Text.Json.JsonSerializer.Deserialize<Instance>(
instancePart.Bytes,
_jsonSerializerOptionsWeb
);
}
}

return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Net;
using System.Net.Http.Headers;
using System.Net.Http.Json;
using System.Text;
using System.Text.Json;
using System.Text.Json.Nodes;
Expand Down Expand Up @@ -273,6 +274,56 @@ public async Task InstationAllowedByOrg_Returns_Forbidden_For_user()
createResponse.StatusCode.Should().Be(HttpStatusCode.Forbidden, createResponseContent);
}

[Fact]
public async Task PostNewInstanceWithInstanceTemplate()
{
string org = "tdd";
string app = "contributer-restriction";
int instanceOwnerPartyId = 501337;
int userId = 1337;
HttpClient client = GetRootedClient(org, app, userId, null);

using var content = JsonContent.Create(
new Instance() { InstanceOwner = new InstanceOwner() { PartyId = instanceOwnerPartyId.ToString() }, }
);

var response = await client.PostAsync($"{org}/{app}/instances", content);
response.Should().HaveStatusCode(HttpStatusCode.Created);
var responseContent = await response.Content.ReadAsStringAsync();
var instance = JsonSerializer.Deserialize<Instance>(responseContent, JsonSerializerOptions);
instance.Should().NotBeNull();
instance!.Id.Should().NotBeNullOrEmpty();

TestData.DeleteInstanceAndData(org, app, instance.Id);
}

[Fact]
public async Task PostNewInstanceWithMissingTemplate()
{
string org = "tdd";
string app = "contributer-restriction";
int instanceOwnerPartyId = 501337;
int userId = 1337;
HttpClient client = GetRootedClient(org, app, userId, null);

using var content = new ByteArrayContent([])
{
Headers = { ContentType = new MediaTypeHeaderValue("application/json") }
};

var response = await client.PostAsync(
$"{org}/{app}/instances?instanceOwnerPartyId={instanceOwnerPartyId}",
content
);
response.Should().HaveStatusCode(HttpStatusCode.Created);
var responseContent = await response.Content.ReadAsStringAsync();
var instance = JsonSerializer.Deserialize<Instance>(responseContent, JsonSerializerOptions);
instance.Should().NotBeNull();
instance!.Id.Should().NotBeNullOrEmpty();

TestData.DeleteInstanceAndData(org, app, instance.Id);
}

[Fact]
public async Task InstationAllowedByOrg_Returns_Forbidden_For_User_SimplifiedEndpoint()
{
Expand Down
Loading