Skip to content

Commit

Permalink
[DMS-446] Convert keycloak error into problem+json format (#375)
Browse files Browse the repository at this point in the history 
* Fix identity error format

* Fix unit tests

* Update e2e error responses
  • Loading branch information
@CSR2017
CSR2017 authored Dec 20, 2024
1 parent 8c55963 commit 4c1d4c4
Show file tree
Hide file tree
Showing 6 changed files with 209 additions and 31 deletions.
34 changes: 22 additions & 12 deletions ...config/datamodel/EdFi.DmsConfigurationService.DataModel/Infrastructure/FailureResponse.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@
using System.Text.Json.Nodes;
using FluentValidation.Results;

namespace EdFi.DmsConfigurationService.Frontend.AspNetCore.Infrastructure;
namespace EdFi.DmsConfigurationService.DataModel.Infrastructure;

public static class FailureResponse
{
private static readonly JsonSerializerOptions _serializerOptions =
new() { Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping };
private static readonly JsonSerializerOptions _serializerOptions = new()
{
Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
};

private static readonly string _typePrefix = "urn:ed-fi:api";
private static readonly string _unauthorizedType = $"{_typePrefix}:security:authentication";
Expand Down Expand Up @@ -49,26 +51,34 @@ private static JsonObject CreateBaseJsonObject(
};
}

public static JsonNode ForUnauthorized(string title, string detail, string correlationId) =>
public static JsonNode ForUnauthorized(
string title,
string detail,
string correlationId,
string[]? errors = null
) =>
CreateBaseJsonObject(
detail: detail,
type: _unauthorizedType,
title: title,
status: 401,
correlationId: correlationId,
validationErrors: [],
errors: []
errors: errors
);

public static JsonNode ForForbidden(string title, string detail, string correlationId) =>
public static JsonNode ForForbidden(
string title,
string detail,
string correlationId,
string[]? errors = null
) =>
CreateBaseJsonObject(
detail: detail,
type: _forbiddenType,
title: title,
status: 403,
correlationId: correlationId,
validationErrors: [],
errors: []
errors: errors
);

public static JsonNode ForBadRequest(string detail, string correlationId) =>
Expand Down Expand Up @@ -106,14 +116,14 @@ string correlationId
.ToDictionary(g => g.Key, g => g.Select(x => x.ErrorMessage).ToArray())
);

public static JsonNode ForBadGateway(string detail, string correlationId) =>
public static JsonNode ForBadGateway(string detail, string correlationId, string[]? errors = null) =>
CreateBaseJsonObject(
detail: detail,
type: _badGatewayTypePrefix,
title: "Bad Gateway",
status: 502,
correlationId: correlationId,
validationErrors: []
errors: errors
);

public static JsonNode ForUnknown(string correlationId) =>
Expand All @@ -123,6 +133,6 @@ public static JsonNode ForUnknown(string correlationId) =>
title: "Internal Server Error",
status: 500,
correlationId: correlationId,
validationErrors: []
errors: []
);
}
97 changes: 83 additions & 14 deletions ...dFi.DmsConfigurationService.Frontend.AspNetCore.Tests.Unit/Modules/IdentityModuleTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,18 @@ public async Task When_provider_has_not_real_admin_role()
}

[Test]
public async Task When_provider_has_invalid_real()
public async Task When_provider_has_invalid_realm()
{
// Arrange
await using var factory = new WebApplicationFactory<Program>().WithWebHostBuilder(builder =>
{
_clientRepository = A.Fake<IClientRepository>();

var error = new IdentityProviderError.NotFound("Invalid realm.");
var error = new IdentityProviderError.NotFound(
"""
{ "error":"Realm does not exist","error_description":"For more on this error consult the server log at the debug level."}
"""
);

A.CallTo(() => _clientRepository.GetAllClientsAsync())
.Returns(new ClientClientsResult.FailureIdentityProvider(error));
Expand All @@ -269,10 +273,25 @@ public async Task When_provider_has_invalid_real()
);
var response = await client.PostAsync("/connect/register", requestContent);
string content = await response.Content.ReadAsStringAsync();

var actualResponse = JsonNode.Parse(content);
var expectedResponse = JsonNode.Parse(
"""
{
"detail": "The request could not be processed. See 'errors' for details.",
"type": "urn:ed-fi:api:bad-gateway",
"title": "Bad Gateway",
"status": 502,
"correlationId": "{correlationId}",
"validationErrors": {},
"errors": [
"Realm does not exist. For more on this error consult the server log at the debug level."
]
}
""".Replace("{correlationId}", actualResponse!["correlationId"]!.GetValue<string>())
);
// Assert
response.StatusCode.Should().Be(HttpStatusCode.BadGateway);
content.Should().Contain("Invalid realm");
JsonNode.DeepEquals(actualResponse, expectedResponse).Should().Be(true);
}

[Test]
Expand Down Expand Up @@ -395,13 +414,15 @@ public async Task When_provider_is_unreachable()
var expectedResponse = JsonNode.Parse(
"""
{
"detail": "No connection could be made because the target machine actively refused it.",
"detail": "The request could not be processed. See 'errors' for details.",
"type": "urn:ed-fi:api:bad-gateway",
"title": "Bad Gateway",
"status": 502,
"correlationId": "{correlationId}",
"validationErrors": {},
"errors": []
"errors": [
"No connection could be made because the target machine actively refused it."
]
}
""".Replace("{correlationId}", actualResponse!["correlationId"]!.GetValue<string>())
);
Expand Down Expand Up @@ -603,13 +624,15 @@ public async Task When_provider_is_unreacheable()
var expectedResponse = JsonNode.Parse(
"""
{
"detail": "No connection could be made because the target machine actively refused it.",
"detail": "The request could not be processed. See 'errors' for details.",
"type": "urn:ed-fi:api:bad-gateway",
"title": "Bad Gateway",
"status": 502,
"correlationId": "{correlationId}",
"validationErrors": {},
"errors": []
"errors": [
"No connection could be made because the target machine actively refused it."
]
}
""".Replace("{correlationId}", actualResponse!["correlationId"]!.GetValue<string>())
);
Expand All @@ -632,7 +655,11 @@ public async Task When_provider_has_invalid_realm()
)
.Returns(
new TokenResult.FailureIdentityProvider(
new IdentityProviderError.NotFound("Invalid realm")
new IdentityProviderError.NotFound(
"""
{ "error":"Realm does not exist","error_description":"For more on this error consult the server log at the debug level."}
"""
)
)
);

Expand Down Expand Up @@ -660,9 +687,25 @@ public async Task When_provider_has_invalid_realm()
var response = await client.PostAsync("/connect/token", requestContent);
string content = await response.Content.ReadAsStringAsync();

var actualResponse = JsonNode.Parse(content);
var expectedResponse = JsonNode.Parse(
"""
{
"detail": "The request could not be processed. See 'errors' for details.",
"type": "urn:ed-fi:api:bad-gateway",
"title": "Bad Gateway",
"status": 502,
"correlationId": "{correlationId}",
"validationErrors": {},
"errors": [
"Realm does not exist. For more on this error consult the server log at the debug level."
]
}
""".Replace("{correlationId}", actualResponse!["correlationId"]!.GetValue<string>())
);
// Assert
response.StatusCode.Should().Be(HttpStatusCode.BadGateway);
content.Should().Contain("Invalid realm");
JsonNode.DeepEquals(actualResponse, expectedResponse).Should().Be(true);
}

[Test]
Expand Down Expand Up @@ -698,13 +741,12 @@ public async Task When_provider_has_not_realm_admin_role()

//Act
var requestContent = new FormUrlEncodedContent(
new[]
{
[
new KeyValuePair<string, string>("client_id", "CSClient1"),
new KeyValuePair<string, string>("client_secret", "test123@Puiu"),
new KeyValuePair<string, string>("grant_type", "client_credentials"),
new KeyValuePair<string, string>("scope", "edfi_admin_api/full_access"),
}
]
);
var response = await client.PostAsync("/connect/token", requestContent);

Expand All @@ -726,7 +768,15 @@ public async Task When_provider_has_bad_credetials()
A<IEnumerable<KeyValuePair<string, string>>>.Ignored
)
)
.Returns(new TokenResult.FailureIdentityProvider(new IdentityProviderError.Unauthorized("")));
.Returns(
new TokenResult.FailureIdentityProvider(
new IdentityProviderError.Unauthorized(
"""
{"error":"invalid_client","error_description":"Invalid client or Invalid client credentials"}
"""
)
)
);

builder.UseEnvironment("Test");
builder.ConfigureServices(
Expand All @@ -750,8 +800,27 @@ public async Task When_provider_has_bad_credetials()
}
);
var response = await client.PostAsync("/connect/token", requestContent);
string content = await response.Content.ReadAsStringAsync();

var actualResponse = JsonNode.Parse(content);
var expectedResponse = JsonNode.Parse(
"""
{
"detail": "The request could not be processed. See 'errors' for details.",
"type": "urn:ed-fi:api:security:authentication",
"title": "Authentication Failed",
"status": 401,
"correlationId": "{correlationId}",
"validationErrors": {},
"errors": [
"invalid_client. Invalid client or Invalid client credentials"
]
}
""".Replace("{correlationId}", actualResponse!["correlationId"]!.GetValue<string>())
);

// Assert
response.StatusCode.Should().Be(HttpStatusCode.Unauthorized);
JsonNode.DeepEquals(actualResponse, expectedResponse).Should().Be(true);
}
}
50 changes: 47 additions & 3 deletions ...rontend/EdFi.DmsConfigurationService.Frontend.AspNetCore/Infrastructure/FailureResults.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,17 @@
// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0.
// See the LICENSE and NOTICES files in the project root for more information.

using System.Text.Json.Nodes;
using EdFi.DmsConfigurationService.DataModel.Infrastructure;

namespace EdFi.DmsConfigurationService.Frontend.AspNetCore.Infrastructure;

internal static class FailureResults
{
private static readonly string _errorDetail =
"The request could not be processed. See 'errors' for details.";
private static readonly string _errorContentType = "application/problem+json";

public static IResult Unknown(string correlationId)
{
return Results.Json(FailureResponse.ForUnknown(correlationId), statusCode: 500);
Expand All @@ -19,22 +26,59 @@ public static IResult NotFound(string detail, string correlationId)

public static IResult BadGateway(string detail, string correlationId)
{
return Results.Json(FailureResponse.ForBadGateway(detail, correlationId), statusCode: 502);
var errors = GetIdentityErrorDetails(detail);
return Results.Json(
FailureResponse.ForBadGateway(_errorDetail, correlationId, errors),
contentType: _errorContentType,
statusCode: 502
);
}

public static IResult Unauthorized(string detail, string correlationId)
{
var errors = GetIdentityErrorDetails(detail, "Unauthorized");
return Results.Json(
FailureResponse.ForUnauthorized("Authentication Failed", detail, correlationId),
FailureResponse.ForUnauthorized("Authentication Failed", _errorDetail, correlationId, errors),
contentType: _errorContentType,
statusCode: 401
);
}

public static IResult Forbidden(string detail, string correlationId)
{
var errors = GetIdentityErrorDetails(detail, "Forbidden");
return Results.Json(
FailureResponse.ForForbidden("Authorization Failed", detail, correlationId),
FailureResponse.ForForbidden("Authorization Failed", _errorDetail, correlationId, errors),
contentType: _errorContentType,
statusCode: 403
);
}

// Attempts to read `{ "error": "...", "error_description": "..."}` from the response
// body, with sensible fallback mechanism if the response is in a different format.
private static string[]? GetIdentityErrorDetails(string detail, string title = "")
{
if (string.IsNullOrEmpty(detail))
{
return null;
}

string error = title;
string errorDescription = detail;

try
{
if (JsonNode.Parse(detail) is JsonNode parsed && parsed is JsonObject obj)
{
error = obj["error"]?.ToString() ?? error;
errorDescription = obj["error_description"]?.ToString() ?? errorDescription;
}
}
catch
{
// Ignoring parsing errors, returning the default formatted message.
}
error = !string.IsNullOrEmpty(error) ? $"{error}. " : "";
return [$"{error}{errorDescription}"];
}
}
1 change: 1 addition & 0 deletions ...EdFi.DmsConfigurationService.Frontend.AspNetCore/Infrastructure/GlobalExceptionHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Net;
using System.Text.Encodings.Web;
using System.Text.Json;
using EdFi.DmsConfigurationService.DataModel.Infrastructure;
using Microsoft.AspNetCore.Diagnostics;

namespace EdFi.DmsConfigurationService.Frontend.AspNetCore.Infrastructure;
Expand Down
Loading

0 comments on commit 4c1d4c4

Please sign in to comment.