diff --git a/.review-pr-ignored-304 b/.review-pr-ignored-304 new file mode 100644 index 000000000..e69de29bb diff --git a/test/Controllers/SessionTimeoutControllerTests.cs b/test/Controllers/SessionTimeoutControllerTests.cs new file mode 100644 index 000000000..989bf35c8 --- /dev/null +++ b/test/Controllers/SessionTimeoutControllerTests.cs @@ -0,0 +1,33 @@ +using System.Reflection; +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)); + } + } + } +} diff --git a/web/Classes/SessionTimeoutStatus.cs b/web/Classes/SessionTimeoutStatus.cs new file mode 100644 index 000000000..4390d4c48 --- /dev/null +++ b/web/Classes/SessionTimeoutStatus.cs @@ -0,0 +1,12 @@ +namespace Viper.Classes +{ + /// + /// Session expiry contract polled by the session timeout dialog. + /// + public class SessionTimeoutStatus + { + public string SessionTimeoutDateTime { get; set; } = string.Empty; + + public int SecondsUntilTimeout { get; set; } + } +} diff --git a/web/Classes/Utilities/SessionTimeoutService.cs b/web/Classes/Utilities/SessionTimeoutService.cs index 6bcff46ae..003fb7940 100644 --- a/web/Classes/Utilities/SessionTimeoutService.cs +++ b/web/Classes/Utilities/SessionTimeoutService.cs @@ -1,3 +1,4 @@ +using Microsoft.EntityFrameworkCore; using NLog; using Viper.Classes.SQLContext; using Viper.Models.AAUD; @@ -7,7 +8,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) { @@ -51,7 +52,8 @@ public static void UpdateSessionTimeout(VIPERContext context) string service = GetService(); if (!string.IsNullOrEmpty(loggedInUserId) && context != null) { - SessionTimeout? record = context.SessionTimeouts.Find(loggedInUserId, service); + SessionTimeout? record = context.SessionTimeouts.AsNoTracking() + .FirstOrDefault(s => s.LoginId == loggedInUserId && s.Service == service); if (record != null) { return record; diff --git a/web/Controllers/SessionTimeoutController.cs b/web/Controllers/SessionTimeoutController.cs new file mode 100644 index 000000000..241bcf1d5 --- /dev/null +++ b/web/Controllers/SessionTimeoutController.cs @@ -0,0 +1,94 @@ +using System.Globalization; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Data.SqlClient; +using NLog; +using Viper.Classes; +using Viper.Classes.SQLContext; +using Viper.Classes.Utilities; + +namespace Viper.Controllers +{ + /// + /// Read-only session expiry poll for the session timeout dialog. Takes no parameters: the user + /// comes from the auth cookie. + /// + /// + /// Inherits ControllerBase, not Viper.Classes.ApiController or Viper.Classes.AreaController. + /// Both of those extend the session on every action, which would stop the session ever expiring, + /// and ApiController also wraps responses in an ApiResponse envelope the dialog cannot read. The + /// [ApiController] attribute below is Microsoft.AspNetCore.Mvc.ApiControllerAttribute and + /// carries none of that behaviour. + /// + /// No [Authorize]: attribute-routed actions do not pick up RequireAuthorization from the + /// conventional routes, and the action takes no parameters and reads identity from the cookie, + /// so an anonymous caller learns only that it has no session. The dialog then offers a log in + /// rather than retrying a call that would 401. + /// + [ApiController] + [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 + // dialog 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; + } + + [HttpGet] + public ActionResult GetSessionTimeout() + { + try + { + Models.VIPER.SessionTimeout? record = SessionTimeoutService.GetSessionTimeout(_viperContext); + if (record != null) + { + return Status(record.SessionTimeoutDateTime, + (int)(record.SessionTimeoutDateTime - DateTime.Now).TotalSeconds); + } + + // Authenticated but untracked. 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. + return User.Identity?.IsAuthenticated == true + ? Status(DateTime.Now.AddSeconds(SessionTimeoutService.SessionTimeoutSeconds), + SessionTimeoutService.SessionTimeoutSeconds) + : Status(DateTime.Now, 0); + } + catch (SqlException ex) + { + return CouldNotRead(ex); + } + catch (InvalidOperationException ex) + { + return CouldNotRead(ex); + } + } + + private static SessionTimeoutStatus CouldNotRead(Exception ex) + { + Logger.Error(ex, "Could not read session timeout"); + return Status(DateTime.Now.AddSeconds(SecondsOnError), SecondsOnError); + } + + // Carry the offset. The column is a bare datetime written from DateTime.Now, so it is local + // wall-clock; without the offset a client in another timezone reads it as its own and shows + // the wrong expiry time. + private static SessionTimeoutStatus Status(DateTime sessionTimeout, int secondsUntilTimeout) + { + DateTimeOffset local = new(DateTime.SpecifyKind(sessionTimeout, DateTimeKind.Local)); + return new SessionTimeoutStatus + { + SessionTimeoutDateTime = local.ToString("yyyy-MM-dd'T'HH:mm:sszzz", CultureInfo.InvariantCulture), + SecondsUntilTimeout = secondsUntilTimeout + }; + } + } +} 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/Views/Shared/Components/SessionTimeout/Default.cshtml b/web/Views/Shared/Components/SessionTimeout/Default.cshtml index c23ce75c2..f1bbf9f7d 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, @@ -42,7 +41,7 @@ methods: { checkSessionTimeout: async function () { // 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 @@ -50,12 +49,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) } @@ -78,10 +78,7 @@ } catch(e) { void e } - 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) @@ -92,9 +89,17 @@ }, hideSessionTimeoutWarning: function() { this.showSessionTimeoutWarning = false + this.sessionExpired = false this.sessionReloaded = false this.sessionExtendFailed = false }, + // Hour 0 is 12 AM, not 0 AM. + formatExpireTime: function(sessionTimeoutDateTime) { + var d = new Date(sessionTimeoutDateTime) + return (d.getHours() % 12 || 12) + ":" + + ("0" + d.getMinutes()).slice(-2) + + (d.getHours() >= 12 ? " PM" : " AM") + }, 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); } }