diff --git a/test/Classes/AuthorizationFallbackTests.cs b/test/Classes/AuthorizationFallbackTests.cs index 16872a2dc..bba663916 100644 --- a/test/Classes/AuthorizationFallbackTests.cs +++ b/test/Classes/AuthorizationFallbackTests.cs @@ -17,7 +17,7 @@ public class AuthorizationFallbackTests // VIPER after signing out of Entra, with nothing but a 401 in the logs to show for it. public static TheoryData AnonymousControllers() => new(typeof(CMSController), typeof(LayoutController), typeof(LoggedInUserController), - typeof(EntraLogoutController)); + typeof(EntraLogoutController), typeof(SessionTimeoutController)); [Theory] [MemberData(nameof(AnonymousControllers))] diff --git a/test/Classes/SessionTimeoutServiceTests.cs b/test/Classes/SessionTimeoutServiceTests.cs new file mode 100644 index 000000000..ed555049f --- /dev/null +++ b/test/Classes/SessionTimeoutServiceTests.cs @@ -0,0 +1,36 @@ +using System.Globalization; +using Viper.Classes.Utilities; +using Viper.Models.VIPER; + +namespace Viper.test.Classes +{ + public class SessionTimeoutServiceTests + { + [Fact] + public void ToStatus_StoredRow_ReportsThatExpiryWithLocalOffset() + { + DateTime expiry = DateTime.Now.AddMinutes(10); + + var status = SessionTimeoutService.ToStatus(new SessionTimeout { SessionTimeoutDateTime = expiry }, authenticated: true); + + Assert.InRange(status.SecondsUntilTimeout, 590, 600); + var parsed = DateTimeOffset.ParseExact(status.SessionTimeoutDateTime, "yyyy-MM-dd'T'HH:mm:sszzz", CultureInfo.InvariantCulture); + Assert.Equal(expiry.ToString("s"), parsed.DateTime.ToString("s")); + Assert.Equal(TimeZoneInfo.Local.GetUtcOffset(expiry), parsed.Offset); + } + + [Fact] + public void ToStatus_AuthenticatedWithoutRow_GrantsFullWindow() + { + var status = SessionTimeoutService.ToStatus(null, authenticated: true); + + Assert.Equal(SessionTimeoutService.SessionTimeoutSeconds, status.SecondsUntilTimeout); + } + + [Fact] + public void ToStatus_Anonymous_ReportsExpired() + { + Assert.Equal(0, SessionTimeoutService.ToStatus(null, authenticated: false).SecondsUntilTimeout); + } + } +} diff --git a/test/Controllers/SessionTimeoutControllerTests.cs b/test/Controllers/SessionTimeoutControllerTests.cs new file mode 100644 index 000000000..2ea7899d2 --- /dev/null +++ b/test/Controllers/SessionTimeoutControllerTests.cs @@ -0,0 +1,57 @@ +using System.Reflection; +using System.Security.Claims; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Controllers; +using Viper.Classes; +using Viper.Controllers; + +namespace Viper.test.Controllers +{ + /// + /// The session expiry poll must never extend the session, or it would never time out. Both + /// controller bases in this app write a fresh expiry on every action, so the guarantee rests + /// entirely on this controller not inheriting either of them. Pin that. + /// + public class SessionTimeoutControllerTests + { + [Fact] + public void Controller_DoesNotInheritASessionExtendingBase() + { + Assert.False(typeof(ApiController).IsAssignableFrom(typeof(SessionTimeoutController))); + Assert.False(typeof(AreaController).IsAssignableFrom(typeof(SessionTimeoutController))); + } + + [Fact] + public void Controller_DoesNotCarryTheSessionUpdateFilter() + { + Assert.Empty(typeof(SessionTimeoutController) + .GetCustomAttributes(typeof(ApiSessionUpdateFilterAttribute), inherit: true)); + + foreach (MethodInfo action in typeof(SessionTimeoutController).GetMethods(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly)) + { + Assert.Empty(action.GetCustomAttributes(typeof(ApiSessionUpdateFilterAttribute), inherit: true)); + } + } + + [Theory] + [InlineData(typeof(SessionTimeoutController), false)] + [InlineData(typeof(HomeController), true)] + public async Task DoNotSlideCookie_RenewsOnlyForOtherEndpoints(Type controller, bool expectedRenew) + { + var httpContext = new DefaultHttpContext(); + var descriptor = new ControllerActionDescriptor { ControllerTypeInfo = controller.GetTypeInfo() }; + httpContext.SetEndpoint(new Endpoint(null, new EndpointMetadataCollection(descriptor), controller.Name)); + var ticket = new AuthenticationTicket(new ClaimsPrincipal(), CookieAuthenticationDefaults.AuthenticationScheme); + var context = new CookieSlidingExpirationContext(httpContext, + new AuthenticationScheme(CookieAuthenticationDefaults.AuthenticationScheme, null, typeof(CookieAuthenticationHandler)), + new CookieAuthenticationOptions(), ticket, TimeSpan.FromHours(7), TimeSpan.FromHours(5)) + { ShouldRenew = true }; + + await SessionTimeoutController.DoNotSlideCookie(context); + + Assert.Equal(expectedRenew, context.ShouldRenew); + } + } +} diff --git a/web/Classes/Utilities/SessionTimeoutService.cs b/web/Classes/Utilities/SessionTimeoutService.cs index 6bcff46ae..90df09a24 100644 --- a/web/Classes/Utilities/SessionTimeoutService.cs +++ b/web/Classes/Utilities/SessionTimeoutService.cs @@ -1,5 +1,7 @@ +using Microsoft.EntityFrameworkCore; using NLog; using Viper.Classes.SQLContext; +using Viper.Models; using Viper.Models.AAUD; using Viper.Models.VIPER; @@ -7,7 +9,7 @@ namespace Viper.Classes.Utilities { public static class SessionTimeoutService { - private const int SessionTimeoutSeconds = (29 * 60) + 30; + internal const int SessionTimeoutSeconds = (29 * 60) + 30; public static void UpdateSessionTimeout(VIPERContext context) { @@ -45,19 +47,28 @@ public static void UpdateSessionTimeout(VIPERContext context) } } - public static SessionTimeout? GetSessionTimeout(VIPERContext context) + public static SessionTimeoutStatus GetStatus(VIPERContext context) { string loggedInUserId = GetLoggedInUserId(); string service = GetService(); - if (!string.IsNullOrEmpty(loggedInUserId) && context != null) + SessionTimeout? record = string.IsNullOrEmpty(loggedInUserId) + ? null + : context.SessionTimeouts.AsNoTracking() + .FirstOrDefault(s => s.LoginId == loggedInUserId && s.Service == service); + return ToStatus(record, HttpHelper.HttpContext?.User.Identity?.IsAuthenticated == true); + } + + // Only AreaController, ApiSessionUpdateFilter and RefreshSession write the row, so a page served by a + // plain Controller leaves a valid session with no row: grant a full window rather than report it expired. + internal static SessionTimeoutStatus ToStatus(SessionTimeout? record, bool authenticated) + { + if (record != null) { - SessionTimeout? record = context.SessionTimeouts.Find(loggedInUserId, service); - if (record != null) - { - return record; - } + return new(record.SessionTimeoutDateTime, (int)(record.SessionTimeoutDateTime - DateTime.Now).TotalSeconds); } - return null; + return authenticated + ? new(DateTime.Now.AddSeconds(SessionTimeoutSeconds), SessionTimeoutSeconds) + : new(DateTime.Now, 0); } private static string GetService() diff --git a/web/Controllers/HomeController.cs b/web/Controllers/HomeController.cs index a8766de73..db8b8b1b4 100644 --- a/web/Controllers/HomeController.cs +++ b/web/Controllers/HomeController.cs @@ -254,7 +254,7 @@ private bool IsUnderPathBase(string? url) public IActionResult RefreshSession() { SessionTimeoutService.UpdateSessionTimeout(_viperContext); - return Ok(SessionTimeoutService.GetSessionTimeout(_viperContext)); + return Ok(SessionTimeoutService.GetStatus(_viperContext)); } /// diff --git a/web/Controllers/SessionTimeoutController.cs b/web/Controllers/SessionTimeoutController.cs new file mode 100644 index 000000000..d35db4e8a --- /dev/null +++ b/web/Controllers/SessionTimeoutController.cs @@ -0,0 +1,52 @@ +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Controllers; +using Viper.Classes.SQLContext; +using Viper.Classes.Utilities; +using Viper.Models; + +namespace Viper.Controllers +{ + /// + /// Read-only session expiry poll for the session timeout dialog. The user comes from the auth cookie. + /// + /// + /// Inherits ControllerBase, not Viper.Classes.ApiController or AreaController: both extend the session on + /// every action. The [ApiController] attribute below is MVC's ApiControllerAttribute and does not. + /// [AllowAnonymous] so a caller whose cookie is gone gets 0 seconds and a Log in button, not a challenge redirect. + /// + [ApiController] + [AllowAnonymous] + [Route("/api/sessionTimeout")] + [ResponseCache(NoStore = true, Location = ResponseCacheLocation.None)] + public class SessionTimeoutController : ControllerBase + { + private readonly VIPERContext _viperContext; + + public SessionTimeoutController(VIPERContext viperContext) + { + _viperContext = viperContext; + } + + // A background read must not slide the auth cookie either, or a tab left open on a page with no + // session row would poll forever and keep the login alive past its 12 hours. + internal static Task DoNotSlideCookie(CookieSlidingExpirationContext context) + { + var action = context.HttpContext.GetEndpoint()?.Metadata.GetMetadata(); + if (action?.ControllerTypeInfo == typeof(SessionTimeoutController)) + { + context.ShouldRenew = false; + } + return Task.CompletedTask; + } + + [HttpGet] + public SessionTimeoutStatus GetSessionTimeout() + { + // No catch: a database failure becomes a 500, which the client treats as a failed poll and retries. + // A made-up expiry here would read as a real one and could hide a live warning. + return SessionTimeoutService.GetStatus(_viperContext); + } + } +} diff --git a/web/Models/SessionTimeoutCheck.cs b/web/Models/SessionTimeoutCheck.cs deleted file mode 100644 index aff937378..000000000 --- a/web/Models/SessionTimeoutCheck.cs +++ /dev/null @@ -1,9 +0,0 @@ -namespace Viper.Models -{ - public class SessionTimeoutCheck - { - public DateTime SessionTimeout { get; set; } - public int SecondsUntilTimeout { get; set; } - public string LoginId { get; set; } = null!; - } -} diff --git a/web/Models/SessionTimeoutStatus.cs b/web/Models/SessionTimeoutStatus.cs new file mode 100644 index 000000000..0d8885fb5 --- /dev/null +++ b/web/Models/SessionTimeoutStatus.cs @@ -0,0 +1,22 @@ +using System.Globalization; + +namespace Viper.Models +{ + /// + /// Session expiry contract polled by the session timeout dialog. + /// + public class SessionTimeoutStatus + { + public SessionTimeoutStatus(DateTime sessionTimeout, int secondsUntilTimeout) + { + // The column is a bare local datetime; carry the offset so a client in another timezone shows the right time. + DateTimeOffset local = new(DateTime.SpecifyKind(sessionTimeout, DateTimeKind.Local)); + SessionTimeoutDateTime = local.ToString("yyyy-MM-dd'T'HH:mm:sszzz", CultureInfo.InvariantCulture); + SecondsUntilTimeout = secondsUntilTimeout; + } + + public string SessionTimeoutDateTime { get; } + + public int SecondsUntilTimeout { get; } + } +} diff --git a/web/Program.cs b/web/Program.cs index d456dd4de..214abf152 100644 --- a/web/Program.cs +++ b/web/Program.cs @@ -45,6 +45,7 @@ using Viper.Classes.Scheduler; using Viper.Classes.SQLContext; using Viper.Classes.Utilities; +using Viper.Controllers; using Viper.EmailTemplates.Services; using Viper.Services; using Web; @@ -165,6 +166,8 @@ options.Events = new CookieAuthenticationEvents { + OnCheckSlidingExpiration = SessionTimeoutController.DoNotSlideCookie, + // Front-channel logout cannot clear this cookie itself: Entra frames the logout URL // on its own origin, and a SameSite=Lax cookie is neither sent nor accepted in that // context. So the sign-out is recorded server side and enforced here instead, on diff --git a/web/Views/Shared/Components/SessionTimeout/Default.cshtml b/web/Views/Shared/Components/SessionTimeout/Default.cshtml index 5e7fbe372..fdfc47887 100644 --- a/web/Views/Shared/Components/SessionTimeout/Default.cshtml +++ b/web/Views/Shared/Components/SessionTimeout/Default.cshtml @@ -30,7 +30,6 @@ createVueApp({ data() { return { - sessionRefreshUrl: '@Html.Raw(ViewData["sessionRefreshUrl"] ?? "")', showSessionTimeoutWarning: false, sessionExpireTime: 0, sessionExpired: false, @@ -45,7 +44,7 @@ // One live timer only: a refresh that overlaps an in-flight poll must not leave two chains running. clearTimeout(this.sessionTimeoutCheckEventId) // Timeout so a request that hangs rather than fails still reaches the catch below. - fetch(this.sessionRefreshUrl, { signal: AbortSignal.timeout(10000) }) + fetch('@Url.Content("~/api/sessionTimeout")', { signal: AbortSignal.timeout(10000) }) .then(r => r.ok ? r.json() : Promise.reject(new Error('Session check returned ' + r.status))) .then(r => { var nextCheck = 300 @@ -58,12 +57,13 @@ if (r.secondsUntilTimeout !== undefined && r.secondsUntilTimeout <= 300) { this.showSessionTimeoutWarning = true this.sessionExpired = r.secondsUntilTimeout < 15 - var d = new Date(r.sessionTimeoutDateTime) - this.sessionExpireTime = (d.getHours() > 12 ? d.getHours() - 12 : d.getHours()) + ":" - + ("0" + d.getMinutes()).slice(-2) - + (d.getHours() >= 12 ? " PM" : " AM") + this.sessionExpireTime = this.formatExpireTime(r.sessionTimeoutDateTime) nextCheck = this.sessionExpired ? 0 : Math.max(r.secondsUntilTimeout - 15, 5) } + else if (r.secondsUntilTimeout !== undefined) { + // Extended elsewhere, in another tab or by an API call, so stand the warning down. + this.hideSessionTimeoutWarning() + } if(nextCheck > 0) { this.sessionTimeoutCheckEventId = window.setTimeout(this.checkSessionTimeout, nextCheck * 1000) } @@ -84,10 +84,7 @@ .then(r => r.ok ? r.json() : Promise.reject(new Error('RefreshSession returned ' + r.status))) .then(r => { clearTimeout(this.sessionTimeoutCheckEventId) - var d = new Date(r.sessionTimeoutDateTime) - this.sessionExpireTime = (d.getHours() > 12 ? d.getHours() - 12 : d.getHours()) + ":" - + ("0" + d.getMinutes()).slice(-2) - + (d.getHours() >= 12 ? " PM" : " AM") + this.sessionExpireTime = this.formatExpireTime(r.sessionTimeoutDateTime) this.sessionReloaded = true this.sessionTimeoutCheckEventId = window.setTimeout(this.checkSessionTimeout, 5000) @@ -98,9 +95,13 @@ }, hideSessionTimeoutWarning: function() { this.showSessionTimeoutWarning = false + this.sessionExpired = false this.sessionReloaded = false this.sessionExtendFailed = false }, + formatExpireTime: function(sessionTimeoutDateTime) { + return new Date(sessionTimeoutDateTime).toLocaleTimeString('en-US', { hour: 'numeric', minute: '2-digit' }) + }, login: function() { var returnUrl = encodeURIComponent(window.location.pathname + window.location.search) window.location = '@Url.Content("~/login")?ReturnUrl=' + returnUrl diff --git a/web/Views/Shared/Components/SessionTimeout/SessionTimeout.cs b/web/Views/Shared/Components/SessionTimeout/SessionTimeout.cs index 2bf3579ba..3ac3f71b9 100644 --- a/web/Views/Shared/Components/SessionTimeout/SessionTimeout.cs +++ b/web/Views/Shared/Components/SessionTimeout/SessionTimeout.cs @@ -7,14 +7,11 @@ public class SessionTimeout : ViewComponent { public IViewComponentResult Invoke() { - UserHelper userHelper = new UserHelper(); - string? loginId = userHelper.GetCurrentUser()?.LoginId; - bool onDev = HttpHelper.Environment?.EnvironmentName == "Development"; - ViewData["sessionRefreshUrl"] = (onDev ? "http://localhost/" : ("https://" + HttpHelper.HttpContext?.Request.Host.Value + "/")) - + "/public/timeout/seconds_until_timeout_v2.cfm?id=" - + (loginId ?? "") - + "&service=" + (onDev ? "Viper2-dev" : "Viper2"); - return View("Default"); + // The layout renders this on public pages too. An anonymous visitor has no session, and + // the poll would report zero seconds left and tell them it had expired. + return UserClaimsPrincipal?.Identity?.IsAuthenticated == true + ? View("Default") + : Content(string.Empty); } }