diff --git a/DESIGN.md b/DESIGN.md index 1aeeeebea..bb2d6e246 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -179,7 +179,7 @@ The system is flat by default. Surfaces sit on the page with borders and tonal f ### Shadow Vocabulary - **Focus ring** (`box-shadow: 0 0 0 0.1rem white, 0 0 0 0.25rem #258cfb`): The keyboard-focus halo on form controls and buttons. On the dark welcome splash this becomes a gold ring (`0 0 0 0.1875rem rgba(255,197,25,0.85)`) since default outlines vanish on the photo. -- **Splash card** (`box-shadow: 0 1.875rem 5rem rgba(0,0,0,0.45)`): The single heavy elevation in the system — the white sign-in card lifting off the full-bleed hero photo. Dramatic on purpose, and used exactly once. +- **Splash card** (`box-shadow: 0 1.875rem 5rem rgba(2,40,81,0.5)`): The single heavy elevation in the system: the white sign-in card lifting off the full-bleed hero photo. Dramatic on purpose, and used exactly once. Tinted Aggie Blue rather than black, like the splash's text halos, so every darkening layer on the page shares the hero's ambient hue. - **Sticky table header** (`background-color: #eee`): Tonal layering, not shadow — the sticky `q-table` header reads as elevated through fill and stickiness alone. ### Named Rules diff --git a/VueApp/src/CAHFS/pages/CAHFSAuth.vue b/VueApp/src/CAHFS/pages/CAHFSAuth.vue index 0feef73de..277560895 100644 --- a/VueApp/src/CAHFS/pages/CAHFSAuth.vue +++ b/VueApp/src/CAHFS/pages/CAHFSAuth.vue @@ -1,5 +1,36 @@ @@ -36,54 +35,6 @@ const currentYear = new Date().getFullYear() height-hint="98" class="bg-white text-dark" > -
- UC Davis Veterinary Medicine logo -
- - - - - VIPER 2.0 - Development - Test - - -
-
+/// Unit tests for HomeController's anonymous landing / login flow, focused on the +/// open-redirect protections and redirect-loop guard added with the welcome page. +/// +public sealed class HomeControllerTests +{ + private readonly HomeController _controller; + + // Real area controllers, one per area. GetAreaNames() derives the area set from the + // "Viper.Areas..…" controller namespace, so these expose ClinicalScheduler/Effort/RAPS/CTS. + // Effort is intentionally an API-only area (its controllers carry no [Area]) — the case the + // namespace-based derivation fixes versus the old [Area] route-value lookup. + private static readonly Type[] _areaControllerTypes = + { + typeof(Viper.Areas.ClinicalScheduler.Controllers.CliniciansController), + typeof(Viper.Areas.Effort.Controllers.ReportsController), + typeof(Viper.Areas.RAPS.Controllers.RAPSController), + typeof(Viper.Areas.CTS.Controllers.CTSController), + }; + + // "/ClinicalScheduler" is a splash-eligible area landing page; "/ClinicalScheduler/rotation" is a deep link. + private static readonly string[] _areas = { "ClinicalScheduler", "Effort", "RAPS", "CTS" }; + + public HomeControllerTests() + { + var actionProvider = Substitute.For(); + var descriptors = _areaControllerTypes + .Select(t => new ControllerActionDescriptor { ControllerTypeInfo = t.GetTypeInfo() }) + .ToList(); + actionProvider.ActionDescriptors.Returns(new ActionDescriptorCollection(descriptors, version: 1)); + + _controller = new HomeController( + Substitute.For(), + Options.Create(new CasSettings { CasBaseUrl = "https://cas.example.edu/" }), + Substitute.For(), + Substitute.For(), + Substitute.For(), + actionProvider); + } + + /// + /// Wires up a controller context with the requested auth state and a URL helper whose + /// IsLocalUrl mirrors framework semantics (local = rooted path, not protocol-relative). + /// + private void Arrange(bool authenticated) + { + var identity = authenticated + ? new ClaimsIdentity(new[] { new Claim(ClaimTypes.Name, "tester") }, authenticationType: "TestAuth") + : new ClaimsIdentity(); + + var httpContext = new DefaultHttpContext + { + User = new ClaimsPrincipal(identity), + RequestServices = new ServiceCollection().BuildServiceProvider(), + }; + httpContext.Request.Scheme = "https"; + httpContext.Request.Host = new HostString("viper.test"); + httpContext.Request.Path = "/login"; + + _controller.ControllerContext = new ControllerContext { HttpContext = httpContext }; + // View() resolves ITempDataDictionaryFactory from DI unless TempData is already set. + _controller.TempData = new TempDataDictionary(httpContext, Substitute.For()); + + var url = Substitute.For(); + url.IsLocalUrl(Arg.Any()).Returns(ci => + { + var candidate = ci.Arg(); + if (string.IsNullOrEmpty(candidate)) + { + return false; + } + + // Mirror framework semantics: rooted "/..." and app-relative "~/..." are + // local, but protocol-relative ("//"), backslash ("/\") and their "~/" + // variants are not. + if (candidate.StartsWith('/')) + { + return !candidate.StartsWith("//") && !candidate.StartsWith("/\\"); + } + + return candidate.StartsWith("~/") + && !candidate.StartsWith("~//") + && !candidate.StartsWith("~/\\"); + }); + _controller.Url = url; + } + + [Theory] + [InlineData(null, false)] + [InlineData("", false)] + [InlineData("/welcome", true)] + [InlineData("/Welcome", true)] + [InlineData("/welcome/", true)] + [InlineData("/login", true)] + [InlineData("/LOGIN?ReturnUrl=/x", true)] + [InlineData("/welcome#frag", true)] + [InlineData("/RAPS/Roles", false)] + [InlineData("/welcomepage", false)] + public void IsWelcomeOrLoginPath_DetectsLoopTargets(string? url, bool expected) + { + Assert.Equal(expected, HomeController.IsWelcomeOrLoginPath(url)); + } + + // Splash appears only for the front door (an empty return path or the bare site root) and for a + // single-segment area landing page. Anything deeper, or a single segment that is not a registered + // area, is treated as a deep link. + [Theory] + [InlineData(null, true)] + [InlineData("", true)] + [InlineData("/", true)] + [InlineData("/ClinicalScheduler", true)] + [InlineData("/clinicalscheduler", true)] // area match is case-insensitive + [InlineData("/Effort/", true)] // trailing slash on an area root still counts + [InlineData("/Effort?tab=1", true)] // query string is ignored + [InlineData("/ClinicalScheduler/rotation", false)] // deep link + [InlineData("/CTS/epa", false)] // CTS is an area, but this is a deep link + [InlineData("/MyPermissions", false)] // single segment, not an area + [InlineData("/cahfs", false)] // not a registered MVC area + public void IsSplashTarget_ClassifiesPassiveLandingsVsDeepLinks(string? url, bool expected) + { + var areas = new HashSet(_areas, StringComparer.OrdinalIgnoreCase); + Assert.Equal(expected, HomeController.IsSplashTarget(url, areas)); + } + + // In a subpath deployment (PathBase "/2") the ReturnUrl is prefixed with the base. StripPathBase + // removes it on a segment boundary so the splash classifier sees an app-relative path, while leaving + // unrelated paths (and the no-base dev case) untouched. + [Theory] + [InlineData(null, "/2", null)] + [InlineData("", "/2", "")] + [InlineData("/2/ClinicalScheduler", "/2", "/ClinicalScheduler")] + [InlineData("/2/ClinicalScheduler/rotation", "/2", "/ClinicalScheduler/rotation")] + [InlineData("/2", "/2", "")] // app root under the subpath + [InlineData("/2/", "/2", "/")] + [InlineData("/2?tab=1", "/2", "?tab=1")] // query is preserved for the classifier to strip + [InlineData("/22/x", "/2", "/22/x")] // segment boundary: "/2" must not strip from "/22" + [InlineData("/ClinicalScheduler", "", "/ClinicalScheduler")] // no base configured (dev) + [InlineData("/ClinicalScheduler", null, "/ClinicalScheduler")] + public void StripPathBase_RemovesBaseOnSegmentBoundary(string? url, string? pathBase, string? expected) + { + Assert.Equal(expected, HomeController.StripPathBase(url, pathBase)); + } + + // Area names come from controller namespaces (Viper.Areas..…), so API-only areas with no + // [Area] attribute are still recognized. Non-area namespaces and near-matches resolve to null. + [Theory] + [InlineData("Viper.Areas.Effort.Controllers", "Effort")] + [InlineData("Viper.Areas.ClinicalScheduler.Controllers.SomethingV2", "ClinicalScheduler")] + [InlineData("Viper.Areas.CMS", "CMS")] + [InlineData("Viper.Controllers", null)] // not an area + [InlineData("Viper.Areas", null)] // no area segment + [InlineData("Viper.AreasButNotReally.X", null)] // prefix must end on a namespace boundary + [InlineData(null, null)] + public void AreaFromControllerNamespace_ExtractsAreaSegment(string? ns, string? expected) + { + Assert.Equal(expected, HomeController.AreaFromControllerNamespace(ns)); + } + + [Fact] + public void Index_Anonymous_RendersWelcomeWithNoStore() + { + Arrange(authenticated: false); + + var result = _controller.Index(); + + var view = Assert.IsType(result); + Assert.Equal("Welcome", view.ViewName); + Assert.Equal("no-store,no-cache", _controller.Response.Headers.CacheControl.ToString()); + } + + // An area landing page ("/ClinicalScheduler") is a passive arrival, so it still gets the splash + // with its ReturnUrl preserved. + [Fact] + public void Welcome_Anonymous_AreaLanding_RendersSplashWithReturnUrl() + { + Arrange(authenticated: false); + + var result = _controller.Welcome("/ClinicalScheduler"); + + var view = Assert.IsType(result); + Assert.Equal("Welcome", view.ViewName); + Assert.Equal("/ClinicalScheduler", view.ViewData["ReturnUrl"]); + } + + // Effort is an API-only area (no [Area] MVC controller); its landing page must still splash. + // Regression guard for the namespace-based area derivation that replaced the [Area] route-value lookup. + [Fact] + public void Welcome_Anonymous_ApiOnlyAreaLanding_RendersSplash() + { + Arrange(authenticated: false); + + var result = _controller.Welcome("/Effort"); + + var view = Assert.IsType(result); + Assert.Equal("Welcome", view.ViewName); + Assert.Equal("/Effort", view.ViewData["ReturnUrl"]); + } + + // A deep link ("/ClinicalScheduler/rotation") skips the interstitial and goes straight to CAS + // via the Login action, carrying the ReturnUrl so the user lands where they intended. + [Fact] + public void Welcome_Anonymous_DeepLink_RedirectsToCasLogin() + { + Arrange(authenticated: false); + + var result = _controller.Welcome("/ClinicalScheduler/rotation"); + + var redirect = Assert.IsType(result); + Assert.Equal(nameof(HomeController.Login), redirect.ActionName); + Assert.Equal("/ClinicalScheduler/rotation", redirect.RouteValues?["ReturnUrl"]); + } + + // Under a subpath deployment (PathBase "/2") the area landing page arrives as "/2/ClinicalScheduler". + // The base is stripped for classification so it still gets the splash, with the friendly area label + // resolved and the full (base-prefixed) ReturnUrl preserved for the round trip. + [Fact] + public void Welcome_Anonymous_SubpathAreaLanding_RendersSplash() + { + Arrange(authenticated: false); + _controller.HttpContext.Request.PathBase = "/2"; + + var result = _controller.Welcome("/2/ClinicalScheduler"); + + var view = Assert.IsType(result); + Assert.Equal("Welcome", view.ViewName); + Assert.Equal("/2/ClinicalScheduler", view.ViewData["ReturnUrl"]); + Assert.Equal("Clinical Scheduler", view.ViewData["DestinationLabel"]); + } + + // A subpath deep link ("/2/ClinicalScheduler/rotation") still bypasses the splash and goes to CAS, + // carrying the full base-prefixed ReturnUrl. + [Fact] + public void Welcome_Anonymous_SubpathDeepLink_RedirectsToCasLogin() + { + Arrange(authenticated: false); + _controller.HttpContext.Request.PathBase = "/2"; + + var result = _controller.Welcome("/2/ClinicalScheduler/rotation"); + + var redirect = Assert.IsType(result); + Assert.Equal(nameof(HomeController.Login), redirect.ActionName); + Assert.Equal("/2/ClinicalScheduler/rotation", redirect.RouteValues?["ReturnUrl"]); + } + + // Under a subpath deployment the loop targets arrive base-prefixed ("/2/welcome", "/2/login"). + // The base is stripped before the loop-guard so they are still caught: the splash renders with a + // null ReturnUrl rather than bouncing back out to CAS. Regression guard for the pre-strip loop-guard. + [Theory] + [InlineData("/2/welcome")] + [InlineData("/2/login")] + [InlineData("/2/Welcome/")] + public void Welcome_Anonymous_SubpathLoopTarget_DropsReturnUrl(string returnUrl) + { + Arrange(authenticated: false); + _controller.HttpContext.Request.PathBase = "/2"; + + var result = _controller.Welcome(returnUrl); + + var view = Assert.IsType(result); + Assert.Equal("Welcome", view.ViewName); + Assert.Null(view.ViewData["ReturnUrl"]); + } + + // Authenticated welcome with no ReturnUrl under a subpath deployment redirects to "~/" so the app + // root keeps its PathBase ("/2/") instead of escaping to the domain root. Regression guard for the + // bare "/" that sent logged-in users out to the legacy site. + [Fact] + public void Welcome_Authenticated_NoReturnUrl_RedirectsToAppRelativeRoot() + { + Arrange(authenticated: true); + _controller.HttpContext.Request.PathBase = "/2"; + + var result = _controller.Welcome(); + + var redirect = Assert.IsType(result); + Assert.Equal("~/", redirect.Url); + } + + // Anonymous users still get the Welcome view, but any ReturnUrl that is non-local + // (open redirect) or points back at /welcome|/login (redirect loop) is dropped. + [Theory] + [InlineData("https://evil.com/phish")] + [InlineData("//evil.com")] + [InlineData("/welcome")] + [InlineData("/login")] + [InlineData("~/welcome")] // app-relative loop targets must still be caught after normalization + [InlineData("~/login")] + public void Welcome_Anonymous_DropsUnsafeReturnUrl(string returnUrl) + { + Arrange(authenticated: false); + + var result = _controller.Welcome(returnUrl); + + var view = Assert.IsType(result); + Assert.Null(view.ViewData["ReturnUrl"]); + } + + [Fact] + public void Welcome_Authenticated_RedirectsToLocalReturnUrl() + { + Arrange(authenticated: true); + + var result = _controller.Welcome("/Effort/Foo"); + + var redirect = Assert.IsType(result); + Assert.Equal("/Effort/Foo", redirect.Url); + } + + // App root is "~/" (not "/") so a subpath deployment keeps its PathBase ("/2/") rather than + // escaping to the domain root (the legacy site). + [Theory] + [InlineData("https://evil.com")] + [InlineData(null)] + public void Welcome_Authenticated_RedirectsToRootWhenReturnUrlInvalidOrMissing(string? returnUrl) + { + Arrange(authenticated: true); + + var result = _controller.Welcome(returnUrl); + + var redirect = Assert.IsType(result); + Assert.Equal("~/", redirect.Url); + } + + // The /api guard matches the api segment case-insensitively (routing is case-insensitive) and + // on a segment boundary, so "/api", "/api/...", "/api?..." are rejected in any casing while + // non-API paths that merely start with "api" (e.g. "/apiary") pass through to CAS. + [Theory] + [InlineData("/api", true)] + [InlineData("/api/foo", true)] + [InlineData("/API/foo", true)] + [InlineData("/Api?x=1", true)] + [InlineData("/api#frag", true)] + [InlineData("/apiary", false)] + [InlineData("/", false)] + public void IsApiPath_MatchesApiSegmentCaseInsensitivelyOnBoundary(string url, bool expected) + { + Assert.Equal(expected, HomeController.IsApiPath(url)); + } + + [Theory] + [InlineData("/api/secret")] + [InlineData("/API/secret")] // routing is case-insensitive, so the guard must be too + [InlineData("~/api/secret")] // app-relative form must not bypass the /api guard + public void Login_RejectsApiReturnUrl_WithUnauthorized(string returnUrl) + { + Arrange(authenticated: false); + + var result = _controller.Login(returnUrl); + + Assert.IsType(result); + } + + // A non-API path that merely starts with "api" is not caught by the guard; it proceeds to the + // normal CAS redirect. + [Fact] + public void Login_ForwardsNonApiPathStartingWithApiToCas() + { + Arrange(authenticated: false); + + var result = _controller.Login("/apiary"); + + Assert.IsType(result); + } + + // Under a subpath deployment the /api ReturnUrl arrives base-prefixed ("/2/api/..."). The base is + // stripped before the guard so it is still rejected and never forwarded to CAS. Regression guard for + // the pre-strip /api check that a "/2/api/..." ReturnUrl slipped past. + [Theory] + [InlineData("/2/api/secret")] + [InlineData("/2/API/secret")] // base-prefixed + mixed case must not bypass the guard + [InlineData("~/2/api/secret")] // app-relative + base-prefixed must not bypass the guard either + public void Login_RejectsSubpathApiReturnUrl_WithUnauthorized(string returnUrl) + { + Arrange(authenticated: false); + _controller.HttpContext.Request.PathBase = "/2"; + + var result = _controller.Login(returnUrl); + + Assert.IsType(result); + } + + [Theory] + [InlineData("https://evil.com/phish")] + [InlineData("//evil.com")] + public void Login_DoesNotForwardNonLocalReturnUrl(string returnUrl) + { + Arrange(authenticated: false); + + var result = _controller.Login(returnUrl); + + var redirect = Assert.IsType(result); + Assert.DoesNotContain("evil.com", redirect.Url, StringComparison.OrdinalIgnoreCase); + } +} diff --git a/web/Classes/Utilities/WelcomePageHelper.cs b/web/Classes/Utilities/WelcomePageHelper.cs new file mode 100644 index 000000000..daa287559 --- /dev/null +++ b/web/Classes/Utilities/WelcomePageHelper.cs @@ -0,0 +1,102 @@ +using System.Collections.Frozen; +using System.Globalization; + +namespace Viper.Classes.Utilities; + +/// +/// Pure helpers for the unauthenticated Welcome (landing) page. +/// +public static class WelcomePageHelper +{ + private static readonly FrozenDictionary AreaLabels = + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["RAPS"] = "RAPS", + ["Effort"] = "Effort Reporting", + ["ClinicalScheduler"] = "Clinical Scheduler", + ["CTS"] = "Competency Tracking System", + ["Directory"] = "Directory", + ["CMS"] = "CMS", + }.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); + + /// + /// Resolve a human-readable destination label for the welcome page's deep-link indicator. + /// Returns null for missing, root-only, or non-local URLs. The check mirrors Url.IsLocalUrl semantics + /// so the helper is safe to call directly without prior validation. + /// + public static string? ResolveDestinationLabel(string? returnUrl) + { + if (!IsLocalUrl(returnUrl)) + { + return null; + } + + // After IsLocalUrl, returnUrl is non-null and non-empty. + var path = returnUrl!; + + int queryIndex = path.IndexOfAny(['?', '#']); + if (queryIndex >= 0) + { + path = path[..queryIndex]; + } + + // Strip a leading ~ (tilde-rooted app path) as well as /, so ~/Area resolves like /Area. + path = path.TrimStart('~', '/'); + if (path.Length == 0) + { + return null; + } + + var segments = path.Split('/', StringSplitOptions.RemoveEmptyEntries); + if (segments.Length == 0) + { + return null; + } + + if (AreaLabels.TryGetValue(segments[0], out var label)) + { + return label; + } + + return ToTitleCase(segments[^1]); + } + + /// + /// Defensive local-URL check matching Url.IsLocalUrl semantics. Rejects null/empty, absolute URLs + /// with a scheme, scheme-relative URLs (//host), and anything not beginning with a single '/'. + /// + private static bool IsLocalUrl(string? url) + { + if (string.IsNullOrEmpty(url)) + { + return false; + } + + if (url[0] == '/') + { + // Reject scheme-relative URLs like //evil.com/x and //\evil.com + if (url.Length == 1) + { + return true; + } + return url[1] != '/' && url[1] != '\\'; + } + + if (url[0] == '~' && url.Length > 1 && url[1] == '/') + { + // Reject ~// and ~/\ for parity with Url.IsLocalUrl semantics + if (url.Length == 2) + { + return true; + } + return url[2] != '/' && url[2] != '\\'; + } + + return false; + } + + private static string ToTitleCase(string value) + { + return char.ToUpper(value[0], CultureInfo.InvariantCulture) + value[1..]; + } +} diff --git a/web/Controllers/HomeController.cs b/web/Controllers/HomeController.cs index 65769ae8e..78b30c675 100644 --- a/web/Controllers/HomeController.cs +++ b/web/Controllers/HomeController.cs @@ -10,7 +10,9 @@ using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Options; using Viper.Areas.CMS.Data; @@ -34,8 +36,9 @@ public class HomeController : AreaController private readonly CasSettings _settings; private readonly List _casAttributesToCapture = new() { "authenticationDate", "credentialType" }; private readonly IUserHelper _userHelper; + private readonly IActionDescriptorCollectionProvider _actionDescriptorProvider; - public HomeController(IHttpClientFactory clientFactory, IOptions settingsOptions, AAUDContext aAUDContext, RAPSContext rapsContext, VIPERContext viperContext) + public HomeController(IHttpClientFactory clientFactory, IOptions settingsOptions, AAUDContext aAUDContext, RAPSContext rapsContext, VIPERContext viperContext, IActionDescriptorCollectionProvider actionDescriptorProvider) { this._clientFactory = clientFactory; this._settings = settingsOptions.Value; @@ -43,6 +46,7 @@ public HomeController(IHttpClientFactory clientFactory, IOptions se this._rapsContext = rapsContext; this._viperContext = viperContext; this._userHelper = new UserHelper(); + this._actionDescriptorProvider = actionDescriptorProvider; } /// /// VIPER 2 home page @@ -52,9 +56,217 @@ public HomeController(IHttpClientFactory clientFactory, IOptions se [SearchName(FriendlyName = "Viper 2 Homepage")] public IActionResult Index() { + if (User.Identity?.IsAuthenticated != true) + { + // Anonymous splash served in-place at "/": mirror /welcome's + // [ResponseCache(NoStore, Location=None)] so it isn't cached. The + // authenticated home response below keeps its default caching. + Response.Headers["Cache-Control"] = "no-store,no-cache"; + Response.Headers["Pragma"] = "no-cache"; + + ViewData["ReturnUrl"] = null; + ViewData["Hero"] = PickRandomHeroKey(); + ViewData["DestinationLabel"] = null; + return View("Welcome"); + } return View(); } + /// + /// Unauthenticated landing/splash page. Anonymous users see the welcome splash; + /// authenticated users are redirected to the validated ReturnUrl or "/". + /// + [Route("/[action]")] + [AllowAnonymous] + [SearchExclude] + [ResponseCache(NoStore = true, Location = ResponseCacheLocation.None)] +#pragma warning disable S6967 // Action only reads ReturnUrl, no model binding required + public IActionResult Welcome([FromQuery] string? ReturnUrl = null) +#pragma warning restore S6967 + { + // Normalize "~/..." to "/..." (mirrors Login) so the loop-guard catches + // ~/welcome and ~/login and we never emit a "~/" redirect target. + ReturnUrl = NormalizeAppRelativeUrl(ReturnUrl); + + // In a subpath deployment the ReturnUrl carries the PathBase (e.g. "/2/ClinicalScheduler"), + // so strip it once here — the loop-guard, classifier, and label resolver all need it + // root-relative. The full ReturnUrl is preserved for the redirect/links back. + var relativeReturnUrl = StripPathBase(ReturnUrl, Request.PathBase.Value); + + // Null out ReturnUrl if it is not local, or if it points back to /welcome or /login (would redirect-loop). + if (!Url.IsLocalUrl(ReturnUrl) || IsWelcomeOrLoginPath(relativeReturnUrl)) + { + ReturnUrl = null; + relativeReturnUrl = null; + } + + if (User.Identity?.IsAuthenticated == true) + { + // "~/" (not "/") so the app root keeps the PathBase ("/2/") in a subpath deployment + // instead of redirecting out to the domain root (the legacy site). + return LocalRedirect(string.IsNullOrEmpty(ReturnUrl) ? "~/" : ReturnUrl); + } + + // Only passive arrivals get the splash: the bare site root or a top-level area + // landing page (e.g. "/ClinicalScheduler"). A deep link (e.g. "/ClinicalScheduler/rotation") + // skips the interstitial and goes straight to CAS so we don't interrupt a targeted workflow. + if (!IsSplashTarget(relativeReturnUrl, GetAreaNames(_actionDescriptorProvider))) + { + return RedirectToAction(nameof(Login), new { ReturnUrl }); + } + + ViewData["ReturnUrl"] = ReturnUrl; + ViewData["Hero"] = PickRandomHeroKey(); + ViewData["DestinationLabel"] = WelcomePageHelper.ResolveDestinationLabel(relativeReturnUrl); + + return View("Welcome"); + } + + private static readonly string[] _heroKeys = + { + "svm_building", + "vetmed_admin", + "ophthalmology", + "guinea_pig", + "horse_foal", + }; + + private static string PickRandomHeroKey() + { + return _heroKeys[Random.Shared.Next(_heroKeys.Length)]; + } + + // Url.IsLocalUrl accepts app-relative "~/..." URLs, but browsers and CAS don't + // understand the "~", so normalize "~/..." to "/..." before validating or + // redirecting. Leaves all other values (including null) unchanged. + private static string? NormalizeAppRelativeUrl(string? returnUrl) + => returnUrl != null && returnUrl.StartsWith("~/") ? returnUrl[1..] : returnUrl; + + // internal (not private) so the redirect-loop guard is unit-testable via InternalsVisibleTo. + internal static bool IsWelcomeOrLoginPath(string? url) + { + if (string.IsNullOrEmpty(url)) + { + return false; + } + + int cut = url.IndexOfAny(['?', '#']); + var path = cut >= 0 ? url[..cut] : url; + path = path.TrimEnd('/'); + + return path.Equals("/welcome", StringComparison.OrdinalIgnoreCase) + || path.Equals("/login", StringComparison.OrdinalIgnoreCase); + } + + // Controllers under web/Areas live in the "Viper.Areas..…" namespace. Deriving the area + // set from controller namespaces (rather than the [Area] route value) covers every area — + // including SPA areas whose controllers are API-only and carry no [Area] attribute — and needs + // no hand-maintained list: add an area the usual way and it is picked up automatically. + private const string AreaNamespacePrefix = "Viper.Areas."; + + // Cached per descriptor collection: the collection is immutable and replaced wholesale + // (new instance) only when endpoints change, so the area set is derived once instead of + // per anonymous /welcome request. Benign race: concurrent first requests may each compute + // the set; last writer wins with an identical result. + private static (ActionDescriptorCollection Source, HashSet Areas)? _areaNamesCache; + + // The set of top-level area names (e.g. "Effort", "ClinicalScheduler"). Used to tell an area + // landing page ("/Effort" → splash) apart from a deep link ("/Effort/Reports" → CAS). + private static HashSet GetAreaNames(IActionDescriptorCollectionProvider actionDescriptorProvider) + { + var descriptors = actionDescriptorProvider.ActionDescriptors; + var cache = _areaNamesCache; + if (cache == null || !ReferenceEquals(cache.Value.Source, descriptors)) + { + var areas = descriptors.Items + .OfType() + .Select(d => AreaFromControllerNamespace(d.ControllerTypeInfo.Namespace)) + .Where(area => area != null) + .Select(area => area!) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + cache = (descriptors, areas); + _areaNamesCache = cache; + } + + return cache.Value.Areas; + } + + // Extracts the area segment from a controller namespace, e.g. "Viper.Areas.Effort.Controllers" + // → "Effort". Returns null for non-area namespaces. internal so it is unit-testable. + internal static string? AreaFromControllerNamespace(string? ns) + { + if (ns == null || !ns.StartsWith(AreaNamespacePrefix, StringComparison.Ordinal)) + { + return null; + } + + var rest = ns[AreaNamespacePrefix.Length..]; + int dot = rest.IndexOf('.'); + var area = dot >= 0 ? rest[..dot] : rest; + return area.Length == 0 ? null : area; + } + + // The welcome splash is reserved for passive arrivals: the bare site root or a top-level + // area landing page (a single path segment matching a registered area). Anything deeper is + // a deep link that should bypass the interstitial. Null/empty ReturnUrl is the front door. + // internal (not private) so the classifier is unit-testable via InternalsVisibleTo. + internal static bool IsSplashTarget(string? url, ISet areaNames) + { + if (string.IsNullOrEmpty(url)) + { + return true; + } + + int cut = url.IndexOfAny(['?', '#']); + var path = (cut >= 0 ? url[..cut] : url).Trim('/'); + + if (path.Length == 0) + { + return true; + } + + if (path.Contains('/')) + { + return false; + } + + return areaNames.Contains(path); + } + + // Routing is case-insensitive, so the /api guard must be too; matching on a segment + // boundary keeps non-API paths that merely start with "api" (e.g. "/apiary") out of + // the guard. internal (not private) so it is unit-testable via InternalsVisibleTo. + internal static bool IsApiPath(string url) + { + if (!url.StartsWith("/api", StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + return url.Length == 4 || url[4] is '/' or '?' or '#'; + } + + // Removes the application's PathBase prefix (e.g. "/2" in a subpath deployment) from a return + // URL so the splash classifier and label resolver can treat it as root-relative. Matches on a + // segment boundary so "/2" never strips from an unrelated "/22/...". Returns the URL unchanged + // when there is no base to strip (e.g. local dev, where PathBase is empty). + // internal (not private) so it is unit-testable via InternalsVisibleTo. + internal static string? StripPathBase(string? url, string? pathBase) + { + if (string.IsNullOrEmpty(url) || string.IsNullOrEmpty(pathBase)) + { + return url; + } + + if (url.StartsWith(pathBase, StringComparison.OrdinalIgnoreCase) + && (url.Length == pathBase.Length || url[pathBase.Length] is '/' or '?' or '#')) + { + return url[pathBase.Length..]; + } + + return url; + } + [Route("/[action]/")] [Authorize(Policy = "2faAuthentication")] [Permission(Allow = "SVMSecure")] @@ -90,6 +302,16 @@ private NavMenu Nav() [SearchExclude] public IActionResult Login([FromQuery] string? ReturnUrl = null) { + // Normalize app-relative "~/..." to "/..." before validating, so the + // /api guard below cannot be bypassed and we never forward an invalid + // browser URL to CAS. + ReturnUrl = NormalizeAppRelativeUrl(ReturnUrl); + + if (!Url.IsLocalUrl(ReturnUrl)) + { + ReturnUrl = null; + } + Uri url = new(Request.GetDisplayUrl()); string baseURl = url.GetLeftPart(UriPartial.Authority); string returnURL = HttpHelper.GetRootURL().Replace(baseURl, ""); @@ -99,7 +321,11 @@ public IActionResult Login([FromQuery] string? ReturnUrl = null) returnURL = ReturnUrl; } - if (returnURL.StartsWith("/api")) + // Strip the PathBase (e.g. "/2") before the /api guard so a base-prefixed + // "/2/api/..." ReturnUrl can't slip past this root-relative check and get + // forwarded to CAS. + var apiCheckUrl = StripPathBase(returnURL, Request.PathBase.Value); + if (apiCheckUrl != null && IsApiPath(apiCheckUrl)) { return Unauthorized(); } @@ -335,7 +561,13 @@ private async Task AuthenticateCasLogin(string? ticket, string? r var user = new ClaimsPrincipal(claimsIdentity); await HttpContext.SignInAsync(CookieAuthenticationDefaults.AuthenticationScheme, user); - return new LocalRedirectResult(!String.IsNullOrWhiteSpace(returnUrl) ? returnUrl : "/"); + if (!Url.IsLocalUrl(returnUrl)) + { + returnUrl = null; + } + + // "~/" (not "/") so a subpath deployment ("/2") lands on the app root, not the domain root. + return new LocalRedirectResult(!String.IsNullOrWhiteSpace(returnUrl) ? returnUrl : "~/"); } } catch (TaskCanceledException ex) diff --git a/web/Program.cs b/web/Program.cs index 8f87dc1e7..90cd64b32 100644 --- a/web/Program.cs +++ b/web/Program.cs @@ -137,7 +137,7 @@ .AddCookie(options => { options.Cookie.Name = "VIPER.Authentication.UCD"; - options.LoginPath = new PathString("/login"); + options.LoginPath = new PathString("/welcome"); options.AccessDeniedPath = new PathString("/Error/403"); options.ExpireTimeSpan = TimeSpan.FromHours(12); }); @@ -352,8 +352,7 @@ void RegisterDbContext(string connectionStringKey) where TContext : Db // Allow fonts to be downloaded from: csp.AllowFonts - .FromSelf()// This domain - .From("fonts.gstatic.com"); + .FromSelf(); // Self-hosted under /fonts - no external font CDN // Allow other sites to put this in an iframe? csp.AllowFraming @@ -377,7 +376,6 @@ void RegisterDbContext(string connectionStringKey) where TContext : Db // Allow styles csp.AllowStyles .FromSelf() // This domain - .From("fonts.googleapis.com") // Google Fonts stylesheets .AllowUnsafeInline(); // Allows inline CSS })); @@ -432,6 +430,18 @@ void RegisterDbContext(string connectionStringKey) where TContext : Db RedirectToAppendTrailingSlash = true }); + // Login screen fonts, served with long-lived cache headers. + app.UseStaticFiles(new StaticFileOptions + { + FileProvider = new PhysicalFileProvider( + Path.Join(builder.Environment.WebRootPath, "fonts")), + RequestPath = "/fonts", + OnPrepareResponse = ctx => + { + ctx.Context.Response.Headers["Cache-Control"] = "public, max-age=31536000, immutable"; // 1 year + } + }); + // General static files (favicon, /css, /js, /images, etc.). app.UseStaticFiles(); diff --git a/web/Views/Home/Welcome.cshtml b/web/Views/Home/Welcome.cshtml new file mode 100644 index 000000000..3d9c09fe9 --- /dev/null +++ b/web/Views/Home/Welcome.cshtml @@ -0,0 +1,97 @@ +@{ + Layout = null; + // Controller has already validated ReturnUrl with Url.IsLocalUrl and nulled it if unsafe. + var returnUrl = ViewData["ReturnUrl"] as string; + var loginUrl = string.IsNullOrEmpty(returnUrl) + ? Url.Content("~/login") + : $"{Url.Content("~/login")}?ReturnUrl={Uri.EscapeDataString(returnUrl)}"; + var hero = ViewData["Hero"] as string ?? "ophthalmology"; + var destinationLabel = ViewData["DestinationLabel"] as string; + var year = DateTime.Now.Year; + // Preload the AVIF that the CSS image-set() picks on capable browsers (must + // match that URL byte-for-byte to be reused). The type hint below lets + // browsers without AVIF skip the preload and fall back to the JPEG. + var heroImageUrl = Url.Content($"~/images/login/photo-{hero.Replace("_", "-")}.avif"); +} + + + + + + Sign in - VIPER 2.0 + + + + +
+
+ +
+ +
+ +
+
+
+

VIPER 2.0

+ +

+ The SVM portal for students, staff, and faculty. +

+
+ +
+
+

Welcome

+ @if (!string.IsNullOrEmpty(destinationLabel)) + { +

+ You'll be taken to @destinationLabel after signing in. +

+ } + + Sign in + + + + Need help signing in? + (opens in new window) + +
+
+
+ + +
+ + diff --git a/web/Views/Shared/Components/ProfilePic/Default.cshtml b/web/Views/Shared/Components/ProfilePic/Default.cshtml index ddabbefc2..b57915ebd 100644 --- a/web/Views/Shared/Components/ProfilePic/Default.cshtml +++ b/web/Views/Shared/Components/ProfilePic/Default.cshtml @@ -1,4 +1,4 @@ -@model Viper.Models.AAUD.AaudUser +@model Viper.Models.AAUD.AaudUser @if (Model != null) { IUserHelper UserHelper = new UserHelper(); @@ -45,7 +45,7 @@ } else { - + Login diff --git a/web/Views/Shared/_VIPERLayout.cshtml b/web/Views/Shared/_VIPERLayout.cshtml index 805e6f629..1304d75e0 100644 --- a/web/Views/Shared/_VIPERLayout.cshtml +++ b/web/Views/Shared/_VIPERLayout.cshtml @@ -25,7 +25,6 @@ } -