diff --git a/VueApp/src/components/SessionTimeout.vue b/VueApp/src/components/SessionTimeout.vue index ddfa77c92..ec4957903 100644 --- a/VueApp/src/components/SessionTimeout.vue +++ b/VueApp/src/components/SessionTimeout.vue @@ -2,82 +2,81 @@ import { ref } from "vue" import { useUserStore } from "@/store/UserStore" import LoginButton from "@/components/LoginButton.vue" +import StatusBanner from "@/components/StatusBanner.vue" const userStore = useUserStore() -//https://" + HttpHelper.HttpContext?.Request.Host.Value -const onDev = import.meta.env.VITE_ENVIRONMENT === "DEVELOPMENT" const viperHome = import.meta.env.VITE_VIPER_HOME -const sessionRefreshUrl = - (onDev ? "http://localhost/" : "/") + - "public/timeout/seconds_until_timeout_v2.cfm?id=" + - userStore.userInfo.loginId + - "&service=" + - (onDev ? "Viper2-dev" : "Viper2") +const sessionTimeoutUrl = `${import.meta.env.VITE_API_URL}sessionTimeout` const showSessionTimeoutWarning = ref(false) const sessionExpireTime = ref("") const sessionExpired = ref(false) let sessionTimeoutCheckEventId = 0 const sessionReloaded = ref(false) +const sessionExtendFailed = ref(false) +function formatExpireTime(sessionTimeoutDateTime: string) { + return new Date(sessionTimeoutDateTime).toLocaleTimeString("en-US", { hour: "numeric", minute: "2-digit" }) +} + +// Plain fetch, not useFetch: that reports every failure to the global error store and fires the +// auth handler, which would spam a silent poll and fight our own dialog's Log in. async function checkSessionTimeout() { - if ( - userStore.userInfo.loginId === undefined || - userStore.userInfo.loginId === null || - userStore.userInfo.loginId.length === 0 - ) { + // One live timer only: a refresh that overlaps an in-flight poll must not leave two chains running. + clearTimeout(sessionTimeoutCheckEventId) + if (!userStore.userInfo.loginId) { return //don't check the session if the user is not logged in } - //try to get the session timeout from an external application - try { - fetch(sessionRefreshUrl) - .then((r) => (r.status === 200 ? r.json() : r)) - .then((r) => { - let nextCheck = 300 - //show timeout warning if the session will time out in 5 minutes or less - if (r.secondsUntilTimeout !== undefined && r.secondsUntilTimeout < 300) { - showSessionTimeoutWarning.value = true - sessionExpired.value = r.secondsUntilTimeout < 15 //consider session timing out in 15 seconds to be timed out already - var d = new Date(r.sessionTimeoutDateTime) - sessionExpireTime.value = - (d.getHours() > 12 ? d.getHours() - 12 : d.getHours()) + - ":" + - ("0" + d.getMinutes()).slice(-2) + - (d.getHours() >= 12 ? " PM" : " AM") - nextCheck = sessionExpired.value ? 0 : Math.max(r.secondsUntilTimeout - 15, 5) - } - if (nextCheck > 0) { - sessionTimeoutCheckEventId = window.setTimeout(checkSessionTimeout, nextCheck * 1000) - } - }) - } catch (e) { - void e - } -} -async function extendSession() { - fetch(viperHome + "RefreshSession") - .then((r) => (r.status === 200 ? r.json() : r)) + // Timeout so a request that hangs rather than fails still reaches the catch below. + fetch(sessionTimeoutUrl, { signal: AbortSignal.timeout(10000) }) + .then((r) => (r.ok ? r.json() : Promise.reject(new Error("Session check returned " + r.status)))) .then((r) => { - try { - clearTimeout(sessionTimeoutCheckEventId) - } catch (e) { - void e + let nextCheck = 300 + //show timeout warning if the session will time out in 5 minutes or less + // "<=" so an exact 300 still warns: otherwise the next poll lands at expiry. + if (r.secondsUntilTimeout !== undefined && r.secondsUntilTimeout <= 300) { + showSessionTimeoutWarning.value = true + sessionExpired.value = r.secondsUntilTimeout < 15 //consider session timing out in 15 seconds to be timed out already + sessionExpireTime.value = formatExpireTime(r.sessionTimeoutDateTime) + nextCheck = sessionExpired.value ? 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. + hideSessionTimeoutWarning() + } + if (nextCheck > 0) { + sessionTimeoutCheckEventId = window.setTimeout(checkSessionTimeout, nextCheck * 1000) } + }) + // Silent, but reschedule: one failed poll must not stop the checks for the life of the page. + // Retry quickly once the warning is up, since expiry is minutes away. + .catch(() => { + const retry = showSessionTimeoutWarning.value ? 15000 : 300000 + sessionTimeoutCheckEventId = window.setTimeout(checkSessionTimeout, retry) + }) +} - var d = new Date(r.sessionTimeoutDateTime) - sessionExpireTime.value = - (d.getHours() > 12 ? d.getHours() - 12 : d.getHours()) + - ":" + - ("0" + d.getMinutes()).slice(-2) + - (d.getHours() >= 12 ? " PM" : " AM") +async function extendSession() { + sessionExtendFailed.value = false + fetch(viperHome + "RefreshSession", { signal: AbortSignal.timeout(10000) }) + .then((r) => (r.ok ? r.json() : Promise.reject(new Error("RefreshSession returned " + r.status)))) + .then((r) => { + clearTimeout(sessionTimeoutCheckEventId) + sessionExpireTime.value = formatExpireTime(r.sessionTimeoutDateTime) sessionReloaded.value = true sessionTimeoutCheckEventId = window.setTimeout(checkSessionTimeout, 5000) window.setTimeout(hideSessionTimeoutWarning, 1000) }) + // Leave the dialog up so the user can retry or log in, and say so: the session was not extended. + .catch(() => { + sessionExtendFailed.value = true + }) } + function hideSessionTimeoutWarning() { showSessionTimeoutWarning.value = false + sessionExpired.value = false sessionReloaded.value = false + sessionExtendFailed.value = false } sessionTimeoutCheckEventId = window.setTimeout(checkSessionTimeout, 60000) @@ -106,7 +105,7 @@ sessionTimeoutCheckEventId = window.setTimeout(checkSessionTimeout, 60000)
Your session has been extended to {{ sessionExpireTime }}.
@@ -129,6 +128,12 @@ sessionTimeoutCheckEventId = window.setTimeout(checkSessionTimeout, 60000) @click="hideSessionTimeoutWarning" > + + Could not extend your session. Please try again. + 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..33ed8b7ce --- /dev/null +++ b/web/Controllers/SessionTimeoutController.cs @@ -0,0 +1,65 @@ +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.Data.SqlClient; +using NLog; +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 + { + // Ten minutes rather than zero, so a database blip cannot strand the user behind a warning they cannot dismiss. + private const int SecondsOnError = 600; + + private static readonly Logger Logger = LogManager.GetCurrentClassLogger(); + + 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() + { + try + { + return SessionTimeoutService.GetStatus(_viperContext); + } + catch (Exception ex) when (ex is SqlException or InvalidOperationException) + { + Logger.Error(ex, "Could not read session timeout"); + return new SessionTimeoutStatus(DateTime.Now.AddSeconds(SecondsOnError), SecondsOnError); + } + } + } +} 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 ddaf014eb..073520734 100644 --- a/web/Views/Shared/Components/SessionTimeout/Default.cshtml +++ b/web/Views/Shared/Components/SessionTimeout/Default.cshtml @@ -14,70 +14,85 @@ Your session has been extended to {{sessionExpireTime}}. - + + + + Could not extend your session. Please try again. + +