dohertj2/scadalink-design
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(central-ui): resolve CentralUI-005 — sliding cookie session expiry (Security AddCookie + AuthEndpoints + SessionExpiry)

Browse Source
This commit is contained in:
Joseph Doherty
2026-05-16 23:54:31 -04:00
parent b1f4251d75
commit 1e2e7d2e7c
6 changed files with 240 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
code-reviews/CentralUI/findings.md
View File

@@ -8,7 +8,7 @@
| Last reviewed | 2026-05-16 |
| Reviewer | claude-agent |
| Commit reviewed | `9c60592` |
| Open findings | 2 |
| Open findings | 1 |
## Summary
@@ -248,7 +248,7 @@ Fixed by the commit whose message references `CentralUI-004`.
|--|--|
| Severity | Medium |
| Category | Design-document adherence |
| Status | Open |
| Status | Resolved |
| Location | `src/ScadaLink.CentralUI/Auth/AuthEndpoints.cs:47-81`; `src/ScadaLink.CentralUI/Components/Shared/SessionExpiry.razor:18-30` |
**Description**
@@ -269,17 +269,48 @@ fixed 30-minute model. The code and the documented decision must agree.
**Resolution**
_Unresolved — requires a cross-module change plus a design decision, both out of
scope for a CentralUI-only fix._ Verified 2026-05-16: the discrepancy is real.
The sliding-expiration mechanism, however, is owned by the cookie
authentication middleware configured in **`ScadaLink.Security`**
(`ServiceCollectionExtensions.AddCookie` — currently sets neither
`ExpireTimeSpan` nor `SlidingExpiration`); `AuthEndpoints` (CentralUI) only sets
the absolute `ExpiresUtc`/`expires_at`. Implementing "15-minute sliding token"
means editing `ScadaLink.Security`, which this module's review cannot touch, and
the alternative — amending the documented decision to a fixed 30-minute model —
is a design decision, not a code fix. Left Open and surfaced for a follow-up
that spans CentralUI + Security, or a design-doc amendment.
Resolved 2026-05-16 (commit `<pending>`) — cross-module fix (CentralUI +
Security), explicitly authorized. Root cause confirmed against the source:
`AddCookie` (`ScadaLink.Security/ServiceCollectionExtensions.cs`) set neither
`ExpireTimeSpan` nor `SlidingExpiration`; `AuthEndpoints` stamped a fixed
`expires_at = UtcNow + 30 min` claim and a 30-minute absolute cookie
`ExpiresUtc`; `SessionExpiry.razor` scheduled one hard redirect at that fixed
instant — a hard 30-minute cap, no sliding renewal, no 15-minute component.
**What was implemented — sliding session window.** ASP.NET Core cookie
authentication exposes a single `ExpireTimeSpan` plus a `SlidingExpiration`
flag; it cannot natively model *both* a 15-minute sliding token *and* a separate
30-minute absolute idle cap. The faithful interpretation implemented: the cookie
session window **is** the idle timeout. `AddSecurity` now post-configures the
cookie options with `ExpireTimeSpan = TimeSpan.FromMinutes(SecurityOptions.IdleTimeoutMinutes)`
(default 30, bound from `appsettings` via the existing options pattern, not
hard-coded) and `SlidingExpiration = true`. The middleware therefore re-issues
the cookie on activity once past the halfway mark of the window: an active user
is continually renewed, an idle user is signed out after the 30-minute idle
timeout — exactly the documented "sliding refresh, 30-minute idle timeout". The
separate 15-minute `JwtExpiryMinutes` governs the lifetime of the *embedded JWT*
itself (`JwtTokenService`) — a distinct layer from the cookie session window;
it is not, and per the ASP.NET cookie model cannot be, a second independent
sliding window inside the same cookie. `AuthEndpoints` no longer imposes a
contradictory absolute cap: the `expires_at` claim and the manual cookie
`ExpiresUtc` were removed, and a new `BuildSignInProperties()` helper sets only
`IsPersistent = true` (no `ExpiresUtc`, `AllowRefresh` left unset) so the
middleware owns expiry. `SessionExpiry.razor` no longer reads a fixed
login-time deadline (the `expires_at` claim is gone) and no longer hard-redirects
at a fixed instant: it now polls the authentication state on a recurring
interval and redirects to `/login` only once the sliding cookie has actually
lapsed server-side — so an active user is never logged out mid-session.
Regression tests fail against the pre-fix code and pass after. Security:
`AddSecurity_AuthCookie_UsesSlidingExpiration`,
`AddSecurity_AuthCookie_ExpireTimeSpanMatchesIdleTimeout` (pre-fix
`ExpireTimeSpan` was the 14-day default — confirmed failing), and
`AddSecurity_AuthCookie_ExpireTimeSpanIsConfigurable` (pins the options-pattern
binding). CentralUI: `SessionExpiryPolicyTests.BuildSignInProperties_DoesNotSetFixedAbsoluteExpiry`,
`..._IsPersistent`, `..._AllowsSlidingRefresh` pin that the login sign-in no
longer imposes a fixed absolute cap. `dotnet build ScadaLink.slnx` clean;
`tests/ScadaLink.Security.Tests` 57 passed, `tests/ScadaLink.CentralUI.Tests`
254 passed.
### CentralUI-006 — Deployment status page polls every 10s despite the documented SignalR-push design

34
src/ScadaLink.CentralUI/Auth/AuthEndpoints.cs
View File

@@ -44,15 +44,17 @@ public static class AuthEndpoints
// Map LDAP groups to roles
var roleMappingResult = await roleMapper.MapGroupsToRolesAsync(authResult.Groups ?? []);
var expiresAt = DateTimeOffset.UtcNow.AddMinutes(30);
// Build claims from LDAP auth + role mapping
// Build claims from LDAP auth + role mapping.
// CentralUI-005: no fixed "expires_at" absolute-cap claim is stamped
// — session expiry is owned by the cookie middleware's sliding window
// (ScadaLink.Security AddCookie: ExpireTimeSpan = idle timeout,
// SlidingExpiration = true). A frozen absolute claim would contradict
// the documented sliding-refresh policy.
var claims = new List<Claim>
{
new(ClaimTypes.Name, authResult.Username ?? username),
new(JwtTokenService.DisplayNameClaimType, authResult.DisplayName ?? username),
new(JwtTokenService.UsernameClaimType, authResult.Username ?? username),
new("expires_at", expiresAt.ToUnixTimeSeconds().ToString()),
};
foreach (var role in roleMappingResult.Roles)
@@ -74,11 +76,7 @@ public static class AuthEndpoints
await context.SignInAsync(
CookieAuthenticationDefaults.AuthenticationScheme,
principal,
new AuthenticationProperties
{
IsPersistent = true,
ExpiresUtc = expiresAt
});
BuildSignInProperties());
context.Response.Redirect("/");
}).DisableAntiforgery();
@@ -138,4 +136,22 @@ public static class AuthEndpoints
return endpoints;
}
/// <summary>
/// Builds the <see cref="AuthenticationProperties"/> for the login sign-in.
/// CentralUI-005: deliberately does <b>not</b> set <see cref="AuthenticationProperties.ExpiresUtc"/>.
/// Session expiry is owned by the cookie authentication middleware's sliding
/// window (configured in <c>ScadaLink.Security</c>'s <c>AddCookie</c>:
/// <c>ExpireTimeSpan</c> = the idle timeout, <c>SlidingExpiration = true</c>).
/// Setting a fixed <c>ExpiresUtc</c> here would re-impose a hard absolute cap
/// that overrides the sliding window and contradicts the documented
/// "sliding refresh, 30-minute idle timeout" policy. <see cref="AuthenticationProperties.IsPersistent"/>
/// is true so the cookie survives a browser restart within the idle window;
/// <see cref="AuthenticationProperties.AllowRefresh"/> is left unset (null)
/// so the middleware is free to slide the expiry on activity.
/// </summary>
public static AuthenticationProperties BuildSignInProperties() => new()
{
IsPersistent = true
};
}

65
src/ScadaLink.CentralUI/Components/Shared/SessionExpiry.razor
View File

@@ -3,37 +3,62 @@
@inject NavigationManager Navigation
@code {
// CentralUI-005: session expiry is a sliding window owned by the cookie
// authentication middleware (ScadaLink.Security AddCookie:
// ExpireTimeSpan = idle timeout, SlidingExpiration = true). An active user's
// cookie is continually renewed; an idle user's cookie lapses after the idle
// timeout. There is therefore no fixed login-time deadline to redirect at —
// the old code read an "expires_at" claim and scheduled a single hard
// redirect, which both contradicted the sliding policy and logged active
// users out mid-session.
//
// Instead this component polls the authentication state on a recurring
// interval. While the session is still valid it does nothing; once the
// sliding cookie has expired (the server-side idle cutoff has been reached)
// the next poll observes an unauthenticated principal and redirects to the
// login page. Re-checking the state is itself circuit activity, so this poll
// alone never keeps a truly idle session alive — only genuine user activity
// refreshes the cookie before it lapses.
/// <summary>How often the session validity is re-checked.</summary>
internal static readonly TimeSpan PollInterval = TimeSpan.FromMinutes(1);
private CancellationTokenSource? _cts;
protected override async Task OnInitializedAsync()
protected override void OnInitialized()
{
// The login page uses the same layout, so this component renders there
// too. Redirecting /login → /login would loop ("too many redirects").
// too. Polling/redirecting on /login → /login would loop.
var path = Navigation.ToBaseRelativePath(Navigation.Uri);
if (path.StartsWith("login", StringComparison.OrdinalIgnoreCase)) return;
var auth = await AuthStateProvider.GetAuthenticationStateAsync();
if (auth.User.Identity?.IsAuthenticated != true) return;
var claim = auth.User.FindFirst("expires_at")?.Value;
if (!long.TryParse(claim, out var unix)) return;
var remaining = DateTimeOffset.FromUnixTimeSeconds(unix) - DateTimeOffset.UtcNow;
if (remaining <= TimeSpan.Zero)
{
Navigation.NavigateTo("/login", forceLoad: true);
return;
}
_cts = new CancellationTokenSource();
_ = ScheduleRedirectAsync(remaining + TimeSpan.FromSeconds(2), _cts.Token);
_ = PollSessionAsync(_cts.Token);
}
private async Task ScheduleRedirectAsync(TimeSpan delay, CancellationToken token)
private async Task PollSessionAsync(CancellationToken token)
{
try { await Task.Delay(delay, token); }
catch (TaskCanceledException) { return; }
await InvokeAsync(() => Navigation.NavigateTo("/login", forceLoad: true));
while (!token.IsCancellationRequested)
{
try { await Task.Delay(PollInterval, token); }
catch (TaskCanceledException) { return; }
AuthenticationState auth;
try
{
auth = await AuthStateProvider.GetAuthenticationStateAsync();
}
catch (ObjectDisposedException)
{
return;
}
if (auth.User.Identity?.IsAuthenticated != true)
{
await InvokeAsync(() => Navigation.NavigateTo("/login", forceLoad: true));
return;
}
}
}
public void Dispose()

23
src/ScadaLink.Security/ServiceCollectionExtensions.cs
View File

@@ -1,5 +1,6 @@
using Microsoft.AspNetCore.Authentication.Cookies;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
namespace ScadaLink.Security;
@@ -25,6 +26,28 @@ public static class ServiceCollectionExtensions
options.Cookie.SecurePolicy = Microsoft.AspNetCore.Http.CookieSecurePolicy.Always;
});
// CentralUI-005: configure the cookie session as a sliding window so the
// code matches the documented policy ("sliding refresh, 30-minute idle
// timeout"). ASP.NET cookie auth exposes a single ExpireTimeSpan plus a
// SlidingExpiration flag — it cannot natively model a 15-minute sliding
// token AND a separate 30-minute absolute idle cap. The faithful
// interpretation: the cookie window IS the idle timeout
// (SecurityOptions.IdleTimeoutMinutes, default 30) and SlidingExpiration
// renews it on activity (the middleware re-issues the cookie once past
// the halfway mark of the window). An active user is therefore kept
// signed in; an idle user is signed out after the idle timeout. The
// 15-minute JwtExpiryMinutes governs the lifetime of the embedded JWT
// itself (see JwtTokenService) — a separate layer from the cookie
// session window. Bound here via PostConfigure so SecurityOptions
// (configured by the Host after AddSecurity) is honoured.
services.AddOptions<CookieAuthenticationOptions>(CookieAuthenticationDefaults.AuthenticationScheme)
.Configure<IOptions<SecurityOptions>>((cookieOptions, securityOptions) =>
{
var idleMinutes = securityOptions.Value.IdleTimeoutMinutes;
cookieOptions.ExpireTimeSpan = TimeSpan.FromMinutes(idleMinutes);
cookieOptions.SlidingExpiration = true;
});
services.AddScadaLinkAuthorization();
return services;

46
tests/ScadaLink.CentralUI.Tests/Auth/SessionExpiryPolicyTests.cs Normal file
View File

@@ -0,0 +1,46 @@
using Microsoft.AspNetCore.Authentication;
using ScadaLink.CentralUI.Auth;
namespace ScadaLink.CentralUI.Tests.Auth;
/// <summary>
/// Regression tests for CentralUI-005. <c>AuthEndpoints</c> previously stamped a
/// fixed <c>expires_at = UtcNow + 30 min</c> claim and a 30-minute absolute cookie
/// <c>ExpiresUtc</c> with no sliding refresh, contradicting the documented
/// "sliding refresh, 30-minute idle timeout" policy. The login handler must now
/// build <see cref="AuthenticationProperties"/> that let the cookie middleware
/// own expiry (sliding window) rather than imposing a contradictory fixed
/// absolute cap.
/// </summary>
public class SessionExpiryPolicyTests
{
[Fact]
public void BuildSignInProperties_DoesNotSetFixedAbsoluteExpiry()
{
var props = AuthEndpoints.BuildSignInProperties();
// A fixed ExpiresUtc would re-introduce the hard 30-minute cap that
// overrides the middleware's sliding window. Expiry must be owned by
// the cookie middleware (ExpireTimeSpan + SlidingExpiration).
Assert.Null(props.ExpiresUtc);
}
[Fact]
public void BuildSignInProperties_IsPersistent()
{
var props = AuthEndpoints.BuildSignInProperties();
Assert.True(props.IsPersistent);
}
[Fact]
public void BuildSignInProperties_AllowsSlidingRefresh()
{
var props = AuthEndpoints.BuildSignInProperties();
// AllowRefresh left null/true lets the cookie middleware slide the
// expiry on activity. A false value would freeze the session to an
// absolute cap — the bug this finding pins.
Assert.NotEqual(false, props.AllowRefresh);
}
}

57
tests/ScadaLink.Security.Tests/SecurityTests.cs
View File

@@ -423,6 +423,63 @@ public class SecurityReviewRegressionTests
Assert.True(cookieOptions.Cookie.HttpOnly);
}
// --- CentralUI-005: cookie auth must use a sliding session window ---
// Documented policy (CLAUDE.md Security & Auth): sliding refresh with a
// 30-minute idle timeout. The cookie middleware must enable SlidingExpiration
// so an active session is renewed on activity and an idle session expires.
[Fact]
public void AddSecurity_AuthCookie_UsesSlidingExpiration()
{
var services = new ServiceCollection();
services.AddLogging();
services.AddDataProtection();
services.AddSecurity();
using var provider = services.BuildServiceProvider();
var cookieOptions = provider
.GetRequiredService<IOptionsMonitor<Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationOptions>>()
.Get(Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme);
Assert.True(cookieOptions.SlidingExpiration);
}
[Fact]
public void AddSecurity_AuthCookie_ExpireTimeSpanMatchesIdleTimeout()
{
var services = new ServiceCollection();
services.AddLogging();
services.AddDataProtection();
services.AddSecurity();
// The idle timeout drives the cookie's expiry window.
services.Configure<SecurityOptions>(o => o.IdleTimeoutMinutes = 30);
using var provider = services.BuildServiceProvider();
var cookieOptions = provider
.GetRequiredService<IOptionsMonitor<Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationOptions>>()
.Get(Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme);
Assert.Equal(TimeSpan.FromMinutes(30), cookieOptions.ExpireTimeSpan);
}
[Fact]
public void AddSecurity_AuthCookie_ExpireTimeSpanIsConfigurable()
{
var services = new ServiceCollection();
services.AddLogging();
services.AddDataProtection();
services.AddSecurity();
services.Configure<SecurityOptions>(o => o.IdleTimeoutMinutes = 45);
using var provider = services.BuildServiceProvider();
var cookieOptions = provider
.GetRequiredService<IOptionsMonitor<Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationOptions>>()
.Get(Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme);
Assert.Equal(TimeSpan.FromMinutes(45), cookieOptions.ExpireTimeSpan);
Assert.True(cookieOptions.SlidingExpiration);
}
// --- Security-001: StartTLS transport must be reachable ---
[Fact]