diff --git a/src/config/datamodel/EdFi.DmsConfigurationService.DataModel/Infrastructure/FailureResponse.cs b/src/config/datamodel/EdFi.DmsConfigurationService.DataModel/Infrastructure/FailureResponse.cs index 5b0676b2b..dee35bc63 100644 --- a/src/config/datamodel/EdFi.DmsConfigurationService.DataModel/Infrastructure/FailureResponse.cs +++ b/src/config/datamodel/EdFi.DmsConfigurationService.DataModel/Infrastructure/FailureResponse.cs @@ -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"; @@ -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) => @@ -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) => @@ -123,6 +133,6 @@ public static JsonNode ForUnknown(string correlationId) => title: "Internal Server Error", status: 500, correlationId: correlationId, - validationErrors: [] + errors: [] ); } diff --git a/src/config/frontend/EdFi.DmsConfigurationService.Frontend.AspNetCore.Tests.Unit/Modules/IdentityModuleTests.cs b/src/config/frontend/EdFi.DmsConfigurationService.Frontend.AspNetCore.Tests.Unit/Modules/IdentityModuleTests.cs index c084a1cb6..ec43e2965 100644 --- a/src/config/frontend/EdFi.DmsConfigurationService.Frontend.AspNetCore.Tests.Unit/Modules/IdentityModuleTests.cs +++ b/src/config/frontend/EdFi.DmsConfigurationService.Frontend.AspNetCore.Tests.Unit/Modules/IdentityModuleTests.cs @@ -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().WithWebHostBuilder(builder => { _clientRepository = A.Fake(); - 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)); @@ -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()) + ); // Assert response.StatusCode.Should().Be(HttpStatusCode.BadGateway); - content.Should().Contain("Invalid realm"); + JsonNode.DeepEquals(actualResponse, expectedResponse).Should().Be(true); } [Test] @@ -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()) ); @@ -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()) ); @@ -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."} + """ + ) ) ); @@ -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()) + ); // Assert response.StatusCode.Should().Be(HttpStatusCode.BadGateway); - content.Should().Contain("Invalid realm"); + JsonNode.DeepEquals(actualResponse, expectedResponse).Should().Be(true); } [Test] @@ -698,13 +741,12 @@ public async Task When_provider_has_not_realm_admin_role() //Act var requestContent = new FormUrlEncodedContent( - new[] - { + [ new KeyValuePair("client_id", "CSClient1"), new KeyValuePair("client_secret", "test123@Puiu"), new KeyValuePair("grant_type", "client_credentials"), new KeyValuePair("scope", "edfi_admin_api/full_access"), - } + ] ); var response = await client.PostAsync("/connect/token", requestContent); @@ -726,7 +768,15 @@ public async Task When_provider_has_bad_credetials() A>>.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( @@ -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()) + ); // Assert response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); + JsonNode.DeepEquals(actualResponse, expectedResponse).Should().Be(true); } } diff --git a/src/config/frontend/EdFi.DmsConfigurationService.Frontend.AspNetCore/Infrastructure/FailureResults.cs b/src/config/frontend/EdFi.DmsConfigurationService.Frontend.AspNetCore/Infrastructure/FailureResults.cs index 84950ffee..91f535ee8 100644 --- a/src/config/frontend/EdFi.DmsConfigurationService.Frontend.AspNetCore/Infrastructure/FailureResults.cs +++ b/src/config/frontend/EdFi.DmsConfigurationService.Frontend.AspNetCore/Infrastructure/FailureResults.cs @@ -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); @@ -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}"]; + } } diff --git a/src/config/frontend/EdFi.DmsConfigurationService.Frontend.AspNetCore/Infrastructure/GlobalExceptionHandler.cs b/src/config/frontend/EdFi.DmsConfigurationService.Frontend.AspNetCore/Infrastructure/GlobalExceptionHandler.cs index a93e60195..533d8dc37 100644 --- a/src/config/frontend/EdFi.DmsConfigurationService.Frontend.AspNetCore/Infrastructure/GlobalExceptionHandler.cs +++ b/src/config/frontend/EdFi.DmsConfigurationService.Frontend.AspNetCore/Infrastructure/GlobalExceptionHandler.cs @@ -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; diff --git a/src/config/tests/EdFi.DmsConfigurationService.Tests.E2E/Features/Connect.feature b/src/config/tests/EdFi.DmsConfigurationService.Tests.E2E/Features/Connect.feature index 1c70d2bd5..cd1ad1f96 100644 --- a/src/config/tests/EdFi.DmsConfigurationService.Tests.E2E/Features/Connect.feature +++ b/src/config/tests/EdFi.DmsConfigurationService.Tests.E2E/Features/Connect.feature @@ -128,6 +128,21 @@ Feature: Connect endpoints "token_type": "Bearer" } """ + + Scenario: 05 Verify token creation with invalid client_secret value + When a Form URL Encoded POST request is made to "/connect/register" with + | Key | Value | + | ClientId | _scenarioRunId | + | ClientSecret | Secr3t:) | + | DisplayName | _scenarioRunId | + Then it should respond with 200 + And the response body is + """ + { + "title": "Registered client {scenarioRunId} successfully.", + "status": 200 + } + """ When a Form URL Encoded POST request is made to "/connect/token" with | Key | Value | | client_id | _scenarioRunId | @@ -138,12 +153,49 @@ Feature: Connect endpoints And the response body is """ { - "detail":"{\"error\":\"unauthorized_client\",\"error_description\":\"Invalid client or Invalid client credentials\"}", + "detail": "The request could not be processed. See 'errors' for details.", "type":"urn:ed-fi:api:security:authentication", "title":"Authentication Failed", "status":401, "validationErrors":{}, - "errors":[] + "errors": [ + "unauthorized_client. Invalid client or Invalid client credentials" + ] + } + """ + + Scenario: 06 Verify token creation with invalid client_id value + When a Form URL Encoded POST request is made to "/connect/register" with + | Key | Value | + | ClientId | _scenarioRunId | + | ClientSecret | Secr3t:) | + | DisplayName | _scenarioRunId | + Then it should respond with 200 + And the response body is + """ + { + "title": "Registered client {scenarioRunId} successfully.", + "status": 200 + } + """ + When a Form URL Encoded POST request is made to "/connect/token" with + | Key | Value | + | client_id | wrong | + | client_secret | Secr3t:) | + | grant_type | client_credentials | + | scope | edfi_admin_api/full_access | + Then it should respond with 401 + And the response body is + """ + { + "detail": "The request could not be processed. See 'errors' for details.", + "type":"urn:ed-fi:api:security:authentication", + "title":"Authentication Failed", + "status":401, + "validationErrors":{}, + "errors": [ + "invalid_client. Invalid client or Invalid client credentials" + ] } """ diff --git a/src/config/tests/EdFi.DmsConfigurationService.Tests.E2E/Hooks/SetupHooks.cs b/src/config/tests/EdFi.DmsConfigurationService.Tests.E2E/Hooks/SetupHooks.cs index 4e3c434d9..e0d406673 100644 --- a/src/config/tests/EdFi.DmsConfigurationService.Tests.E2E/Hooks/SetupHooks.cs +++ b/src/config/tests/EdFi.DmsConfigurationService.Tests.E2E/Hooks/SetupHooks.cs @@ -33,7 +33,9 @@ public static async Task AfterFeature() using var conn = new NpgsqlConnection(hostConnectionString); await conn.OpenAsync(); + await DeleteData("dmscs.Application"); await DeleteData("dmscs.Vendor"); + async Task DeleteData(string tableName) { var deleteRefCmd = new NpgsqlCommand($"DELETE FROM {tableName};", conn);