From 786016344b0a63b2f105e14cb96bf5bd3b25055a Mon Sep 17 00:00:00 2001 From: Paul Schneider Date: Sun, 12 Jul 2026 17:56:23 +0100 Subject: [PATCH] refacto error handling --- .../Accounting/AccountController.cs | 10 ++-- .../Accounting/ManageController.cs | 14 ----- .../Controllers/Consent/ConsentController.cs | 17 +++--- .../Controllers/Device/DeviceController.cs | 20 ++++++- src/Yavsc.Org/Controllers/HomeController.cs | 56 +++++++++++++++---- src/Yavsc.Org/Extensions/HostingExtensions.cs | 1 - src/Yavsc.Org/Helpers/ErrorViewHelpers.cs | 52 +++++++++++++++++ src/Yavsc.Server/Models/ErrorViewModel.cs | 1 + 8 files changed, 126 insertions(+), 45 deletions(-) create mode 100644 src/Yavsc.Org/Helpers/ErrorViewHelpers.cs diff --git a/src/Yavsc.Org/Controllers/Accounting/AccountController.cs b/src/Yavsc.Org/Controllers/Accounting/AccountController.cs index e8a63163..43565442 100644 --- a/src/Yavsc.Org/Controllers/Accounting/AccountController.cs +++ b/src/Yavsc.Org/Controllers/Accounting/AccountController.cs @@ -791,12 +791,12 @@ IHtmlLocalizerFactory htmlLocalizerFactory, { if (userId == null || code == null) { - return View("Error"); + return this.ErrorView("Error: userId or code is null."); } var user = await _userManager.FindByIdAsync(userId); if (user == null) { - return View("Error"); + return this.ErrorView("Error: user not found."); } IdentityResult result = null; try @@ -819,12 +819,12 @@ IHtmlLocalizerFactory htmlLocalizerFactory, { if (userId == null || code == null) { - return View("Error"); + return this.ErrorView("Error: userId or code is null."); } var user = await _userManager.FindByIdAsync(userId); if (user == null) { - return View("Error"); + return this.ErrorView("Error: user not found."); } bool result = false; try @@ -837,7 +837,7 @@ IHtmlLocalizerFactory htmlLocalizerFactory, _logger.LogError(ex.StackTrace); _logger.LogError(ex.Message); } - return View(result ? "EmailConfirmed" : "Error"); + return result ? View("EmailConfirmed") : this.ErrorView("Error confirming two factor token."); } // diff --git a/src/Yavsc.Org/Controllers/Accounting/ManageController.cs b/src/Yavsc.Org/Controllers/Accounting/ManageController.cs index 7419c889..43e11cd2 100644 --- a/src/Yavsc.Org/Controllers/Accounting/ManageController.cs +++ b/src/Yavsc.Org/Controllers/Accounting/ManageController.cs @@ -1,6 +1,5 @@ using System.Security.Claims; -using System.IO; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Localization; @@ -490,12 +489,7 @@ namespace Yavsc.Controllers : message == ManageMessageId.Error ? "An error has occurred." : ""; var user = await GetCurrentUserAsync(); - if (user == null) - { - return View("Error"); - } var userLogins = await _userManager.GetLoginsAsync(user); - ViewBag.ShowRemoveButton = user.PasswordHash != null || userLogins.Count > 1; return View(new ManageLoginsViewModel @@ -522,15 +516,7 @@ namespace Yavsc.Controllers public async Task LinkLoginCallback() { var user = await GetCurrentUserAsync(); - if (user == null) - { - return View("Error"); - } var info = await _signInManager.GetExternalLoginInfoAsync(User.GetUserId()); - if (info == null) - { - return RedirectToAction(nameof(ManageLogins), new { Message = ManageMessageId.Error }); - } var result = await _userManager.AddLoginAsync(user, info); var message = result.Succeeded ? ManageMessageId.AddLoginSuccess : ManageMessageId.Error; return RedirectToAction(nameof(ManageLogins), new { Message = message }); diff --git a/src/Yavsc.Org/Controllers/Consent/ConsentController.cs b/src/Yavsc.Org/Controllers/Consent/ConsentController.cs index 15c4a93b..b7b6eff8 100644 --- a/src/Yavsc.Org/Controllers/Consent/ConsentController.cs +++ b/src/Yavsc.Org/Controllers/Consent/ConsentController.cs @@ -16,6 +16,7 @@ using System.Collections.Generic; using System; using Yavsc; using Yavsc.Extensions; +using Yavsc.Models; namespace IdentityServerHost.Quickstart.UI { @@ -53,10 +54,11 @@ namespace IdentityServerHost.Quickstart.UI { return View("Index", vm); } - - return View("Error"); + return this.ErrorView("No consent request matching request: " + returnUrl); } + + /// /// Handles the consent screen postback /// @@ -88,8 +90,8 @@ namespace IdentityServerHost.Quickstart.UI { return View("Index", result.ViewModel); } - - return View("Error"); + return this.ErrorView($"ReturnUrl: {model}, result: {result}" ); + } /*****************************************/ @@ -170,11 +172,6 @@ namespace IdentityServerHost.Quickstart.UI { return CreateConsentViewModel(model, returnUrl, request); } - else - { - _logger.LogError("No consent request matching request: {0}", returnUrl); - } - return null; } @@ -199,7 +196,7 @@ namespace IdentityServerHost.Quickstart.UI vm.IdentityScopes = request.ValidatedResources.Resources.IdentityResources.Select(x => CreateScopeViewModel(x, vm.ScopesConsented.Contains(x.Name) || model == null)).ToArray(); var apiScopes = new List(); - foreach(var parsedScope in request.ValidatedResources.ParsedScopes) + foreach (var parsedScope in request.ValidatedResources.ParsedScopes) { var apiScope = request.ValidatedResources.Resources.FindApiScope(parsedScope.ParsedName); if (apiScope != null) diff --git a/src/Yavsc.Org/Controllers/Device/DeviceController.cs b/src/Yavsc.Org/Controllers/Device/DeviceController.cs index 5e0c7780..2e516aa6 100644 --- a/src/Yavsc.Org/Controllers/Device/DeviceController.cs +++ b/src/Yavsc.Org/Controllers/Device/DeviceController.cs @@ -16,6 +16,7 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Yavsc.Models; using Yavsc.Models.Access; namespace Yavsc.Controllers @@ -49,7 +50,7 @@ namespace Yavsc.Controllers if (string.IsNullOrWhiteSpace(userCode)) return View("UserCodeCapture"); var vm = await BuildViewModelAsync(userCode); - if (vm == null) return View("Error"); + if (vm == null) return this.ErrorView($"ViewModel is null! userCodeParamName: {userCodeParamName}, userCode: {userCode}" );; vm.ConfirmUserCode = true; return View("UserCodeConfirmation", vm); @@ -60,7 +61,7 @@ namespace Yavsc.Controllers public async Task UserCodeCapture(string userCode) { var vm = await BuildViewModelAsync(userCode); - if (vm == null) return View("Error"); + if (vm == null) return this.ErrorView($"UserCodeCapture: ViewModel is null! userCode: {userCode}" ); return View("UserCodeConfirmation", vm); } @@ -72,7 +73,20 @@ namespace Yavsc.Controllers if (model == null) throw new ArgumentNullException(nameof(model)); var result = await ProcessConsent(model); - if (result.HasValidationError) return View("Error"); + if (result.HasValidationError) + { + if (HttpContext.RequestServices.GetRequiredService().IsDevelopment()) + { + throw new InvalidOperationException("Device Authorization Input validation error: " + result.ValidationError); + } + + return View("Error", + new ErrorViewModel { + RequestId = HttpContext.TraceIdentifier, + Description = "Device Authorization Input validation error: " + result.ValidationError + } + ); + } return View("Success"); } diff --git a/src/Yavsc.Org/Controllers/HomeController.cs b/src/Yavsc.Org/Controllers/HomeController.cs index 2602a1a3..67c19fed 100644 --- a/src/Yavsc.Org/Controllers/HomeController.cs +++ b/src/Yavsc.Org/Controllers/HomeController.cs @@ -15,18 +15,24 @@ namespace Yavsc.Controllers public class HomeController : Controller { readonly ApplicationDbContext _dbContext; - + readonly ILogger _logger; + private readonly bool _isDevelopment; readonly IHtmlLocalizer _localizer; private SiteSettings siteSettings; public HomeController(ILogger logger, IHtmlLocalizer localizer, ApplicationDbContext context, - IOptions settingsOptions) + IOptions settingsOptions, + IWebHostEnvironment env + ) { _localizer = localizer; _dbContext = context; siteSettings = settingsOptions.Value; + _logger = logger; + _isDevelopment = env.IsDevelopment(); + } public async Task Index(string id) @@ -99,18 +105,44 @@ namespace Yavsc.Controllers public IActionResult Error() { - var feature = this.HttpContext.Features.Get(); - if (feature == null) return View(); - var errorType = feature?.Error; - if (errorType == null) return View(); - if (errorType is NotSupportedException notSupported) + if (_isDevelopment) { - return View(new ErrorViewModel { - Description = notSupported.Message, - RequestId = this.HttpContext.TraceIdentifier - }); + _logger.LogInformation( + "Home/Error requested in Development. This endpoint is disabled because DeveloperExceptionPage should handle unhandled exceptions."); + + return NotFound( + "In Development, /Home/Error is disabled. Unhandled exceptions are rendered by DeveloperExceptionPage."); } - return View("~/Views/Shared/Error.cshtml", feature?.Error); + + var errorViewModel = new ErrorViewModel + { + RequestId = HttpContext.TraceIdentifier + }; + + var exceptionHandlerPathFeature = + HttpContext.Features.Get(); + + if (exceptionHandlerPathFeature is null) + { + _logger.LogWarning( + "Home/Error called without IExceptionHandlerPathFeature in non-development environment."); + + return View("~/Views/Shared/Error.cshtml", errorViewModel); + } + + if (exceptionHandlerPathFeature?.Error is FileNotFoundException) + { + errorViewModel.Description = "The file was not found."; + } + + if (exceptionHandlerPathFeature?.Path == "/") + { + errorViewModel.Description ??= string.Empty; + errorViewModel.Description += " Page: Home."; + } + + + return View("~/Views/Shared/Error.cshtml", errorViewModel); } public IActionResult Status(int id) { diff --git a/src/Yavsc.Org/Extensions/HostingExtensions.cs b/src/Yavsc.Org/Extensions/HostingExtensions.cs index 74a1a53e..b3f90639 100644 --- a/src/Yavsc.Org/Extensions/HostingExtensions.cs +++ b/src/Yavsc.Org/Extensions/HostingExtensions.cs @@ -967,7 +967,6 @@ public static class HostingExtensions if (app.Environment.IsDevelopment()) { app.UseDeveloperExceptionPage(); - await app.MigrateDatabaseAsync(); } else { diff --git a/src/Yavsc.Org/Helpers/ErrorViewHelpers.cs b/src/Yavsc.Org/Helpers/ErrorViewHelpers.cs new file mode 100644 index 00000000..3d038c3f --- /dev/null +++ b/src/Yavsc.Org/Helpers/ErrorViewHelpers.cs @@ -0,0 +1,52 @@ +using Microsoft.AspNetCore.Mvc; +using Yavsc.Models; + +public static class ErrorViewHelpers +{ + public static IActionResult ErrorView(this Controller controller, string message) + { + var logger = controller.HttpContext.RequestServices.GetRequiredService() + .CreateLogger(); + + logger.LogError(message); + Dictionary dictionary = new Dictionary(); + + if (!controller.ModelState.IsValid) + { + foreach (var modelState in controller.ModelState.Values) + { + foreach (var error in modelState.Errors) + { + logger.LogError("ModelState error: {0}", error.ErrorMessage); + foreach (var key in controller.ModelState.Keys) + { + logger.LogError("ModelState key: {0}", key); + dictionary.Add(key, + string.Join("\n", + controller.ModelState[key].Errors.Select( e => e.ErrorMessage).ToArray())); + } + } + } + } + + if (controller.HttpContext.Request.Headers.ContainsKey("Accept") + && controller.HttpContext.Request.Headers["Accept"].ToString().Contains("application/json")) + { + return controller.Json(new + { + RequestId = controller.HttpContext.TraceIdentifier, + Description = message, + ModelErrors = dictionary + }); + } + + return controller.View("Error", + new ErrorViewModel + { + RequestId = controller.HttpContext.TraceIdentifier, + Description = message, + ModelErrors = dictionary + } + ); + } +} \ No newline at end of file diff --git a/src/Yavsc.Server/Models/ErrorViewModel.cs b/src/Yavsc.Server/Models/ErrorViewModel.cs index 1b779ead..17819476 100644 --- a/src/Yavsc.Server/Models/ErrorViewModel.cs +++ b/src/Yavsc.Server/Models/ErrorViewModel.cs @@ -7,4 +7,5 @@ public class ErrorViewModel public bool ShowRequestId => !string.IsNullOrEmpty(RequestId); + public Dictionary ModelErrors { get; set; } }