Tidy up, fix errors

Author: stusherwin

SAPSec.Web/Authentication/DsiAuthenticationExtensions.cs

@@ -88,7 +88,23 @@ private static void ConfigureCookieOptions(CookieAuthenticationOptions options,
         options.ExpireTimeSpan = TimeSpan.FromMinutes(config.TokenExpiryMinutes);
         options.SlidingExpiration = true;
         options.LoginPath = Routes.SignIn;
-        options.AccessDeniedPath = Routes.AccessDenied;
+        options.AccessDeniedPath = Routes.AccessDenied;
+
+        // Override login redirect for unauthenticated requests
+        options.Events.OnRedirectToLogin = context =>
+        {
+            // Set status code to 401 Unauthorized
+            context.Response.StatusCode = StatusCodes.Status401Unauthorized;
+            return Task.CompletedTask;
+        };
+
+        // Override access-denied redirect for unauthorized requests
+        options.Events.OnRedirectToAccessDenied = context =>
+        {
+            // Set status code to 403 Forbidden
+            context.Response.StatusCode = StatusCodes.Status403Forbidden;
+            return Task.CompletedTask;
+        };
     }
 
     private static void ConfigureOpenIdConnectOptions(OpenIdConnectOptions options, DsiConfiguration config)

SAPSec.Web/Authentication/DsiAuthenticationHandler.cs

@@ -149,7 +149,7 @@ await userService.SetCurrentOrganisationAsync(
         }
         else if (user.Organisations.Count > 1)
         {
-            context.Properties!.RedirectUri = Routes.SchoolSearch;
+            context.Properties!.RedirectUri = Routes.FindASchool;
         }
     }
 

SAPSec.Web/Authorization/DsiAuthorizationHandler.cs

@@ -38,7 +38,6 @@ void Fail(string message)
         if (!org.IsEstablishment || org.Urn is null)
         {
             logger.LogInformation("User Organisation is not an Establishment or has a null Urn.");
-            return;
         }
 
         context.Succeed(requirement);

SAPSec.Web/Constants/Routes.cs

@@ -2,10 +2,10 @@
 
 public static class Routes
 {
-    public const string SignIn = "/Auth/sign-in";
-    public const string AccessDenied = "/error/403";
     public const string Home = "/";
-    public const string SchoolSearch = "/find-a-school";
-    public const string Error = "/error";
+    public const string SignIn = "/auth/signin";
+    public const string FindASchool = "/find-a-school";
     public static string School(string urn) => $"/school/{urn}";
+    public const string Error = "/error";
+    public const string AccessDenied = "/error/403";
 }

SAPSec.Web/Controllers/AuthController.cs

@@ -3,13 +3,11 @@
 using Microsoft.AspNetCore.Authentication.OpenIdConnect;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
-using Microsoft.AspNetCore.Mvc.ViewEngines;
 using SAPSec.Core.Interfaces.Services;
-using SAPSec.Core.Model;
 
 namespace SAPSec.Web.Controllers;
 
-[Route("[controller]")]
+[Route("auth")]
 public class AuthController(
     IUserService userService,
     ILogger<AuthController> logger) : Controller
@@ -19,29 +17,14 @@ public class AuthController(
 
     private static class Routes
     {
-        public const string SignIn = "sign-in";
-        public const string SignOut = "sign-out";
+        public const string SignIn = "signin";
+        public const string SignOut = "signout";
         public const string SignedOut = "signed-out";
-        public const string SignOutCallback = "SignOutCallback";
-        public const string SelectOrganisation = "select-organisation";
-    }
-
-    private static class Defaults
-    {
-        public const string ReturnUrl = "/find-a-school";
     }
 
     private static class LogMessages
     {
         public const string UserSigningOut = "User {UserId} signing out";
-        public const string OrganisationSelected = "User {UserId} selected organisation {OrganisationId}";
-        public const string OrganisationSelectionFailed = "Failed to set organisation {OrganisationId} for user {UserId}";
-        public const string UserNotFound = "User not found or has no organisations";
-    }
-
-    private static class ErrorMessages
-    {
-        public const string OrganisationIdRequired = "Organisation ID is required";
     }
 
     [HttpGet(Routes.SignIn)]
@@ -56,46 +39,7 @@ public IActionResult SignIn(string? returnUrl = null)
         return ChallengeWithRedirect(returnUrl);
     }
 
-    [HttpGet(Routes.SelectOrganisation)]
-    [Authorize]
-    public async Task<IActionResult> SelectOrganisation(string? returnUrl = null)
-    {
-        var user = await _userService.GetUserFromClaimsAsync(User);
-
-        if (!HasValidOrganisations(user))
-        {
-            _logger.LogWarning(LogMessages.UserNotFound);
-            return RedirectToProblem();
-        }
-
-        ViewBag.ReturnUrl = returnUrl;
-        return View(user);
-    }
-
-    [HttpPost(Routes.SelectOrganisation)]
-    [Authorize]
-    [ValidateAntiForgeryToken]
-    public async Task<IActionResult> SelectOrganisationPost(string organisationId, string? returnUrl = null)
-    {
-        if (string.IsNullOrEmpty(organisationId))
-        {
-            return BadRequest(ErrorMessages.OrganisationIdRequired);
-        }
-
-        var success = await _userService.SetCurrentOrganisationAsync(User, organisationId);
-
-        if (!success)
-        {
-            LogOrganisationSelectionFailed(organisationId);
-            return RedirectToProblem();
-        }
-
-        LogOrganisationSelected(organisationId);
-        return RedirectToLocal(returnUrl);
-    }
-
     [HttpGet(Routes.SignOut)]
-    [HttpGet(Routes.SignOutCallback)]
     [AllowAnonymous]
     public new async Task<IActionResult> SignOut()
     {
@@ -127,16 +71,11 @@ private bool IsUserAuthenticated()
         return _userService.IsAuthenticated(User);
     }
 
-    private static bool HasValidOrganisations(User? user)
-    {
-        return user?.Organisations.Any() == true;
-    }
-
     private IActionResult ChallengeWithRedirect(string? returnUrl)
     {
         var properties = new AuthenticationProperties
         {
-            RedirectUri = returnUrl ?? Defaults.ReturnUrl
+            RedirectUri = returnUrl ?? Constants.Routes.FindASchool
         };
 
         return Challenge(properties, OpenIdConnectDefaults.AuthenticationScheme);
@@ -169,11 +108,6 @@ private IActionResult RedirectToHome()
         return RedirectToAction("Index", "Home");
     }
 
-    private IActionResult RedirectToProblem()
-    {
-        return RedirectToAction("StatusCodeError", "Error", new { statusCode = 500 });
-    }
-
     #endregion
 
     #region Logging Methods
@@ -184,17 +118,5 @@ private void LogUserSigningOut()
         _logger.LogInformation(LogMessages.UserSigningOut, userId);
     }
 
-    private void LogOrganisationSelected(string organisationId)
-    {
-        var userId = _userService.GetUserId(User);
-        _logger.LogInformation(LogMessages.OrganisationSelected, userId, organisationId);
-    }
-
-    private void LogOrganisationSelectionFailed(string organisationId)
-    {
-        var userId = _userService.GetUserId(User);
-        _logger.LogWarning(LogMessages.OrganisationSelectionFailed, organisationId, userId);
-    }
-
     #endregion
 }

SAPSec.Web/Controllers/ConnectSchoolController.cs

@@ -5,15 +5,17 @@
 
 namespace SAPSec.Web.Controllers;
 
-[Authorize]
-public class SchoolHomeController(
+[Authorize]
+[Route("user")]
+public class UserController(
     IUserService userService,
-    ILogger<SchoolHomeController> logger) : Controller
+    ILogger<UserController> logger) : Controller
 {
     private readonly IUserService _userService = userService ?? throw new ArgumentNullException(nameof(userService));
-    private readonly ILogger<SchoolHomeController> _logger = logger ?? throw new ArgumentNullException(nameof(logger));
+    private readonly ILogger<UserController> _logger = logger ?? throw new ArgumentNullException(nameof(logger));
 
-    [HttpGet]
+    [HttpGet]
+    [Route("redirect")]
     public async Task<IActionResult> Index()
     {
         var user = await _userService.GetUserFromClaimsAsync(User);
@@ -31,7 +33,7 @@ public async Task<IActionResult> Index()
         if (!org.IsEstablishment || org.Urn is null)
         {
             _logger.LogInformation("User Organisation is not an Establishment or has a null Urn, redirecting to school search.");
-            return Redirect(Routes.SchoolSearch);
+            return Redirect(Routes.FindASchool);
         }
 
         return Redirect(Routes.School(org.Urn));

SAPSec.Web/Controllers/OrganisationController.cs

@@ -40,7 +40,7 @@
 
                 <a asp-controller="Auth"
                    asp-action="SignIn"
-                   asp-route-returnUrl="/SchoolHome"
+                   asp-route-returnUrl="/user/redirect"
                    role="button"
                    draggable="false"
                    class="govuk-button govuk-button--start govuk-!-margin-top-2 govuk-!-margin-bottom-8"