diff --git a/src/Modules/Grand.Module.Api/Infrastructure/ApiAuthenticationRegistrar.cs b/src/Modules/Grand.Module.Api/Infrastructure/ApiAuthenticationRegistrar.cs index c7b5215d1..e3c8400a3 100644 --- a/src/Modules/Grand.Module.Api/Infrastructure/ApiAuthenticationRegistrar.cs +++ b/src/Modules/Grand.Module.Api/Infrastructure/ApiAuthenticationRegistrar.cs @@ -1,9 +1,10 @@ -using Grand.Module.Api.Infrastructure.Extensions; +using Grand.Module.Api.Infrastructure.Extensions; using Grand.Business.Core.Interfaces.Authentication; using Grand.Infrastructure.Configuration; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.IdentityModel.Tokens; @@ -32,29 +33,39 @@ public void AddAuthentication(AuthenticationBuilder builder, IConfiguration conf OnAuthenticationFailed = async context => { context.NoResult(); - context.Response.StatusCode = 401; - context.Response.ContentType = "text/plain"; - await context.Response.WriteAsync(context.Exception.Message); - }, + context.Response.StatusCode = StatusCodes.Status401Unauthorized; + var problemDetailsService = context.HttpContext.RequestServices.GetService(); + if (problemDetailsService != null) + { + await problemDetailsService.WriteAsync(new ProblemDetailsContext { + HttpContext = context.HttpContext, + ProblemDetails = new ProblemDetails { + Status = StatusCodes.Status401Unauthorized, + Title = "Authentication failed" + } + }); + } + else + { + context.Response.ContentType = "application/problem+json"; + await context.Response.WriteAsJsonAsync(new ProblemDetails { + Status = StatusCodes.Status401Unauthorized, + Title = "Authentication failed" + }); + } + }, OnTokenValidated = async context => { - try + if (config.Enabled) { - if (config.Enabled) - { - var jwtAuthentication = context.HttpContext.RequestServices - .GetRequiredService(); - if (!await jwtAuthentication.Valid(context)) - throw new Exception(await jwtAuthentication.ErrorMessage()); - } - else - { - throw new Exception("API is disable"); - } + var jwtAuthentication = context.HttpContext.RequestServices + .GetRequiredService(); + if (!await jwtAuthentication.Valid(context)) + throw new Exception(await jwtAuthentication.ErrorMessage()); } - catch (Exception ex) + else { - throw new Exception(ex.Message); + throw new Exception("API is disabled"); } } }; @@ -80,30 +91,40 @@ public void AddAuthentication(AuthenticationBuilder builder, IConfiguration conf OnAuthenticationFailed = async context => { context.NoResult(); - context.Response.StatusCode = 401; - context.Response.ContentType = "text/plain"; - await context.Response.WriteAsync(context.Exception.Message); - }, + context.Response.StatusCode = StatusCodes.Status401Unauthorized; + var problemDetailsService = context.HttpContext.RequestServices.GetService(); + if (problemDetailsService != null) + { + await problemDetailsService.WriteAsync(new ProblemDetailsContext { + HttpContext = context.HttpContext, + ProblemDetails = new ProblemDetails { + Status = StatusCodes.Status401Unauthorized, + Title = "Authentication failed" + } + }); + } + else + { + context.Response.ContentType = "application/problem+json"; + await context.Response.WriteAsJsonAsync(new ProblemDetails { + Status = StatusCodes.Status401Unauthorized, + Title = "Authentication failed" + }); + } + }, OnTokenValidated = async context => { - try + if (config.Enabled) { - if (config.Enabled) - { - var jwtAuthentication = context.HttpContext.RequestServices - .GetRequiredService(); - var isValid = await jwtAuthentication.Valid(context); - if (!isValid) - throw new Exception(await jwtAuthentication.ErrorMessage()); - } - else - { - throw new Exception("API is disable"); - } + var jwtAuthentication = context.HttpContext.RequestServices + .GetRequiredService(); + var isValid = await jwtAuthentication.Valid(context); + if (!isValid) + throw new Exception(await jwtAuthentication.ErrorMessage()); } - catch (Exception ex) + else { - throw new Exception(ex.Message); + throw new Exception("API is disabled"); } } }; @@ -111,4 +132,4 @@ public void AddAuthentication(AuthenticationBuilder builder, IConfiguration conf } public int Priority => 900; -} \ No newline at end of file +} diff --git a/src/Web/Grand.Web.Common/Controllers/BaseController.cs b/src/Web/Grand.Web.Common/Controllers/BaseController.cs index 3a15c34f1..a69861e44 100644 --- a/src/Web/Grand.Web.Common/Controllers/BaseController.cs +++ b/src/Web/Grand.Web.Common/Controllers/BaseController.cs @@ -116,7 +116,7 @@ protected void Error(Exception exception, bool persistNextRequest = true, bool l private void LogException(Exception exception) { var logger = HttpContext.RequestServices.GetRequiredService().CreateLogger("BaseController"); - logger.LogError(exception, exception.Message); + logger.LogError(exception, "An error occurred"); } /// diff --git a/src/Web/Grand.Web.Common/Infrastructure/ApplicationBuilderExtensions.cs b/src/Web/Grand.Web.Common/Infrastructure/ApplicationBuilderExtensions.cs index f4ded74f3..e06e3bbf1 100644 --- a/src/Web/Grand.Web.Common/Infrastructure/ApplicationBuilderExtensions.cs +++ b/src/Web/Grand.Web.Common/Infrastructure/ApplicationBuilderExtensions.cs @@ -1,5 +1,4 @@ -using Grand.Data; -using Grand.Infrastructure.Configuration; +using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Endpoints; using Grand.Infrastructure.Plugins; using Grand.Infrastructure.TypeSearch; @@ -26,6 +25,21 @@ namespace Grand.Web.Common.Infrastructure; /// public static class ApplicationBuilderExtensions { + // Reused across requests — FileExtensionContentTypeProvider is stateless and thread-safe + private static readonly FileExtensionContentTypeProvider ContentTypeProvider = new(); + + internal static bool IsApiRequest(HttpRequest request) + { + string authHeader = request.Headers[HeaderNames.Authorization]; + return authHeader != null && + authHeader.StartsWith(JwtBearerDefaults.AuthenticationScheme + " ", StringComparison.OrdinalIgnoreCase); + } + + private static bool IsStaticFileRequest(PathString path) + { + return ContentTypeProvider.TryGetContentType(path, out _); + } + /// /// Add exception handling /// @@ -39,64 +53,46 @@ public static void UseGrandExceptionHandler(this WebApplication application) //get detailed exceptions for developing and testing purposes application.UseDeveloperExceptionPage(); else - //or use special exception handler - application.UseExceptionHandler("/errorpage.htm"); - - //log errors - application.UseExceptionHandler(handler => - { - handler.Run(async context => - { - var exception = context.Features.Get()?.Error; - if (exception == null) - return; - - string authHeader = context.Request.Headers["Authorization"]; - var apiRequest = authHeader != null && authHeader.Split(' ')[0] == "Bearer"; - if (apiRequest) - { - await context.Response.WriteAsync(exception.Message); - return; - } - - if (DataSettingsManager.DatabaseIsInstalled()) - { - var logger = context.RequestServices.GetRequiredService() - .CreateLogger("UseExceptionHandler"); - // Log the error - logger.LogError(exception, exception.Message); - } - - }); - }); + //use registered IExceptionHandler services (GrandExceptionHandler handles both API and web requests) + application.UseExceptionHandler(); } /// - /// Adds a special handler that checks for responses with the 404 status code that do not have a body + /// Adds a special handler that checks for responses with the 404 status code that do not have a body. + /// Re-executes the pipeline at /page-not-found (preserving the original 404 status code) while + /// skipping the re-execution for API and static-resource requests. /// /// Builder for configuring an application's request pipeline public static void UsePageNotFound(this WebApplication application) { - application.UseStatusCodePages(async context => + // UseStatusCodePagesWithReExecute sets IStatusCodePagesFeature.Enabled = true and re-executes + // the pipeline at the specified path when a 404 occurs, preserving the original 404 status code. + application.UseStatusCodePagesWithReExecute("/page-not-found"); + + // Disable status code pages for API (Bearer) requests and static resource requests so that + // those callers receive the original response rather than the HTML not-found page. + // For all other requests, also restrict re-execution to actual 404 responses so that + // 400/401/403/405/500 etc. are not mistakenly routed to /page-not-found. + application.Use(async (context, next) => { - //handle 404 Not Found - if (context.HttpContext.Response.StatusCode == 404) + if (IsApiRequest(context.Request) || IsStaticFileRequest(context.Request.Path)) { - string authHeader = context.HttpContext.Request.Headers[HeaderNames.Authorization]; - var apiRequest = authHeader != null && - authHeader.Split(' ')[0] == JwtBearerDefaults.AuthenticationScheme; + var feature = context.Features.Get(); + if (feature != null) + feature.Enabled = false; + await next(context); + return; + } - var contentTypeProvider = new FileExtensionContentTypeProvider(); - var staticResource = contentTypeProvider.TryGetContentType(context.HttpContext.Request.Path, out _); + await next(context); - if (!apiRequest && !staticResource) - { - const string location = "/page-not-found"; - context.HttpContext.Response.Redirect(context.HttpContext.Request.PathBase + location); - } + // Only re-execute for 404 Not Found; all other error codes are handled elsewhere. + if (context.Response.StatusCode != StatusCodes.Status404NotFound) + { + var feature = context.Features.Get(); + if (feature != null) + feature.Enabled = false; } - - await Task.CompletedTask; }); } @@ -112,10 +108,7 @@ public static void UseBadRequestResult(this WebApplication application) if (context.HttpContext.Response.StatusCode != StatusCodes.Status400BadRequest) return Task.CompletedTask; - string authHeader = context.HttpContext.Request.Headers[HeaderNames.Authorization]; - var apiRequest = authHeader != null && authHeader.Split(' ')[0] == JwtBearerDefaults.AuthenticationScheme; - - if (apiRequest) return Task.CompletedTask; + if (IsApiRequest(context.HttpContext.Request)) return Task.CompletedTask; var logger = context.HttpContext.RequestServices.GetRequiredService() .CreateLogger("UseBadRequestResult"); logger.LogError("Error 400. Bad request"); diff --git a/src/Web/Grand.Web.Common/Infrastructure/GrandExceptionHandler.cs b/src/Web/Grand.Web.Common/Infrastructure/GrandExceptionHandler.cs new file mode 100644 index 000000000..82a043ad5 --- /dev/null +++ b/src/Web/Grand.Web.Common/Infrastructure/GrandExceptionHandler.cs @@ -0,0 +1,72 @@ +using Grand.Data; +using Microsoft.AspNetCore.Diagnostics; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Grand.Web.Common.Infrastructure; + +/// +/// Handles unhandled exceptions according to ASP.NET Core best practices. +/// For API requests (Bearer token) it writes an RFC 7807 ProblemDetails JSON response. +/// For regular web (Razor/MVC) requests it redirects to the static error page (/errorpage.htm). +/// +public class GrandExceptionHandler : IExceptionHandler +{ + private readonly ILogger _logger; + + public GrandExceptionHandler(ILogger logger) + { + _logger = logger; + } + + public async ValueTask TryHandleAsync( + HttpContext httpContext, + Exception exception, + CancellationToken cancellationToken) + { + if (httpContext.Response.HasStarted) + return false; + + // Log the exception when the database is available + if (DataSettingsManager.DatabaseIsInstalled()) + _logger.LogError(exception, "An unhandled exception has occurred"); + + if (!ApplicationBuilderExtensions.IsApiRequest(httpContext.Request)) + { + // For Razor/MVC web requests, redirect to the static error page so the + // browser always sees a user-friendly page (the path-based re-execution + // fallback in UseExceptionHandler is unreliable in an MVC pipeline). + httpContext.Response.Redirect("/errorpage.htm", permanent: false); + return true; + } + + httpContext.Response.StatusCode = StatusCodes.Status500InternalServerError; + + var problemDetailsService = httpContext.RequestServices.GetService(); + if (problemDetailsService != null) + { + await problemDetailsService.WriteAsync(new ProblemDetailsContext { + HttpContext = httpContext, + Exception = exception, + ProblemDetails = new ProblemDetails { + Status = StatusCodes.Status500InternalServerError, + Title = "An error occurred while processing your request", + Instance = httpContext.Request.Path + } + }); + } + else + { + httpContext.Response.ContentType = "application/problem+json"; + await httpContext.Response.WriteAsJsonAsync(new ProblemDetails { + Status = StatusCodes.Status500InternalServerError, + Title = "An error occurred while processing your request", + Instance = httpContext.Request.Path + }, cancellationToken); + } + + return true; + } +} diff --git a/src/Web/Grand.Web.Common/Startup/ErrorHandlerStartup.cs b/src/Web/Grand.Web.Common/Startup/ErrorHandlerStartup.cs index 061d8b9c5..c2b9bd133 100644 --- a/src/Web/Grand.Web.Common/Startup/ErrorHandlerStartup.cs +++ b/src/Web/Grand.Web.Common/Startup/ErrorHandlerStartup.cs @@ -19,6 +19,11 @@ public class ErrorHandlerStartup : IStartupApplication /// Configuration root of the application public void ConfigureServices(IServiceCollection services, IConfiguration configuration) { + // Register RFC 7807 ProblemDetails support (used by GrandExceptionHandler for API errors) + services.AddProblemDetails(); + + // Register the IExceptionHandler implementation used by UseExceptionHandler() + services.AddExceptionHandler(); } /// diff --git a/src/Web/Grand.Web/App_Data/appsettings.json b/src/Web/Grand.Web/App_Data/appsettings.json index 7ca546b1b..4c2d343e3 100644 --- a/src/Web/Grand.Web/App_Data/appsettings.json +++ b/src/Web/Grand.Web/App_Data/appsettings.json @@ -1,7 +1,7 @@ { "Application": { //Enable if you want to see the full error in production environment. It's ignored (always enabled) in development environment - "DisplayFullErrorStack": true, + "DisplayFullErrorStack": false, //Value of "Cache-Control" header value for static content "StaticFilesCacheControl": "public,max-age=31536000", //Enable the session-based TempData provider