[ODS-6283] Refine problem details responses for 401, 404, 405 status …

…codes (#988)

Co-authored-by: Axel Marquez <[email protected]>

Application/EdFi.Ods.Api/Container/Modules/ApplicationModule.cs

@@ -175,11 +175,6 @@ protected override void Load(ContainerBuilder builder)
                 .As<ISystemDateProvider>()
                 .SingleInstance();
 
-            builder.RegisterType<FeatureDisabledProfileResourceModelProvider>()
-                .As<IProfileResourceModelProvider>()
-                .PreserveExistingDefaults()
-                .SingleInstance();
-
             builder.RegisterType<EdFiOdsInstanceIdentificationProvider>()
                 .As<IEdFiOdsInstanceIdentificationProvider>()
                 .SingleInstance();
@@ -464,7 +459,7 @@ void RegisterMiddleware()
                     .AsSelf()
                     .SingleInstance();
 
-                builder.RegisterType<ComplementErrorDetailsMiddleware>()
+                builder.RegisterType<ProblemDetailsErrorEnrichmentMiddleware>()
                     .As<IMiddleware>()
                     .AsSelf()
                     .SingleInstance();
@@ -498,8 +493,6 @@ void RegisterMiddleware()
 
                 builder.RegisterType<ErrorTranslator>().SingleInstance();
 
-                builder.RegisterType<AuthenticateResultTranslator>().SingleInstance();
-
                 builder.RegisterType<ModelStateKeyConverter>().EnableClassInterceptors().SingleInstance();
 
                 builder.RegisterType<CachingInterceptor>()

Application/EdFi.Ods.Api/Controllers/DataManagementControllerBase.cs

@@ -6,7 +6,6 @@
 using System;
 using System.Collections.Generic;
 using System.ComponentModel.DataAnnotations;
-using System.Linq;
 using System.Net.Mime;
 using System.Threading;
 using System.Threading.Tasks;
@@ -20,9 +19,9 @@
 using EdFi.Ods.Api.Infrastructure.Pipelines.Put;
 using EdFi.Ods.Common;
 using EdFi.Ods.Common.Configuration;
-using EdFi.Ods.Common.Constants;
 using EdFi.Ods.Common.Context;
 using EdFi.Ods.Common.Exceptions;
+using EdFi.Ods.Common.Extensions;
 using EdFi.Ods.Common.Infrastructure.Pipelines.Delete;
 using EdFi.Ods.Common.Infrastructure.Pipelines.GetMany;
 using EdFi.Ods.Common.Logging;
@@ -36,7 +35,6 @@
 using log4net;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
-using Microsoft.AspNetCore.Mvc.ModelBinding;
 using Microsoft.Net.Http.Headers;
 using Polly;
 using Polly.Retry;
@@ -183,7 +181,7 @@ public virtual async Task<IActionResult> GetAll(
                     "The limit parameter was incorrect.",
                     new[] { $"Limit must be omitted or set to a value between 0 and {_defaultPageLimitSize}." })
                 {
-                    CorrelationId = (string)_logContextAccessor.GetValue(CorrelationConstants.LogContextKey)
+                    CorrelationId = _logContextAccessor.GetCorrelationId()
                 }.AsSerializableModel();
 
                 return BadRequest(problemDetails);
@@ -253,6 +251,32 @@ public virtual async Task<IActionResult> Get(Guid id)
             return Ok(result.Resource);
         }
 
+        [HttpPut][HttpDelete]
+        [ProducesResponseType(StatusCodes.Status405MethodNotAllowed)]
+        [Produces(MediaTypeNames.Application.Json)]
+        public virtual Task<IActionResult> PutOrDelete()
+        {
+            MethodNotAllowedException problemDetails;
+
+            if (Request.Method == HttpMethods.Put)
+            {
+                problemDetails = new MethodNotAllowedException("Resource collections cannot be replaced. To \"upsert\" an item in the collection, use POST. To update a specific item, use PUT and include the \"id\" in the route.")
+                {
+                    CorrelationId = _logContextAccessor.GetCorrelationId()
+                };
+            }
+            else
+            {
+                problemDetails = new MethodNotAllowedException("Resource collections cannot be deleted. To delete a specific item, use DELETE and include the \"id\" in the route.")
+                {
+                    CorrelationId = _logContextAccessor.GetCorrelationId()
+                };
+            }
+
+            return Task.FromResult<IActionResult>(
+                new ObjectResult(problemDetails.AsSerializableModel()) { StatusCode = problemDetails.Status });
+        }
+
         [HttpPut("{id}")]
         [ServiceFilter(typeof(EnforceAssignedProfileUsageFilter), IsReusable = true)]
         [ProducesResponseType(StatusCodes.Status201Created)]
@@ -320,6 +344,20 @@ private IActionResult GetActionResultForNullRequest(Resource resource)
             return StatusCode(problemDetails.Status, problemDetails);
         }
 
+        [HttpPost("{id}")]
+        [ProducesResponseType(StatusCodes.Status405MethodNotAllowed)]
+        [Produces(MediaTypeNames.Application.Json)]
+        public virtual Task<IActionResult> Post(Guid id)
+        {
+            var problemDetails = new MethodNotAllowedException("Resource items can only be updated using PUT. To \"upsert\" an item in the resource collection using POST, remove the \"id\" from the route.")
+            {
+                CorrelationId = _logContextAccessor.GetCorrelationId()
+            };
+
+            return Task.FromResult<IActionResult>(
+                new ObjectResult(problemDetails.AsSerializableModel()) { StatusCode = problemDetails.Status });
+        }
+
         [HttpPost]
         [ServiceFilter(typeof(EnforceAssignedProfileUsageFilter), IsReusable = true)]
         [ProducesResponseType(StatusCodes.Status200OK)]
@@ -343,7 +381,7 @@ public virtual async Task<IActionResult> Post([FromBody] TPostRequest request)
                     "The request data was constructed incorrectly.",
                     new[] { "Resource identifiers cannot be assigned by the client. The 'id' property should not be included in the request body." })
                 {
-                    CorrelationId = (string)_logContextAccessor.GetValue(CorrelationConstants.LogContextKey)
+                    CorrelationId = _logContextAccessor.GetCorrelationId()
                 }.AsSerializableModel();
 
                 return BadRequest(problemDetails);
@@ -398,7 +436,7 @@ private IActionResult ValidationFailedResult(IEnumerable<ValidationResult> valid
                 _dataManagementResourceContextProvider.Get().Resource,
                 validationResults);
 
-            problemDetails.CorrelationId = (string)_logContextAccessor.GetValue(CorrelationConstants.LogContextKey);
+            problemDetails.CorrelationId = _logContextAccessor.GetCorrelationId();
 
             return BadRequest(problemDetails);
         }

Application/EdFi.Ods.Api/ExceptionHandling/EdFiProblemDetailsProvider.cs

@@ -7,6 +7,7 @@
 using System.Collections.Generic;
 using EdFi.Ods.Common.Constants;
 using EdFi.Ods.Common.Exceptions;
+using EdFi.Ods.Common.Extensions;
 using EdFi.Ods.Common.Logging;
 using EdFi.Ods.Common.ProblemDetails;
 using log4net;
@@ -29,7 +30,7 @@ public EdFiProblemDetailsProvider(IEnumerable<IProblemDetailsExceptionTranslator
     /// <inheritdoc cref="IEdFiProblemDetailsProvider.GetProblemDetails" />
     public IEdFiProblemDetails GetProblemDetails(Exception exception)
     {
-        string correlationId = (string) _logContextAccessor.GetValue(CorrelationConstants.LogContextKey);
+        string correlationId = _logContextAccessor.GetCorrelationId();
 
         // Handle Problem Details exceptions directly
         if (exception is EdFiProblemDetailsExceptionBase problemDetailsException)

Application/EdFi.Ods.Api/Extensions/ApplicationBuilderExtensions.cs

@@ -22,8 +22,8 @@ public static IApplicationBuilder UseTenantIdentification(this IApplicationBuild
         public static IApplicationBuilder UseRequestCorrelation(this IApplicationBuilder builder)
             => builder.UseMiddleware<RequestCorrelationMiddleware>();
 
-        public static IApplicationBuilder UseComplementErrorDetails(this IApplicationBuilder builder)
-            => builder.UseMiddleware<ComplementErrorDetailsMiddleware>();
+        public static IApplicationBuilder UseProblemDetailsEnrichment(this IApplicationBuilder builder)
+            => builder.UseMiddleware<ProblemDetailsErrorEnrichmentMiddleware>();
 
         /// <summary>
         /// Adds the <see cref="EdFiApiAuthenticationMiddleware"/> to the specified <see cref="IApplicationBuilder"/>, which enables authentication capabilities.

Application/EdFi.Ods.Api/Filters/EnforceAssignedProfileUsageFilter.cs

@@ -7,9 +7,9 @@
 using System.Linq;
 using System.Threading.Tasks;
 using EdFi.Ods.Common.Configuration;
-using EdFi.Ods.Common.Constants;
 using EdFi.Ods.Common.Context;
 using EdFi.Ods.Common.Exceptions;
+using EdFi.Ods.Common.Extensions;
 using EdFi.Ods.Common.Logging;
 using EdFi.Ods.Common.Models;
 using EdFi.Ods.Common.Profiles;
@@ -93,14 +93,7 @@ public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionE
 
         var resourceFullName = dataManagementResourceContext.Resource.FullName;
 
-        var assignedProfilesForRequest = apiClientContext.Profiles.Where(
-                p => _profileResourceModelProvider.GetProfileResourceModel(p)
-                        .ResourceByName.TryGetValue(resourceFullName, out var contentTypes)
-                    && (relevantContentTypeUsage == ContentTypeUsage.Readable
-                        ? contentTypes.Readable
-                        : contentTypes.Writable)
-                    != null)
-            .ToArray();
+        string[] assignedProfilesForRequest = GetAssignedProfilesForRequest();
 
         // If there are no assigned profiles relevant for this request, skip additional processing here now.
         if (assignedProfilesForRequest.Length == 0)
@@ -160,10 +153,22 @@ void SetForbiddenResponse()
                 SecurityAuthorizationException.DefaultDetail,
                 errorMessage)
             {
-                CorrelationId = (string) _logContextAccessor.GetValue(CorrelationConstants.LogContextKey)
+                CorrelationId = _logContextAccessor.GetCorrelationId()
             }.AsSerializableModel();
 
             context.Result = new ObjectResult(problemDetails) { StatusCode = problemDetails.Status };
         }
+
+        string[] GetAssignedProfilesForRequest()
+        {
+            return apiClientContext.Profiles.Where(
+                    p => _profileResourceModelProvider.GetProfileResourceModel(p)
+                            .ResourceByName.TryGetValue(resourceFullName, out var contentTypes)
+                        && (relevantContentTypeUsage == ContentTypeUsage.Readable
+                            ? contentTypes.Readable
+                            : contentTypes.Writable)
+                        != null)
+                .ToArray();
+        }
     }
 }

Application/EdFi.Ods.Api/Helpers/ControllerHelpers.cs

@@ -3,26 +3,42 @@
 // 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.Net;
 using EdFi.Ods.Common.Exceptions;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 
 namespace EdFi.Ods.Api.Helpers
 {
     public static class ControllerHelpers
     {
         /// <summary>
-        /// Gets an IActionResult returning a 404 Not Found status with no response body.
+        /// Gets an IActionResult returning a 404 Not Found status with a default response body indicating the resource could not be found.
         /// </summary>
         /// <returns></returns>
-        public static IActionResult NotFound(string error)
-        {
-            var problemDetails = new NotFoundException(error);
+        public static IActionResult NotFound(string correlationId = null)
+        {
+            var problemDetails = new NotFoundException()
+            {
+                CorrelationId = correlationId
+            };
 
-            return new ObjectResult(problemDetails)
+            return new ObjectResult(problemDetails.AsSerializableModel())
             {
                 StatusCode = problemDetails.Status,
             };
         }
+
+        public static IActionResult FeatureDisabled(string featureName, string correlationId = null)
+        {
+            return new ObjectResult(
+                new FeatureDisabledException(featureName, StatusCodes.Status404NotFound)
+                    {
+                        CorrelationId = correlationId
+                    }
+                    .AsSerializableModel())
+            {
+                StatusCode = StatusCodes.Status404NotFound
+            };
+        }
     }
 }

Application/EdFi.Ods.Api/Middleware/AuthenticationFailureMessages.cs

@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: Apache-2.0
+// Licensed to the Ed-Fi Alliance under one or more agreements.
+// 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.
+
+namespace EdFi.Ods.Api.Middleware;
+
+public class AuthenticationFailureMessages
+{
+    public const string UnknownAuthorizationHeaderScheme = "Unknown Authorization header scheme.";
+    public const string MissingAuthorizationHeaderBearerTokenValue = "Missing Authorization header bearer token value.";
+    public const string MissingAuthorizationHeader = "Authorization header is missing.";
+    public const string InvalidAuthorizationHeader = "Invalid Authorization header.";
+}

Application/EdFi.Ods.Api/Middleware/EdFiApiAuthenticationMiddleware.cs

@@ -5,10 +5,9 @@
 
 using System;
 using System.Threading.Tasks;
-using EdFi.Ods.Api.ExceptionHandling;
 using EdFi.Ods.Api.Extensions;
-using EdFi.Ods.Common.Constants;
 using EdFi.Ods.Common.Exceptions;
+using EdFi.Ods.Common.Extensions;
 using EdFi.Ods.Common.Logging;
 using EdFi.Ods.Common.Security;
 using log4net;
@@ -21,21 +20,18 @@ namespace EdFi.Ods.Api.Middleware
     public class EdFiApiAuthenticationMiddleware
     {
         private readonly IApiClientContextProvider _apiClientContextProvider;
-        private readonly AuthenticateResultTranslator _authenticateResultTranslator;
         private readonly RequestDelegate _next;
         private readonly ILogContextAccessor _logContextAccessor;
 
         public EdFiApiAuthenticationMiddleware(RequestDelegate next,
             IAuthenticationSchemeProvider schemes,
             IApiClientContextProvider apiClientContextProvider,
-            AuthenticateResultTranslator authenticateResultTranslator,
             ILogContextAccessor logContextAccessor)
         {
             if (schemes != null)
             {
                 _next = next ?? throw new ArgumentNullException(nameof(next));
                 _apiClientContextProvider = apiClientContextProvider;
-                _authenticateResultTranslator = authenticateResultTranslator;
                 Schemes = schemes;
                 _logContextAccessor = logContextAccessor;
             }
@@ -90,17 +86,32 @@ public async Task Invoke(HttpContext context)
                         context.Request.Path.StartsWithSegments("/composites") ||
                         context.Request.Path.StartsWithSegments("/changeQueries"))
                 {
-                    string correlationId = (string)_logContextAccessor.GetValue(CorrelationConstants.LogContextKey);
-
-                    var problemDetails = (EdFiProblemDetailsExceptionBase)_authenticateResultTranslator.GetProblemDetails(result);
-
-                    problemDetails.CorrelationId = correlationId;
+                    var problemDetails = GetProblemDetailsExceptionFromResult(result);
+                    problemDetails.CorrelationId = _logContextAccessor.GetCorrelationId();;
 
                     await context.Response.WriteProblemDetailsAsync(problemDetails);
+
+                    return;
                 }
             }
 
-        await _next(context);
+            await _next(context);
+
+            EdFiProblemDetailsExceptionBase GetProblemDetailsExceptionFromResult(AuthenticateResult result)
+            {
+                EdFiProblemDetailsExceptionBase problemDetails;
+
+                if (result.None)
+                {
+                    problemDetails = new SecurityAuthenticationException(AuthenticationFailureMessages.MissingAuthorizationHeader);
+                }
+                else
+                {
+                    problemDetails = new SecurityAuthenticationException(result.Failure.Message, result.Failure);
+                }
+
+                return problemDetails;
+            }
+        }
     }
 }
-}