From 8577a1ab3650444d58915d9ee969bea146b8fcd5 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Sun, 11 Oct 2026 22:11:31 +0200 Subject: [PATCH 01/15] OIDC sessions: OidcLogin configuration in core, OidcSession entity, back-office account check --- .../Services/OidcCustomerResolver.cs | 6 +-- .../Extensions/BackOfficeAccountExtensions.cs | 18 ++++++++ .../Customers/ExternalAuthentication.cs | 6 +++ .../Grand.Domain/Customers/OidcSession.cs | 42 ++++++++++++++++++ .../Configuration/OidcLoginConfig.cs | 44 +++++++++++++++++++ src/Core/Grand.Infrastructure/StartupBase.cs | 1 + .../Infrastructure/StartupApplication.cs | 4 -- .../OidcLoginConfig.cs | 16 ------- .../BackOfficeAccountExtensionsTests.cs | 36 +++++++++++++++ .../Services/OidcLoginConfigTests.cs | 41 +++++++++++++++++ src/Web/Grand.Web/App_Data/appsettings.json | 14 ++++++ 11 files changed, 205 insertions(+), 23 deletions(-) create mode 100644 src/Business/Grand.Business.Core/Extensions/BackOfficeAccountExtensions.cs create mode 100644 src/Core/Grand.Domain/Customers/OidcSession.cs create mode 100644 src/Core/Grand.Infrastructure/Configuration/OidcLoginConfig.cs delete mode 100644 src/Plugins/Authentication.OpenIdConnect/OidcLoginConfig.cs create mode 100644 src/Tests/Grand.Business.Authentication.Tests/Services/BackOfficeAccountExtensionsTests.cs create mode 100644 src/Tests/Grand.Business.Authentication.Tests/Services/OidcLoginConfigTests.cs diff --git a/src/Business/Grand.Business.Authentication/Services/OidcCustomerResolver.cs b/src/Business/Grand.Business.Authentication/Services/OidcCustomerResolver.cs index c013a3fa15..e69024cf76 100644 --- a/src/Business/Grand.Business.Authentication/Services/OidcCustomerResolver.cs +++ b/src/Business/Grand.Business.Authentication/Services/OidcCustomerResolver.cs @@ -1,4 +1,5 @@ using Grand.Business.Core.Events.Customers; +using Grand.Business.Core.Extensions; using Grand.Business.Core.Interfaces.Authentication; using Grand.Business.Core.Interfaces.Common.Configuration; using Grand.Business.Core.Interfaces.Common.Directory; @@ -451,7 +452,6 @@ private async Task HasTwoFactor(Customer customer, string storeId) => private async Task RequiresPassword(Customer customer, string storeId) => await IsBackOfficeAccount(customer) || await HasTwoFactor(customer, storeId); - private async Task IsBackOfficeAccount(Customer customer) => - !string.IsNullOrEmpty(customer.VendorId) || !string.IsNullOrEmpty(customer.StaffStoreId) || - await _permissionService.Authorize(StandardPermission.ManageAccessAdminPanel, customer); + private Task IsBackOfficeAccount(Customer customer) => + _permissionService.IsBackOfficeAccount(customer); } diff --git a/src/Business/Grand.Business.Core/Extensions/BackOfficeAccountExtensions.cs b/src/Business/Grand.Business.Core/Extensions/BackOfficeAccountExtensions.cs new file mode 100644 index 0000000000..62cda25789 --- /dev/null +++ b/src/Business/Grand.Business.Core/Extensions/BackOfficeAccountExtensions.cs @@ -0,0 +1,18 @@ +using Grand.Business.Core.Interfaces.Common.Security; +using Grand.Domain.Customers; +using Grand.Domain.Permissions; + +namespace Grand.Business.Core.Extensions; + +public static class BackOfficeAccountExtensions +{ + /// + /// A back-office account: vendor, store staff, or an account with admin panel access + /// + public static async Task IsBackOfficeAccount(this IPermissionService permissionService, Customer customer) + { + ArgumentNullException.ThrowIfNull(customer); + return !string.IsNullOrEmpty(customer.VendorId) || !string.IsNullOrEmpty(customer.StaffStoreId) || + await permissionService.Authorize(StandardPermission.ManageAccessAdminPanel, customer); + } +} diff --git a/src/Core/Grand.Domain/Customers/ExternalAuthentication.cs b/src/Core/Grand.Domain/Customers/ExternalAuthentication.cs index d657c2c5ce..1fb8169a56 100644 --- a/src/Core/Grand.Domain/Customers/ExternalAuthentication.cs +++ b/src/Core/Grand.Domain/Customers/ExternalAuthentication.cs @@ -44,4 +44,10 @@ public class ExternalAuthentication : BaseEntity /// Gets or sets a value indicating whether the link was made by the e-mail address alone, without the account's password /// public bool LinkedByEmail { get; set; } + + /// + /// Gets or sets the time before which the identity provider's access tokens of this subject are refused (provider + /// sign-out, or the provider refused the account); empty = never + /// + public DateTime? TokensRevokedBeforeUtc { get; set; } } \ No newline at end of file diff --git a/src/Core/Grand.Domain/Customers/OidcSession.cs b/src/Core/Grand.Domain/Customers/OidcSession.cs new file mode 100644 index 0000000000..f673a45ccc --- /dev/null +++ b/src/Core/Grand.Domain/Customers/OidcSession.cs @@ -0,0 +1,42 @@ +namespace Grand.Domain.Customers; + +/// +/// A browser session started by a sign-in at the OpenID Connect identity provider; the main authentication cookie +/// carries its id, so the session can be ended on the server (sign-out, back-channel logout, staff policy) +/// +public class OidcSession : BaseEntity +{ + public string CustomerId { get; set; } + + /// The store of the sign-in + public string StoreId { get; set; } + + /// Provider subject (sub) + public string Subject { get; set; } + + /// Provider session id (sid) from the ID token; empty when the provider sends none + public string Sid { get; set; } + + public DateTime CreatedUtc { get; set; } + public DateTime LastActivityUtc { get; set; } + + /// Last successful check of the account at the provider + public DateTime LastValidatedUtc { get; set; } + + /// Refresh token protected with Data Protection; kept only for back-office accounts + public string ProtectedRefreshToken { get; set; } + + public DateTime? EndedUtc { get; set; } + public OidcSessionEndReason? EndReason { get; set; } +} + +public enum OidcSessionEndReason +{ + SignOut = 1, + BackChannel = 2, + Idle = 3, + MaxLifetime = 4, + RevalidationRefused = 5, + RevalidationUnavailable = 6, + NoRefreshToken = 7 +} diff --git a/src/Core/Grand.Infrastructure/Configuration/OidcLoginConfig.cs b/src/Core/Grand.Infrastructure/Configuration/OidcLoginConfig.cs new file mode 100644 index 0000000000..558ada5306 --- /dev/null +++ b/src/Core/Grand.Infrastructure/Configuration/OidcLoginConfig.cs @@ -0,0 +1,44 @@ +namespace Grand.Infrastructure.Configuration; + +/// +/// appsettings section OidcLogin: the client of this store at the identity provider configured in OidcResourceServer, +/// and the policy for back-office sessions started by a provider sign-in +/// +public class OidcLoginConfig +{ + private const int DefaultStaffIdleTimeoutMinutes = 30; + private const int DefaultStaffMaxSessionHours = 10; + private const int DefaultStaffRevalidationMinutes = 10; + + public string ClientId { get; set; } + public string ClientSecret { get; set; } + public string Scopes { get; set; } = "openid profile email"; + + /// A back-office session ends after this much inactivity (sliding) + public int StaffIdleTimeoutMinutes { get; set; } = DefaultStaffIdleTimeoutMinutes; + + /// A back-office session ends this long after the sign-in + public int StaffMaxSessionHours { get; set; } = DefaultStaffMaxSessionHours; + + /// A back-office session checks the account at the identity provider this often (refresh token exchange) + public int StaffRevalidationMinutes { get; set; } = DefaultStaffRevalidationMinutes; + + /// + /// Back-office accounts connected to the identity provider with their password sign in only through the provider; + /// an account without such a connection keeps its password (break-glass) + /// + public bool StaffSsoOnly { get; set; } + + //zero or less is a configuration mistake, never "check on every request" or "end every session at once" + public TimeSpan StaffIdleTimeout => + TimeSpan.FromMinutes(StaffIdleTimeoutMinutes > 0 ? StaffIdleTimeoutMinutes : DefaultStaffIdleTimeoutMinutes); + + public TimeSpan StaffMaxSession => + TimeSpan.FromHours(StaffMaxSessionHours > 0 ? StaffMaxSessionHours : DefaultStaffMaxSessionHours); + + public TimeSpan StaffRevalidationInterval => + TimeSpan.FromMinutes(StaffRevalidationMinutes > 0 ? StaffRevalidationMinutes : DefaultStaffRevalidationMinutes); + + public bool IsConfigured(OidcResourceServerConfig resourceServer) => + !string.IsNullOrWhiteSpace(resourceServer?.Authority) && !string.IsNullOrWhiteSpace(ClientId); +} diff --git a/src/Core/Grand.Infrastructure/StartupBase.cs b/src/Core/Grand.Infrastructure/StartupBase.cs index 777c132313..e2a1af27bb 100644 --- a/src/Core/Grand.Infrastructure/StartupBase.cs +++ b/src/Core/Grand.Infrastructure/StartupBase.cs @@ -222,6 +222,7 @@ private static void RegisterConfigurations(IServiceCollection services, IConfigu services.StartupConfig(configuration.GetSection("BackendAPI")); services.StartupConfig(configuration.GetSection("FrontendAPI")); services.StartupConfig(configuration.GetSection("OidcResourceServer")); + services.StartupConfig(configuration.GetSection("OidcLogin")); services.StartupConfig(configuration.GetSection("Database")); services.StartupConfig(configuration.GetSection("Amazon")); services.StartupConfig(configuration.GetSection("Azure")); diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs index 6b5dbe7689..c7bd259abd 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs @@ -11,10 +11,6 @@ public class StartupApplication : IStartupApplication { public void ConfigureServices(IServiceCollection services, IConfiguration configuration) { - var config = new OidcLoginConfig(); - configuration.GetSection(OidcLoginDefaults.ConfigSection).Bind(config); - services.AddSingleton(config); - services.AddScoped(); } diff --git a/src/Plugins/Authentication.OpenIdConnect/OidcLoginConfig.cs b/src/Plugins/Authentication.OpenIdConnect/OidcLoginConfig.cs deleted file mode 100644 index 7b743af677..0000000000 --- a/src/Plugins/Authentication.OpenIdConnect/OidcLoginConfig.cs +++ /dev/null @@ -1,16 +0,0 @@ -using Grand.Infrastructure.Configuration; - -namespace Authentication.OpenIdConnect; - -/// -/// appsettings section OidcLogin: the client of this store at the identity provider configured in OidcResourceServer -/// -public class OidcLoginConfig -{ - public string ClientId { get; set; } - public string ClientSecret { get; set; } - public string Scopes { get; set; } = "openid profile email"; - - public bool IsConfigured(OidcResourceServerConfig resourceServer) => - !string.IsNullOrWhiteSpace(resourceServer?.Authority) && !string.IsNullOrWhiteSpace(ClientId); -} diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/BackOfficeAccountExtensionsTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/BackOfficeAccountExtensionsTests.cs new file mode 100644 index 0000000000..3af9bbee6a --- /dev/null +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/BackOfficeAccountExtensionsTests.cs @@ -0,0 +1,36 @@ +using Grand.Business.Core.Extensions; +using Grand.Business.Core.Interfaces.Common.Security; +using Grand.Domain.Customers; +using Grand.Domain.Permissions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Business.Authentication.Tests.Services; + +[TestClass] +public class BackOfficeAccountExtensionsTests +{ + private static Mock Permissions(bool adminPanel) + { + var permissions = new Mock(); + permissions.Setup(x => x.Authorize(StandardPermission.ManageAccessAdminPanel, It.IsAny())) + .ReturnsAsync(adminPanel); + return permissions; + } + + [TestMethod] + public async Task Customer_NotBackOffice() => + Assert.IsFalse(await Permissions(false).Object.IsBackOfficeAccount(new Customer())); + + [TestMethod] + public async Task AdminPanelAccess_BackOffice() => + Assert.IsTrue(await Permissions(true).Object.IsBackOfficeAccount(new Customer())); + + [TestMethod] + public async Task Vendor_BackOffice() => + Assert.IsTrue(await Permissions(false).Object.IsBackOfficeAccount(new Customer { VendorId = "v1" })); + + [TestMethod] + public async Task StoreStaff_BackOffice() => + Assert.IsTrue(await Permissions(false).Object.IsBackOfficeAccount(new Customer { StaffStoreId = "s1" })); +} diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcLoginConfigTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcLoginConfigTests.cs new file mode 100644 index 0000000000..69cb3863b1 --- /dev/null +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcLoginConfigTests.cs @@ -0,0 +1,41 @@ +using Grand.Infrastructure.Configuration; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Grand.Business.Authentication.Tests.Services; + +[TestClass] +public class OidcLoginConfigTests +{ + [TestMethod] + public void Defaults() + { + var config = new OidcLoginConfig(); + Assert.AreEqual("openid profile email", config.Scopes); + Assert.AreEqual(TimeSpan.FromMinutes(30), config.StaffIdleTimeout); + Assert.AreEqual(TimeSpan.FromHours(10), config.StaffMaxSession); + Assert.AreEqual(TimeSpan.FromMinutes(10), config.StaffRevalidationInterval); + Assert.IsFalse(config.StaffSsoOnly); + } + + //a zero in appsettings must not mean "refresh at the provider on every request" or "end every session at once" + [TestMethod] + [DataRow(0)] + [DataRow(-5)] + public void NonPositiveValues_FallBackToDefaults(int value) + { + var config = new OidcLoginConfig { + StaffIdleTimeoutMinutes = value, StaffMaxSessionHours = value, StaffRevalidationMinutes = value + }; + Assert.AreEqual(TimeSpan.FromMinutes(30), config.StaffIdleTimeout); + Assert.AreEqual(TimeSpan.FromHours(10), config.StaffMaxSession); + Assert.AreEqual(TimeSpan.FromMinutes(10), config.StaffRevalidationInterval); + } + + [TestMethod] + [DataRow("https://idp", "client", true)] + [DataRow("", "client", false)] + [DataRow("https://idp", " ", false)] + public void IsConfigured(string authority, string clientId, bool expected) => + Assert.AreEqual(expected, new OidcLoginConfig { ClientId = clientId } + .IsConfigured(new OidcResourceServerConfig { Authority = authority })); +} diff --git a/src/Web/Grand.Web/App_Data/appsettings.json b/src/Web/Grand.Web/App_Data/appsettings.json index b3481d1065..85f5f86b47 100644 --- a/src/Web/Grand.Web/App_Data/appsettings.json +++ b/src/Web/Grand.Web/App_Data/appsettings.json @@ -273,6 +273,20 @@ ] }, + //browser sign-in with the identity provider of OidcResourceServer (plugin Authentication.OpenIdConnect). + //See docs/oidc-resource-server.md, section 11 and 12. + "OidcLogin": { + "ClientId": "", + //keep it out of source control (env var OidcLogin__ClientSecret or a secret store) + "ClientSecret": "", + "Scopes": "openid profile email", + //back-office accounts (admin panel, store staff, vendors) signed in through the provider + "StaffIdleTimeoutMinutes": 30, + "StaffMaxSessionHours": 10, + "StaffRevalidationMinutes": 10, + //true: back-office accounts connected to the provider sign in only through it; accounts without a connection keep the password + "StaffSsoOnly": false + }, "Logging": { "LogLevel": { "Default": "Information", From f8c34f3c36a4f39aa43688504862cbabd94a6976 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Sun, 11 Oct 2026 22:16:42 +0200 Subject: [PATCH 02/15] OIDC sessions: refuse provider tokens of a subject issued before a revocation time --- .../Services/OidcTokenRevocationService.cs | 30 +++++++++ .../Startup/StartupApplication.cs | 1 + .../IOidcTokenRevocationService.cs | 14 +++++ .../OidcTokenRevocationServiceTests.cs | 63 +++++++++++++++++++ 4 files changed, 108 insertions(+) create mode 100644 src/Business/Grand.Business.Authentication/Services/OidcTokenRevocationService.cs create mode 100644 src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcTokenRevocationService.cs create mode 100644 src/Tests/Grand.Business.Authentication.Tests/Services/OidcTokenRevocationServiceTests.cs diff --git a/src/Business/Grand.Business.Authentication/Services/OidcTokenRevocationService.cs b/src/Business/Grand.Business.Authentication/Services/OidcTokenRevocationService.cs new file mode 100644 index 0000000000..5eaddd766c --- /dev/null +++ b/src/Business/Grand.Business.Authentication/Services/OidcTokenRevocationService.cs @@ -0,0 +1,30 @@ +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Data; +using Grand.Domain.Customers; + +namespace Grand.Business.Authentication.Services; + +/// +/// Keeps the revocation time on the subject's links (one per store with per-store identity). Not cached: the check +/// must hold at once on every instance, and the lookup uses the provider + identifier index +/// +public class OidcTokenRevocationService(IRepository externalAuthenticationRepository) + : IOidcTokenRevocationService +{ + public Task RevokeTokens(string subject) + { + ArgumentException.ThrowIfNullOrEmpty(subject); + return externalAuthenticationRepository.UpdateManyAsync( + x => x.ProviderSystemName == OidcCustomerResolver.ProviderSystemName && x.ExternalIdentifier == subject, + UpdateBuilder.Create().Set(x => x.TokensRevokedBeforeUtc, DateTime.UtcNow)); + } + + public async Task IsRevoked(string subject, DateTime issuedUtc) + { + if (string.IsNullOrEmpty(subject)) + return false; + return await externalAuthenticationRepository.GetOneAsync(x => + x.ProviderSystemName == OidcCustomerResolver.ProviderSystemName && x.ExternalIdentifier == subject && + x.TokensRevokedBeforeUtc != null && x.TokensRevokedBeforeUtc > issuedUtc) != null; + } +} diff --git a/src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs b/src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs index 20bc77417f..5c52685b7a 100644 --- a/src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs +++ b/src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs @@ -23,6 +23,7 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); } public void Configure(WebApplication application, IWebHostEnvironment webHostEnvironment) diff --git a/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcTokenRevocationService.cs b/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcTokenRevocationService.cs new file mode 100644 index 0000000000..e00e5386ec --- /dev/null +++ b/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcTokenRevocationService.cs @@ -0,0 +1,14 @@ +namespace Grand.Business.Core.Interfaces.Authentication; + +/// +/// Refuses identity provider access tokens of a subject issued before the provider ended the subject's session +/// or refused the account +/// +public interface IOidcTokenRevocationService +{ + /// Tokens of the subject issued until now are refused + Task RevokeTokens(string subject); + + /// The token of the subject issued at is refused + Task IsRevoked(string subject, DateTime issuedUtc); +} diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcTokenRevocationServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcTokenRevocationServiceTests.cs new file mode 100644 index 0000000000..8b9e23cbcb --- /dev/null +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcTokenRevocationServiceTests.cs @@ -0,0 +1,63 @@ +using Grand.Business.Authentication.Services; +using Grand.Data; +using Grand.Data.Tests.MongoDb; +using Grand.Domain.Customers; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Grand.Business.Authentication.Tests.Services; + +[TestClass] +public class OidcTokenRevocationServiceTests +{ + private IRepository _links; + private OidcTokenRevocationService _service; + + [TestInitialize] + public void Init() + { + _links = new MongoDBRepositoryTest(); + _service = new OidcTokenRevocationService(_links); + } + + private Task Link(string customerId, string sub, string provider = OidcCustomerResolver.ProviderSystemName, string storeId = "") => + _links.InsertAsync(new ExternalAuthentication { + CustomerId = customerId, ExternalIdentifier = sub, ProviderSystemName = provider, StoreId = storeId + }); + + [TestMethod] + public async Task NeverRevoked_NotRevoked() + { + await Link("c1", "sub-1"); + Assert.IsFalse(await _service.IsRevoked("sub-1", DateTime.UtcNow.AddHours(-1))); + } + + [TestMethod] + public async Task Revoke_OlderTokenRevoked_NewerAccepted() + { + await Link("c1", "sub-1"); + await _service.RevokeTokens("sub-1"); + Assert.IsTrue(await _service.IsRevoked("sub-1", DateTime.UtcNow.AddMinutes(-1))); + Assert.IsFalse(await _service.IsRevoked("sub-1", DateTime.UtcNow.AddMinutes(1))); + } + + //with per-store identity a subject has one link per store: all of them stop the old tokens + [TestMethod] + public async Task Revoke_EveryLinkOfTheSubject_OnlyOpenIdConnect() + { + await Link("c1", "sub-1", storeId: "s1"); + await Link("c2", "sub-1", storeId: "s2"); + await Link("c3", "sub-1", provider: "ExternalAuth.Google"); + await Link("c4", "sub-2"); + await _service.RevokeTokens("sub-1"); + + var revoked = _links.Table.Where(x => x.TokensRevokedBeforeUtc != null).Select(x => x.CustomerId).ToList(); + CollectionAssert.AreEquivalent(new[] { "c1", "c2" }, revoked); + } + + [TestMethod] + public async Task UnknownSubject_NotRevoked_RevokeDoesNothing() + { + await _service.RevokeTokens("nobody"); + Assert.IsFalse(await _service.IsRevoked("nobody", DateTime.UtcNow.AddHours(-1))); + } +} From dd877a4ae1f0325073698c0cf24263917dbc9b0e Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Sun, 11 Oct 2026 22:19:38 +0200 Subject: [PATCH 03/15] OIDC sessions: server-side session record, end by provider session or subject, cleanup query --- .../Services/OidcSessionService.cs | 161 ++++++++++++++++ .../Startup/StartupApplication.cs | 1 + .../Authentication/IOidcSessionService.cs | 32 +++ .../Authentication/IOidcTokenRefresher.cs | 12 ++ .../Authentication/OidcRefreshResult.cs | 22 +++ .../Authentication/OidcSessionDefaults.cs | 13 ++ .../Caching/Constants/CustomerCacheKey.cs | 13 ++ .../Services/OidcSessionServiceTests.cs | 182 ++++++++++++++++++ 8 files changed, 436 insertions(+) create mode 100644 src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs create mode 100644 src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcSessionService.cs create mode 100644 src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcTokenRefresher.cs create mode 100644 src/Business/Grand.Business.Core/Utilities/Authentication/OidcRefreshResult.cs create mode 100644 src/Business/Grand.Business.Core/Utilities/Authentication/OidcSessionDefaults.cs create mode 100644 src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs diff --git a/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs b/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs new file mode 100644 index 0000000000..7e13feab09 --- /dev/null +++ b/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs @@ -0,0 +1,161 @@ +using Grand.Business.Core.Extensions; +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Business.Core.Interfaces.Common.Security; +using Grand.Business.Core.Utilities.Authentication; +using Grand.Data; +using Grand.Domain.Customers; +using Grand.Infrastructure.Caching; +using Grand.Infrastructure.Caching.Constants; +using Grand.Infrastructure.Configuration; +using Microsoft.AspNetCore.DataProtection; +using Microsoft.Extensions.Logging; +using System.Collections.Concurrent; +using System.Security.Cryptography; + +namespace Grand.Business.Authentication.Services; + +/// +/// Browser sessions started by an identity provider sign-in. Every session can be ended on the server (sign-out, +/// back-channel logout); a back-office session also ends after the idle time or the maximum lifetime, and is +/// confirmed at the provider at an interval by exchanging its refresh token - the only way to notice that the +/// provider disabled or deleted the user, since providers send no logout for that +/// +public class OidcSessionService : IOidcSessionService +{ + //how often the activity time of a back-office session is written + private static readonly TimeSpan ActivityWriteInterval = TimeSpan.FromMinutes(1); + + //minutes; ended sessions are removed from the cache, so only the activity time may be this stale + private const int CacheTimeMinutes = 1; + + //one refresh at a time per session in this process: refresh tokens rotate + private static readonly ConcurrentDictionary RefreshesInProgress = new(); + + private readonly IRepository _sessionRepository; + private readonly ICacheBase _cache; + private readonly IPermissionService _permissionService; + private readonly IOidcTokenRevocationService _tokenRevocationService; + private readonly IEnumerable _tokenRefreshers; + private readonly IDataProtector _protector; + private readonly OidcLoginConfig _config; + private readonly SecurityConfig _securityConfig; + private readonly ILogger _logger; + + public OidcSessionService( + IRepository sessionRepository, + ICacheBase cache, + IPermissionService permissionService, + IOidcTokenRevocationService tokenRevocationService, + IEnumerable tokenRefreshers, + IDataProtectionProvider dataProtectionProvider, + OidcLoginConfig config, + SecurityConfig securityConfig, + ILogger logger) + { + _sessionRepository = sessionRepository; + _cache = cache; + _permissionService = permissionService; + _tokenRevocationService = tokenRevocationService; + _tokenRefreshers = tokenRefreshers; + _protector = dataProtectionProvider.CreateProtector(OidcSessionDefaults.RefreshTokenPurpose); + _config = config; + _securityConfig = securityConfig; + _logger = logger; + } + + public virtual async Task Start(Customer customer, string subject, string sid, string refreshToken, string storeId) + { + ArgumentNullException.ThrowIfNull(customer); + ArgumentException.ThrowIfNullOrEmpty(subject); + + var backOffice = await _permissionService.IsBackOfficeAccount(customer); + if (backOffice && string.IsNullOrEmpty(refreshToken)) + { + _logger.LogError("Provider sign-in of back-office customer {CustomerId} refused: the provider issued no refresh " + + "token, so the session could not be revalidated. Add offline_access to OidcLogin:Scopes " + + "if the provider issues refresh tokens only for it", customer.Id); + return null; + } + + var now = DateTime.UtcNow; + var session = new OidcSession { + CustomerId = customer.Id, + StoreId = storeId ?? "", + Subject = subject, + Sid = sid ?? "", + CreatedUtc = now, + LastActivityUtc = now, + LastValidatedUtc = now, + ProtectedRefreshToken = backOffice ? _protector.Protect(refreshToken) : null + }; + await _sessionRepository.InsertAsync(session); + return session.Id; + } + + public virtual Task Validate(string sessionId, Customer customer) => + throw new NotImplementedException(); + + public virtual async Task End(string sessionId, OidcSessionEndReason reason) + { + if (string.IsNullOrEmpty(sessionId)) + return; + //the first end wins: its reason is the one that explains the sign-out + await _sessionRepository.UpdateOneAsync(x => x.Id == sessionId && x.EndedUtc == null, + UpdateBuilder.Create().Set(x => x.EndedUtc, DateTime.UtcNow).Set(x => x.EndReason, reason)); + await _cache.RemoveAsync(string.Format(CacheKey.OIDC_SESSION_BY_ID_KEY, sessionId)); + _logger.LogInformation("OpenID Connect session {SessionId} ended: {Reason}", sessionId, reason); + } + + public virtual async Task> EndBySid(string subject, string sid) + { + ArgumentException.ThrowIfNullOrEmpty(sid); + var query = _sessionRepository.Table.Where(x => x.Sid == sid && x.EndedUtc == null); + if (!string.IsNullOrEmpty(subject)) + query = query.Where(x => x.Subject == subject); + return await EndAll(await _sessionRepository.ToListAsync(query)); + } + + public virtual async Task> EndBySubject(string subject) + { + ArgumentException.ThrowIfNullOrEmpty(subject); + return await EndAll(await _sessionRepository.ToListAsync( + _sessionRepository.Table.Where(x => x.Subject == subject && x.EndedUtc == null))); + } + + public virtual async Task DeleteExpired() + { + var now = DateTime.UtcNow; + var ended = now - OidcSessionDefaults.Retention; + await _sessionRepository.DeleteManyAsync(x => x.EndedUtc != null && x.EndedUtc < ended); + + //sessions nobody used again end lazily, at the next request: these will never get one + var staffCreated = now - _config.StaffMaxSession - OidcSessionDefaults.Retention; + await _sessionRepository.DeleteManyAsync(x => x.ProtectedRefreshToken != null && x.CreatedUtc < staffCreated); + var customerCreated = now - TimeSpan.FromHours(_securityConfig.CookieAuthExpires) - OidcSessionDefaults.Retention; + await _sessionRepository.DeleteManyAsync(x => x.CreatedUtc < customerCreated); + } + + private async Task> EndAll(IList sessions) + { + foreach (var session in sessions) + await End(session.Id, OidcSessionEndReason.BackChannel); + return sessions; + } + + private Task Get(string sessionId) => + _cache.GetAsync(string.Format(CacheKey.OIDC_SESSION_BY_ID_KEY, sessionId), + () => _sessionRepository.GetByIdAsync(sessionId), CacheTimeMinutes); + + private string Unprotect(string protectedToken) + { + try + { + return _protector.Unprotect(protectedToken); + } + catch (CryptographicException) + { + //key ring lost or rotated away: the token cannot be used any more + return null; + } + } +} diff --git a/src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs b/src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs index 5c52685b7a..e03aee9535 100644 --- a/src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs +++ b/src/Business/Grand.Business.Authentication/Startup/StartupApplication.cs @@ -24,6 +24,7 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); } public void Configure(WebApplication application, IWebHostEnvironment webHostEnvironment) diff --git a/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcSessionService.cs b/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcSessionService.cs new file mode 100644 index 0000000000..23142d3a59 --- /dev/null +++ b/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcSessionService.cs @@ -0,0 +1,32 @@ +using Grand.Domain.Customers; + +namespace Grand.Business.Core.Interfaces.Authentication; + +/// +/// Server-side state of browser sessions started by an identity provider sign-in +/// +public interface IOidcSessionService +{ + /// + /// Records a new session and returns its id; the refresh token is kept (protected) only for a back-office account. + /// Null when a back-office account brings no refresh token: such a session could never be revalidated + /// + Task Start(Customer customer, string subject, string sid, string refreshToken, string storeId); + + /// + /// The session may continue: not ended, of this customer, and for a back-office account within the idle and + /// maximum lifetime and confirmed by the provider at the configured interval (ends the session otherwise) + /// + Task Validate(string sessionId, Customer customer); + + Task End(string sessionId, OidcSessionEndReason reason); + + /// Ends the open sessions with the provider session id (and the subject, when given); returns them + Task> EndBySid(string subject, string sid); + + /// Ends every open session of the subject; returns them + Task> EndBySubject(string subject); + + /// Deletes ended sessions and sessions past their lifetime, after the retention period + Task DeleteExpired(); +} diff --git a/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcTokenRefresher.cs b/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcTokenRefresher.cs new file mode 100644 index 0000000000..76157deb8f --- /dev/null +++ b/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcTokenRefresher.cs @@ -0,0 +1,12 @@ +using Grand.Business.Core.Utilities.Authentication; + +namespace Grand.Business.Core.Interfaces.Authentication; + +/// +/// Exchanges a refresh token at the identity provider (implemented by the OpenID Connect login plugin; with no +/// implementation registered a revalidation counts as refused) +/// +public interface IOidcTokenRefresher +{ + Task Refresh(string refreshToken, CancellationToken cancellationToken = default); +} diff --git a/src/Business/Grand.Business.Core/Utilities/Authentication/OidcRefreshResult.cs b/src/Business/Grand.Business.Core/Utilities/Authentication/OidcRefreshResult.cs new file mode 100644 index 0000000000..442a8b2adf --- /dev/null +++ b/src/Business/Grand.Business.Core/Utilities/Authentication/OidcRefreshResult.cs @@ -0,0 +1,22 @@ +namespace Grand.Business.Core.Utilities.Authentication; + +public enum OidcRefreshOutcome +{ + /// The provider accepted the refresh token: the account is still usable there + Refreshed, + + /// A definitive refusal (invalid_grant, invalid_request, any 400/401 of the token endpoint) + Refused, + + /// No answer: network error, timeout, 5xx + Unavailable +} + +/// The answer of the provider +/// The rotated refresh token, or null when the provider keeps the old one +public sealed record OidcRefreshResult(OidcRefreshOutcome Outcome, string RefreshToken = null) +{ + public static OidcRefreshResult Refreshed(string refreshToken) => new(OidcRefreshOutcome.Refreshed, refreshToken); + public static OidcRefreshResult Refused() => new(OidcRefreshOutcome.Refused); + public static OidcRefreshResult Unavailable() => new(OidcRefreshOutcome.Unavailable); +} diff --git a/src/Business/Grand.Business.Core/Utilities/Authentication/OidcSessionDefaults.cs b/src/Business/Grand.Business.Core/Utilities/Authentication/OidcSessionDefaults.cs new file mode 100644 index 0000000000..1dc59af1d5 --- /dev/null +++ b/src/Business/Grand.Business.Core/Utilities/Authentication/OidcSessionDefaults.cs @@ -0,0 +1,13 @@ +namespace Grand.Business.Core.Utilities.Authentication; + +public static class OidcSessionDefaults +{ + /// Claim of the main authentication cookie holding the OidcSession id + public const string SessionIdClaimType = "grand:oidcSession"; + + /// Data Protection purpose of stored refresh tokens + public const string RefreshTokenPurpose = "Grand.OidcSession.RefreshToken"; + + /// How long an ended or expired session record is kept before the cleanup task deletes it + public static readonly TimeSpan Retention = TimeSpan.FromDays(7); +} diff --git a/src/Core/Grand.Infrastructure/Caching/Constants/CustomerCacheKey.cs b/src/Core/Grand.Infrastructure/Caching/Constants/CustomerCacheKey.cs index 49f6c642cd..61238fc10e 100644 --- a/src/Core/Grand.Infrastructure/Caching/Constants/CustomerCacheKey.cs +++ b/src/Core/Grand.Infrastructure/Caching/Constants/CustomerCacheKey.cs @@ -113,5 +113,18 @@ public static partial class CacheKey /// public static string SALESEMPLOYEE_PATTERN_KEY => "Grand.salesemployee."; + /// + /// Key for an OpenID Connect session + /// + /// + /// {0} : session ID + /// + public static string OIDC_SESSION_BY_ID_KEY => "Grand.oidcsession.id-{0}"; + + /// + /// Key pattern to clear cache + /// + public static string OIDC_SESSION_PATTERN_KEY => "Grand.oidcsession."; + #endregion } \ No newline at end of file diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs new file mode 100644 index 0000000000..6a817cc801 --- /dev/null +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs @@ -0,0 +1,182 @@ +using Grand.Business.Authentication.Services; +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Business.Core.Interfaces.Common.Security; +using Grand.Business.Core.Utilities.Authentication; +using Grand.Data; +using Grand.Data.Tests.MongoDb; +using Grand.Domain.Customers; +using Grand.Domain.Permissions; +using Grand.Infrastructure.Caching; +using Grand.Infrastructure.Caching.Constants; +using Grand.Infrastructure.Configuration; +using Microsoft.AspNetCore.DataProtection; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Business.Authentication.Tests.Services; + +[TestClass] +public class OidcSessionServiceTests +{ + private IRepository _sessions; + private Mock _cache; + private Mock _permissions; + private Mock _revocation; + private Mock _refresher; + private List _refreshers; + private IDataProtectionProvider _dataProtection; + private OidcLoginConfig _config; + private SecurityConfig _securityConfig; + private bool _staff; + + private readonly Customer _customer = new() { Id = "c1" }; + + [TestInitialize] + public void Init() + { + _sessions = new MongoDBRepositoryTest(); + _cache = new Mock(); + //read-through without caching: every call sees the repository + _cache.Setup(x => x.GetAsync(It.IsAny(), It.IsAny>>(), It.IsAny())) + .Returns((string _, Func> acquire, int _) => acquire()); + _permissions = new Mock(); + _permissions.Setup(x => x.Authorize(StandardPermission.ManageAccessAdminPanel, It.IsAny())) + .ReturnsAsync(() => _staff); + _revocation = new Mock(); + _refresher = new Mock(); + _refreshers = [_refresher.Object]; + _dataProtection = new EphemeralDataProtectionProvider(); + _config = new OidcLoginConfig(); + _securityConfig = new SecurityConfig { CookieAuthExpires = 24 }; + _staff = false; + } + + private OidcSessionService Service() => + new(_sessions, _cache.Object, _permissions.Object, _revocation.Object, _refreshers, _dataProtection, _config, + _securityConfig, NullLogger.Instance); + + private string Protect(string token) => + _dataProtection.CreateProtector(OidcSessionDefaults.RefreshTokenPurpose).Protect(token); + + private string Unprotect(string token) => + _dataProtection.CreateProtector(OidcSessionDefaults.RefreshTokenPurpose).Unprotect(token); + + //a session as it would be stored, with clocks moved back by the given amounts + private async Task Session(TimeSpan? age = null, TimeSpan? idle = null, TimeSpan? sinceValidated = null, + string refreshToken = "rt-1", string sid = "sid-1", string subject = "sub-1", string customerId = "c1") + { + var now = DateTime.UtcNow; + var session = new OidcSession { + CustomerId = customerId, StoreId = "s1", Subject = subject, Sid = sid, + CreatedUtc = now - (age ?? TimeSpan.Zero), + LastActivityUtc = now - (idle ?? TimeSpan.Zero), + LastValidatedUtc = now - (sinceValidated ?? TimeSpan.Zero), + ProtectedRefreshToken = refreshToken == null ? null : Protect(refreshToken) + }; + await _sessions.InsertAsync(session); + return session; + } + + private OidcSession Stored(string id) => _sessions.Table.First(x => x.Id == id); + + [TestMethod] + public async Task Start_Customer_NoRefreshTokenKept() + { + var id = await Service().Start(_customer, "sub-1", "sid-1", "rt-1", "s1"); + var session = Stored(id); + Assert.AreEqual("c1", session.CustomerId); + Assert.AreEqual("sub-1", session.Subject); + Assert.AreEqual("sid-1", session.Sid); + Assert.AreEqual("s1", session.StoreId); + Assert.IsNull(session.ProtectedRefreshToken); + Assert.IsNull(session.EndedUtc); + } + + [TestMethod] + public async Task Start_Staff_RefreshTokenProtected() + { + _staff = true; + var id = await Service().Start(_customer, "sub-1", null, "rt-1", "s1"); + var session = Stored(id); + Assert.AreNotEqual("rt-1", session.ProtectedRefreshToken); + Assert.AreEqual("rt-1", Unprotect(session.ProtectedRefreshToken)); + Assert.AreEqual("", session.Sid); + } + + //a back-office sign-in without a refresh token could never be revalidated: no session, the sign-in fails + [TestMethod] + public async Task Start_StaffWithoutRefreshToken_Null() + { + _staff = true; + Assert.IsNull(await Service().Start(_customer, "sub-1", "sid-1", null, "s1")); + Assert.AreEqual(0, _sessions.Table.Count()); + } + + [TestMethod] + public async Task End_SetsReason_OnlyOnce() + { + var session = await Session(); + await Service().End(session.Id, OidcSessionEndReason.SignOut); + var ended = Stored(session.Id).EndedUtc; + await Service().End(session.Id, OidcSessionEndReason.Idle); + Assert.AreEqual(OidcSessionEndReason.SignOut, Stored(session.Id).EndReason); + Assert.AreEqual(ended, Stored(session.Id).EndedUtc); + _cache.Verify(x => x.RemoveAsync(string.Format(CacheKey.OIDC_SESSION_BY_ID_KEY, session.Id), true), Times.AtLeastOnce); + } + + [TestMethod] + public async Task EndBySid_MatchingSidAndSubject_Only() + { + var match = await Session(sid: "sid-1", subject: "sub-1"); + var otherSid = await Session(sid: "sid-2", subject: "sub-1"); + var otherSubject = await Session(sid: "sid-1", subject: "sub-2"); + + var ended = await Service().EndBySid("sub-1", "sid-1"); + + CollectionAssert.AreEqual(new[] { match.Id }, ended.Select(x => x.Id).ToArray()); + Assert.AreEqual(OidcSessionEndReason.BackChannel, Stored(match.Id).EndReason); + Assert.IsNull(Stored(otherSid.Id).EndedUtc); + Assert.IsNull(Stored(otherSubject.Id).EndedUtc); + } + + //a logout token with sid only: every session of that provider session, whatever the subject + [TestMethod] + public async Task EndBySid_NoSubject_AllWithSid() + { + var a = await Session(sid: "sid-1", subject: "sub-1"); + var b = await Session(sid: "sid-1", subject: "sub-2"); + var ended = await Service().EndBySid(null, "sid-1"); + CollectionAssert.AreEquivalent(new[] { a.Id, b.Id }, ended.Select(x => x.Id).ToArray()); + } + + [TestMethod] + public async Task EndBySubject_AllOpenSessionsOfSubject() + { + var a = await Session(sid: "sid-1"); + var b = await Session(sid: ""); + var other = await Session(subject: "sub-2"); + var ended = await Service().EndBySubject("sub-1"); + CollectionAssert.AreEquivalent(new[] { a.Id, b.Id }, ended.Select(x => x.Id).ToArray()); + Assert.IsNull(Stored(other.Id).EndedUtc); + } + + [TestMethod] + public async Task DeleteExpired_KeepsRecentAndOpen() + { + var endedLongAgo = await Session(); + await _sessions.UpdateField(endedLongAgo.Id, x => x.EndedUtc, DateTime.UtcNow.AddDays(-8)); + var endedRecently = await Session(); + await _sessions.UpdateField(endedRecently.Id, x => x.EndedUtc, DateTime.UtcNow.AddDays(-1)); + //staff (refresh token kept) past the maximum lifetime + retention + var oldStaff = await Session(age: TimeSpan.FromHours(10) + TimeSpan.FromDays(8)); + //customer (no refresh token) past the cookie lifetime (24 h here) + retention + var oldCustomer = await Session(age: TimeSpan.FromHours(24) + TimeSpan.FromDays(8), refreshToken: null); + var openCustomer = await Session(age: TimeSpan.FromHours(30), refreshToken: null); + + await Service().DeleteExpired(); + + CollectionAssert.AreEquivalent(new[] { endedRecently.Id, openCustomer.Id }, + _sessions.Table.Select(x => x.Id).ToArray()); + } +} From 75267d2d60e37fc866c01a655f62605dc9b35a78 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Sun, 11 Oct 2026 22:23:10 +0200 Subject: [PATCH 04/15] OIDC sessions: idle and maximum lifetime, provider revalidation of back-office sessions --- .../Services/OidcSessionService.cs | 115 ++++++++- .../Services/OidcSessionServiceTests.cs | 218 +++++++++++++++++- 2 files changed, 329 insertions(+), 4 deletions(-) diff --git a/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs b/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs index 7e13feab09..1f2ab0706f 100644 --- a/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs +++ b/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs @@ -92,8 +92,119 @@ public virtual async Task Start(Customer customer, string subject, strin return session.Id; } - public virtual Task Validate(string sessionId, Customer customer) => - throw new NotImplementedException(); + public virtual async Task Validate(string sessionId, Customer customer) + { + if (string.IsNullOrEmpty(sessionId) || customer == null) + return false; + + var session = await Get(sessionId); + if (session == null || session.EndedUtc.HasValue || session.CustomerId != customer.Id) + return false; + + //back-office is evaluated now: an account promoted or demoted during the session follows its current role + if (!await _permissionService.IsBackOfficeAccount(customer)) + return true; + + var now = DateTime.UtcNow; + if (now - session.LastActivityUtc > _config.StaffIdleTimeout) + return await EndInvalid(session, OidcSessionEndReason.Idle); + if (now - session.CreatedUtc > _config.StaffMaxSession) + return await EndInvalid(session, OidcSessionEndReason.MaxLifetime); + if (string.IsNullOrEmpty(session.ProtectedRefreshToken)) + return await EndInvalid(session, OidcSessionEndReason.NoRefreshToken); + + if (now - session.LastValidatedUtc >= _config.StaffRevalidationInterval && !await Revalidate(session, now)) + return false; + + if (now - session.LastActivityUtc >= ActivityWriteInterval) + { + await _sessionRepository.UpdateField(session.Id, x => x.LastActivityUtc, now); + await _cache.RemoveAsync(string.Format(CacheKey.OIDC_SESSION_BY_ID_KEY, session.Id)); + } + + return true; + } + + private async Task EndInvalid(OidcSession session, OidcSessionEndReason reason) + { + await End(session.Id, reason); + return false; + } + + /// + /// Confirms the account at the provider. A refusal ends the session and refuses the subject's older access tokens; + /// no answer keeps the session until two intervals passed since the last confirmation + /// + private async Task Revalidate(OidcSession session, DateTime now) + { + var grace = _config.StaffRevalidationInterval * 2; + if (!RefreshesInProgress.TryAdd(session.Id, 0)) + return now - session.LastValidatedUtc < grace; + + try + { + var refresher = _tokenRefreshers.FirstOrDefault(); + var refreshToken = Unprotect(session.ProtectedRefreshToken); + OidcRefreshResult result; + if (refresher == null || refreshToken == null) + result = OidcRefreshResult.Refused(); + else + { + try + { + result = await refresher.Refresh(refreshToken); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + //a failing refresher is no answer from the provider + _logger.LogWarning("Refreshing session {SessionId} failed: {ExceptionType}", session.Id, ex.GetType().Name); + result = OidcRefreshResult.Unavailable(); + } + } + + switch (result.Outcome) + { + case OidcRefreshOutcome.Refreshed: + { + var update = UpdateBuilder.Create().Set(x => x.LastValidatedUtc, now); + if (!string.IsNullOrEmpty(result.RefreshToken)) + update.Set(x => x.ProtectedRefreshToken, _protector.Protect(result.RefreshToken)); + //conditional on the token read: a newer token stored meanwhile by another instance is never overwritten + await _sessionRepository.UpdateOneAsync( + x => x.Id == session.Id && x.ProtectedRefreshToken == session.ProtectedRefreshToken, update); + await _cache.RemoveAsync(string.Format(CacheKey.OIDC_SESSION_BY_ID_KEY, session.Id)); + return true; + } + case OidcRefreshOutcome.Refused: + { + //the token may have been rotated by another instance in the meantime: then the refusal is about the old one + var current = await _sessionRepository.GetByIdAsync(session.Id); + if (current is { EndedUtc: null } && current.ProtectedRefreshToken != session.ProtectedRefreshToken) + { + await _cache.RemoveAsync(string.Format(CacheKey.OIDC_SESSION_BY_ID_KEY, session.Id)); + return true; + } + + _logger.LogWarning("The identity provider refused the session {SessionId} of customer {CustomerId}", + session.Id, session.CustomerId); + await End(session.Id, OidcSessionEndReason.RevalidationRefused); + await _tokenRevocationService.RevokeTokens(session.Subject); + return false; + } + default: + _logger.LogWarning("The identity provider could not confirm session {SessionId}; last confirmed {LastValidatedUtc}", + session.Id, session.LastValidatedUtc); + if (now - session.LastValidatedUtc < grace) + return true; + await End(session.Id, OidcSessionEndReason.RevalidationUnavailable); + return false; + } + } + finally + { + RefreshesInProgress.TryRemove(session.Id, out _); + } + } public virtual async Task End(string sessionId, OidcSessionEndReason reason) { diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs index 6a817cc801..25e04ae563 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs @@ -164,14 +164,16 @@ public async Task EndBySubject_AllOpenSessionsOfSubject() [TestMethod] public async Task DeleteExpired_KeepsRecentAndOpen() { + //the staff rule alone must remove the old staff session: the customer lifetime is longer than its age + _securityConfig.CookieAuthExpires = 100; var endedLongAgo = await Session(); await _sessions.UpdateField(endedLongAgo.Id, x => x.EndedUtc, DateTime.UtcNow.AddDays(-8)); var endedRecently = await Session(); await _sessions.UpdateField(endedRecently.Id, x => x.EndedUtc, DateTime.UtcNow.AddDays(-1)); //staff (refresh token kept) past the maximum lifetime + retention var oldStaff = await Session(age: TimeSpan.FromHours(10) + TimeSpan.FromDays(8)); - //customer (no refresh token) past the cookie lifetime (24 h here) + retention - var oldCustomer = await Session(age: TimeSpan.FromHours(24) + TimeSpan.FromDays(8), refreshToken: null); + //customer (no refresh token) past the cookie lifetime (100 h here) + retention + var oldCustomer = await Session(age: TimeSpan.FromHours(100) + TimeSpan.FromDays(8), refreshToken: null); var openCustomer = await Session(age: TimeSpan.FromHours(30), refreshToken: null); await Service().DeleteExpired(); @@ -179,4 +181,216 @@ public async Task DeleteExpired_KeepsRecentAndOpen() CollectionAssert.AreEquivalent(new[] { endedRecently.Id, openCustomer.Id }, _sessions.Table.Select(x => x.Id).ToArray()); } + + private static readonly TimeSpan Interval = TimeSpan.FromMinutes(10); + + [TestMethod] + public async Task Validate_MissingOrEmptyId_False() + { + Assert.IsFalse(await Service().Validate("000000000000000000000000", _customer)); + Assert.IsFalse(await Service().Validate("", _customer)); + } + + [TestMethod] + public async Task Validate_Ended_False() + { + var session = await Session(); + await Service().End(session.Id, OidcSessionEndReason.BackChannel); + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + } + + //a cookie whose session claim was copied into another customer's ticket + [TestMethod] + public async Task Validate_OtherCustomer_False() + { + var session = await Session(customerId: "c2"); + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + } + + //customers keep the long cookie: no idle, maximum or provider check + [TestMethod] + public async Task Validate_Customer_OldAndIdle_True_NoRefresh() + { + var session = await Session(age: TimeSpan.FromDays(30), idle: TimeSpan.FromDays(20), + sinceValidated: TimeSpan.FromDays(30), refreshToken: null); + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + _refresher.Verify(x => x.Refresh(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task Validate_StaffIdle_Ends() + { + _staff = true; + var session = await Session(idle: TimeSpan.FromMinutes(31)); + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + Assert.AreEqual(OidcSessionEndReason.Idle, Stored(session.Id).EndReason); + } + + [TestMethod] + public async Task Validate_StaffPastMaximum_Ends() + { + _staff = true; + var session = await Session(age: TimeSpan.FromHours(10) + TimeSpan.FromMinutes(1)); + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + Assert.AreEqual(OidcSessionEndReason.MaxLifetime, Stored(session.Id).EndReason); + } + + //a customer promoted to staff during the session has no refresh token to check the account with + [TestMethod] + public async Task Validate_StaffWithoutRefreshToken_Ends() + { + _staff = true; + var session = await Session(refreshToken: null); + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + Assert.AreEqual(OidcSessionEndReason.NoRefreshToken, Stored(session.Id).EndReason); + } + + [TestMethod] + public async Task Validate_StaffWithinInterval_NoRefresh() + { + _staff = true; + var session = await Session(sinceValidated: TimeSpan.FromMinutes(9)); + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + _refresher.Verify(x => x.Refresh(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task Validate_StaffRefreshed_RotatedTokenStored() + { + _staff = true; + _refresher.Setup(x => x.Refresh("rt-1", It.IsAny())).ReturnsAsync(OidcRefreshResult.Refreshed("rt-2")); + var session = await Session(sinceValidated: Interval); + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + var stored = Stored(session.Id); + Assert.AreEqual("rt-2", Unprotect(stored.ProtectedRefreshToken)); + Assert.IsTrue(stored.LastValidatedUtc > DateTime.UtcNow.AddMinutes(-1)); + } + + [TestMethod] + public async Task Validate_StaffRefreshedWithoutRotation_OldTokenKept() + { + _staff = true; + _refresher.Setup(x => x.Refresh("rt-1", It.IsAny())).ReturnsAsync(OidcRefreshResult.Refreshed(null)); + var session = await Session(sinceValidated: Interval); + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + Assert.AreEqual("rt-1", Unprotect(Stored(session.Id).ProtectedRefreshToken)); + } + + [TestMethod] + public async Task Validate_StaffRefused_EndsAndRevokesTokens() + { + _staff = true; + _refresher.Setup(x => x.Refresh("rt-1", It.IsAny())).ReturnsAsync(OidcRefreshResult.Refused()); + var session = await Session(sinceValidated: Interval); + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + Assert.AreEqual(OidcSessionEndReason.RevalidationRefused, Stored(session.Id).EndReason); + _revocation.Verify(x => x.RevokeTokens("sub-1"), Times.Once); + } + + //another request (or instance) used the rotated token first and stored the new one: the refusal is about the old token + [TestMethod] + public async Task Refused_AfterAnotherRefreshStoredNewToken_StaysValid() + { + _staff = true; + var session = await Session(sinceValidated: Interval); + _refresher.Setup(x => x.Refresh("rt-1", It.IsAny())).Returns(async () => + { + await _sessions.UpdateField(session.Id, x => x.ProtectedRefreshToken, Protect("rt-2")); + return OidcRefreshResult.Refused(); + }); + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + Assert.IsNull(Stored(session.Id).EndedUtc); + _revocation.Verify(x => x.RevokeTokens(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task Validate_StaffUnavailable_WithinTwoIntervals_Valid() + { + _staff = true; + _refresher.Setup(x => x.Refresh(It.IsAny(), It.IsAny())).ReturnsAsync(OidcRefreshResult.Unavailable()); + var session = await Session(sinceValidated: Interval + TimeSpan.FromMinutes(5)); + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + Assert.IsNull(Stored(session.Id).EndedUtc); + } + + [TestMethod] + public async Task Validate_StaffUnavailable_PastTwoIntervals_Ends() + { + _staff = true; + _refresher.Setup(x => x.Refresh(It.IsAny(), It.IsAny())).ReturnsAsync(OidcRefreshResult.Unavailable()); + var session = await Session(sinceValidated: Interval * 2 + TimeSpan.FromMinutes(1)); + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + Assert.AreEqual(OidcSessionEndReason.RevalidationUnavailable, Stored(session.Id).EndReason); + _revocation.Verify(x => x.RevokeTokens(It.IsAny()), Times.Never); + } + + //a refresher that fails is no answer from the provider: the session is kept within the grace time + [TestMethod] + public async Task Validate_RefresherThrows_TreatedAsUnavailable() + { + _staff = true; + _refresher.Setup(x => x.Refresh(It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException("boom")); + var session = await Session(sinceValidated: Interval + TimeSpan.FromMinutes(5)); + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + Assert.IsNull(Stored(session.Id).EndedUtc); + _revocation.Verify(x => x.RevokeTokens(It.IsAny()), Times.Never); + } + + //the login plugin was uninstalled: staff sessions end at the next check, customers are not affected + [TestMethod] + public async Task NoRefresher_CountsAsRefused() + { + _staff = true; + _refreshers.Clear(); + var session = await Session(sinceValidated: Interval); + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + Assert.AreEqual(OidcSessionEndReason.RevalidationRefused, Stored(session.Id).EndReason); + } + + //a parallel request finds the refresh in progress and goes on while the last check is recent enough + [TestMethod] + public async Task RefreshInProgress_RecentCheck_Valid() + { + _staff = true; + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var started = new TaskCompletionSource(); + _refresher.Setup(x => x.Refresh(It.IsAny(), It.IsAny())) + .Returns(() => + { + started.TrySetResult(); + return release.Task; + }); + var session = await Session(sinceValidated: Interval); + + var first = Service().Validate(session.Id, _customer); + try + { + //the first request holds the refresh before the second one starts + await started.Task.WaitAsync(TimeSpan.FromSeconds(10)); + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + } + finally + { + //always released: the in-progress marker is process-wide and must not outlive the test + release.TrySetResult(OidcRefreshResult.Refreshed("rt-2")); + } + + Assert.IsTrue(await first); + _refresher.Verify(x => x.Refresh(It.IsAny(), It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task Validate_StaffActivity_WrittenAtMostOncePerMinute() + { + _staff = true; + var fresh = await Session(idle: TimeSpan.FromSeconds(10)); + var before = Stored(fresh.Id).LastActivityUtc; + Assert.IsTrue(await Service().Validate(fresh.Id, _customer)); + Assert.AreEqual(before, Stored(fresh.Id).LastActivityUtc); + + var stale = await Session(idle: TimeSpan.FromMinutes(2)); + Assert.IsTrue(await Service().Validate(stale.Id, _customer)); + Assert.IsTrue(Stored(stale.Id).LastActivityUtc > DateTime.UtcNow.AddMinutes(-1)); + } } From 954d53896de016a214939f5287cc1653c6a4905b Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Sun, 11 Oct 2026 22:26:38 +0200 Subject: [PATCH 05/15] OIDC sessions: revalidate from the stored record, keep newer tokens, treat refresher failures as no answer --- .../Services/OidcSessionService.cs | 33 ++++++++++---- .../Services/OidcSessionServiceTests.cs | 44 +++++++++++++++++++ 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs b/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs index 1f2ab0706f..85d6d57f36 100644 --- a/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs +++ b/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs @@ -135,28 +135,41 @@ private async Task EndInvalid(OidcSession session, OidcSessionEndReason re /// Confirms the account at the provider. A refusal ends the session and refuses the subject's older access tokens; /// no answer keeps the session until two intervals passed since the last confirmation /// - private async Task Revalidate(OidcSession session, DateTime now) + private async Task Revalidate(OidcSession cached, DateTime now) { var grace = _config.StaffRevalidationInterval * 2; - if (!RefreshesInProgress.TryAdd(session.Id, 0)) - return now - session.LastValidatedUtc < grace; + if (!RefreshesInProgress.TryAdd(cached.Id, 0)) + return now - cached.LastValidatedUtc < grace; try { + //the cached record may predate a refresh that just finished: its token is already rotated + var session = await _sessionRepository.GetByIdAsync(cached.Id); + if (session == null || session.EndedUtc.HasValue) + return false; + if (now - session.LastValidatedUtc < _config.StaffRevalidationInterval) + return true; + var refresher = _tokenRefreshers.FirstOrDefault(); var refreshToken = Unprotect(session.ProtectedRefreshToken); OidcRefreshResult result; + var unreadable = refresher != null && refreshToken == null; if (refresher == null || refreshToken == null) + { + if (unreadable) + _logger.LogWarning("The stored refresh token of session {SessionId} cannot be read (Data Protection keys changed)", + session.Id); result = OidcRefreshResult.Refused(); + } else { try { result = await refresher.Refresh(refreshToken); } - catch (Exception ex) when (ex is not OperationCanceledException) + catch (Exception ex) { - //a failing refresher is no answer from the provider + //no cancellation token is passed, so a cancellation is a provider timeout: no answer, like any failure _logger.LogWarning("Refreshing session {SessionId} failed: {ExceptionType}", session.Id, ex.GetType().Name); result = OidcRefreshResult.Unavailable(); } @@ -171,7 +184,8 @@ private async Task Revalidate(OidcSession session, DateTime now) update.Set(x => x.ProtectedRefreshToken, _protector.Protect(result.RefreshToken)); //conditional on the token read: a newer token stored meanwhile by another instance is never overwritten await _sessionRepository.UpdateOneAsync( - x => x.Id == session.Id && x.ProtectedRefreshToken == session.ProtectedRefreshToken, update); + x => x.Id == session.Id && x.EndedUtc == null && x.ProtectedRefreshToken == session.ProtectedRefreshToken, + update); await _cache.RemoveAsync(string.Format(CacheKey.OIDC_SESSION_BY_ID_KEY, session.Id)); return true; } @@ -185,8 +199,9 @@ await _sessionRepository.UpdateOneAsync( return true; } - _logger.LogWarning("The identity provider refused the session {SessionId} of customer {CustomerId}", - session.Id, session.CustomerId); + if (!unreadable) + _logger.LogWarning("The identity provider refused the session {SessionId} of customer {CustomerId}", + session.Id, session.CustomerId); await End(session.Id, OidcSessionEndReason.RevalidationRefused); await _tokenRevocationService.RevokeTokens(session.Subject); return false; @@ -202,7 +217,7 @@ await _sessionRepository.UpdateOneAsync( } finally { - RefreshesInProgress.TryRemove(session.Id, out _); + RefreshesInProgress.TryRemove(cached.Id, out _); } } diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs index 25e04ae563..6fb5a300a0 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs @@ -337,6 +337,49 @@ public async Task Validate_RefresherThrows_TreatedAsUnavailable() _revocation.Verify(x => x.RevokeTokens(It.IsAny()), Times.Never); } + //a request holding a cached record from before another request's refresh must not use the already rotated token + [TestMethod] + public async Task Validate_CachedRecordStale_StoredRecentlyValidated_NoRefresh() + { + _staff = true; + var session = await Session(sinceValidated: Interval + TimeSpan.FromMinutes(1)); + var stale = Stored(session.Id); + await _sessions.UpdateField(session.Id, x => x.LastValidatedUtc, DateTime.UtcNow); + await _sessions.UpdateField(session.Id, x => x.ProtectedRefreshToken, Protect("rt-2")); + _cache.Setup(x => x.GetAsync(It.IsAny(), It.IsAny>>(), It.IsAny())) + .ReturnsAsync(stale); + + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + _refresher.Verify(x => x.Refresh(It.IsAny(), It.IsAny()), Times.Never); + } + + //the token was replaced by another instance while this refresh ran: its token is never overwritten + [TestMethod] + public async Task Refreshed_AfterAnotherRefreshStoredNewToken_NewerTokenKept() + { + _staff = true; + var session = await Session(sinceValidated: Interval); + _refresher.Setup(x => x.Refresh("rt-1", It.IsAny())).Returns(async () => + { + await _sessions.UpdateField(session.Id, x => x.ProtectedRefreshToken, Protect("rt-X")); + return OidcRefreshResult.Refreshed("rt-2"); + }); + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + Assert.AreEqual("rt-X", Unprotect(Stored(session.Id).ProtectedRefreshToken)); + } + + //no cancellation token is passed, so a cancellation is a provider timeout + [TestMethod] + public async Task Validate_RefresherCancelled_TreatedAsUnavailable() + { + _staff = true; + _refresher.Setup(x => x.Refresh(It.IsAny(), It.IsAny())) + .ThrowsAsync(new TaskCanceledException()); + var session = await Session(sinceValidated: Interval + TimeSpan.FromMinutes(5)); + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + Assert.IsNull(Stored(session.Id).EndedUtc); + } + //the login plugin was uninstalled: staff sessions end at the next check, customers are not affected [TestMethod] public async Task NoRefresher_CountsAsRefused() @@ -346,6 +389,7 @@ public async Task NoRefresher_CountsAsRefused() var session = await Session(sinceValidated: Interval); Assert.IsFalse(await Service().Validate(session.Id, _customer)); Assert.AreEqual(OidcSessionEndReason.RevalidationRefused, Stored(session.Id).EndReason); + _revocation.Verify(x => x.RevokeTokens("sub-1"), Times.Once); } //a parallel request finds the refresh in progress and goes on while the last check is recent enough From 53867426f8c6ac79891b17a7b4d359c6a44d030a Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 06:38:36 +0200 Subject: [PATCH 06/15] OIDC sessions: the authentication cookie references the server-side session --- .../Services/CookieAuthenticationService.cs | 41 +++++++- .../IGrandAuthenticationService.cs | 9 ++ .../CookieAuthenticationServiceTests.cs | 97 ++++++++++++++++++- 3 files changed, 143 insertions(+), 4 deletions(-) diff --git a/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs b/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs index 508e1e1b6a..2dae116e7a 100644 --- a/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs +++ b/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs @@ -26,13 +26,15 @@ public class CookieAuthenticationService : IGrandAuthenticationService /// Group service /// HTTP context accessor /// SecurityConfig + /// Server-side sessions of provider sign-ins public CookieAuthenticationService( CustomerSettings customerSettings, ICustomerService customerService, IGroupService groupService, IHttpContextAccessor httpContextAccessor, ICookieOptionsFactory cookieOptionsFactory, - SecurityConfig securityConfig) + SecurityConfig securityConfig, + IOidcSessionService oidcSessionService) { _customerSettings = customerSettings; _customerService = customerService; @@ -40,6 +42,7 @@ public CookieAuthenticationService( _httpContextAccessor = httpContextAccessor; _cookieOptionsFactory = cookieOptionsFactory; _securityConfig = securityConfig; + _oidcSessionService = oidcSessionService; } #endregion @@ -63,6 +66,7 @@ public CookieAuthenticationService( private readonly ICookieOptionsFactory _cookieOptionsFactory; private readonly SecurityConfig _securityConfig; + private readonly IOidcSessionService _oidcSessionService; #endregion @@ -73,7 +77,18 @@ public CookieAuthenticationService( /// /// Customer /// Whether the authentication session is persisted across multiple requests - public virtual async Task SignIn(Customer customer, bool isPersistent) + public virtual Task SignIn(Customer customer, bool isPersistent) + { + return SignIn(customer, isPersistent, null); + } + + /// + /// Sign in with a session started at the identity provider + /// + /// Customer + /// Whether the authentication session is persisted across multiple requests + /// Id of the OidcSession; null or empty = an ordinary sign-in + public virtual async Task SignIn(Customer customer, bool isPersistent, string oidcSessionId) { ArgumentNullException.ThrowIfNull(customer); @@ -109,6 +124,11 @@ public virtual async Task SignIn(Customer customer, bool isPersistent) _securityConfig.CookieClaimsIssuer)); } + //the server-side session of a provider sign-in: ending it ends this cookie + if (!string.IsNullOrEmpty(oidcSessionId)) + claims.Add(new Claim(OidcSessionDefaults.SessionIdClaimType, oidcSessionId, ClaimValueTypes.String, + _securityConfig.CookieClaimsIssuer)); + //create principal for the present scheme of authentication var userIdentity = new ClaimsIdentity(claims, GrandCookieAuthenticationDefaults.AuthenticationScheme); var userPrincipal = new ClaimsPrincipal(userIdentity); @@ -137,6 +157,12 @@ public virtual async Task SignOut() //and then sign out customer from the present scheme of authentication if (_httpContextAccessor.HttpContext != null) { + var authenticateResult = await _httpContextAccessor.HttpContext.AuthenticateAsync( + GrandCookieAuthenticationDefaults.AuthenticationScheme); + var oidcSessionId = authenticateResult?.Succeeded == true ? OidcSessionId(authenticateResult.Principal) : null; + if (!string.IsNullOrEmpty(oidcSessionId)) + await _oidcSessionService.End(oidcSessionId, OidcSessionEndReason.SignOut); + await _httpContextAccessor.HttpContext.SignOutAsync(GrandCookieAuthenticationDefaults .AuthenticationScheme); //sign out also from other schema @@ -215,8 +241,19 @@ private async Task IsValidCustomer(Customer customer, ClaimsPrincipal prin if (!customer.Active || customer.Deleted || !await _groupService.IsRegistered(customer)) return false; + //a provider sign-in: the server-side session decides (ended, idle, refused by the provider) + var oidcSessionId = OidcSessionId(principal); + if (!string.IsNullOrEmpty(oidcSessionId) && !await _oidcSessionService.Validate(oidcSessionId, customer)) + return false; + return true; } + + private string OidcSessionId(ClaimsPrincipal principal) => + principal?.FindFirst(claim => + claim.Type == OidcSessionDefaults.SessionIdClaimType && + claim.Issuer.Equals(_securityConfig.CookieClaimsIssuer, StringComparison.InvariantCultureIgnoreCase))?.Value; + /// /// Get customer cookie /// diff --git a/src/Business/Grand.Business.Core/Interfaces/Authentication/IGrandAuthenticationService.cs b/src/Business/Grand.Business.Core/Interfaces/Authentication/IGrandAuthenticationService.cs index 3fe8086a40..71e0c1b4c3 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Authentication/IGrandAuthenticationService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Authentication/IGrandAuthenticationService.cs @@ -14,6 +14,15 @@ public interface IGrandAuthenticationService /// A value indicating whether to create a persistent cookie Task SignIn(Customer customer, bool createPersistentCookie); + /// + /// Sign in with a session started at the identity provider; the cookie references the session, so it can be ended + /// on the server + /// + /// Customer + /// A value indicating whether to create a persistent cookie + /// Id of the OidcSession; null or empty = an ordinary sign-in + Task SignIn(Customer customer, bool createPersistentCookie, string oidcSessionId); + /// /// Sign out /// diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs index 2599bcbd9c..63c24f6c15 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs @@ -1,7 +1,9 @@ using Grand.Business.Authentication.Services; +using Grand.Business.Core.Interfaces.Authentication; using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Customers; using Grand.Business.Core.Utilities.Authentication; +using Grand.Domain.Common; using Grand.Domain.Customers; using Grand.Infrastructure.Configuration; using Microsoft.AspNetCore.Authentication; @@ -25,6 +27,7 @@ public class CookieAuthenticationServiceTests private DefaultHttpContext _httpContext; private Mock serviceProviderMock; private CookieOptionsFactory _cookieOptionsFactory; + private Mock _oidcSessionService; [TestInitialize] public void Init() @@ -39,8 +42,10 @@ public void Init() CookiePrefix = ".Grand." }; _cookieOptionsFactory = new CookieOptionsFactory(_config); + _oidcSessionService = new Mock(); _cookieAuthService = new CookieAuthenticationService(_customerSettings, _customerServiceMock.Object, - _groupServiceMock.Object, _httpAccessorMock.Object, _cookieOptionsFactory, _config); + _groupServiceMock.Object, _httpAccessorMock.Object, _cookieOptionsFactory, _config, + _oidcSessionService.Object); //For mock HttpContext extension methods like SignOutAsync ,SignInAsync etc.. _authServiceMock = new Mock(); serviceProviderMock = new Mock(); @@ -155,4 +160,92 @@ public async Task GetAuthenticatedCustomer_UsernameEnableGuests_ReturnNull() Assert.IsNull(customer); //_customerServiceMock.Verify(c => c.GetCustomerByUsername(It.IsAny()), Times.Once); } -} \ No newline at end of file + + private static AuthenticateResult Ticket(params Claim[] extra) + { + var claims = new List { + new("grand:customerId", "c1", ClaimValueTypes.String, "grandnode"), + new(ClaimTypes.UserData, "token", ClaimValueTypes.String, "grandnode") + }; + claims.AddRange(extra); + return AuthenticateResult.Success(new AuthenticationTicket( + new ClaimsPrincipal(new ClaimsIdentity(claims, GrandCookieAuthenticationDefaults.AuthenticationScheme)), + GrandCookieAuthenticationDefaults.AuthenticationScheme)); + } + + private Customer RegisteredCustomer() + { + var customer = new Customer { Id = "c1", Active = true }; + customer.UserFields.Add(new UserField { Key = SystemCustomerFieldNames.PasswordToken, Value = "token", StoreId = "" }); + _customerServiceMock.Setup(x => x.GetCustomerById("c1")).ReturnsAsync(customer); + _groupServiceMock.Setup(x => x.IsRegistered(customer)).ReturnsAsync(true); + return customer; + } + + [TestMethod] + public async Task SignIn_WithOidcSession_AddsClaim() + { + ClaimsPrincipal signedIn = null; + _authServiceMock.Setup(x => x.SignInAsync(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny())) + .Callback((HttpContext _, string _, ClaimsPrincipal p, AuthenticationProperties _) => signedIn = p) + .Returns(Task.CompletedTask); + var customer = new Customer { Id = "c1", Email = "a@b.c" }; + customer.UserFields.Add(new UserField { Key = SystemCustomerFieldNames.PasswordToken, Value = "token", StoreId = "" }); + + await _cookieAuthService.SignIn(customer, false, "session-1"); + + var claim = signedIn.FindFirst(OidcSessionDefaults.SessionIdClaimType); + Assert.AreEqual("session-1", claim.Value); + Assert.AreEqual("grandnode", claim.Issuer); + } + + [TestMethod] + public async Task GetAuthenticatedCustomer_NoSessionClaim_SessionServiceNotAsked() + { + var customer = RegisteredCustomer(); + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), It.IsAny())).ReturnsAsync(Ticket()); + Assert.AreSame(customer, await _cookieAuthService.GetAuthenticatedCustomer()); + _oidcSessionService.Verify(x => x.Validate(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task GetAuthenticatedCustomer_EndedSession_Null() + { + var customer = RegisteredCustomer(); + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(Ticket(new Claim(OidcSessionDefaults.SessionIdClaimType, "session-1", ClaimValueTypes.String, "grandnode"))); + _oidcSessionService.Setup(x => x.Validate("session-1", customer)).ReturnsAsync(false); + Assert.IsNull(await _cookieAuthService.GetAuthenticatedCustomer()); + } + + [TestMethod] + public async Task GetAuthenticatedCustomer_ValidSession_ReturnCustomer() + { + var customer = RegisteredCustomer(); + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(Ticket(new Claim(OidcSessionDefaults.SessionIdClaimType, "session-1", ClaimValueTypes.String, "grandnode"))); + _oidcSessionService.Setup(x => x.Validate("session-1", customer)).ReturnsAsync(true); + Assert.AreSame(customer, await _cookieAuthService.GetAuthenticatedCustomer()); + } + + //a claim not issued by the cookie (another issuer) is not a session reference + [TestMethod] + public async Task GetAuthenticatedCustomer_SessionClaimOfOtherIssuer_Ignored() + { + var customer = RegisteredCustomer(); + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(Ticket(new Claim(OidcSessionDefaults.SessionIdClaimType, "session-1", ClaimValueTypes.String, "other"))); + Assert.AreSame(customer, await _cookieAuthService.GetAuthenticatedCustomer()); + _oidcSessionService.Verify(x => x.Validate(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task SignOut_EndsOidcSession() + { + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme)) + .ReturnsAsync(Ticket(new Claim(OidcSessionDefaults.SessionIdClaimType, "session-1", ClaimValueTypes.String, "grandnode"))); + await _cookieAuthService.SignOut(); + _oidcSessionService.Verify(x => x.End("session-1", OidcSessionEndReason.SignOut), Times.Once); + } +} From b79ac224def8277e41f8c6b36189c57f59c4fc7e Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 06:47:57 +0200 Subject: [PATCH 07/15] OIDC sessions: cleanup task, indexes and 2.5 migration --- .../Services/InstallDataScheduleTasks.cs | 6 ++ .../Services/InstallationService.cs | 13 +++++ .../Migrations/2.5/MigrationOidcSessions.cs | 56 +++++++++++++++++++ .../DeleteOidcSessionsScheduleTask.cs | 12 ++++ .../Startup/StartupApplication.cs | 1 + .../DeleteOidcSessionsScheduleTaskTests.cs | 18 ++++++ 6 files changed, 106 insertions(+) create mode 100644 src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationOidcSessions.cs create mode 100644 src/Modules/Grand.Module.ScheduledTasks/BackgroundServices/DeleteOidcSessionsScheduleTask.cs create mode 100644 src/Tests/Grand.Modules.Tests/Services/BackgroundService/DeleteOidcSessionsScheduleTaskTests.cs diff --git a/src/Modules/Grand.Module.Installer/Services/InstallDataScheduleTasks.cs b/src/Modules/Grand.Module.Installer/Services/InstallDataScheduleTasks.cs index c5ec64e539..5837de4c14 100644 --- a/src/Modules/Grand.Module.Installer/Services/InstallDataScheduleTasks.cs +++ b/src/Modules/Grand.Module.Installer/Services/InstallDataScheduleTasks.cs @@ -51,6 +51,12 @@ protected virtual Task InstallScheduleTasks() Enabled = false, StopOnError = false, TimeInterval = 1440 + }, + new() { + ScheduleTaskName = "Delete OpenID Connect sessions", + Enabled = true, + StopOnError = false, + TimeInterval = 1440 } }; tasks.ForEach(x => _scheduleTaskRepository.Insert(x)); diff --git a/src/Modules/Grand.Module.Installer/Services/InstallationService.cs b/src/Modules/Grand.Module.Installer/Services/InstallationService.cs index 10f5fc5ca2..e61e4557ba 100644 --- a/src/Modules/Grand.Module.Installer/Services/InstallationService.cs +++ b/src/Modules/Grand.Module.Installer/Services/InstallationService.cs @@ -122,6 +122,7 @@ public InstallationService( IRepository permissionRepository, IRepository permissionAction, IRepository externalAuthenticationRepository, + IRepository oidcSessionRepository, IRepository merchandiseReturnReasonRepository, IRepository merchandiseReturnActionRepository, IRepository contactUsRepository, @@ -218,6 +219,7 @@ public InstallationService( _permissionAction = permissionAction; _vendorRepository = vendorRepository; _externalAuthenticationRepository = externalAuthenticationRepository; + _oidcSessionRepository = oidcSessionRepository; _discountusageRepository = discountusageRepository; _merchandiseReturnReasonRepository = merchandiseReturnReasonRepository; _contactUsRepository = contactUsRepository; @@ -384,6 +386,7 @@ public virtual async Task InstallData(string httpscheme, HostString host, string private readonly IRepository _permissionRepository; private readonly IRepository _permissionAction; private readonly IRepository _externalAuthenticationRepository; + private readonly IRepository _oidcSessionRepository; private readonly IRepository _merchandiseReturnReasonRepository; private readonly IRepository _merchandiseReturnActionRepository; private readonly IRepository _contactUsRepository; @@ -918,6 +921,16 @@ await dbContext.CreateIndex(_externalAuthenticationRepository, .Ascending(x => x.ExternalIdentifier).Ascending(x => x.StoreId), "ProviderSystemName_ExternalIdentifier_StoreId", true); + //OpenID Connect sessions: back-channel logout looks up by sid and subject, the cleanup by end time + await dbContext.CreateIndex(_oidcSessionRepository, + OrderBuilder.Create().Ascending(x => x.Sid), "Sid"); + await dbContext.CreateIndex(_oidcSessionRepository, + OrderBuilder.Create().Ascending(x => x.Subject), "Subject"); + await dbContext.CreateIndex(_oidcSessionRepository, + OrderBuilder.Create().Ascending(x => x.CustomerId), "CustomerId"); + await dbContext.CreateIndex(_oidcSessionRepository, + OrderBuilder.Create().Ascending(x => x.EndedUtc), "EndedUtc"); + //merchandise return await dbContext.CreateIndex(_merchandiseReturnRepository, OrderBuilder.Create().Ascending(x => x.ReturnNumber), "ReturnNumber", true); diff --git a/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationOidcSessions.cs b/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationOidcSessions.cs new file mode 100644 index 0000000000..a30c7073c1 --- /dev/null +++ b/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationOidcSessions.cs @@ -0,0 +1,56 @@ +using Grand.Data; +using Grand.Domain.Customers; +using Grand.Domain.Tasks; +using Grand.Infrastructure.Migrations; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Grand.Module.Migration.Migrations._2._5; + +public class MigrationOidcSessions : IMigration +{ + private const string TaskName = "Delete OpenID Connect sessions"; + + public int Priority => 2; + public DbVersion Version => new(2, 5); + public Guid Identity => new("3f6a9e21-7c4b-4d0e-9b8a-5e2c1d7f4a63"); + public string Name => "OpenID Connect sessions: indexes and cleanup task"; + + public bool UpgradeProcess(IServiceProvider serviceProvider) + { + var sessionRepository = serviceProvider.GetRequiredService>(); + var taskRepository = serviceProvider.GetRequiredService>(); + var databaseContext = serviceProvider.GetRequiredService(); + var logService = serviceProvider.GetRequiredService>(); + + try + { + databaseContext.CreateIndex(sessionRepository, + OrderBuilder.Create().Ascending(x => x.Sid), "Sid").GetAwaiter().GetResult(); + databaseContext.CreateIndex(sessionRepository, + OrderBuilder.Create().Ascending(x => x.Subject), "Subject").GetAwaiter().GetResult(); + databaseContext.CreateIndex(sessionRepository, + OrderBuilder.Create().Ascending(x => x.CustomerId), "CustomerId").GetAwaiter().GetResult(); + databaseContext.CreateIndex(sessionRepository, + OrderBuilder.Create().Ascending(x => x.EndedUtc), "EndedUtc").GetAwaiter().GetResult(); + } + catch (Exception ex) + { + logService.LogError(ex, "UpgradeProcess - OidcSessions indexes (2.5)"); + } + + try + { + if (!taskRepository.Table.Any(x => x.ScheduleTaskName == TaskName)) + taskRepository.Insert(new ScheduleTask { + ScheduleTaskName = TaskName, Enabled = true, StopOnError = false, TimeInterval = 1440 + }); + } + catch (Exception ex) + { + logService.LogError(ex, "UpgradeProcess - OidcSessions schedule task (2.5)"); + } + + return true; + } +} diff --git a/src/Modules/Grand.Module.ScheduledTasks/BackgroundServices/DeleteOidcSessionsScheduleTask.cs b/src/Modules/Grand.Module.ScheduledTasks/BackgroundServices/DeleteOidcSessionsScheduleTask.cs new file mode 100644 index 0000000000..c004f06685 --- /dev/null +++ b/src/Modules/Grand.Module.ScheduledTasks/BackgroundServices/DeleteOidcSessionsScheduleTask.cs @@ -0,0 +1,12 @@ +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Business.Core.Interfaces.System.ScheduleTasks; + +namespace Grand.Module.ScheduledTasks.BackgroundServices; + +/// +/// Deletes the records of ended OpenID Connect sessions and of sessions past their lifetime +/// +public class DeleteOidcSessionsScheduleTask(IOidcSessionService oidcSessionService) : IScheduleTask +{ + public Task Execute() => oidcSessionService.DeleteExpired(); +} diff --git a/src/Modules/Grand.Module.ScheduledTasks/Startup/StartupApplication.cs b/src/Modules/Grand.Module.ScheduledTasks/Startup/StartupApplication.cs index 6f620d782f..894d5b4dcc 100644 --- a/src/Modules/Grand.Module.ScheduledTasks/Startup/StartupApplication.cs +++ b/src/Modules/Grand.Module.ScheduledTasks/Startup/StartupApplication.cs @@ -30,6 +30,7 @@ private static void RegisterTask(IServiceCollection serviceCollection) serviceCollection.AddKeyedScoped("Clear cache"); serviceCollection.AddKeyedScoped("Generate sitemap XML file"); serviceCollection.AddKeyedScoped("Delete guests"); + serviceCollection.AddKeyedScoped("Delete OpenID Connect sessions"); serviceCollection.AddKeyedScoped("Update currency exchange rates"); serviceCollection.AddKeyedScoped("End of the auctions"); serviceCollection.AddKeyedScoped("Cancel unpaid and pending orders"); diff --git a/src/Tests/Grand.Modules.Tests/Services/BackgroundService/DeleteOidcSessionsScheduleTaskTests.cs b/src/Tests/Grand.Modules.Tests/Services/BackgroundService/DeleteOidcSessionsScheduleTaskTests.cs new file mode 100644 index 0000000000..951817977c --- /dev/null +++ b/src/Tests/Grand.Modules.Tests/Services/BackgroundService/DeleteOidcSessionsScheduleTaskTests.cs @@ -0,0 +1,18 @@ +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Module.ScheduledTasks.BackgroundServices; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Modules.Tests.Services.BackgroundService; + +[TestClass] +public class DeleteOidcSessionsScheduleTaskTests +{ + [TestMethod] + public async Task Execute_DeletesExpiredSessions() + { + var sessions = new Mock(); + await new DeleteOidcSessionsScheduleTask(sessions.Object).Execute(); + sessions.Verify(x => x.DeleteExpired(), Times.Once); + } +} From e5eb2014c036f62f994aad78834c120614f8bc70 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 09:15:49 +0200 Subject: [PATCH 08/15] OIDC sessions: keep the session claim when the same customer is signed in again A re-sign-in after a password, e-mail or username change used the two-argument SignIn, which dropped grand:oidcSession, so the new cookie escaped idle and maximum lifetime, revalidation and back-channel logout. The two-argument SignIn now carries the session of the current cookie over when it belongs to the same customer. --- .../Services/CookieAuthenticationService.cs | 25 +++++- .../CookieAuthenticationServiceTests.cs | 87 +++++++++++++++++++ 2 files changed, 110 insertions(+), 2 deletions(-) diff --git a/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs b/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs index 2dae116e7a..6191c917d7 100644 --- a/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs +++ b/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs @@ -77,9 +77,30 @@ public CookieAuthenticationService( /// /// Customer /// Whether the authentication session is persisted across multiple requests - public virtual Task SignIn(Customer customer, bool isPersistent) + public virtual async Task SignIn(Customer customer, bool isPersistent) { - return SignIn(customer, isPersistent, null); + ArgumentNullException.ThrowIfNull(customer); + + //a re-sign-in of the same customer (password, e-mail or username change) keeps the server-side + //session of a provider sign-in, so idle/max lifetime, revalidation and back-channel logout still apply + await SignIn(customer, isPersistent, await CurrentOidcSessionIdOf(customer)); + } + + private async Task CurrentOidcSessionIdOf(Customer customer) + { + if (_httpContextAccessor.HttpContext == null || string.IsNullOrEmpty(customer.Id)) + return null; + + var authenticateResult = await _httpContextAccessor.HttpContext.AuthenticateAsync( + GrandCookieAuthenticationDefaults.AuthenticationScheme); + if (authenticateResult?.Succeeded != true) + return null; + + var customerId = authenticateResult.Principal?.FindFirst(claim => + claim.Type == CustomerIdClaimType && + claim.Issuer.Equals(_securityConfig.CookieClaimsIssuer, StringComparison.InvariantCultureIgnoreCase))?.Value; + + return customerId == customer.Id ? OidcSessionId(authenticateResult.Principal) : null; } /// diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs index 63c24f6c15..88e0e90299 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs @@ -248,4 +248,91 @@ public async Task SignOut_EndsOidcSession() await _cookieAuthService.SignOut(); _oidcSessionService.Verify(x => x.End("session-1", OidcSessionEndReason.SignOut), Times.Once); } + + private (Customer customer, Func signedIn) PrepareSignIn(string customerId) + { + ClaimsPrincipal signedIn = null; + _authServiceMock.Setup(x => x.SignInAsync(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny())) + .Callback((HttpContext _, string _, ClaimsPrincipal p, AuthenticationProperties _) => signedIn = p) + .Returns(Task.CompletedTask); + var customer = new Customer { Id = customerId, Email = "a@b.c" }; + customer.UserFields.Add(new UserField { Key = SystemCustomerFieldNames.PasswordToken, Value = "token", StoreId = "" }); + return (customer, () => signedIn); + } + + //re-sign-in (password, e-mail or username change) keeps the session of a provider sign-in + [TestMethod] + public async Task SignIn_WithoutSessionId_SameCustomerCookieWithSession_CarriesSessionOver() + { + var (customer, signedIn) = PrepareSignIn("c1"); + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme)) + .ReturnsAsync(Ticket(new Claim(OidcSessionDefaults.SessionIdClaimType, "session-1", ClaimValueTypes.String, "grandnode"))); + + await _cookieAuthService.SignIn(customer, false); + + var claim = signedIn().FindFirst(OidcSessionDefaults.SessionIdClaimType); + Assert.IsNotNull(claim); + Assert.AreEqual("session-1", claim.Value); + Assert.AreEqual("grandnode", claim.Issuer); + } + + [TestMethod] + public async Task SignIn_WithoutSessionId_OtherCustomerCookieWithSession_NoCarryOver() + { + var (customer, signedIn) = PrepareSignIn("c2"); + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme)) + .ReturnsAsync(Ticket(new Claim(OidcSessionDefaults.SessionIdClaimType, "session-1", ClaimValueTypes.String, "grandnode"))); + + await _cookieAuthService.SignIn(customer, false); + + Assert.IsNull(signedIn().FindFirst(OidcSessionDefaults.SessionIdClaimType)); + } + + [TestMethod] + public async Task SignIn_WithoutSessionId_SessionClaimOfOtherIssuer_NoCarryOver() + { + var (customer, signedIn) = PrepareSignIn("c1"); + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme)) + .ReturnsAsync(Ticket(new Claim(OidcSessionDefaults.SessionIdClaimType, "session-1", ClaimValueTypes.String, "other"))); + + await _cookieAuthService.SignIn(customer, false); + + Assert.IsNull(signedIn().FindFirst(OidcSessionDefaults.SessionIdClaimType)); + } + + [TestMethod] + public async Task SignIn_WithoutSessionId_NoCurrentCookie_NoClaim() + { + var (customer, signedIn) = PrepareSignIn("c1"); + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme)) + .ReturnsAsync(AuthenticateResult.NoResult()); + + await _cookieAuthService.SignIn(customer, false); + + Assert.IsNull(signedIn().FindFirst(OidcSessionDefaults.SessionIdClaimType)); + } + + [TestMethod] + public async Task SignIn_WithoutSessionId_CurrentCookieWithoutSession_NoClaim() + { + var (customer, signedIn) = PrepareSignIn("c1"); + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme)) + .ReturnsAsync(Ticket()); + + await _cookieAuthService.SignIn(customer, false); + + Assert.IsNull(signedIn().FindFirst(OidcSessionDefaults.SessionIdClaimType)); + } + + [TestMethod] + public async Task SignOut_NoSessionClaim_EndNeverCalled() + { + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme)) + .ReturnsAsync(Ticket()); + await _cookieAuthService.SignOut(); + _oidcSessionService.Verify(x => x.End(It.IsAny(), It.IsAny()), Times.Never); + _authServiceMock.Verify(x => x.SignOutAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme, + It.IsAny()), Times.Once); + } } From a83a73e4afe6f59fbe70c7677a755ae6b5610a58 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 09:17:19 +0200 Subject: [PATCH 09/15] OIDC sessions: sign out even when the session record cannot be ended If ending the server-side session threw, SignOutAsync never ran and the customer stayed signed in. The failure is now logged as a warning with the session id and both schemes are always signed out. --- .../Services/CookieAuthenticationService.cs | 17 ++++++++++++++-- .../CookieAuthenticationServiceTests.cs | 20 ++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs b/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs index 6191c917d7..b89402becd 100644 --- a/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs +++ b/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs @@ -7,6 +7,7 @@ using Grand.Infrastructure.Configuration; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging; using System.Security.Claims; namespace Grand.Business.Authentication.Services; @@ -27,6 +28,7 @@ public class CookieAuthenticationService : IGrandAuthenticationService /// HTTP context accessor /// SecurityConfig /// Server-side sessions of provider sign-ins + /// Logger public CookieAuthenticationService( CustomerSettings customerSettings, ICustomerService customerService, @@ -34,7 +36,8 @@ public CookieAuthenticationService( IHttpContextAccessor httpContextAccessor, ICookieOptionsFactory cookieOptionsFactory, SecurityConfig securityConfig, - IOidcSessionService oidcSessionService) + IOidcSessionService oidcSessionService, + ILogger logger) { _customerSettings = customerSettings; _customerService = customerService; @@ -43,6 +46,7 @@ public CookieAuthenticationService( _cookieOptionsFactory = cookieOptionsFactory; _securityConfig = securityConfig; _oidcSessionService = oidcSessionService; + _logger = logger; } #endregion @@ -67,6 +71,7 @@ public CookieAuthenticationService( private readonly SecurityConfig _securityConfig; private readonly IOidcSessionService _oidcSessionService; + private readonly ILogger _logger; #endregion @@ -182,7 +187,15 @@ public virtual async Task SignOut() GrandCookieAuthenticationDefaults.AuthenticationScheme); var oidcSessionId = authenticateResult?.Succeeded == true ? OidcSessionId(authenticateResult.Principal) : null; if (!string.IsNullOrEmpty(oidcSessionId)) - await _oidcSessionService.End(oidcSessionId, OidcSessionEndReason.SignOut); + try + { + await _oidcSessionService.End(oidcSessionId, OidcSessionEndReason.SignOut); + } + catch (Exception ex) + { + //the cookie is removed below anyway; the session ends by idle or maximum lifetime + _logger.LogWarning(ex, "Could not end the OpenID Connect session {SessionId} on sign-out", oidcSessionId); + } await _httpContextAccessor.HttpContext.SignOutAsync(GrandCookieAuthenticationDefaults .AuthenticationScheme); diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs index 88e0e90299..d21c520b00 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs @@ -8,6 +8,7 @@ using Grand.Infrastructure.Configuration; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using System.Security.Claims; @@ -45,7 +46,7 @@ public void Init() _oidcSessionService = new Mock(); _cookieAuthService = new CookieAuthenticationService(_customerSettings, _customerServiceMock.Object, _groupServiceMock.Object, _httpAccessorMock.Object, _cookieOptionsFactory, _config, - _oidcSessionService.Object); + _oidcSessionService.Object, NullLogger.Instance); //For mock HttpContext extension methods like SignOutAsync ,SignInAsync etc.. _authServiceMock = new Mock(); serviceProviderMock = new Mock(); @@ -335,4 +336,21 @@ public async Task SignOut_NoSessionClaim_EndNeverCalled() _authServiceMock.Verify(x => x.SignOutAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme, It.IsAny()), Times.Once); } + + //a failing session store must not keep the customer signed in + [TestMethod] + public async Task SignOut_EndThrows_StillSignsOut() + { + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme)) + .ReturnsAsync(Ticket(new Claim(OidcSessionDefaults.SessionIdClaimType, "session-1", ClaimValueTypes.String, "grandnode"))); + _oidcSessionService.Setup(x => x.End("session-1", OidcSessionEndReason.SignOut)) + .ThrowsAsync(new InvalidOperationException("database down")); + + await _cookieAuthService.SignOut(); + + _authServiceMock.Verify(x => x.SignOutAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme, + It.IsAny()), Times.Once); + _authServiceMock.Verify(x => x.SignOutAsync(It.IsAny(), GrandCookieAuthenticationDefaults.ExternalAuthenticationScheme, + It.IsAny()), Times.Once); + } } From dc352dbda596bf0e8576ae2555309ebe281826cd Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 09:18:19 +0200 Subject: [PATCH 10/15] LiteDB: UpdateOneAsync with no matching document updates nothing It dereferenced the missing entity and threw, while MongoDB treats no match as a no-op. Conditional updates such as ending an OpenID Connect session that has already ended rely on the MongoDB behavior. --- .../Grand.Data/LiteDb/LiteDBRepository.cs | 4 ++- .../LiteDb/LiteDbRepositoryTests.cs | 27 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/Core/Grand.Data/LiteDb/LiteDBRepository.cs b/src/Core/Grand.Data/LiteDb/LiteDBRepository.cs index 9d2403986f..32a1dbbfeb 100644 --- a/src/Core/Grand.Data/LiteDb/LiteDBRepository.cs +++ b/src/Core/Grand.Data/LiteDb/LiteDBRepository.cs @@ -210,7 +210,9 @@ public virtual Task IncField(string id, Expression> expression, U public virtual Task UpdateOneAsync(Expression> filterexpression, UpdateBuilder updateBuilder) { var entity = Collection.FindOne(filterexpression); - Update(entity, updateBuilder); + //like MongoDB: a filter that matches nothing updates nothing + if (entity != null) + Update(entity, updateBuilder); return Task.CompletedTask; } diff --git a/src/Tests/Grand.Data.Tests/LiteDb/LiteDbRepositoryTests.cs b/src/Tests/Grand.Data.Tests/LiteDb/LiteDbRepositoryTests.cs index 58c58d2538..346abc4f36 100644 --- a/src/Tests/Grand.Data.Tests/LiteDb/LiteDbRepositoryTests.cs +++ b/src/Tests/Grand.Data.Tests/LiteDb/LiteDbRepositoryTests.cs @@ -508,6 +508,33 @@ await myRepository.UpdateOneAsync(x => x.Name == "Test", Assert.AreEqual("user", pUpdated.FirstOrDefault()!.UpdatedBy); } + //like MongoDB: a filter that matches nothing updates nothing + [TestMethod] + public async Task UpdateOneAsync_NoMatch_UpdatesNothing() + { + var myRepository = new LiteDBRepositoryMock(); + myRepository.Insert(new SampleCollection { Id = "1", Name = "Test" }); + + await myRepository.UpdateOneAsync(x => x.Name == "Missing", + UpdateBuilder.Create().Set(x => x.Name, "UpdateTest")); + + Assert.AreEqual(1, myRepository.Table.Count()); + Assert.AreEqual("Test", myRepository.GetById("1").Name); + Assert.IsFalse(myRepository.GetById("1").UpdatedOnUtc.HasValue); + } + + [TestMethod] + public async Task UpdateManyAsync_NoMatch_UpdatesNothing() + { + var myRepository = new LiteDBRepositoryMock(); + myRepository.Insert(new SampleCollection { Id = "1", Name = "Test" }); + + await myRepository.UpdateManyAsync(x => x.Name == "Missing", + UpdateBuilder.Create().Set(x => x.Name, "UpdateTest")); + + Assert.AreEqual("Test", myRepository.GetById("1").Name); + } + [TestMethod] public async Task UpdateToSet_LiteRepository_Success() { From ac2e6d20068e619398e201245bbf380861c5e31f Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 09:18:45 +0200 Subject: [PATCH 11/15] Docs: list the OpenID Connect session cleanup task in the scheduled task inventory --- .ai/skills/scheduled-task.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.ai/skills/scheduled-task.md b/.ai/skills/scheduled-task.md index 81d6bdd323..2e66f6d08c 100644 --- a/.ai/skills/scheduled-task.md +++ b/.ai/skills/scheduled-task.md @@ -135,6 +135,7 @@ public class ScheduleTask : BaseEntity | `GenerateSitemapXmlTask` | `"Generate sitemap XML file"` | 10080 min | false | | `EndAuctionsTask` | `"End of the auctions"` | 60 min | false | | `CancelOrderScheduledTask` | `"Cancel unpaid and pending orders"` | 1440 min | false | +| `DeleteOidcSessionsScheduleTask` | `"Delete OpenID Connect sessions"` | 1440 min | true | ## Expected Output Produce one of these results: From 255190910cc24831c1c3c95cf6bbf56fedf70d53 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 10:57:48 +0200 Subject: [PATCH 12/15] OIDC sessions: confirm every provider session at the identity provider, not only back-office ones --- .../Services/OidcSessionService.cs | 48 +++++----- .../Configuration/OidcLoginConfig.cs | 17 ++-- .../Services/OidcLoginConfigTests.cs | 8 +- .../Services/OidcSessionServiceTests.cs | 95 ++++++++++++++----- src/Web/Grand.Web/App_Data/appsettings.json | 7 +- 5 files changed, 115 insertions(+), 60 deletions(-) diff --git a/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs b/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs index 85d6d57f36..eba4957c24 100644 --- a/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs +++ b/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs @@ -16,9 +16,9 @@ namespace Grand.Business.Authentication.Services; /// /// Browser sessions started by an identity provider sign-in. Every session can be ended on the server (sign-out, -/// back-channel logout); a back-office session also ends after the idle time or the maximum lifetime, and is -/// confirmed at the provider at an interval by exchanging its refresh token - the only way to notice that the -/// provider disabled or deleted the user, since providers send no logout for that +/// back-channel logout); a back-office session also ends after the idle time or the maximum lifetime, and every +/// session is confirmed at the provider at an interval by exchanging its refresh token - the only way to notice +/// that the provider disabled or deleted the user, since providers send no logout for that /// public class OidcSessionService : IOidcSessionService { @@ -68,12 +68,11 @@ public virtual async Task Start(Customer customer, string subject, strin ArgumentNullException.ThrowIfNull(customer); ArgumentException.ThrowIfNullOrEmpty(subject); - var backOffice = await _permissionService.IsBackOfficeAccount(customer); - if (backOffice && string.IsNullOrEmpty(refreshToken)) + if (string.IsNullOrEmpty(refreshToken)) { - _logger.LogError("Provider sign-in of back-office customer {CustomerId} refused: the provider issued no refresh " + - "token, so the session could not be revalidated. Add offline_access to OidcLogin:Scopes " + - "if the provider issues refresh tokens only for it", customer.Id); + _logger.LogError("Provider sign-in of customer {CustomerId} refused: the provider issued no refresh token, so the " + + "session could not be revalidated. Request offline_access in OidcLogin:Scopes and allow the " + + "Refresh Token grant for the client at the provider", customer.Id); return null; } @@ -86,7 +85,7 @@ public virtual async Task Start(Customer customer, string subject, strin CreatedUtc = now, LastActivityUtc = now, LastValidatedUtc = now, - ProtectedRefreshToken = backOffice ? _protector.Protect(refreshToken) : null + ProtectedRefreshToken = _protector.Protect(refreshToken) }; await _sessionRepository.InsertAsync(session); return session.Id; @@ -101,22 +100,25 @@ public virtual async Task Validate(string sessionId, Customer customer) if (session == null || session.EndedUtc.HasValue || session.CustomerId != customer.Id) return false; + var now = DateTime.UtcNow; //back-office is evaluated now: an account promoted or demoted during the session follows its current role - if (!await _permissionService.IsBackOfficeAccount(customer)) - return true; + var backOffice = await _permissionService.IsBackOfficeAccount(customer); + if (backOffice) + { + if (now - session.LastActivityUtc > _config.StaffIdleTimeout) + return await EndInvalid(session, OidcSessionEndReason.Idle); + if (now - session.CreatedUtc > _config.StaffMaxSession) + return await EndInvalid(session, OidcSessionEndReason.MaxLifetime); + } - var now = DateTime.UtcNow; - if (now - session.LastActivityUtc > _config.StaffIdleTimeout) - return await EndInvalid(session, OidcSessionEndReason.Idle); - if (now - session.CreatedUtc > _config.StaffMaxSession) - return await EndInvalid(session, OidcSessionEndReason.MaxLifetime); if (string.IsNullOrEmpty(session.ProtectedRefreshToken)) return await EndInvalid(session, OidcSessionEndReason.NoRefreshToken); - if (now - session.LastValidatedUtc >= _config.StaffRevalidationInterval && !await Revalidate(session, now)) + if (now - session.LastValidatedUtc >= _config.RevalidationInterval && !await Revalidate(session, now)) return false; - if (now - session.LastActivityUtc >= ActivityWriteInterval) + //only the staff idle rule reads the activity time + if (backOffice && now - session.LastActivityUtc >= ActivityWriteInterval) { await _sessionRepository.UpdateField(session.Id, x => x.LastActivityUtc, now); await _cache.RemoveAsync(string.Format(CacheKey.OIDC_SESSION_BY_ID_KEY, session.Id)); @@ -137,7 +139,7 @@ private async Task EndInvalid(OidcSession session, OidcSessionEndReason re /// private async Task Revalidate(OidcSession cached, DateTime now) { - var grace = _config.StaffRevalidationInterval * 2; + var grace = _config.RevalidationInterval * 2; if (!RefreshesInProgress.TryAdd(cached.Id, 0)) return now - cached.LastValidatedUtc < grace; @@ -147,7 +149,7 @@ private async Task Revalidate(OidcSession cached, DateTime now) var session = await _sessionRepository.GetByIdAsync(cached.Id); if (session == null || session.EndedUtc.HasValue) return false; - if (now - session.LastValidatedUtc < _config.StaffRevalidationInterval) + if (now - session.LastValidatedUtc < _config.RevalidationInterval) return true; var refresher = _tokenRefreshers.FirstOrDefault(); @@ -255,10 +257,8 @@ public virtual async Task DeleteExpired() await _sessionRepository.DeleteManyAsync(x => x.EndedUtc != null && x.EndedUtc < ended); //sessions nobody used again end lazily, at the next request: these will never get one - var staffCreated = now - _config.StaffMaxSession - OidcSessionDefaults.Retention; - await _sessionRepository.DeleteManyAsync(x => x.ProtectedRefreshToken != null && x.CreatedUtc < staffCreated); - var customerCreated = now - TimeSpan.FromHours(_securityConfig.CookieAuthExpires) - OidcSessionDefaults.Retention; - await _sessionRepository.DeleteManyAsync(x => x.CreatedUtc < customerCreated); + var created = now - TimeSpan.FromHours(_securityConfig.CookieAuthExpires) - OidcSessionDefaults.Retention; + await _sessionRepository.DeleteManyAsync(x => x.CreatedUtc < created); } private async Task> EndAll(IList sessions) diff --git a/src/Core/Grand.Infrastructure/Configuration/OidcLoginConfig.cs b/src/Core/Grand.Infrastructure/Configuration/OidcLoginConfig.cs index 558ada5306..352d148778 100644 --- a/src/Core/Grand.Infrastructure/Configuration/OidcLoginConfig.cs +++ b/src/Core/Grand.Infrastructure/Configuration/OidcLoginConfig.cs @@ -2,17 +2,17 @@ namespace Grand.Infrastructure.Configuration; /// /// appsettings section OidcLogin: the client of this store at the identity provider configured in OidcResourceServer, -/// and the policy for back-office sessions started by a provider sign-in +/// and the policy for sessions started by a provider sign-in /// public class OidcLoginConfig { private const int DefaultStaffIdleTimeoutMinutes = 30; private const int DefaultStaffMaxSessionHours = 10; - private const int DefaultStaffRevalidationMinutes = 10; + private const int DefaultRevalidationMinutes = 10; public string ClientId { get; set; } public string ClientSecret { get; set; } - public string Scopes { get; set; } = "openid profile email"; + public string Scopes { get; set; } = "openid profile email offline_access"; /// A back-office session ends after this much inactivity (sliding) public int StaffIdleTimeoutMinutes { get; set; } = DefaultStaffIdleTimeoutMinutes; @@ -20,8 +20,11 @@ public class OidcLoginConfig /// A back-office session ends this long after the sign-in public int StaffMaxSessionHours { get; set; } = DefaultStaffMaxSessionHours; - /// A back-office session checks the account at the identity provider this often (refresh token exchange) - public int StaffRevalidationMinutes { get; set; } = DefaultStaffRevalidationMinutes; + /// + /// Every session started by a provider sign-in confirms the account at the identity provider this often (refresh + /// token exchange): an account disabled or deleted there loses GrandNode within this time + /// + public int RevalidationMinutes { get; set; } = DefaultRevalidationMinutes; /// /// Back-office accounts connected to the identity provider with their password sign in only through the provider; @@ -36,8 +39,8 @@ public class OidcLoginConfig public TimeSpan StaffMaxSession => TimeSpan.FromHours(StaffMaxSessionHours > 0 ? StaffMaxSessionHours : DefaultStaffMaxSessionHours); - public TimeSpan StaffRevalidationInterval => - TimeSpan.FromMinutes(StaffRevalidationMinutes > 0 ? StaffRevalidationMinutes : DefaultStaffRevalidationMinutes); + public TimeSpan RevalidationInterval => + TimeSpan.FromMinutes(RevalidationMinutes > 0 ? RevalidationMinutes : DefaultRevalidationMinutes); public bool IsConfigured(OidcResourceServerConfig resourceServer) => !string.IsNullOrWhiteSpace(resourceServer?.Authority) && !string.IsNullOrWhiteSpace(ClientId); diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcLoginConfigTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcLoginConfigTests.cs index 69cb3863b1..bc46d7403e 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcLoginConfigTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcLoginConfigTests.cs @@ -10,10 +10,10 @@ public class OidcLoginConfigTests public void Defaults() { var config = new OidcLoginConfig(); - Assert.AreEqual("openid profile email", config.Scopes); + Assert.AreEqual("openid profile email offline_access", config.Scopes); Assert.AreEqual(TimeSpan.FromMinutes(30), config.StaffIdleTimeout); Assert.AreEqual(TimeSpan.FromHours(10), config.StaffMaxSession); - Assert.AreEqual(TimeSpan.FromMinutes(10), config.StaffRevalidationInterval); + Assert.AreEqual(TimeSpan.FromMinutes(10), config.RevalidationInterval); Assert.IsFalse(config.StaffSsoOnly); } @@ -24,11 +24,11 @@ public void Defaults() public void NonPositiveValues_FallBackToDefaults(int value) { var config = new OidcLoginConfig { - StaffIdleTimeoutMinutes = value, StaffMaxSessionHours = value, StaffRevalidationMinutes = value + StaffIdleTimeoutMinutes = value, StaffMaxSessionHours = value, RevalidationMinutes = value }; Assert.AreEqual(TimeSpan.FromMinutes(30), config.StaffIdleTimeout); Assert.AreEqual(TimeSpan.FromHours(10), config.StaffMaxSession); - Assert.AreEqual(TimeSpan.FromMinutes(10), config.StaffRevalidationInterval); + Assert.AreEqual(TimeSpan.FromMinutes(10), config.RevalidationInterval); } [TestMethod] diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs index 6fb5a300a0..f2304cc2cd 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs @@ -81,16 +81,12 @@ private async Task Session(TimeSpan? age = null, TimeSpan? idle = n private OidcSession Stored(string id) => _sessions.Table.First(x => x.Id == id); [TestMethod] - public async Task Start_Customer_NoRefreshTokenKept() + public async Task Start_Customer_RefreshTokenProtected() { var id = await Service().Start(_customer, "sub-1", "sid-1", "rt-1", "s1"); var session = Stored(id); - Assert.AreEqual("c1", session.CustomerId); - Assert.AreEqual("sub-1", session.Subject); + Assert.AreEqual("rt-1", Unprotect(session.ProtectedRefreshToken)); Assert.AreEqual("sid-1", session.Sid); - Assert.AreEqual("s1", session.StoreId); - Assert.IsNull(session.ProtectedRefreshToken); - Assert.IsNull(session.EndedUtc); } [TestMethod] @@ -104,11 +100,13 @@ public async Task Start_Staff_RefreshTokenProtected() Assert.AreEqual("", session.Sid); } - //a back-office sign-in without a refresh token could never be revalidated: no session, the sign-in fails + //any account: a session without a refresh token could never be revalidated [TestMethod] - public async Task Start_StaffWithoutRefreshToken_Null() + [DataRow(false)] + [DataRow(true)] + public async Task Start_WithoutRefreshToken_Null(bool staff) { - _staff = true; + _staff = staff; Assert.IsNull(await Service().Start(_customer, "sub-1", "sid-1", null, "s1")); Assert.AreEqual(0, _sessions.Table.Count()); } @@ -164,22 +162,28 @@ public async Task EndBySubject_AllOpenSessionsOfSubject() [TestMethod] public async Task DeleteExpired_KeepsRecentAndOpen() { - //the staff rule alone must remove the old staff session: the customer lifetime is longer than its age - _securityConfig.CookieAuthExpires = 100; + _securityConfig.CookieAuthExpires = 24; var endedLongAgo = await Session(); await _sessions.UpdateField(endedLongAgo.Id, x => x.EndedUtc, DateTime.UtcNow.AddDays(-8)); var endedRecently = await Session(); await _sessions.UpdateField(endedRecently.Id, x => x.EndedUtc, DateTime.UtcNow.AddDays(-1)); - //staff (refresh token kept) past the maximum lifetime + retention - var oldStaff = await Session(age: TimeSpan.FromHours(10) + TimeSpan.FromDays(8)); - //customer (no refresh token) past the cookie lifetime (100 h here) + retention - var oldCustomer = await Session(age: TimeSpan.FromHours(100) + TimeSpan.FromDays(8), refreshToken: null); - var openCustomer = await Session(age: TimeSpan.FromHours(30), refreshToken: null); + //past the cookie lifetime (24 h here) + retention, whoever it belonged to + var expired = await Session(age: TimeSpan.FromHours(24) + TimeSpan.FromDays(8)); + var open = await Session(age: TimeSpan.FromHours(30)); await Service().DeleteExpired(); - CollectionAssert.AreEquivalent(new[] { endedRecently.Id, openCustomer.Id }, - _sessions.Table.Select(x => x.Id).ToArray()); + CollectionAssert.AreEquivalent(new[] { endedRecently.Id, open.Id }, _sessions.Table.Select(x => x.Id).ToArray()); + } + + //every session keeps a refresh token now: the old staff rule (refresh token kept + past staff maximum) would delete live customers + [TestMethod] + public async Task DeleteExpired_KeepsActiveCustomerWithRefreshToken() + { + _securityConfig.CookieAuthExpires = 8760; + var customer = await Session(age: TimeSpan.FromDays(20)); + await Service().DeleteExpired(); + Assert.AreEqual(1, _sessions.Table.Count(x => x.Id == customer.Id)); } private static readonly TimeSpan Interval = TimeSpan.FromMinutes(10); @@ -207,14 +211,61 @@ public async Task Validate_OtherCustomer_False() Assert.IsFalse(await Service().Validate(session.Id, _customer)); } - //customers keep the long cookie: no idle, maximum or provider check + //customers keep the long session: no idle or maximum lifetime, only the provider check [TestMethod] - public async Task Validate_Customer_OldAndIdle_True_NoRefresh() + public async Task Validate_Customer_OldAndIdle_WithinInterval_True_NoRefresh() { var session = await Session(age: TimeSpan.FromDays(30), idle: TimeSpan.FromDays(20), - sinceValidated: TimeSpan.FromDays(30), refreshToken: null); + sinceValidated: TimeSpan.FromMinutes(5)); Assert.IsTrue(await Service().Validate(session.Id, _customer)); _refresher.Verify(x => x.Refresh(It.IsAny(), It.IsAny()), Times.Never); + Assert.IsNull(Stored(session.Id).EndedUtc); + } + + [TestMethod] + public async Task Validate_CustomerRefused_EndsAndRevokesTokens() + { + _refresher.Setup(x => x.Refresh("rt-1", It.IsAny())).ReturnsAsync(OidcRefreshResult.Refused()); + var session = await Session(age: TimeSpan.FromDays(3), idle: TimeSpan.FromDays(2), sinceValidated: Interval); + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + Assert.AreEqual(OidcSessionEndReason.RevalidationRefused, Stored(session.Id).EndReason); + _revocation.Verify(x => x.RevokeTokens("sub-1"), Times.Once); + } + + [TestMethod] + public async Task Validate_CustomerRefreshed_RotatedTokenStored() + { + _refresher.Setup(x => x.Refresh("rt-1", It.IsAny())).ReturnsAsync(OidcRefreshResult.Refreshed("rt-2")); + var session = await Session(sinceValidated: Interval); + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + Assert.AreEqual("rt-2", Unprotect(Stored(session.Id).ProtectedRefreshToken)); + } + + [TestMethod] + public async Task Validate_CustomerUnavailable_WithinGrace_Valid() + { + _refresher.Setup(x => x.Refresh(It.IsAny(), It.IsAny())).ReturnsAsync(OidcRefreshResult.Unavailable()); + var session = await Session(sinceValidated: Interval + TimeSpan.FromMinutes(5)); + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + } + + //a customer session started before every session kept a refresh token + [TestMethod] + public async Task Validate_CustomerWithoutRefreshToken_Ends() + { + var session = await Session(refreshToken: null); + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + Assert.AreEqual(OidcSessionEndReason.NoRefreshToken, Stored(session.Id).EndReason); + } + + //customers' activity is not written: only the staff idle rule needs it + [TestMethod] + public async Task Validate_Customer_ActivityNotWritten() + { + var session = await Session(idle: TimeSpan.FromMinutes(5)); + var before = Stored(session.Id).LastActivityUtc; + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + Assert.AreEqual(before, Stored(session.Id).LastActivityUtc); } [TestMethod] @@ -380,7 +431,7 @@ public async Task Validate_RefresherCancelled_TreatedAsUnavailable() Assert.IsNull(Stored(session.Id).EndedUtc); } - //the login plugin was uninstalled: staff sessions end at the next check, customers are not affected + //the login plugin was uninstalled: every session ends at the next check [TestMethod] public async Task NoRefresher_CountsAsRefused() { diff --git a/src/Web/Grand.Web/App_Data/appsettings.json b/src/Web/Grand.Web/App_Data/appsettings.json index 85f5f86b47..92da6a2d67 100644 --- a/src/Web/Grand.Web/App_Data/appsettings.json +++ b/src/Web/Grand.Web/App_Data/appsettings.json @@ -279,11 +279,12 @@ "ClientId": "", //keep it out of source control (env var OidcLogin__ClientSecret or a secret store) "ClientSecret": "", - "Scopes": "openid profile email", - //back-office accounts (admin panel, store staff, vendors) signed in through the provider + "Scopes": "openid profile email offline_access", + //every session signed in through the provider confirms the account there this often (minutes); a user disabled at the provider loses access within this time + "RevalidationMinutes": 10, + //back-office accounts only (admin panel, store staff, vendors) signed in through the provider "StaffIdleTimeoutMinutes": 30, "StaffMaxSessionHours": 10, - "StaffRevalidationMinutes": 10, //true: back-office accounts connected to the provider sign in only through it; accounts without a connection keep the password "StaffSsoOnly": false }, From 90e79197f9eeded2396c22d5a7d44388fc7cc108 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 11:58:58 +0200 Subject: [PATCH 13/15] OIDC sessions: an ended session does not keep its refresh token --- .../Services/OidcSessionService.cs | 6 ++++-- src/Core/Grand.Domain/Customers/OidcSession.cs | 2 +- .../Services/OidcSessionServiceTests.cs | 9 +++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs b/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs index eba4957c24..8cbad1240c 100644 --- a/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs +++ b/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs @@ -227,9 +227,11 @@ public virtual async Task End(string sessionId, OidcSessionEndReason reason) { if (string.IsNullOrEmpty(sessionId)) return; - //the first end wins: its reason is the one that explains the sign-out + //the first end wins: its reason is the one that explains the sign-out; an ended session is never confirmed + //again, so its refresh token is not kept for the retention period await _sessionRepository.UpdateOneAsync(x => x.Id == sessionId && x.EndedUtc == null, - UpdateBuilder.Create().Set(x => x.EndedUtc, DateTime.UtcNow).Set(x => x.EndReason, reason)); + UpdateBuilder.Create().Set(x => x.EndedUtc, DateTime.UtcNow).Set(x => x.EndReason, reason) + .Set(x => x.ProtectedRefreshToken, null)); await _cache.RemoveAsync(string.Format(CacheKey.OIDC_SESSION_BY_ID_KEY, sessionId)); _logger.LogInformation("OpenID Connect session {SessionId} ended: {Reason}", sessionId, reason); } diff --git a/src/Core/Grand.Domain/Customers/OidcSession.cs b/src/Core/Grand.Domain/Customers/OidcSession.cs index f673a45ccc..aa3b4ae085 100644 --- a/src/Core/Grand.Domain/Customers/OidcSession.cs +++ b/src/Core/Grand.Domain/Customers/OidcSession.cs @@ -23,7 +23,7 @@ public class OidcSession : BaseEntity /// Last successful check of the account at the provider public DateTime LastValidatedUtc { get; set; } - /// Refresh token protected with Data Protection; kept only for back-office accounts + /// Refresh token protected with Data Protection; cleared when the session ends public string ProtectedRefreshToken { get; set; } public DateTime? EndedUtc { get; set; } diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs index f2304cc2cd..2c6ad4463f 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs @@ -123,6 +123,15 @@ public async Task End_SetsReason_OnlyOnce() _cache.Verify(x => x.RemoveAsync(string.Format(CacheKey.OIDC_SESSION_BY_ID_KEY, session.Id), true), Times.AtLeastOnce); } + //an ended session is never confirmed again: its refresh token is not kept for the retention period + [TestMethod] + public async Task End_ClearsRefreshToken() + { + var session = await Session(); + await Service().End(session.Id, OidcSessionEndReason.SignOut); + Assert.IsNull(Stored(session.Id).ProtectedRefreshToken); + } + [TestMethod] public async Task EndBySid_MatchingSidAndSubject_Only() { From f6aa28dafa3f951c24cb633848ea94be23e3eb10 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Tue, 13 Oct 2026 19:58:03 +0200 Subject: [PATCH 14/15] OIDC sessions: refresh lease shared by every instance, abandoned sessions deleted, access tokens revoked only on a provider refusal --- .../Services/OidcSessionService.cs | 59 ++++++++----- .../Authentication/IOidcTokenRefresher.cs | 2 +- .../Authentication/OidcRefreshResult.cs | 9 +- .../Authentication/OidcSessionDefaults.cs | 6 ++ .../Grand.Domain/Customers/OidcSession.cs | 6 ++ .../Services/OidcSessionServiceTests.cs | 83 ++++++++++++++++++- 6 files changed, 142 insertions(+), 23 deletions(-) diff --git a/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs b/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs index 8cbad1240c..0f27e4c48e 100644 --- a/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs +++ b/src/Business/Grand.Business.Authentication/Services/OidcSessionService.cs @@ -9,7 +9,6 @@ using Grand.Infrastructure.Configuration; using Microsoft.AspNetCore.DataProtection; using Microsoft.Extensions.Logging; -using System.Collections.Concurrent; using System.Security.Cryptography; namespace Grand.Business.Authentication.Services; @@ -28,8 +27,9 @@ public class OidcSessionService : IOidcSessionService //minutes; ended sessions are removed from the cache, so only the activity time may be this stale private const int CacheTimeMinutes = 1; - //one refresh at a time per session in this process: refresh tokens rotate - private static readonly ConcurrentDictionary RefreshesInProgress = new(); + //one refresh at a time per session on every instance (refresh tokens rotate): a lease stored on the session; + //longer than the refresh request may take, so it lapses only when its holder died + private static readonly TimeSpan RefreshLeaseTime = TimeSpan.FromSeconds(30); private readonly IRepository _sessionRepository; private readonly ICacheBase _cache; @@ -140,28 +140,38 @@ private async Task EndInvalid(OidcSession session, OidcSessionEndReason re private async Task Revalidate(OidcSession cached, DateTime now) { var grace = _config.RevalidationInterval * 2; - if (!RefreshesInProgress.TryAdd(cached.Id, 0)) - return now - cached.LastValidatedUtc < grace; + + //atomic on MongoDB: of the requests that find the check due, on any instance, one takes the lease + var leaseId = Guid.NewGuid().ToString("N"); + await _sessionRepository.UpdateOneAsync( + x => x.Id == cached.Id && x.EndedUtc == null && + (x.RefreshLeaseUntilUtc == null || x.RefreshLeaseUntilUtc < now), + UpdateBuilder.Create().Set(x => x.RefreshLeaseId, leaseId) + .Set(x => x.RefreshLeaseUntilUtc, now + RefreshLeaseTime)); + + //the cached record may predate a refresh that just finished: its token is already rotated + var session = await _sessionRepository.GetByIdAsync(cached.Id); + if (session == null || session.EndedUtc.HasValue) + return false; + //another request is refreshing: go on while the last confirmation is recent enough + if (session.RefreshLeaseId != leaseId) + return now - session.LastValidatedUtc < grace; try { - //the cached record may predate a refresh that just finished: its token is already rotated - var session = await _sessionRepository.GetByIdAsync(cached.Id); - if (session == null || session.EndedUtc.HasValue) - return false; if (now - session.LastValidatedUtc < _config.RevalidationInterval) return true; var refresher = _tokenRefreshers.FirstOrDefault(); var refreshToken = Unprotect(session.ProtectedRefreshToken); OidcRefreshResult result; - var unreadable = refresher != null && refreshToken == null; - if (refresher == null || refreshToken == null) + if (refresher == null) + result = OidcRefreshResult.NotAsked(); + else if (refreshToken == null) { - if (unreadable) - _logger.LogWarning("The stored refresh token of session {SessionId} cannot be read (Data Protection keys changed)", - session.Id); - result = OidcRefreshResult.Refused(); + _logger.LogWarning("The stored refresh token of session {SessionId} cannot be read (Data Protection keys changed)", + session.Id); + result = OidcRefreshResult.NotAsked(); } else { @@ -201,13 +211,17 @@ await _sessionRepository.UpdateOneAsync( return true; } - if (!unreadable) - _logger.LogWarning("The identity provider refused the session {SessionId} of customer {CustomerId}", - session.Id, session.CustomerId); + _logger.LogWarning("The identity provider refused the session {SessionId} of customer {CustomerId}", + session.Id, session.CustomerId); await End(session.Id, OidcSessionEndReason.RevalidationRefused); await _tokenRevocationService.RevokeTokens(session.Subject); return false; } + case OidcRefreshOutcome.NotAsked: + //nothing can confirm the session any more; the provider refused nothing, so the subject's access + //tokens stay valid + await End(session.Id, OidcSessionEndReason.RevalidationRefused); + return false; default: _logger.LogWarning("The identity provider could not confirm session {SessionId}; last confirmed {LastValidatedUtc}", session.Id, session.LastValidatedUtc); @@ -219,7 +233,9 @@ await _sessionRepository.UpdateOneAsync( } finally { - RefreshesInProgress.TryRemove(cached.Id, out _); + await _sessionRepository.UpdateOneAsync(x => x.Id == session.Id && x.RefreshLeaseId == leaseId, + UpdateBuilder.Create().Set(x => x.RefreshLeaseId, null) + .Set(x => x.RefreshLeaseUntilUtc, null)); } } @@ -261,6 +277,11 @@ public virtual async Task DeleteExpired() //sessions nobody used again end lazily, at the next request: these will never get one var created = now - TimeSpan.FromHours(_securityConfig.CookieAuthExpires) - OidcSessionDefaults.Retention; await _sessionRepository.DeleteManyAsync(x => x.CreatedUtc < created); + + //a session in use is confirmed every RevalidationMinutes: one not confirmed for this long was left (most end with + //the browser, without a sign-out) and must not keep its refresh token any longer + var abandoned = now - OidcSessionDefaults.AbandonedAfter; + await _sessionRepository.DeleteManyAsync(x => x.EndedUtc == null && x.LastValidatedUtc < abandoned); } private async Task> EndAll(IList sessions) diff --git a/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcTokenRefresher.cs b/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcTokenRefresher.cs index 76157deb8f..fee3d1ac28 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcTokenRefresher.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcTokenRefresher.cs @@ -4,7 +4,7 @@ namespace Grand.Business.Core.Interfaces.Authentication; /// /// Exchanges a refresh token at the identity provider (implemented by the OpenID Connect login plugin; with no -/// implementation registered a revalidation counts as refused) +/// implementation registered a session cannot be confirmed and ends) /// public interface IOidcTokenRefresher { diff --git a/src/Business/Grand.Business.Core/Utilities/Authentication/OidcRefreshResult.cs b/src/Business/Grand.Business.Core/Utilities/Authentication/OidcRefreshResult.cs index 442a8b2adf..4426bfdc38 100644 --- a/src/Business/Grand.Business.Core/Utilities/Authentication/OidcRefreshResult.cs +++ b/src/Business/Grand.Business.Core/Utilities/Authentication/OidcRefreshResult.cs @@ -9,7 +9,13 @@ public enum OidcRefreshOutcome Refused, /// No answer: network error, timeout, 5xx - Unavailable + Unavailable, + + /// + /// The provider was not asked: this installation cannot (sign-in not configured, stored token unreadable). The + /// session cannot be confirmed any more, but the provider refused nothing + /// + NotAsked } /// The answer of the provider @@ -19,4 +25,5 @@ public sealed record OidcRefreshResult(OidcRefreshOutcome Outcome, string Refres public static OidcRefreshResult Refreshed(string refreshToken) => new(OidcRefreshOutcome.Refreshed, refreshToken); public static OidcRefreshResult Refused() => new(OidcRefreshOutcome.Refused); public static OidcRefreshResult Unavailable() => new(OidcRefreshOutcome.Unavailable); + public static OidcRefreshResult NotAsked() => new(OidcRefreshOutcome.NotAsked); } diff --git a/src/Business/Grand.Business.Core/Utilities/Authentication/OidcSessionDefaults.cs b/src/Business/Grand.Business.Core/Utilities/Authentication/OidcSessionDefaults.cs index 1dc59af1d5..478e63468f 100644 --- a/src/Business/Grand.Business.Core/Utilities/Authentication/OidcSessionDefaults.cs +++ b/src/Business/Grand.Business.Core/Utilities/Authentication/OidcSessionDefaults.cs @@ -10,4 +10,10 @@ public static class OidcSessionDefaults /// How long an ended or expired session record is kept before the cleanup task deletes it public static readonly TimeSpan Retention = TimeSpan.FromDays(7); + + /// + /// An open session not confirmed at the provider for this long is deleted by the cleanup task (its browser is gone; + /// providers drop an unused offline session after about as long) + /// + public static readonly TimeSpan AbandonedAfter = TimeSpan.FromDays(30); } diff --git a/src/Core/Grand.Domain/Customers/OidcSession.cs b/src/Core/Grand.Domain/Customers/OidcSession.cs index aa3b4ae085..73d46eb325 100644 --- a/src/Core/Grand.Domain/Customers/OidcSession.cs +++ b/src/Core/Grand.Domain/Customers/OidcSession.cs @@ -26,6 +26,12 @@ public class OidcSession : BaseEntity /// Refresh token protected with Data Protection; cleared when the session ends public string ProtectedRefreshToken { get; set; } + /// Holder of the refresh in progress (one at a time on every instance); empty when none + public string RefreshLeaseId { get; set; } + + /// The lease lapses at this time, so a holder that died does not block the next refresh + public DateTime? RefreshLeaseUntilUtc { get; set; } + public DateTime? EndedUtc { get; set; } public OidcSessionEndReason? EndReason { get; set; } } diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs index 2c6ad4463f..52ac0220c4 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcSessionServiceTests.cs @@ -185,6 +185,19 @@ public async Task DeleteExpired_KeepsRecentAndOpen() CollectionAssert.AreEquivalent(new[] { endedRecently.Id, open.Id }, _sessions.Table.Select(x => x.Id).ToArray()); } + //most sessions end with the browser: one not confirmed for the abandonment period is deleted with its refresh token + [TestMethod] + public async Task DeleteExpired_OpenSessionNotConfirmedForLong_Deleted() + { + _securityConfig.CookieAuthExpires = 8760; + var abandoned = await Session(age: TimeSpan.FromDays(40), sinceValidated: OidcSessionDefaults.AbandonedAfter + TimeSpan.FromHours(1)); + var inUse = await Session(age: TimeSpan.FromDays(40), sinceValidated: TimeSpan.FromMinutes(5)); + + await Service().DeleteExpired(); + + CollectionAssert.AreEquivalent(new[] { inUse.Id }, _sessions.Table.Select(x => x.Id).ToArray()); + } + //every session keeps a refresh token now: the old staff rule (refresh token kept + past staff maximum) would delete live customers [TestMethod] public async Task DeleteExpired_KeepsActiveCustomerWithRefreshToken() @@ -449,7 +462,73 @@ public async Task NoRefresher_CountsAsRefused() var session = await Session(sinceValidated: Interval); Assert.IsFalse(await Service().Validate(session.Id, _customer)); Assert.AreEqual(OidcSessionEndReason.RevalidationRefused, Stored(session.Id).EndReason); - _revocation.Verify(x => x.RevokeTokens("sub-1"), Times.Once); + //the provider refused nothing: the subject's access tokens stay valid + _revocation.Verify(x => x.RevokeTokens(It.IsAny()), Times.Never); + } + + //the Data Protection keys changed: the stored token cannot be used, but the provider was never asked + [TestMethod] + public async Task UnreadableRefreshToken_Ends_TokensNotRevoked() + { + var session = await Session(sinceValidated: Interval); + await _sessions.UpdateField(session.Id, x => x.ProtectedRefreshToken, + new EphemeralDataProtectionProvider().CreateProtector(OidcSessionDefaults.RefreshTokenPurpose).Protect("rt-1")); + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + Assert.AreEqual(OidcSessionEndReason.RevalidationRefused, Stored(session.Id).EndReason); + _refresher.Verify(x => x.Refresh(It.IsAny(), It.IsAny()), Times.Never); + _revocation.Verify(x => x.RevokeTokens(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task NotAsked_Ends_TokensNotRevoked() + { + _refresher.Setup(x => x.Refresh("rt-1", It.IsAny())).ReturnsAsync(OidcRefreshResult.NotAsked()); + var session = await Session(sinceValidated: Interval); + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + Assert.IsNotNull(Stored(session.Id).EndedUtc); + _revocation.Verify(x => x.RevokeTokens(It.IsAny()), Times.Never); + } + + //another instance holds the refresh: this one does not use the same refresh token + [TestMethod] + public async Task LeaseHeldElsewhere_RecentCheck_Valid_NoRefresh() + { + var session = await Session(sinceValidated: Interval); + await _sessions.UpdateField(session.Id, x => x.RefreshLeaseId, "other"); + await _sessions.UpdateField(session.Id, x => x.RefreshLeaseUntilUtc, DateTime.UtcNow.AddSeconds(20)); + + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + _refresher.Verify(x => x.Refresh(It.IsAny(), It.IsAny()), Times.Never); + Assert.AreEqual("other", Stored(session.Id).RefreshLeaseId); + } + + [TestMethod] + public async Task LeaseHeldElsewhere_PastGrace_False_NotEnded() + { + var session = await Session(sinceValidated: Interval * 2); + await _sessions.UpdateField(session.Id, x => x.RefreshLeaseId, "other"); + await _sessions.UpdateField(session.Id, x => x.RefreshLeaseUntilUtc, DateTime.UtcNow.AddSeconds(20)); + + Assert.IsFalse(await Service().Validate(session.Id, _customer)); + //the holder decides the outcome + Assert.IsNull(Stored(session.Id).EndedUtc); + } + + //the holder died: its lease lapses and the next request refreshes + [TestMethod] + public async Task LeaseLapsed_Taken_RefreshedAndReleased() + { + _refresher.Setup(x => x.Refresh("rt-1", It.IsAny())) + .ReturnsAsync(OidcRefreshResult.Refreshed("rt-2")); + var session = await Session(sinceValidated: Interval); + await _sessions.UpdateField(session.Id, x => x.RefreshLeaseId, "dead"); + await _sessions.UpdateField(session.Id, x => x.RefreshLeaseUntilUtc, DateTime.UtcNow.AddSeconds(-1)); + + Assert.IsTrue(await Service().Validate(session.Id, _customer)); + var stored = Stored(session.Id); + Assert.AreEqual("rt-2", Unprotect(stored.ProtectedRefreshToken)); + Assert.IsNull(stored.RefreshLeaseId); + Assert.IsNull(stored.RefreshLeaseUntilUtc); } //a parallel request finds the refresh in progress and goes on while the last check is recent enough @@ -476,7 +555,7 @@ public async Task RefreshInProgress_RecentCheck_Valid() } finally { - //always released: the in-progress marker is process-wide and must not outlive the test + //always released, so the first request completes release.TrySetResult(OidcRefreshResult.Refreshed("rt-2")); } From d0eeff9ef6ac720f11dda11cda36a93ea8296b8d Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Tue, 13 Oct 2026 20:00:34 +0200 Subject: [PATCH 15/15] OIDC sessions: a new provider sign-in of the same customer ends the session it replaces --- .../Services/CookieAuthenticationService.cs | 29 +++++++++++++------ .../CookieAuthenticationServiceTests.cs | 26 +++++++++++++++++ 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs b/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs index b89402becd..bea8d2e451 100644 --- a/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs +++ b/src/Business/Grand.Business.Authentication/Services/CookieAuthenticationService.cs @@ -118,6 +118,12 @@ public virtual async Task SignIn(Customer customer, bool isPersistent, string oi { ArgumentNullException.ThrowIfNull(customer); + //the same customer signing in through the provider again: the cookie that referenced the previous session is + //replaced, so that session would never be used or ended again + if (!string.IsNullOrEmpty(oidcSessionId) && + await CurrentOidcSessionIdOf(customer) is { Length: > 0 } previous && previous != oidcSessionId) + await EndOidcSession(previous); + //create claims for customer's username and email var claims = new List(); @@ -187,15 +193,7 @@ public virtual async Task SignOut() GrandCookieAuthenticationDefaults.AuthenticationScheme); var oidcSessionId = authenticateResult?.Succeeded == true ? OidcSessionId(authenticateResult.Principal) : null; if (!string.IsNullOrEmpty(oidcSessionId)) - try - { - await _oidcSessionService.End(oidcSessionId, OidcSessionEndReason.SignOut); - } - catch (Exception ex) - { - //the cookie is removed below anyway; the session ends by idle or maximum lifetime - _logger.LogWarning(ex, "Could not end the OpenID Connect session {SessionId} on sign-out", oidcSessionId); - } + await EndOidcSession(oidcSessionId); await _httpContextAccessor.HttpContext.SignOutAsync(GrandCookieAuthenticationDefaults .AuthenticationScheme); @@ -283,6 +281,19 @@ private async Task IsValidCustomer(Customer customer, ClaimsPrincipal prin return true; } + private async Task EndOidcSession(string sessionId) + { + try + { + await _oidcSessionService.End(sessionId, OidcSessionEndReason.SignOut); + } + catch (Exception ex) + { + //the cookie that referenced it is replaced anyway; the record goes with the cleanup task + _logger.LogWarning(ex, "Could not end the OpenID Connect session {SessionId}", sessionId); + } + } + private string OidcSessionId(ClaimsPrincipal principal) => principal?.FindFirst(claim => claim.Type == OidcSessionDefaults.SessionIdClaimType && diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs index d21c520b00..09fb202606 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/CookieAuthenticationServiceTests.cs @@ -302,6 +302,32 @@ public async Task SignIn_WithoutSessionId_SessionClaimOfOtherIssuer_NoCarryOver( Assert.IsNull(signedIn().FindFirst(OidcSessionDefaults.SessionIdClaimType)); } + //signing in through the provider again: the replaced session is ended, not left open with its refresh token + [TestMethod] + public async Task SignIn_NewSession_SameCustomerCookieWithOtherSession_EndsPrevious() + { + var (customer, signedIn) = PrepareSignIn("c1"); + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme)) + .ReturnsAsync(Ticket(new Claim(OidcSessionDefaults.SessionIdClaimType, "session-1", ClaimValueTypes.String, "grandnode"))); + + await _cookieAuthService.SignIn(customer, false, "session-2"); + + _oidcSessionService.Verify(x => x.End("session-1", OidcSessionEndReason.SignOut), Times.Once); + Assert.AreEqual("session-2", signedIn().FindFirst(OidcSessionDefaults.SessionIdClaimType)!.Value); + } + + [TestMethod] + public async Task SignIn_NewSession_OtherCustomerCookie_PreviousNotEnded() + { + var (customer, _) = PrepareSignIn("c2"); + _authServiceMock.Setup(x => x.AuthenticateAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme)) + .ReturnsAsync(Ticket(new Claim(OidcSessionDefaults.SessionIdClaimType, "session-1", ClaimValueTypes.String, "grandnode"))); + + await _cookieAuthService.SignIn(customer, false, "session-2"); + + _oidcSessionService.Verify(x => x.End(It.IsAny(), It.IsAny()), Times.Never); + } + [TestMethod] public async Task SignIn_WithoutSessionId_NoCurrentCookie_NoClaim() {