From 8577a1ab3650444d58915d9ee969bea146b8fcd5 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Sun, 11 Oct 2026 22:11:31 +0200 Subject: [PATCH 01/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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() { From f912be9791409273dd87398239e4eefb10bd6c7d Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 06:57:31 +0200 Subject: [PATCH 16/28] OpenID Connect login: start a server-side session with the refresh token and provider session id --- .../Controllers/OidcLoginController.cs | 20 ++++- .../OidcLoginAuthenticationBuilder.cs | 6 ++ .../Infrastructure/OidcLoginSessionData.cs | 55 ++++++++++++++ .../Models/FailedModel.cs | 3 + .../OidcLoginPlugin.cs | 2 + .../Views/OidcLogin/Failed.cshtml | 2 + .../OpenIdConnect/OidcLoginControllerTests.cs | 74 +++++++++++++++++-- .../OidcLoginSessionDataTests.cs | 51 +++++++++++++ 8 files changed, 204 insertions(+), 9 deletions(-) create mode 100644 src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLoginSessionData.cs create mode 100644 src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginSessionDataTests.cs diff --git a/src/Plugins/Authentication.OpenIdConnect/Controllers/OidcLoginController.cs b/src/Plugins/Authentication.OpenIdConnect/Controllers/OidcLoginController.cs index bfbcd1ae36..29c66a5145 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Controllers/OidcLoginController.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Controllers/OidcLoginController.cs @@ -1,3 +1,4 @@ +using Authentication.OpenIdConnect.Infrastructure; using Authentication.OpenIdConnect.Models; using Grand.Business.Core.Events.Customers; using Grand.Business.Core.Interfaces.Authentication; @@ -23,6 +24,7 @@ public OidcLoginController( IExternalRegistrationContext externalRegistrationContext, IOidcCustomerResolver oidcCustomerResolver, IGrandAuthenticationService authenticationService, + IOidcSessionService oidcSessionService, IMediator mediator, IExternalAuthenticationService externalAuthenticationService, IAuthenticationSchemeProvider authenticationSchemeProvider, @@ -33,6 +35,7 @@ public OidcLoginController( _externalRegistrationContext = externalRegistrationContext; _oidcCustomerResolver = oidcCustomerResolver; _authenticationService = authenticationService; + _oidcSessionService = oidcSessionService; _mediator = mediator; _externalAuthenticationService = externalAuthenticationService; _authenticationSchemeProvider = authenticationSchemeProvider; @@ -50,6 +53,7 @@ public OidcLoginController( private readonly IExternalRegistrationContext _externalRegistrationContext; private readonly IOidcCustomerResolver _oidcCustomerResolver; private readonly IGrandAuthenticationService _authenticationService; + private readonly IOidcSessionService _oidcSessionService; private readonly IMediator _mediator; private readonly IExternalAuthenticationService _externalAuthenticationService; private readonly IAuthenticationSchemeProvider _authenticationSchemeProvider; @@ -114,11 +118,20 @@ public async Task Callback(string returnUrl) switch (result.Outcome) { case OidcLoginOutcome.SignIn: + { + //read before the pending identity is cleared: the same cookie carries them + var (refreshToken, sid) = await OidcLoginSessionData.Read(HttpContext); await _externalRegistrationContext.Clear(); + var sessionId = await _oidcSessionService.Start(result.Customer, pending.Subject, sid, refreshToken, + _contextAccessor.StoreContext.CurrentStore.Id); + if (sessionId == null) + return RedirectToRoute("OidcLoginFailed", new { session = true }); + //the identity provider is the sign-in: its multi-factor policy applies, not the store's two-factor code await _mediator.Publish(new CustomerLoggedInEvent(result.Customer)); - await _authenticationService.SignIn(result.Customer, false); + await _authenticationService.SignIn(result.Customer, false, sessionId); return RedirectLocal(returnUrl); + } case OidcLoginOutcome.RegistrationRequired: return RedirectToRoute("Register", new { returnUrl }); case OidcLoginOutcome.LinkRequired: @@ -199,10 +212,11 @@ public async Task LinkComplete(string returnUrl) }; } - public IActionResult Failed(int? reason, bool mismatch = false, bool taken = false, bool signedIn = false) => + public IActionResult Failed(int? reason, bool mismatch = false, bool taken = false, bool signedIn = false, + bool session = false) => View(new FailedModel { Reason = (OidcLoginRefusal)(reason ?? 0), Mismatch = mismatch, IdentityTaken = taken, - SignedInAsOtherAccount = signedIn + SignedInAsOtherAccount = signedIn, SessionUnavailable = session }); #endregion diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLoginAuthenticationBuilder.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLoginAuthenticationBuilder.cs index dcf5d4dfe4..d0bbcf6ebc 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLoginAuthenticationBuilder.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLoginAuthenticationBuilder.cs @@ -49,6 +49,12 @@ public void AddAuthentication(AuthenticationBuilder builder, IConfiguration conf options.ClaimActions.MapUniqueJsonKey(claim, claim); options.Events = new OpenIdConnectEvents { + //the refresh token stays out of the ticket's tokens (SaveTokens is off): only the server-side session keeps it + OnTokenValidated = ctx => + { + OidcLoginSessionData.Capture(ctx.Properties, ctx.TokenEndpointResponse); + return Task.CompletedTask; + }, OnTicketReceived = async ctx => { var identity = OidcLoginIdentityMapper.Map(ctx.Principal, resourceServer.Claims); diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLoginSessionData.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLoginSessionData.cs new file mode 100644 index 0000000000..c83b2eeafa --- /dev/null +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLoginSessionData.cs @@ -0,0 +1,55 @@ +using Grand.Business.Core.Utilities.Authentication; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Http; +using Microsoft.IdentityModel.JsonWebTokens; +using Microsoft.IdentityModel.Protocols.OpenIdConnect; + +namespace Authentication.OpenIdConnect.Infrastructure; + +/// +/// What the server-side session needs from the provider sign-in (refresh token, provider session id), carried from +/// the token response to the callback in the external authentication cookie next to the pending identity +/// +public static class OidcLoginSessionData +{ + public const string RefreshTokenItem = ".Grand.OidcLogin.RefreshToken"; + public const string SidItem = ".Grand.OidcLogin.Sid"; + + public static void Capture(AuthenticationProperties properties, OpenIdConnectMessage tokenResponse) + { + if (properties == null || tokenResponse == null) + return; + if (!string.IsNullOrEmpty(tokenResponse.RefreshToken)) + properties.Items[RefreshTokenItem] = tokenResponse.RefreshToken; + var sid = Sid(tokenResponse.IdToken); + if (sid != null) + properties.Items[SidItem] = sid; + } + + /// The sid claim of an ID token already validated by the handler; null when absent or unreadable + public static string Sid(string idToken) + { + if (string.IsNullOrEmpty(idToken)) + return null; + try + { + return new JsonWebToken(idToken).TryGetPayloadValue("sid", out var sid) && !string.IsNullOrEmpty(sid) + ? sid + : null; + } + catch (ArgumentException) + { + return null; + } + } + + public static async Task<(string RefreshToken, string Sid)> Read(HttpContext context) + { + var result = await context.AuthenticateAsync(GrandCookieAuthenticationDefaults.ExternalAuthenticationScheme); + var items = result?.Succeeded == true ? result.Properties?.Items : null; + string refreshToken = null, sid = null; + items?.TryGetValue(RefreshTokenItem, out refreshToken); + items?.TryGetValue(SidItem, out sid); + return (refreshToken, sid); + } +} diff --git a/src/Plugins/Authentication.OpenIdConnect/Models/FailedModel.cs b/src/Plugins/Authentication.OpenIdConnect/Models/FailedModel.cs index a5273d8085..111bff8145 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Models/FailedModel.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Models/FailedModel.cs @@ -12,4 +12,7 @@ public class FailedModel /// Another account is signed in to the store public bool SignedInAsOtherAccount { get; set; } + + /// A back-office sign-in without a refresh token from the provider: the session could not be checked later + public bool SessionUnavailable { get; set; } } diff --git a/src/Plugins/Authentication.OpenIdConnect/OidcLoginPlugin.cs b/src/Plugins/Authentication.OpenIdConnect/OidcLoginPlugin.cs index 7f2e391da4..a3c47a2ba7 100644 --- a/src/Plugins/Authentication.OpenIdConnect/OidcLoginPlugin.cs +++ b/src/Plugins/Authentication.OpenIdConnect/OidcLoginPlugin.cs @@ -33,6 +33,8 @@ private static readonly (string Key, string Value)[] Resources = [ "Your identity provider account is already connected to another account in this store. Nothing was connected."), ("Failed.SignedInAsOtherAccount", "You are already signed in with another account. Sign out first, then sign in with your identity provider."), + ("Failed.SessionUnavailable", + "Signing in to the back office with the identity provider is not set up completely. Use your password, or ask the administrator."), ("DisplayOrder", "Display order"), ("ClientId", "Client ID"), ("ClientSecret", "Client secret"), diff --git a/src/Plugins/Authentication.OpenIdConnect/Views/OidcLogin/Failed.cshtml b/src/Plugins/Authentication.OpenIdConnect/Views/OidcLogin/Failed.cshtml index 2e50dd0848..bbfc7f06f1 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Views/OidcLogin/Failed.cshtml +++ b/src/Plugins/Authentication.OpenIdConnect/Views/OidcLogin/Failed.cshtml @@ -8,6 +8,8 @@ ? "Plugins.ExternalAuth.OpenIdConnect.Failed.IdentityLinkedToOtherAccount" : Model.SignedInAsOtherAccount ? "Plugins.ExternalAuth.OpenIdConnect.Failed.SignedInAsOtherAccount" + : Model.SessionUnavailable + ? "Plugins.ExternalAuth.OpenIdConnect.Failed.SessionUnavailable" : Model.Reason switch { OidcLoginRefusal.AccountInactive => "Plugins.ExternalAuth.OpenIdConnect.Failed.AccountInactive", OidcLoginRefusal.RegistrationDisabled => "Plugins.ExternalAuth.OpenIdConnect.Failed.RegistrationDisabled", diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginControllerTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginControllerTests.cs index 9a5307fc3f..01830b36c9 100644 --- a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginControllerTests.cs +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginControllerTests.cs @@ -1,5 +1,6 @@ using Authentication.OpenIdConnect; using Authentication.OpenIdConnect.Controllers; +using Authentication.OpenIdConnect.Infrastructure; using Authentication.OpenIdConnect.Models; using Grand.Business.Core.Events.Customers; using Grand.Business.Core.Interfaces.Authentication; @@ -33,6 +34,8 @@ public class OidcLoginControllerTests private Mock _external; private Mock _resolver; private Mock _auth; + private Mock _sessions; + private Dictionary _externalItems; private Mock _mediator; private Mock _externalAuth; private Mock _schemes; @@ -66,6 +69,8 @@ public void Init() _external = new Mock(); _resolver = new Mock(); _auth = new Mock(); + _sessions = new Mock(); + _sessions.Setup(x => x.Start(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync("session-1"); _mediator = new Mock(); _externalAuth = new Mock(); _externalAuth.Setup(x => x.AuthenticationProviderIsAvailable(OidcLoginDefaults.ProviderSystemName)).Returns(true); @@ -87,12 +92,18 @@ public void Init() _httpAuth.Setup(x => x.AuthenticateAsync(It.IsAny(), GrandCookieAuthenticationDefaults.AuthenticationScheme)) .ReturnsAsync(() => AuthenticateResult.Success(new AuthenticationTicket(new ClaimsPrincipal(new ClaimsIdentity("test")), new AuthenticationProperties { IssuedUtc = _ticketIssued }, GrandCookieAuthenticationDefaults.AuthenticationScheme))); + _externalItems = new Dictionary { + [OidcLoginSessionData.RefreshTokenItem] = "rt-1", [OidcLoginSessionData.SidItem] = "sid-1" + }; + _httpAuth.Setup(x => x.AuthenticateAsync(It.IsAny(), GrandCookieAuthenticationDefaults.ExternalAuthenticationScheme)) + .ReturnsAsync(() => AuthenticateResult.Success(new AuthenticationTicket(new ClaimsPrincipal(new ClaimsIdentity("ext")), + new AuthenticationProperties(_externalItems), GrandCookieAuthenticationDefaults.ExternalAuthenticationScheme))); _logger = new Mock>(); var http = new DefaultHttpContext { Session = new TestSession(), RequestServices = new ServiceCollection().AddSingleton(_httpAuth.Object).BuildServiceProvider() }; - _controller = new OidcLoginController(_external.Object, _resolver.Object, _auth.Object, _mediator.Object, + _controller = new OidcLoginController(_external.Object, _resolver.Object, _auth.Object, _sessions.Object, _mediator.Object, _externalAuth.Object, _schemes.Object, _groups.Object, ctx.Object, _logger.Object) { ControllerContext = new ControllerContext { HttpContext = http } }; @@ -148,7 +159,7 @@ public async Task Callback_SignIn_SignsInClearsRedirects() _external.Setup(x => x.GetPending()).ReturnsAsync(Pending); _resolver.Setup(x => x.ResolveForLogin("sub-1", "john@example.com", true, "s1")).ReturnsAsync(OidcLoginResult.SignIn(customer)); var result = await _controller.Callback("/cart"); - _auth.Verify(x => x.SignIn(customer, false), Times.Once); + _auth.Verify(x => x.SignIn(customer, false, "session-1"), Times.Once); _mediator.Verify(x => x.Publish(It.Is(e => e.Customer == customer), It.IsAny()), Times.Once); _external.Verify(x => x.Clear(), Times.Once); Assert.AreEqual("/cart", ((RedirectResult)result).Url); @@ -183,7 +194,7 @@ public async Task Callback_Refused_ClearsAndFailsWithReason() Assert.AreEqual("OidcLoginFailed", result.RouteName); Assert.AreEqual((int)OidcLoginRefusal.AccountInactive, result.RouteValues!["reason"]); _external.Verify(x => x.Clear(), Times.Once); - _auth.Verify(x => x.SignIn(It.IsAny(), It.IsAny()), Times.Never); + _auth.Verify(x => x.SignIn(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } [TestMethod] @@ -301,7 +312,7 @@ public async Task Callback_SignIn_TwoFactorCustomer_SignsInWithoutStoreCode() _resolver.Setup(x => x.ResolveForLogin(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync(OidcLoginResult.SignIn(customer)); Assert.AreEqual("/cart", ((RedirectResult)await _controller.Callback("/cart")).Url); - _auth.Verify(x => x.SignIn(customer, false), Times.Once); + _auth.Verify(x => x.SignIn(customer, false, "session-1"), Times.Once); } [TestMethod] @@ -315,7 +326,7 @@ public async Task Callback_SignIn_OtherAccountSignedIn_ClearsAndFails_SessionKep Assert.AreEqual("OidcLoginFailed", result.RouteName); Assert.AreEqual(true, result.RouteValues!["signedIn"]); _external.Verify(x => x.Clear(), Times.Once); - _auth.Verify(x => x.SignIn(It.IsAny(), It.IsAny()), Times.Never); + _auth.Verify(x => x.SignIn(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); _mediator.Verify(x => x.Publish(It.IsAny(), It.IsAny()), Times.Never); } @@ -328,7 +339,7 @@ public async Task Callback_SignIn_SameAccountSignedIn_RedirectsWithoutNewSignIn( .ReturnsAsync(OidcLoginResult.SignIn(_current)); Assert.AreEqual("/cart", ((RedirectResult)await _controller.Callback("/cart")).Url); _external.Verify(x => x.Clear(), Times.Once); - _auth.Verify(x => x.SignIn(It.IsAny(), It.IsAny()), Times.Never); + _auth.Verify(x => x.SignIn(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } [TestMethod] @@ -414,6 +425,57 @@ public async Task LinkComplete_TicketIssuedAfterLink_Links() _external.Verify(x => x.Complete(_current, "s1"), Times.Once); } + [TestMethod] + public async Task Callback_SignIn_StartsSessionWithTokensOfTheSignIn() + { + var customer = new Customer { Id = "c2" }; + _external.Setup(x => x.GetPending()).ReturnsAsync(Pending); + _resolver.Setup(x => x.ResolveForLogin("sub-1", "john@example.com", true, "s1")).ReturnsAsync(OidcLoginResult.SignIn(customer)); + + await _controller.Callback("/"); + + _sessions.Verify(x => x.Start(customer, "sub-1", "sid-1", "rt-1", "s1"), Times.Once); + _auth.Verify(x => x.SignIn(customer, false, "session-1"), Times.Once); + } + + //a back-office account whose provider sent no refresh token (offline_access missing): no session that dies at once + [TestMethod] + public async Task Callback_SessionNotStarted_FailedPage_NotSignedIn() + { + var customer = new Customer { Id = "c2" }; + _external.Setup(x => x.GetPending()).ReturnsAsync(Pending); + _resolver.Setup(x => x.ResolveForLogin(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(OidcLoginResult.SignIn(customer)); + _sessions.Setup(x => x.Start(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((string)null); + + var result = (RedirectToRouteResult)await _controller.Callback("/"); + + Assert.AreEqual("OidcLoginFailed", result.RouteName); + Assert.AreEqual(true, result.RouteValues!["session"]); + _auth.Verify(x => x.SignIn(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _external.Verify(x => x.Clear(), Times.AtLeastOnce); + } + + [TestMethod] + public void Failed_SessionUnavailable_Model() + { + var model = (FailedModel)((ViewResult)_controller.Failed(null, session: true)).Model!; + Assert.IsTrue(model.SessionUnavailable); + } + + [TestMethod] + public async Task Callback_Signin_ExternalItemsMissing_StartsSessionWithoutTokens() + { + var customer = new Customer { Id = "c2" }; + _externalItems.Clear(); + _external.Setup(x => x.GetPending()).ReturnsAsync(Pending); + _resolver.Setup(x => x.ResolveForLogin(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(OidcLoginResult.SignIn(customer)); + await _controller.Callback("/"); + _sessions.Verify(x => x.Start(customer, "sub-1", null, null, "s1"), Times.Once); + } + [TestMethod] public async Task Callback_SignIn_ExternalReturnUrl_GoesHome() { diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginSessionDataTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginSessionDataTests.cs new file mode 100644 index 0000000000..47af09705e --- /dev/null +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginSessionDataTests.cs @@ -0,0 +1,51 @@ +using Authentication.OpenIdConnect.Infrastructure; +using Microsoft.AspNetCore.Authentication; +using Microsoft.IdentityModel.JsonWebTokens; +using Microsoft.IdentityModel.Protocols.OpenIdConnect; +using Microsoft.IdentityModel.Tokens; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System.Security.Claims; +using System.Text; + +namespace Grand.Modules.Tests.Plugins.OpenIdConnect; + +[TestClass] +public class OidcLoginSessionDataTests +{ + private static string IdToken(params Claim[] claims) => + new JsonWebTokenHandler().CreateToken(new SecurityTokenDescriptor { + Subject = new ClaimsIdentity(claims), + SigningCredentials = new SigningCredentials( + new SymmetricSecurityKey(Encoding.UTF8.GetBytes(new string('k', 32))), SecurityAlgorithms.HmacSha256) + }); + + [TestMethod] + public void Capture_RefreshTokenAndSid() + { + var properties = new AuthenticationProperties(); + OidcLoginSessionData.Capture(properties, new OpenIdConnectMessage { + RefreshToken = "rt-1", IdToken = IdToken(new Claim("sub", "s"), new Claim("sid", "sid-1")) + }); + Assert.AreEqual("rt-1", properties.Items[OidcLoginSessionData.RefreshTokenItem]); + Assert.AreEqual("sid-1", properties.Items[OidcLoginSessionData.SidItem]); + } + + [TestMethod] + public void Capture_NoRefreshTokenNoSid_NothingStored() + { + var properties = new AuthenticationProperties(); + OidcLoginSessionData.Capture(properties, new OpenIdConnectMessage { IdToken = IdToken(new Claim("sub", "s")) }); + Assert.IsFalse(properties.Items.ContainsKey(OidcLoginSessionData.RefreshTokenItem)); + Assert.IsFalse(properties.Items.ContainsKey(OidcLoginSessionData.SidItem)); + } + + [TestMethod] + public void Capture_NullResponse_NoThrow() => + OidcLoginSessionData.Capture(new AuthenticationProperties(), null); + + [TestMethod] + [DataRow(null)] + [DataRow("")] + [DataRow("not-a-jwt")] + public void Sid_Unreadable_Null(string idToken) => Assert.IsNull(OidcLoginSessionData.Sid(idToken)); +} From 41dbec5d67e2225f26894c0d36b1d213adb17a12 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 07:02:27 +0200 Subject: [PATCH 17/28] OpenID Connect login: an unreadable sid claim never fails the sign-in callback --- .../Infrastructure/OidcLoginSessionData.cs | 3 ++- .../Plugins/OpenIdConnect/OidcLoginSessionDataTests.cs | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLoginSessionData.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLoginSessionData.cs index c83b2eeafa..aef9f092b3 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLoginSessionData.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLoginSessionData.cs @@ -31,13 +31,14 @@ public static string Sid(string idToken) { if (string.IsNullOrEmpty(idToken)) return null; + //sid is optional: whatever the token holds, the sign-in must go on try { return new JsonWebToken(idToken).TryGetPayloadValue("sid", out var sid) && !string.IsNullOrEmpty(sid) ? sid : null; } - catch (ArgumentException) + catch (Exception) { return null; } diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginSessionDataTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginSessionDataTests.cs index 47af09705e..fbf31fdeb5 100644 --- a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginSessionDataTests.cs +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginSessionDataTests.cs @@ -48,4 +48,9 @@ public void Capture_NullResponse_NoThrow() => [DataRow("")] [DataRow("not-a-jwt")] public void Sid_Unreadable_Null(string idToken) => Assert.IsNull(OidcLoginSessionData.Sid(idToken)); + + //sid is optional: a value of another type must never fail the sign-in callback + [TestMethod] + public void Sid_NonStringValue_NoThrow() => + OidcLoginSessionData.Sid(IdToken(new Claim("sub", "s"), new Claim("sid", "5", ClaimValueTypes.Integer))); } From ca01b2c6f78e21a1e336d891d4a2479107917f86 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 07:08:47 +0200 Subject: [PATCH 18/28] OpenID Connect login: confirm back-office sessions by exchanging the refresh token --- .../Infrastructure/OidcTokenRefresher.cs | 93 +++++++++++ .../Infrastructure/StartupApplication.cs | 2 + .../OidcLoginDefaults.cs | 1 + .../OpenIdConnect/OidcTokenRefresherTests.cs | 144 ++++++++++++++++++ 4 files changed, 240 insertions(+) create mode 100644 src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcTokenRefresher.cs create mode 100644 src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcTokenRefresherTests.cs diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcTokenRefresher.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcTokenRefresher.cs new file mode 100644 index 0000000000..bece43dde4 --- /dev/null +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcTokenRefresher.cs @@ -0,0 +1,93 @@ +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Business.Core.Utilities.Authentication; +using Grand.Infrastructure.Configuration; +using Microsoft.AspNetCore.Authentication.OpenIdConnect; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using System.Net; +using System.Text.Json; + +namespace Authentication.OpenIdConnect.Infrastructure; + +/// +/// Confirms a back-office session at the identity provider by exchanging its refresh token (RFC 6749, section 6) +/// +public class OidcTokenRefresher( + IHttpClientFactory httpClientFactory, + IOptionsMonitor optionsMonitor, + OidcLoginConfig config, + ILogger logger) : IOidcTokenRefresher +{ + public async Task Refresh(string refreshToken, CancellationToken cancellationToken = default) + { + ArgumentException.ThrowIfNullOrEmpty(refreshToken); + + var options = optionsMonitor.Get(OidcLoginDefaults.AuthenticationScheme); + if (options.ConfigurationManager == null) + { + logger.LogWarning("OpenID Connect login is not configured: back-office sessions cannot be confirmed"); + return OidcRefreshResult.Refused(); + } + + try + { + var configuration = await options.ConfigurationManager.GetConfigurationAsync(cancellationToken); + if (string.IsNullOrEmpty(configuration.TokenEndpoint)) + { + logger.LogWarning("The identity provider metadata has no token endpoint"); + return OidcRefreshResult.Unavailable(); + } + + var form = new Dictionary { + ["grant_type"] = "refresh_token", ["refresh_token"] = refreshToken, ["client_id"] = config.ClientId + }; + if (!string.IsNullOrEmpty(config.ClientSecret)) + form["client_secret"] = config.ClientSecret; + + using var client = httpClientFactory.CreateClient(OidcLoginDefaults.HttpClientName); + using var response = await client.PostAsync(configuration.TokenEndpoint, new FormUrlEncodedContent(form), + cancellationToken); + var body = await response.Content.ReadAsStringAsync(cancellationToken); + + if (response.StatusCode is HttpStatusCode.BadRequest or HttpStatusCode.Unauthorized) + { + //the error code only, never the body: it may echo the token + logger.LogInformation("The identity provider refused a refresh token: {Status} {Error}", + (int)response.StatusCode, ErrorCode(body)); + return OidcRefreshResult.Refused(); + } + + if (!response.IsSuccessStatusCode) + { + logger.LogWarning("The identity provider token endpoint answered {Status}", (int)response.StatusCode); + return OidcRefreshResult.Unavailable(); + } + + using var document = JsonDocument.Parse(body); + var rotated = document.RootElement.TryGetProperty("refresh_token", out var value) && + value.ValueKind == JsonValueKind.String + ? value.GetString() + : null; + return OidcRefreshResult.Refreshed(rotated); + } + catch (Exception ex) when (ex is HttpRequestException or TaskCanceledException or JsonException + or InvalidOperationException or IOException) + { + logger.LogWarning("The identity provider could not be reached to confirm a session ({Exception})", ex.GetType().Name); + return OidcRefreshResult.Unavailable(); + } + } + + private static string ErrorCode(string body) + { + try + { + using var document = JsonDocument.Parse(body); + return document.RootElement.TryGetProperty("error", out var error) ? error.GetString() : null; + } + catch (JsonException) + { + return null; + } + } +} diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs index c7bd259abd..bec3536603 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs @@ -12,6 +12,8 @@ public class StartupApplication : IStartupApplication public void ConfigureServices(IServiceCollection services, IConfiguration configuration) { services.AddScoped(); + services.AddHttpClient(OidcLoginDefaults.HttpClientName, client => client.Timeout = TimeSpan.FromSeconds(10)); + services.AddScoped(); } public int Priority => 10; diff --git a/src/Plugins/Authentication.OpenIdConnect/OidcLoginDefaults.cs b/src/Plugins/Authentication.OpenIdConnect/OidcLoginDefaults.cs index c7a2b898ac..d6d23ce29d 100644 --- a/src/Plugins/Authentication.OpenIdConnect/OidcLoginDefaults.cs +++ b/src/Plugins/Authentication.OpenIdConnect/OidcLoginDefaults.cs @@ -10,6 +10,7 @@ public static class OidcLoginDefaults public const string AuthenticationScheme = "OidcLogin"; public const string CallbackPath = "/signin-oidc"; public const string ConfigSection = "OidcLogin"; + public const string HttpClientName = "Grand.OidcLogin"; public const string FailedPath = "/oidc/failed"; public const string ConfigurationUrl = "../OidcLoginSettings/Configure"; public static readonly TimeSpan PendingLifetime = TimeSpan.FromMinutes(15); diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcTokenRefresherTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcTokenRefresherTests.cs new file mode 100644 index 0000000000..cd288cd133 --- /dev/null +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcTokenRefresherTests.cs @@ -0,0 +1,144 @@ +using Authentication.OpenIdConnect; +using Authentication.OpenIdConnect.Infrastructure; +using Grand.Business.Core.Utilities.Authentication; +using Grand.Infrastructure.Configuration; +using Microsoft.AspNetCore.Authentication.OpenIdConnect; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Microsoft.IdentityModel.Protocols; +using Microsoft.IdentityModel.Protocols.OpenIdConnect; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using System.Net; + +namespace Grand.Modules.Tests.Plugins.OpenIdConnect; + +[TestClass] +public class OidcTokenRefresherTests +{ + private sealed class StubHandler(Func> answer) : HttpMessageHandler + { + public string LastBody { get; private set; } + + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + LastBody = request.Content == null ? null : await request.Content.ReadAsStringAsync(cancellationToken); + return await answer(request); + } + } + + private StubHandler _handler; + private OpenIdConnectOptions _options; + private OidcLoginConfig _config; + + [TestInitialize] + public void Init() + { + _options = new OpenIdConnectOptions { + ConfigurationManager = new StaticConfigurationManager( + new OpenIdConnectConfiguration { TokenEndpoint = "https://idp.test/token" }) + }; + _config = new OidcLoginConfig { ClientId = "shop", ClientSecret = "secret" }; + Answer(HttpStatusCode.OK, "{\"access_token\":\"at\",\"refresh_token\":\"rt-2\"}"); + } + + private void Answer(HttpStatusCode status, string json) => + _handler = new StubHandler(_ => Task.FromResult(new HttpResponseMessage(status) { + Content = new StringContent(json ?? "", System.Text.Encoding.UTF8, "application/json") + })); + + private OidcTokenRefresher Refresher() + { + var factory = new Mock(); + factory.Setup(x => x.CreateClient(OidcLoginDefaults.HttpClientName)).Returns(() => new HttpClient(_handler)); + var monitor = new Mock>(); + monitor.Setup(x => x.Get(OidcLoginDefaults.AuthenticationScheme)).Returns(_options); + return new OidcTokenRefresher(factory.Object, monitor.Object, _config, NullLogger.Instance); + } + + [TestMethod] + public async Task Ok_Rotated() + { + var result = await Refresher().Refresh("rt-1"); + Assert.AreEqual(OidcRefreshOutcome.Refreshed, result.Outcome); + Assert.AreEqual("rt-2", result.RefreshToken); + StringAssert.Contains(_handler.LastBody, "grant_type=refresh_token"); + StringAssert.Contains(_handler.LastBody, "refresh_token=rt-1"); + StringAssert.Contains(_handler.LastBody, "client_id=shop"); + StringAssert.Contains(_handler.LastBody, "client_secret=secret"); + } + + [TestMethod] + public async Task Ok_NotRotated_NullToken() + { + Answer(HttpStatusCode.OK, "{\"access_token\":\"at\"}"); + var result = await Refresher().Refresh("rt-1"); + Assert.AreEqual(OidcRefreshOutcome.Refreshed, result.Outcome); + Assert.IsNull(result.RefreshToken); + } + + [TestMethod] + public async Task PublicClient_NoSecretSent() + { + _config.ClientSecret = ""; + await Refresher().Refresh("rt-1"); + Assert.IsFalse(_handler.LastBody.Contains("client_secret")); + } + + [TestMethod] + [DataRow(HttpStatusCode.BadRequest, "{\"error\":\"invalid_grant\"}")] + [DataRow(HttpStatusCode.Unauthorized, "{\"error\":\"invalid_client\"}")] + public async Task ClientErrors_Refused(HttpStatusCode status, string json) + { + Answer(status, json); + Assert.AreEqual(OidcRefreshOutcome.Refused, (await Refresher().Refresh("rt-1")).Outcome); + } + + [TestMethod] + [DataRow(HttpStatusCode.InternalServerError)] + [DataRow(HttpStatusCode.BadGateway)] + [DataRow(HttpStatusCode.TooManyRequests)] + public async Task OtherStatus_Unavailable(HttpStatusCode status) + { + Answer(status, ""); + Assert.AreEqual(OidcRefreshOutcome.Unavailable, (await Refresher().Refresh("rt-1")).Outcome); + } + + [TestMethod] + public async Task NetworkError_Unavailable() + { + _handler = new StubHandler(_ => throw new HttpRequestException("down")); + Assert.AreEqual(OidcRefreshOutcome.Unavailable, (await Refresher().Refresh("rt-1")).Outcome); + } + + [TestMethod] + public async Task Timeout_Unavailable() + { + _handler = new StubHandler(_ => throw new TaskCanceledException("timeout")); + Assert.AreEqual(OidcRefreshOutcome.Unavailable, (await Refresher().Refresh("rt-1")).Outcome); + } + + [TestMethod] + public async Task InvalidJson_Unavailable() + { + Answer(HttpStatusCode.OK, ""); + Assert.AreEqual(OidcRefreshOutcome.Unavailable, (await Refresher().Refresh("rt-1")).Outcome); + } + + [TestMethod] + public async Task MetadataUnobtainable_Unavailable() + { + var manager = new Mock>(); + manager.Setup(x => x.GetConfigurationAsync(It.IsAny())).ThrowsAsync(new InvalidOperationException("IDX20803")); + _options.ConfigurationManager = manager.Object; + Assert.AreEqual(OidcRefreshOutcome.Unavailable, (await Refresher().Refresh("rt-1")).Outcome); + } + + //no sign-in scheme (not configured): nothing can ever confirm the session + [TestMethod] + public async Task NotConfigured_Refused() + { + _options.ConfigurationManager = null; + Assert.AreEqual(OidcRefreshOutcome.Refused, (await Refresher().Refresh("rt-1")).Outcome); + } +} From 3ca37a56e075f6226bc72c41a0733b74f8142bd2 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 07:48:29 +0200 Subject: [PATCH 19/28] OpenID Connect login: validate back-channel logout tokens --- .../Infrastructure/OidcLogoutToken.cs | 7 + .../OidcLogoutTokenValidator.cs | 96 ++++++++++ .../Infrastructure/StartupApplication.cs | 1 + .../OidcLogoutTokenValidatorTests.cs | 173 ++++++++++++++++++ 4 files changed, 277 insertions(+) create mode 100644 src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutToken.cs create mode 100644 src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs create mode 100644 src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutToken.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutToken.cs new file mode 100644 index 0000000000..bdf58370bb --- /dev/null +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutToken.cs @@ -0,0 +1,7 @@ +namespace Authentication.OpenIdConnect.Infrastructure; + +public sealed record OidcLogoutToken(bool IsValid, string Subject, string Sid, string Error) +{ + public static OidcLogoutToken Valid(string subject, string sid) => new(true, subject, sid, null); + public static OidcLogoutToken Invalid(string error) => new(false, null, null, error); +} diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs new file mode 100644 index 0000000000..6a1a04d6bb --- /dev/null +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs @@ -0,0 +1,96 @@ +using Microsoft.Extensions.Caching.Memory; +using Microsoft.IdentityModel.JsonWebTokens; +using Microsoft.IdentityModel.Protocols.OpenIdConnect; +using Microsoft.IdentityModel.Tokens; +using System.Text.Json; + +namespace Authentication.OpenIdConnect.Infrastructure; + +/// +/// Validates a logout token of OpenID Connect Back-Channel Logout 1.0 +/// +public class OidcLogoutTokenValidator(IMemoryCache cache) +{ + public const string BackChannelLogoutEvent = "http://schemas.openid.net/event/backchannel-logout"; + + /// A logout token older than this is refused (it is sent at once by the provider) + public static readonly TimeSpan MaxAge = TimeSpan.FromMinutes(5); + + private static readonly string[] AsymmetricAlgorithms = [ + SecurityAlgorithms.RsaSha256, SecurityAlgorithms.RsaSha384, SecurityAlgorithms.RsaSha512, + SecurityAlgorithms.RsaSsaPssSha256, SecurityAlgorithms.RsaSsaPssSha384, SecurityAlgorithms.RsaSsaPssSha512, + SecurityAlgorithms.EcdsaSha256, SecurityAlgorithms.EcdsaSha384, SecurityAlgorithms.EcdsaSha512 + ]; + + public static bool TypeAccepted(string typ) => + string.IsNullOrEmpty(typ) || typ.Equals("logout+jwt", StringComparison.OrdinalIgnoreCase) || + typ.Equals("JWT", StringComparison.OrdinalIgnoreCase); + + public async Task Validate(string token, OpenIdConnectConfiguration configuration, string clientId, + TimeSpan clockSkew) + { + if (string.IsNullOrEmpty(token) || configuration == null || string.IsNullOrEmpty(clientId)) + return OidcLogoutToken.Invalid("The logout token is missing"); + + var parameters = new TokenValidationParameters { + ValidIssuer = configuration.Issuer, + ValidAudience = clientId, + IssuerSigningKeys = configuration.SigningKeys, + ValidAlgorithms = AsymmetricAlgorithms, + RequireSignedTokens = true, + ValidateIssuerSigningKey = true, + //exp is optional in a logout token; when present it is checked + RequireExpirationTime = false, + ValidateLifetime = true, + ClockSkew = clockSkew, + TypeValidator = (type, securityToken, _) => + TypeAccepted(type) ? type : throw new SecurityTokenInvalidTypeException("The token type is not accepted") + }; + + var result = await new JsonWebTokenHandler().ValidateTokenAsync(token, parameters); + if (!result.IsValid || result.SecurityToken is not JsonWebToken jwt) + return OidcLogoutToken.Invalid("The logout token is invalid"); + + if (!HasBackChannelEvent(jwt)) + return OidcLogoutToken.Invalid("The logout token has no back-channel logout event"); + if (jwt.TryGetPayloadValue("nonce", out _)) + return OidcLogoutToken.Invalid("A logout token must not have a nonce"); + + var now = DateTime.UtcNow; + if (jwt.IssuedAt == DateTime.MinValue || jwt.IssuedAt > now + clockSkew || now - jwt.IssuedAt > MaxAge + clockSkew) + return OidcLogoutToken.Invalid("The logout token is not recent"); + + var subject = jwt.TryGetPayloadValue("sub", out var sub) && !string.IsNullOrEmpty(sub) ? sub : null; + var sid = jwt.TryGetPayloadValue("sid", out var sidValue) && !string.IsNullOrEmpty(sidValue) ? sidValue : null; + if (subject == null && sid == null) + return OidcLogoutToken.Invalid("The logout token names no subject and no session"); + + if (!string.IsNullOrEmpty(jwt.Id)) + { + var key = "Grand.OidcLogin.LogoutJti." + jwt.Issuer + "|" + jwt.Id; + if (cache.TryGetValue(key, out _)) + return OidcLogoutToken.Invalid("The logout token was already used"); + var until = (jwt.ValidTo != DateTime.MinValue ? jwt.ValidTo : jwt.IssuedAt + MaxAge) + clockSkew; + cache.Set(key, true, new DateTimeOffset(DateTime.SpecifyKind(until, DateTimeKind.Utc))); + } + + return OidcLogoutToken.Valid(subject, sid); + } + + private static bool HasBackChannelEvent(JsonWebToken jwt) + { + var events = jwt.Claims.FirstOrDefault(c => c.Type == "events")?.Value; + if (string.IsNullOrEmpty(events)) + return false; + try + { + using var document = JsonDocument.Parse(events); + return document.RootElement.ValueKind == JsonValueKind.Object && + document.RootElement.TryGetProperty(BackChannelLogoutEvent, out _); + } + catch (JsonException) + { + return false; + } + } +} diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs index bec3536603..3154f83dbd 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs @@ -14,6 +14,7 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config services.AddScoped(); services.AddHttpClient(OidcLoginDefaults.HttpClientName, client => client.Timeout = TimeSpan.FromSeconds(10)); services.AddScoped(); + services.AddSingleton(); } public int Priority => 10; diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs new file mode 100644 index 0000000000..042fd96eba --- /dev/null +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs @@ -0,0 +1,173 @@ +using Authentication.OpenIdConnect.Infrastructure; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.IdentityModel.JsonWebTokens; +using Microsoft.IdentityModel.Protocols.OpenIdConnect; +using Microsoft.IdentityModel.Tokens; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System.Security.Cryptography; +using System.Text; + +namespace Grand.Modules.Tests.Plugins.OpenIdConnect; + +[TestClass] +public class OidcLogoutTokenValidatorTests +{ + private const string Issuer = "https://idp.test/realms/shop"; + private const string ClientId = "shop"; + private static readonly RsaSecurityKey Key = NewKey(); + private static readonly RsaSecurityKey OtherKey = NewKey(); + private OidcLogoutTokenValidator _validator; + private OpenIdConnectConfiguration _configuration; + + private static RsaSecurityKey NewKey() + { + using var rsa = RSA.Create(2048); + return new RsaSecurityKey(rsa.ExportParameters(true)) { KeyId = Guid.NewGuid().ToString("N") }; + } + + [TestInitialize] + public void Init() + { + _validator = new OidcLogoutTokenValidator(new MemoryCache(new MemoryCacheOptions())); + _configuration = new OpenIdConnectConfiguration { Issuer = Issuer }; + _configuration.SigningKeys.Add(Key); + } + + private static string Token(Dictionary claims = null, string typ = "logout+jwt", SecurityKey key = null, + string algorithm = SecurityAlgorithms.RsaSha256, string issuer = Issuer, object audience = null, + DateTime? issuedAt = null, bool events = true, string jti = null) + { + var payload = new Dictionary { + ["iss"] = issuer, + ["aud"] = audience ?? ClientId, + ["iat"] = EpochTime.GetIntDate(issuedAt ?? DateTime.UtcNow), + ["jti"] = jti ?? Guid.NewGuid().ToString("N"), + ["sub"] = "sub-1", + ["sid"] = "sid-1" + }; + if (events) + payload["events"] = new Dictionary { [OidcLogoutTokenValidator.BackChannelLogoutEvent] = new Dictionary() }; + foreach (var (name, value) in claims ?? []) + if (value == null) payload.Remove(name); else payload[name] = value; + + var descriptor = new SecurityTokenDescriptor { + Claims = payload, + SigningCredentials = new SigningCredentials(key ?? Key, algorithm), + TokenType = typ + }; + return new JsonWebTokenHandler { SetDefaultTimesOnTokenCreation = false }.CreateToken(descriptor); + } + + private Task Validate(string token) => + _validator.Validate(token, _configuration, ClientId, TimeSpan.FromSeconds(60)); + + [TestMethod] + [DataRow("logout+jwt")] + [DataRow("JWT")] + public async Task Valid_BothTypes(string typ) + { + var result = await Validate(Token(typ: typ)); + Assert.IsTrue(result.IsValid, result.Error); + Assert.AreEqual("sub-1", result.Subject); + Assert.AreEqual("sid-1", result.Sid); + } + + //Zitadel sends aud as an array + [TestMethod] + public async Task Valid_AudienceArray() => + Assert.IsTrue((await Validate(Token(audience: new[] { ClientId, "other" }))).IsValid); + + [TestMethod] + public async Task Valid_SidOnly() + { + var result = await Validate(Token(new() { ["sub"] = null })); + Assert.IsTrue(result.IsValid, result.Error); + Assert.IsNull(result.Subject); + } + + [TestMethod] + public async Task Valid_SubOnly() + { + var result = await Validate(Token(new() { ["sid"] = null })); + Assert.IsTrue(result.IsValid, result.Error); + Assert.IsNull(result.Sid); + } + + [TestMethod] + public async Task Invalid_NeitherSubNorSid() => + Assert.IsFalse((await Validate(Token(new() { ["sub"] = null, ["sid"] = null }))).IsValid); + + [TestMethod] + public async Task Invalid_NoEvents() => Assert.IsFalse((await Validate(Token(events: false))).IsValid); + + [TestMethod] + public async Task Invalid_OtherEvent() => + Assert.IsFalse((await Validate(Token(new() { + ["events"] = new Dictionary { ["urn:other"] = new Dictionary() } + }, events: false))).IsValid); + + //an ID token replayed as a logout token + [TestMethod] + public async Task Invalid_Nonce() => Assert.IsFalse((await Validate(Token(new() { ["nonce"] = "n" }))).IsValid); + + [TestMethod] + public async Task Invalid_OldIat() => + Assert.IsFalse((await Validate(Token(issuedAt: DateTime.UtcNow.AddMinutes(-10)))).IsValid); + + [TestMethod] + public async Task Invalid_FutureIat() => + Assert.IsFalse((await Validate(Token(issuedAt: DateTime.UtcNow.AddMinutes(10)))).IsValid); + + [TestMethod] + public async Task Invalid_NoIat() => Assert.IsFalse((await Validate(Token(new() { ["iat"] = null }))).IsValid); + + [TestMethod] + public async Task Invalid_Expired() => + Assert.IsFalse((await Validate(Token(new() { ["exp"] = EpochTime.GetIntDate(DateTime.UtcNow.AddMinutes(-3)) }))).IsValid); + + [TestMethod] + public async Task Invalid_Replay() + { + var token = Token(jti: "jti-1"); + Assert.IsTrue((await Validate(token)).IsValid); + var replay = await Validate(token); + Assert.IsFalse(replay.IsValid); + } + + [TestMethod] + public async Task Valid_NoJti() => Assert.IsTrue((await Validate(Token(new() { ["jti"] = null }))).IsValid); + + [TestMethod] + public async Task Invalid_WrongIssuer() => Assert.IsFalse((await Validate(Token(issuer: "https://evil.test"))).IsValid); + + [TestMethod] + public async Task Invalid_WrongAudience() => Assert.IsFalse((await Validate(Token(audience: "other"))).IsValid); + + [TestMethod] + public async Task Invalid_UnknownKey() => Assert.IsFalse((await Validate(Token(key: OtherKey))).IsValid); + + //a symmetric token: the client secret is known to the client, so it must never sign a logout + [TestMethod] + public async Task Invalid_Symmetric() + { + var secret = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(new string('s', 32))); + _configuration.SigningKeys.Add(secret); + Assert.IsFalse((await Validate(Token(key: secret, algorithm: SecurityAlgorithms.HmacSha256))).IsValid); + } + + [TestMethod] + public async Task Invalid_OtherTyp() => Assert.IsFalse((await Validate(Token(typ: "at+jwt"))).IsValid); + + [TestMethod] + public async Task Invalid_Garbage() => Assert.IsFalse((await Validate("not.a.token")).IsValid); + + [TestMethod] + [DataRow(null, true)] + [DataRow("", true)] + [DataRow("logout+jwt", true)] + [DataRow("LOGOUT+JWT", true)] + [DataRow("jwt", true)] + [DataRow("at+jwt", false)] + public void TypeAccepted(string typ, bool expected) => + Assert.AreEqual(expected, OidcLogoutTokenValidator.TypeAccepted(typ)); +} From c86d7f0fa2fe7ef8534a31deee61f5b8bc267500 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 07:55:48 +0200 Subject: [PATCH 20/28] OpenID Connect login: bound the logout token replay cache and name the failure type --- .../Infrastructure/OidcLogoutTokenValidator.cs | 7 +++++-- .../OpenIdConnect/OidcLogoutTokenValidatorTests.cs | 13 +++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs index 6a1a04d6bb..8b05840114 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs @@ -49,7 +49,8 @@ public async Task Validate(string token, OpenIdConnectConfigura var result = await new JsonWebTokenHandler().ValidateTokenAsync(token, parameters); if (!result.IsValid || result.SecurityToken is not JsonWebToken jwt) - return OidcLogoutToken.Invalid("The logout token is invalid"); + return OidcLogoutToken.Invalid( + $"The logout token is invalid ({result.Exception?.GetType().Name ?? "unknown"})"); if (!HasBackChannelEvent(jwt)) return OidcLogoutToken.Invalid("The logout token has no back-channel logout event"); @@ -70,7 +71,9 @@ public async Task Validate(string token, OpenIdConnectConfigura var key = "Grand.OidcLogin.LogoutJti." + jwt.Issuer + "|" + jwt.Id; if (cache.TryGetValue(key, out _)) return OidcLogoutToken.Invalid("The logout token was already used"); - var until = (jwt.ValidTo != DateTime.MinValue ? jwt.ValidTo : jwt.IssuedAt + MaxAge) + clockSkew; + //the iat check above already refuses the token after IssuedAt + MaxAge + skew, so that bound is replay-safe + var latest = jwt.IssuedAt + MaxAge; + var until = (jwt.ValidTo != DateTime.MinValue && jwt.ValidTo < latest ? jwt.ValidTo : latest) + clockSkew; cache.Set(key, true, new DateTimeOffset(DateTime.SpecifyKind(until, DateTimeKind.Utc))); } diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs index 042fd96eba..0d04626508 100644 --- a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs @@ -125,6 +125,19 @@ public async Task Invalid_FutureIat() => public async Task Invalid_Expired() => Assert.IsFalse((await Validate(Token(new() { ["exp"] = EpochTime.GetIntDate(DateTime.UtcNow.AddMinutes(-3)) }))).IsValid); + //an extreme exp must not overflow the replay-cache expiry + [TestMethod] + public async Task Valid_ExtremeExp_NoOverflow() => + Assert.IsTrue((await Validate(Token(new() { ["exp"] = 253402300799L }, jti: "jti-far"))).IsValid); + + [TestMethod] + public async Task Invalid_ErrorNamesExceptionType_NotTokenContent() + { + var result = await Validate(Token(issuer: "https://evil.test")); + Assert.IsFalse(result.IsValid); + StringAssert.Contains(result.Error, "SecurityTokenInvalidIssuerException"); + } + [TestMethod] public async Task Invalid_Replay() { From f4f10d42f670427929341de55c12d1815cf67660 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 08:03:36 +0200 Subject: [PATCH 21/28] OpenID Connect login: back-channel logout endpoint --- .../EndpointProvider.cs | 5 + .../OidcBackChannelLogoutEndpoint.cs | 79 +++++++++ .../OidcLoginDefaults.cs | 1 + .../OidcBackChannelLogoutEndpointTests.cs | 155 ++++++++++++++++++ 4 files changed, 240 insertions(+) create mode 100644 src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcBackChannelLogoutEndpoint.cs create mode 100644 src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcBackChannelLogoutEndpointTests.cs diff --git a/src/Plugins/Authentication.OpenIdConnect/EndpointProvider.cs b/src/Plugins/Authentication.OpenIdConnect/EndpointProvider.cs index 1c889aa002..a1535f9688 100644 --- a/src/Plugins/Authentication.OpenIdConnect/EndpointProvider.cs +++ b/src/Plugins/Authentication.OpenIdConnect/EndpointProvider.cs @@ -1,5 +1,7 @@ +using Authentication.OpenIdConnect.Infrastructure; using Grand.Infrastructure.Endpoints; using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; namespace Authentication.OpenIdConnect; @@ -18,6 +20,9 @@ public void RegisterEndpoint(IEndpointRouteBuilder endpointRouteBuilder) new { controller = "OidcLogin", action = "LinkComplete" }); endpointRouteBuilder.MapControllerRoute("OidcLoginFailed", "oidc/failed", new { controller = "OidcLogin", action = "Failed" }); + //called by the identity provider, server to server: no antiforgery, no store context, no MVC filters + endpointRouteBuilder.MapPost(OidcLoginDefaults.BackChannelLogoutPath, + (Func>)OidcBackChannelLogoutEndpoint.Handle); } public int Priority => 10; diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcBackChannelLogoutEndpoint.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcBackChannelLogoutEndpoint.cs new file mode 100644 index 0000000000..51437c41f1 --- /dev/null +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcBackChannelLogoutEndpoint.cs @@ -0,0 +1,79 @@ +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Infrastructure.Configuration; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.OpenIdConnect; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace Authentication.OpenIdConnect.Infrastructure; + +/// +/// OpenID Connect Back-Channel Logout 1.0: the identity provider tells the store that a provider session ended; the +/// store ends its sessions of that provider session (or subject) and refuses the subject's older access tokens +/// +public static class OidcBackChannelLogoutEndpoint +{ + public static async Task Handle(HttpContext context) + { + context.Response.Headers.CacheControl = "no-store"; + var services = context.RequestServices; + var logger = services.GetRequiredService().CreateLogger(typeof(OidcBackChannelLogoutEndpoint)); + + if (!context.Request.HasFormContentType) + return Error("The request must be a form post"); + var form = await context.Request.ReadFormAsync(context.RequestAborted); + var token = form["logout_token"].ToString(); + if (string.IsNullOrEmpty(token)) + return Error("The logout_token parameter is missing"); + + if (await services.GetRequiredService() + .GetSchemeAsync(OidcLoginDefaults.AuthenticationScheme) == null) + return Error("OpenID Connect sign-in is not configured"); + + var options = services.GetRequiredService>() + .Get(OidcLoginDefaults.AuthenticationScheme); + Microsoft.IdentityModel.Protocols.OpenIdConnect.OpenIdConnectConfiguration configuration; + try + { + configuration = await options.ConfigurationManager!.GetConfigurationAsync(context.RequestAborted); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + logger.LogError(ex, "Back-channel logout: the identity provider metadata could not be obtained"); + return Results.StatusCode(StatusCodes.Status503ServiceUnavailable); + } + + var clockSkew = TimeSpan.FromSeconds(services.GetRequiredService().ClockSkewSeconds); + var logout = await services.GetRequiredService().Validate(token, configuration, + services.GetRequiredService().ClientId, clockSkew); + if (!logout.IsValid) + { + logger.LogWarning("Back-channel logout refused: {Reason}", logout.Error); + return Error(logout.Error); + } + + var sessionService = services.GetRequiredService(); + var ended = logout.Sid != null + ? await sessionService.EndBySid(logout.Subject, logout.Sid) + : await sessionService.EndBySubject(logout.Subject); + + //a logout token with sid only names the subject through the sessions it ended + var subjects = logout.Subject != null + ? [logout.Subject] + : ended.Select(x => x.Subject).Where(x => !string.IsNullOrEmpty(x)).Distinct().ToList(); + var revocation = services.GetRequiredService(); + foreach (var subject in subjects) + await revocation.RevokeTokens(subject); + + logger.LogInformation("Back-channel logout: {Count} session(s) ended for subject {Subject}, session {Sid}", + ended.Count, logout.Subject, logout.Sid); + return Results.Ok(); + } + + private static IResult Error(string description) => + Results.Json(new Dictionary { + ["error"] = "invalid_request", ["error_description"] = description + }, statusCode: StatusCodes.Status400BadRequest); +} diff --git a/src/Plugins/Authentication.OpenIdConnect/OidcLoginDefaults.cs b/src/Plugins/Authentication.OpenIdConnect/OidcLoginDefaults.cs index d6d23ce29d..2cce9a04a4 100644 --- a/src/Plugins/Authentication.OpenIdConnect/OidcLoginDefaults.cs +++ b/src/Plugins/Authentication.OpenIdConnect/OidcLoginDefaults.cs @@ -11,6 +11,7 @@ public static class OidcLoginDefaults public const string CallbackPath = "/signin-oidc"; public const string ConfigSection = "OidcLogin"; public const string HttpClientName = "Grand.OidcLogin"; + public const string BackChannelLogoutPath = "oidc/backchannel-logout"; public const string FailedPath = "/oidc/failed"; public const string ConfigurationUrl = "../OidcLoginSettings/Configure"; public static readonly TimeSpan PendingLifetime = TimeSpan.FromMinutes(15); diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcBackChannelLogoutEndpointTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcBackChannelLogoutEndpointTests.cs new file mode 100644 index 0000000000..abdb66c8b2 --- /dev/null +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcBackChannelLogoutEndpointTests.cs @@ -0,0 +1,155 @@ +using Authentication.OpenIdConnect; +using Authentication.OpenIdConnect.Infrastructure; +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Domain.Customers; +using Grand.Infrastructure.Configuration; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.OpenIdConnect; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Microsoft.IdentityModel.JsonWebTokens; +using Microsoft.IdentityModel.Protocols; +using Microsoft.IdentityModel.Protocols.OpenIdConnect; +using Microsoft.IdentityModel.Tokens; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using System.Security.Cryptography; + +namespace Grand.Modules.Tests.Plugins.OpenIdConnect; + +[TestClass] +public class OidcBackChannelLogoutEndpointTests +{ + private const string Issuer = "https://idp.test/realms/shop"; + private static readonly RsaSecurityKey Key = NewKey(); + private Mock _sessions; + private Mock _revocation; + private OpenIdConnectOptions _options; + private bool _schemeRegistered; + + private static RsaSecurityKey NewKey() + { + using var rsa = RSA.Create(2048); + return new RsaSecurityKey(rsa.ExportParameters(true)) { KeyId = "k1" }; + } + + [TestInitialize] + public void Init() + { + _sessions = new Mock(); + _sessions.Setup(x => x.EndBySid(It.IsAny(), It.IsAny())).ReturnsAsync([]); + _sessions.Setup(x => x.EndBySubject(It.IsAny())).ReturnsAsync([]); + _revocation = new Mock(); + var configuration = new OpenIdConnectConfiguration { Issuer = Issuer }; + configuration.SigningKeys.Add(Key); + _options = new OpenIdConnectOptions { + ConfigurationManager = new StaticConfigurationManager(configuration) + }; + _schemeRegistered = true; + } + + private static string Token(string sub = "sub-1", string sid = "sid-1") + { + var claims = new Dictionary { + ["iss"] = Issuer, ["aud"] = "shop", ["iat"] = EpochTime.GetIntDate(DateTime.UtcNow), + ["jti"] = Guid.NewGuid().ToString("N"), + ["events"] = new Dictionary { [OidcLogoutTokenValidator.BackChannelLogoutEvent] = new Dictionary() } + }; + if (sub != null) claims["sub"] = sub; + if (sid != null) claims["sid"] = sid; + return new JsonWebTokenHandler { SetDefaultTimesOnTokenCreation = false }.CreateToken(new SecurityTokenDescriptor { + Claims = claims, SigningCredentials = new SigningCredentials(Key, SecurityAlgorithms.RsaSha256), TokenType = "logout+jwt" + }); + } + + private async Task Post(string logoutToken) + { + var schemes = new Mock(); + schemes.Setup(x => x.GetSchemeAsync(OidcLoginDefaults.AuthenticationScheme)).ReturnsAsync(() => _schemeRegistered + ? new AuthenticationScheme(OidcLoginDefaults.AuthenticationScheme, null, typeof(IAuthenticationHandler)) + : null); + var monitor = new Mock>(); + monitor.Setup(x => x.Get(OidcLoginDefaults.AuthenticationScheme)).Returns(_options); + var services = new ServiceCollection() + .AddSingleton(schemes.Object) + .AddSingleton(monitor.Object) + .AddSingleton(new OidcLogoutTokenValidator(new MemoryCache(new MemoryCacheOptions()))) + .AddSingleton(_sessions.Object) + .AddSingleton(_revocation.Object) + .AddSingleton(new OidcLoginConfig { ClientId = "shop" }) + .AddSingleton(new OidcResourceServerConfig { ClockSkewSeconds = 60 }) + .AddSingleton(NullLoggerFactory.Instance) + .BuildServiceProvider(); + var context = new DefaultHttpContext { RequestServices = services }; + context.Request.Method = "POST"; + context.Request.ContentType = "application/x-www-form-urlencoded"; + context.Request.Form = new FormCollection(logoutToken == null ? [] : new() { ["logout_token"] = logoutToken }); + context.Response.Body = new MemoryStream(); + var result = await OidcBackChannelLogoutEndpoint.Handle(context); + await result.ExecuteAsync(context); + return context; + } + + [TestMethod] + public async Task SubAndSid_EndsMatchingSessions_RevokesSubject() + { + var context = await Post(Token()); + Assert.AreEqual(200, context.Response.StatusCode); + Assert.AreEqual("no-store", context.Response.Headers.CacheControl.ToString()); + _sessions.Verify(x => x.EndBySid("sub-1", "sid-1"), Times.Once); + _revocation.Verify(x => x.RevokeTokens("sub-1"), Times.Once); + } + + //the subject comes from the ended sessions + [TestMethod] + public async Task SidOnly_RevokesSubjectsOfEndedSessions() + { + _sessions.Setup(x => x.EndBySid(null, "sid-1")).ReturnsAsync([ + new OidcSession { Subject = "sub-1" }, new OidcSession { Subject = "sub-1" }, new OidcSession { Subject = "sub-2" } + ]); + await Post(Token(sub: null)); + _revocation.Verify(x => x.RevokeTokens("sub-1"), Times.Once); + _revocation.Verify(x => x.RevokeTokens("sub-2"), Times.Once); + } + + [TestMethod] + public async Task SubOnly_EndsAllSessionsOfSubject() + { + await Post(Token(sid: null)); + _sessions.Verify(x => x.EndBySubject("sub-1"), Times.Once); + _revocation.Verify(x => x.RevokeTokens("sub-1"), Times.Once); + } + + [TestMethod] + [DataRow(null)] + [DataRow("not-a-token")] + public async Task MissingOrInvalidToken_400_NothingEnded(string token) + { + var context = await Post(token); + Assert.AreEqual(400, context.Response.StatusCode); + context.Response.Body.Position = 0; + StringAssert.Contains(await new StreamReader(context.Response.Body).ReadToEndAsync(), "invalid_request"); + _sessions.VerifyNoOtherCalls(); + _revocation.VerifyNoOtherCalls(); + } + + [TestMethod] + public async Task NotConfigured_400() + { + _schemeRegistered = false; + Assert.AreEqual(400, (await Post(Token())).Response.StatusCode); + } + + [TestMethod] + public async Task MetadataUnobtainable_503() + { + var manager = new Mock>(); + manager.Setup(x => x.GetConfigurationAsync(It.IsAny())).ThrowsAsync(new InvalidOperationException("IDX20803")); + _options.ConfigurationManager = manager.Object; + Assert.AreEqual(503, (await Post(Token())).Response.StatusCode); + } +} From 6b049de48f1980da9c22111375fada982ddfa1f3 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 08:09:22 +0200 Subject: [PATCH 22/28] OpenID Connect login: release the logout token when ending sessions fails, tolerate unreadable bodies --- .../OidcBackChannelLogoutEndpoint.cs | 47 +++++++++++++------ .../Infrastructure/OidcLogoutToken.cs | 5 +- .../OidcLogoutTokenValidator.cs | 12 ++++- .../OidcBackChannelLogoutEndpointTests.cs | 43 +++++++++++++++-- .../OidcLogoutTokenValidatorTests.cs | 12 +++++ 5 files changed, 98 insertions(+), 21 deletions(-) diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcBackChannelLogoutEndpoint.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcBackChannelLogoutEndpoint.cs index 51437c41f1..f443e986b0 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcBackChannelLogoutEndpoint.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcBackChannelLogoutEndpoint.cs @@ -23,7 +23,15 @@ public static async Task Handle(HttpContext context) if (!context.Request.HasFormContentType) return Error("The request must be a form post"); - var form = await context.Request.ReadFormAsync(context.RequestAborted); + IFormCollection form; + try + { + form = await context.Request.ReadFormAsync(context.RequestAborted); + } + catch (Exception ex) when (ex is InvalidDataException or BadHttpRequestException or IOException) + { + return Error("The request body could not be read"); + } var token = form["logout_token"].ToString(); if (string.IsNullOrEmpty(token)) return Error("The logout_token parameter is missing"); @@ -54,21 +62,32 @@ public static async Task Handle(HttpContext context) return Error(logout.Error); } - var sessionService = services.GetRequiredService(); - var ended = logout.Sid != null - ? await sessionService.EndBySid(logout.Subject, logout.Sid) - : await sessionService.EndBySubject(logout.Subject); + try + { + var sessionService = services.GetRequiredService(); + var ended = logout.Sid != null + ? await sessionService.EndBySid(logout.Subject, logout.Sid) + : await sessionService.EndBySubject(logout.Subject); + + //a logout token with sid only names the subject through the sessions it ended + var subjects = logout.Subject != null + ? [logout.Subject] + : ended.Select(x => x.Subject).Where(x => !string.IsNullOrEmpty(x)).Distinct().ToList(); + var revocation = services.GetRequiredService(); + foreach (var subject in subjects) + await revocation.RevokeTokens(subject); - //a logout token with sid only names the subject through the sessions it ended - var subjects = logout.Subject != null - ? [logout.Subject] - : ended.Select(x => x.Subject).Where(x => !string.IsNullOrEmpty(x)).Distinct().ToList(); - var revocation = services.GetRequiredService(); - foreach (var subject in subjects) - await revocation.RevokeTokens(subject); + logger.LogInformation("Back-channel logout: {Count} session(s) ended for subject {Subject}, session {Sid}", + ended.Count, logout.Subject, logout.Sid); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + //the provider retries a failed request: the same logout token must be accepted again + services.GetRequiredService().Release(logout.ReplayKey); + logger.LogError(ex, "Back-channel logout: ending the sessions failed"); + return Results.StatusCode(StatusCodes.Status500InternalServerError); + } - logger.LogInformation("Back-channel logout: {Count} session(s) ended for subject {Subject}, session {Sid}", - ended.Count, logout.Subject, logout.Sid); return Results.Ok(); } diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutToken.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutToken.cs index bdf58370bb..6da93ccdeb 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutToken.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutToken.cs @@ -1,7 +1,8 @@ namespace Authentication.OpenIdConnect.Infrastructure; -public sealed record OidcLogoutToken(bool IsValid, string Subject, string Sid, string Error) +public sealed record OidcLogoutToken(bool IsValid, string Subject, string Sid, string Error, string ReplayKey = null) { - public static OidcLogoutToken Valid(string subject, string sid) => new(true, subject, sid, null); + public static OidcLogoutToken Valid(string subject, string sid, string replayKey = null) => + new(true, subject, sid, null, replayKey); public static OidcLogoutToken Invalid(string error) => new(false, null, null, error); } diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs index 8b05840114..dcf8ffe7d8 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs @@ -66,9 +66,10 @@ public async Task Validate(string token, OpenIdConnectConfigura if (subject == null && sid == null) return OidcLogoutToken.Invalid("The logout token names no subject and no session"); + string key = null; if (!string.IsNullOrEmpty(jwt.Id)) { - var key = "Grand.OidcLogin.LogoutJti." + jwt.Issuer + "|" + jwt.Id; + key = "Grand.OidcLogin.LogoutJti." + jwt.Issuer + "|" + jwt.Id; if (cache.TryGetValue(key, out _)) return OidcLogoutToken.Invalid("The logout token was already used"); //the iat check above already refuses the token after IssuedAt + MaxAge + skew, so that bound is replay-safe @@ -77,7 +78,14 @@ public async Task Validate(string token, OpenIdConnectConfigura cache.Set(key, true, new DateTimeOffset(DateTime.SpecifyKind(until, DateTimeKind.Utc))); } - return OidcLogoutToken.Valid(subject, sid); + return OidcLogoutToken.Valid(subject, sid, key); + } + + /// Lets a token be validated again, when the work it asked for failed and the provider retries + public void Release(string replayKey) + { + if (replayKey != null) + cache.Remove(replayKey); } private static bool HasBackChannelEvent(JsonWebToken jwt) diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcBackChannelLogoutEndpointTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcBackChannelLogoutEndpointTests.cs index abdb66c8b2..9b97cec5a7 100644 --- a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcBackChannelLogoutEndpointTests.cs +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcBackChannelLogoutEndpointTests.cs @@ -66,7 +66,7 @@ private static string Token(string sub = "sub-1", string sid = "sid-1") }); } - private async Task Post(string logoutToken) + private IServiceProvider Services() { var schemes = new Mock(); schemes.Setup(x => x.GetSchemeAsync(OidcLoginDefaults.AuthenticationScheme)).ReturnsAsync(() => _schemeRegistered @@ -84,7 +84,12 @@ private async Task Post(string logoutToken) .AddSingleton(new OidcResourceServerConfig { ClockSkewSeconds = 60 }) .AddSingleton(NullLoggerFactory.Instance) .BuildServiceProvider(); - var context = new DefaultHttpContext { RequestServices = services }; + return services; + } + + private async Task Post(string logoutToken, IServiceProvider services = null) + { + var context = new DefaultHttpContext { RequestServices = services ?? Services() }; context.Request.Method = "POST"; context.Request.ContentType = "application/x-www-form-urlencoded"; context.Request.Form = new FormCollection(logoutToken == null ? [] : new() { ["logout_token"] = logoutToken }); @@ -131,12 +136,42 @@ public async Task MissingOrInvalidToken_400_NothingEnded(string token) { var context = await Post(token); Assert.AreEqual(400, context.Response.StatusCode); + Assert.AreEqual("no-store", context.Response.Headers.CacheControl.ToString()); context.Response.Body.Position = 0; StringAssert.Contains(await new StreamReader(context.Response.Body).ReadToEndAsync(), "invalid_request"); _sessions.VerifyNoOtherCalls(); _revocation.VerifyNoOtherCalls(); } + [TestMethod] + public async Task EndingFails_500_RetryOfSameTokenIsAccepted() + { + var token = Token(); + _sessions.SetupSequence(x => x.EndBySid("sub-1", "sid-1")) + .ThrowsAsync(new InvalidOperationException("db")) + .ReturnsAsync([]); + var services = Services(); + var first = await Post(token, services); + Assert.AreEqual(500, first.Response.StatusCode); + Assert.AreEqual("no-store", first.Response.Headers.CacheControl.ToString()); + Assert.AreEqual(200, (await Post(token, services)).Response.StatusCode); + _sessions.Verify(x => x.EndBySid("sub-1", "sid-1"), Times.Exactly(2)); + } + + [TestMethod] + public async Task UnreadableBody_400() + { + var context = new DefaultHttpContext { RequestServices = Services() }; + context.Request.Method = "POST"; + context.Request.ContentType = "multipart/form-data"; + context.Request.Body = new MemoryStream("x"u8.ToArray()); + context.Response.Body = new MemoryStream(); + var result = await OidcBackChannelLogoutEndpoint.Handle(context); + await result.ExecuteAsync(context); + Assert.AreEqual(400, context.Response.StatusCode); + Assert.AreEqual("no-store", context.Response.Headers.CacheControl.ToString()); + } + [TestMethod] public async Task NotConfigured_400() { @@ -150,6 +185,8 @@ public async Task MetadataUnobtainable_503() var manager = new Mock>(); manager.Setup(x => x.GetConfigurationAsync(It.IsAny())).ThrowsAsync(new InvalidOperationException("IDX20803")); _options.ConfigurationManager = manager.Object; - Assert.AreEqual(503, (await Post(Token())).Response.StatusCode); + var context = await Post(Token()); + Assert.AreEqual(503, context.Response.StatusCode); + Assert.AreEqual("no-store", context.Response.Headers.CacheControl.ToString()); } } diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs index 0d04626508..b143b8b695 100644 --- a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs @@ -138,6 +138,18 @@ public async Task Invalid_ErrorNamesExceptionType_NotTokenContent() StringAssert.Contains(result.Error, "SecurityTokenInvalidIssuerException"); } + [TestMethod] + public async Task Release_AllowsTheSameTokenAgain() + { + var token = Token(jti: "jti-rel"); + var first = await Validate(token); + Assert.IsTrue(first.IsValid); + Assert.IsFalse((await Validate(token)).IsValid); + _validator.Release(first.ReplayKey); + Assert.IsTrue((await Validate(token)).IsValid); + _validator.Release(null); + } + [TestMethod] public async Task Invalid_Replay() { From bc1a56bcb75f9359be3024fb4c641fc4ae9613f0 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 09:25:32 +0200 Subject: [PATCH 23/28] OpenID Connect login: the token endpoint client does not follow redirects A 307 or 308 from the token endpoint would have re-posted the refresh token and the client secret to another address. The named client no longer follows redirects, and a redirect answer counts as the provider being unavailable. --- .../Infrastructure/StartupApplication.cs | 4 ++- .../OpenIdConnect/OidcTokenRefresherTests.cs | 35 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs index 3154f83dbd..f0772704c3 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/StartupApplication.cs @@ -12,7 +12,9 @@ public class StartupApplication : IStartupApplication public void ConfigureServices(IServiceCollection services, IConfiguration configuration) { services.AddScoped(); - services.AddHttpClient(OidcLoginDefaults.HttpClientName, client => client.Timeout = TimeSpan.FromSeconds(10)); + //no redirects: a 307/308 from the token endpoint would re-post the refresh token and client secret elsewhere + services.AddHttpClient(OidcLoginDefaults.HttpClientName, client => client.Timeout = TimeSpan.FromSeconds(10)) + .ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler { AllowAutoRedirect = false }); services.AddScoped(); services.AddSingleton(); } diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcTokenRefresherTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcTokenRefresherTests.cs index cd288cd133..6b8bc0a9f4 100644 --- a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcTokenRefresherTests.cs +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcTokenRefresherTests.cs @@ -3,6 +3,9 @@ using Grand.Business.Core.Utilities.Authentication; using Grand.Infrastructure.Configuration; using Microsoft.AspNetCore.Authentication.OpenIdConnect; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Http; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Microsoft.IdentityModel.Protocols; @@ -141,4 +144,36 @@ public async Task NotConfigured_Refused() _options.ConfigurationManager = null; Assert.AreEqual(OidcRefreshOutcome.Refused, (await Refresher().Refresh("rt-1")).Outcome); } + + //a redirect is not an answer of the token endpoint: the token is never re-posted elsewhere + [TestMethod] + [DataRow(HttpStatusCode.TemporaryRedirect)] + [DataRow(HttpStatusCode.PermanentRedirect)] + [DataRow(HttpStatusCode.Found)] + public async Task Redirect_Unavailable(HttpStatusCode status) + { + _handler = new StubHandler(_ => { + var response = new HttpResponseMessage(status); + response.Headers.Location = new Uri("https://elsewhere.test/collect"); + return Task.FromResult(response); + }); + Assert.AreEqual(OidcRefreshOutcome.Unavailable, (await Refresher().Refresh("rt-1")).Outcome); + } + + [TestMethod] + public void HttpClient_DoesNotFollowRedirects() + { + var services = new ServiceCollection(); + new StartupApplication().ConfigureServices(services, new ConfigurationBuilder().Build()); + using var provider = services.BuildServiceProvider(); + + var builder = provider.GetRequiredService(); + builder.Name = OidcLoginDefaults.HttpClientName; + var options = provider.GetRequiredService>().Get(OidcLoginDefaults.HttpClientName); + foreach (var action in options.HttpMessageHandlerBuilderActions) + action(builder); + + Assert.IsInstanceOfType(builder.PrimaryHandler); + Assert.IsFalse(((HttpClientHandler)builder.PrimaryHandler).AllowAutoRedirect); + } } From b0e04bc12c04cd0b9f2a4b65358e63fca5423b00 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 09:38:34 +0200 Subject: [PATCH 24/28] OpenID Connect login: refresh the provider metadata when a logout token names an unknown signing key After the provider rotated its signing keys, logout tokens signed with the new key were refused until the cached metadata expired, and providers do not retry a 400. The validator now flags an unknown signing key (also when the handler reports it as an expired token without exp); the endpoint then requests a metadata refresh, waits briefly for the background refresh of the configuration manager and validates the token once more. --- .../OidcBackChannelLogoutEndpoint.cs | 64 ++++++++++++--- .../Infrastructure/OidcLogoutToken.cs | 7 +- .../OidcLogoutTokenValidator.cs | 23 +++++- .../OidcBackChannelLogoutEndpointTests.cs | 78 ++++++++++++++++++- .../OidcLogoutTokenValidatorTests.cs | 25 ++++++ 5 files changed, 179 insertions(+), 18 deletions(-) diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcBackChannelLogoutEndpoint.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcBackChannelLogoutEndpoint.cs index f443e986b0..ca94d31bfb 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcBackChannelLogoutEndpoint.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcBackChannelLogoutEndpoint.cs @@ -42,20 +42,25 @@ public static async Task Handle(HttpContext context) var options = services.GetRequiredService>() .Get(OidcLoginDefaults.AuthenticationScheme); - Microsoft.IdentityModel.Protocols.OpenIdConnect.OpenIdConnectConfiguration configuration; - try - { - configuration = await options.ConfigurationManager!.GetConfigurationAsync(context.RequestAborted); - } - catch (Exception ex) when (ex is not OperationCanceledException) - { - logger.LogError(ex, "Back-channel logout: the identity provider metadata could not be obtained"); + var configuration = await Configuration(options, context, logger); + if (configuration == null) return Results.StatusCode(StatusCodes.Status503ServiceUnavailable); - } + var validator = services.GetRequiredService(); + var clientId = services.GetRequiredService().ClientId; var clockSkew = TimeSpan.FromSeconds(services.GetRequiredService().ClockSkewSeconds); - var logout = await services.GetRequiredService().Validate(token, configuration, - services.GetRequiredService().ClientId, clockSkew); + var logout = await validator.Validate(token, configuration, clientId, clockSkew); + if (logout.SigningKeyNotFound) + { + //the provider may have rotated its signing keys since the metadata was cached, and it does not retry + //a refused logout token: refresh the metadata and validate once more (a failed validation consumed no jti) + logger.LogInformation("Back-channel logout: the signing key is unknown, refreshing the identity provider metadata"); + configuration = await RefreshedConfiguration(options, configuration, context, logger); + if (configuration == null) + return Results.StatusCode(StatusCodes.Status503ServiceUnavailable); + logout = await validator.Validate(token, configuration, clientId, clockSkew); + } + if (!logout.IsValid) { logger.LogWarning("Back-channel logout refused: {Reason}", logout.Error); @@ -83,7 +88,7 @@ public static async Task Handle(HttpContext context) catch (Exception ex) when (ex is not OperationCanceledException) { //the provider retries a failed request: the same logout token must be accepted again - services.GetRequiredService().Release(logout.ReplayKey); + validator.Release(logout.ReplayKey); logger.LogError(ex, "Back-channel logout: ending the sessions failed"); return Results.StatusCode(StatusCodes.Status500InternalServerError); } @@ -91,6 +96,41 @@ public static async Task Handle(HttpContext context) return Results.Ok(); } + private static async Task Configuration( + OpenIdConnectOptions options, HttpContext context, ILogger logger) + { + try + { + return await options.ConfigurationManager!.GetConfigurationAsync(context.RequestAborted); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + logger.LogError(ex, "Back-channel logout: the identity provider metadata could not be obtained"); + return null; + } + } + + /// + /// The configuration manager refreshes in the background and answers with the cached metadata meanwhile, so + /// wait (briefly) for a new instance; a refresh requested too soon after the last one is ignored by the manager + /// + private static async Task RefreshedConfiguration( + OpenIdConnectOptions options, Microsoft.IdentityModel.Protocols.OpenIdConnect.OpenIdConnectConfiguration cached, + HttpContext context, ILogger logger) + { + options.ConfigurationManager!.RequestRefresh(); + for (var attempt = 0;; attempt++) + { + var configuration = await Configuration(options, context, logger); + if (configuration == null || !ReferenceEquals(configuration, cached) || attempt == RefreshAttempts) + return configuration; + await Task.Delay(RefreshPollInterval, context.RequestAborted); + } + } + + private const int RefreshAttempts = 10; + private static readonly TimeSpan RefreshPollInterval = TimeSpan.FromMilliseconds(200); + private static IResult Error(string description) => Results.Json(new Dictionary { ["error"] = "invalid_request", ["error_description"] = description diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutToken.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutToken.cs index 6da93ccdeb..f8a262cbb2 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutToken.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutToken.cs @@ -1,8 +1,11 @@ namespace Authentication.OpenIdConnect.Infrastructure; -public sealed record OidcLogoutToken(bool IsValid, string Subject, string Sid, string Error, string ReplayKey = null) +/// The token is signed with a key the metadata does not list (yet): the provider may have rotated its keys +public sealed record OidcLogoutToken(bool IsValid, string Subject, string Sid, string Error, string ReplayKey = null, + bool SigningKeyNotFound = false) { public static OidcLogoutToken Valid(string subject, string sid, string replayKey = null) => new(true, subject, sid, null, replayKey); - public static OidcLogoutToken Invalid(string error) => new(false, null, null, error); + public static OidcLogoutToken Invalid(string error, bool signingKeyNotFound = false) => + new(false, null, null, error, SigningKeyNotFound: signingKeyNotFound); } diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs index dcf8ffe7d8..69d4092673 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcLogoutTokenValidator.cs @@ -47,10 +47,12 @@ public async Task Validate(string token, OpenIdConnectConfigura TypeAccepted(type) ? type : throw new SecurityTokenInvalidTypeException("The token type is not accepted") }; - var result = await new JsonWebTokenHandler().ValidateTokenAsync(token, parameters); + var handler = new JsonWebTokenHandler(); + var result = await handler.ValidateTokenAsync(token, parameters); if (!result.IsValid || result.SecurityToken is not JsonWebToken jwt) return OidcLogoutToken.Invalid( - $"The logout token is invalid ({result.Exception?.GetType().Name ?? "unknown"})"); + $"The logout token is invalid ({result.Exception?.GetType().Name ?? "unknown"})", + result.Exception is SecurityTokenSignatureKeyNotFoundException || HasUnknownKeyId(handler, token, configuration)); if (!HasBackChannelEvent(jwt)) return OidcLogoutToken.Invalid("The logout token has no back-channel logout event"); @@ -88,6 +90,23 @@ public void Release(string replayKey) cache.Remove(replayKey); } + //the handler reports a token without exp whose key is unknown as expired, so the key id is checked here too + private static bool HasUnknownKeyId(JsonWebTokenHandler handler, string token, OpenIdConnectConfiguration configuration) + { + if (!handler.CanReadToken(token)) + return false; + try + { + var kid = handler.ReadJsonWebToken(token).Kid; + return !string.IsNullOrEmpty(kid) && + configuration.SigningKeys.All(key => !string.Equals(key.KeyId, kid, StringComparison.Ordinal)); + } + catch (ArgumentException) + { + return false; + } + } + private static bool HasBackChannelEvent(JsonWebToken jwt) { var events = jwt.Claims.FirstOrDefault(c => c.Type == "events")?.Value; diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcBackChannelLogoutEndpointTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcBackChannelLogoutEndpointTests.cs index 9b97cec5a7..e82326130d 100644 --- a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcBackChannelLogoutEndpointTests.cs +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcBackChannelLogoutEndpointTests.cs @@ -52,7 +52,7 @@ public void Init() _schemeRegistered = true; } - private static string Token(string sub = "sub-1", string sid = "sid-1") + private static string Token(string sub = "sub-1", string sid = "sid-1", SecurityKey key = null) { var claims = new Dictionary { ["iss"] = Issuer, ["aud"] = "shop", ["iat"] = EpochTime.GetIntDate(DateTime.UtcNow), @@ -62,7 +62,7 @@ private static string Token(string sub = "sub-1", string sid = "sid-1") if (sub != null) claims["sub"] = sub; if (sid != null) claims["sid"] = sid; return new JsonWebTokenHandler { SetDefaultTimesOnTokenCreation = false }.CreateToken(new SecurityTokenDescriptor { - Claims = claims, SigningCredentials = new SigningCredentials(Key, SecurityAlgorithms.RsaSha256), TokenType = "logout+jwt" + Claims = claims, SigningCredentials = new SigningCredentials(key ?? Key, SecurityAlgorithms.RsaSha256), TokenType = "logout+jwt" }); } @@ -189,4 +189,78 @@ public async Task MetadataUnobtainable_503() Assert.AreEqual(503, context.Response.StatusCode); Assert.AreEqual("no-store", context.Response.Headers.CacheControl.ToString()); } + + private static RsaSecurityKey NewKey(string keyId) + { + using var rsa = RSA.Create(2048); + return new RsaSecurityKey(rsa.ExportParameters(true)) { KeyId = keyId }; + } + + //the configuration manager answers with the old keys until a refresh is requested + private Mock> RotatingManager(SecurityKey newKey, int staleAnswers = 0) + { + var before = new OpenIdConnectConfiguration { Issuer = Issuer }; + before.SigningKeys.Add(Key); + var after = new OpenIdConnectConfiguration { Issuer = Issuer }; + after.SigningKeys.Add(newKey); + var refreshed = false; + var manager = new Mock>(); + manager.Setup(x => x.GetConfigurationAsync(It.IsAny())) + .ReturnsAsync(() => refreshed && staleAnswers-- <= 0 ? after : before); + manager.Setup(x => x.RequestRefresh()).Callback(() => refreshed = true); + return manager; + } + + [TestMethod] + public async Task RotatedSigningKey_RefreshesMetadata_EndsSessions() + { + var newKey = NewKey("k2"); + var manager = RotatingManager(newKey); + _options.ConfigurationManager = manager.Object; + + var context = await Post(Token(key: newKey)); + + Assert.AreEqual(200, context.Response.StatusCode); + manager.Verify(x => x.RequestRefresh(), Times.Once); + _sessions.Verify(x => x.EndBySid("sub-1", "sid-1"), Times.Once); + _revocation.Verify(x => x.RevokeTokens("sub-1"), Times.Once); + } + + //the real configuration manager refreshes in the background: the cached metadata is answered a while longer + [TestMethod] + public async Task RotatedSigningKey_BackgroundRefresh_WaitsForNewMetadata() + { + var newKey = NewKey("k2"); + var manager = RotatingManager(newKey, staleAnswers: 2); + _options.ConfigurationManager = manager.Object; + + var context = await Post(Token(key: newKey)); + + Assert.AreEqual(200, context.Response.StatusCode); + _sessions.Verify(x => x.EndBySid("sub-1", "sid-1"), Times.Once); + } + + [TestMethod] + public async Task UnknownSigningKeyAfterRefresh_400_NothingEnded() + { + var manager = RotatingManager(NewKey("k2")); + _options.ConfigurationManager = manager.Object; + + var context = await Post(Token(key: NewKey("k3"))); + + Assert.AreEqual(400, context.Response.StatusCode); + manager.Verify(x => x.RequestRefresh(), Times.Once); + _sessions.VerifyNoOtherCalls(); + } + + //only an unknown key is worth a metadata refresh + [TestMethod] + public async Task InvalidToken_NoMetadataRefresh() + { + var manager = RotatingManager(NewKey("k2")); + _options.ConfigurationManager = manager.Object; + + Assert.AreEqual(400, (await Post("not-a-token")).Response.StatusCode); + manager.Verify(x => x.RequestRefresh(), Times.Never); + } } diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs index b143b8b695..83b46655f5 100644 --- a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLogoutTokenValidatorTests.cs @@ -195,4 +195,29 @@ public async Task Invalid_Symmetric() [DataRow("at+jwt", false)] public void TypeAccepted(string typ, bool expected) => Assert.AreEqual(expected, OidcLogoutTokenValidator.TypeAccepted(typ)); + + //after a key rotation at the provider the endpoint refreshes the metadata and validates again + [TestMethod] + public async Task UnknownSigningKey_Flagged() + { + var result = await Validate(Token(key: OtherKey)); + Assert.IsFalse(result.IsValid); + Assert.IsTrue(result.SigningKeyNotFound, result.Error); + } + + [TestMethod] + public async Task UnknownSigningKey_WithExpiry_Flagged() + { + var result = await Validate(Token(new() { ["exp"] = EpochTime.GetIntDate(DateTime.UtcNow.AddMinutes(2)) }, key: OtherKey)); + Assert.IsFalse(result.IsValid); + Assert.IsTrue(result.SigningKeyNotFound, result.Error); + } + + [TestMethod] + public async Task OtherFailure_NotFlaggedAsUnknownKey() + { + var result = await Validate(Token(issuer: "https://other.test")); + Assert.IsFalse(result.IsValid); + Assert.IsFalse(result.SigningKeyNotFound); + } } From 8f428e27a70e09c884b2e052ae87dc4a73749994 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 11:12:45 +0200 Subject: [PATCH 25/28] OpenID Connect login: connecting an account and signing in again as the same account end with a session-backed cookie --- .../Controllers/OidcLoginController.cs | 15 +++---- .../OpenIdConnect/OidcLoginControllerTests.cs | 42 +++++++++++++++---- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/Plugins/Authentication.OpenIdConnect/Controllers/OidcLoginController.cs b/src/Plugins/Authentication.OpenIdConnect/Controllers/OidcLoginController.cs index 29c66a5145..8bf44cc330 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Controllers/OidcLoginController.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Controllers/OidcLoginController.cs @@ -101,17 +101,16 @@ public async Task Callback(string returnUrl) var result = await _oidcCustomerResolver.ResolveForLogin(pending.Subject, pending.Email, pending.EmailVerified, _contextAccessor.StoreContext.CurrentStore.Id); - //a store session is never replaced by another account, and a signed-in visitor does not register; - //connecting accounts (LinkRequired) asks for the password of the account itself + //a store session is never replaced by another account, and a signed-in visitor does not register; the same + //account signing in again (for example right after "Connect your account") gets a session-backed cookie if (result.Outcome is OidcLoginOutcome.SignIn or OidcLoginOutcome.RegistrationRequired) { var current = _contextAccessor.WorkContext.CurrentCustomer; - if (await _groupService.IsRegistered(current)) + if (await _groupService.IsRegistered(current) && + (result.Outcome == OidcLoginOutcome.RegistrationRequired || result.Customer?.Id != current.Id)) { await _externalRegistrationContext.Clear(); - return result.Customer?.Id == current.Id - ? RedirectLocal(returnUrl) - : RedirectToRoute("OidcLoginFailed", new { signedIn = true }); + return RedirectToRoute("OidcLoginFailed", new { signedIn = true }); } } @@ -205,7 +204,9 @@ public async Task LinkComplete(string returnUrl) } return await _externalRegistrationContext.Complete(customer, _contextAccessor.StoreContext.CurrentStore.Id) switch { - OidcLinkResult.Linked => RedirectLocal(returnUrl), + //the password sign-in that connected the account is replaced at once: the provider session is still open, + //so the provider answers without a prompt and the callback starts a session-backed cookie + OidcLinkResult.Linked => RedirectToRoute("OidcLoginLogin", new { returnUrl }), //the provider identity, not this account, is the one already connected elsewhere OidcLinkResult.SubjectLinkedToOtherCustomer => RedirectToRoute("OidcLoginFailed", new { taken = true }), _ => RedirectToRoute("OidcLoginFailed", new { reason = (int)OidcLoginRefusal.LinkedToOtherIdentity }) diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginControllerTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginControllerTests.cs index 01830b36c9..bc246860f9 100644 --- a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginControllerTests.cs +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcLoginControllerTests.cs @@ -211,7 +211,23 @@ public async Task LinkComplete_EmailMatches_Links() Marker("sub-1"); _external.Setup(x => x.GetPending()).ReturnsAsync(Pending); _external.Setup(x => x.Complete(_current, "s1")).ReturnsAsync(OidcLinkResult.Linked); - Assert.AreEqual("/account", ((RedirectResult)await _controller.LinkComplete("/account")).Url); + var result = (RedirectToRouteResult)await _controller.LinkComplete("/account"); + Assert.AreEqual("OidcLoginLogin", result.RouteName); + Assert.AreEqual("/account", result.RouteValues!["returnUrl"]); + } + + //the password session of "Connect your account" is replaced at once: the provider session is still open, so the + //browser comes straight back and the callback starts a session-backed cookie + [TestMethod] + public async Task LinkComplete_Linked_GoesThroughProviderSignIn() + { + _groups.Setup(x => x.IsRegistered(_current)).ReturnsAsync(true); + Marker("sub-1"); + _external.Setup(x => x.GetPending()).ReturnsAsync(Pending); + _external.Setup(x => x.Complete(_current, "s1")).ReturnsAsync(OidcLinkResult.Linked); + var result = (RedirectToRouteResult)await _controller.LinkComplete("/account"); + Assert.AreEqual("OidcLoginLogin", result.RouteName); + Assert.AreEqual("/account", result.RouteValues!["returnUrl"]); } [TestMethod] @@ -330,16 +346,20 @@ public async Task Callback_SignIn_OtherAccountSignedIn_ClearsAndFails_SessionKep _mediator.Verify(x => x.Publish(It.IsAny(), It.IsAny()), Times.Never); } + //the same account signing in again (right after "Connect your account") replaces its cookie with a session-backed one [TestMethod] - public async Task Callback_SignIn_SameAccountSignedIn_RedirectsWithoutNewSignIn() + public async Task Callback_SameCustomerSignedIn_ReplacesCookieWithSession() { _groups.Setup(x => x.IsRegistered(_current)).ReturnsAsync(true); _external.Setup(x => x.GetPending()).ReturnsAsync(Pending); _resolver.Setup(x => x.ResolveForLogin(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync(OidcLoginResult.SignIn(_current)); - Assert.AreEqual("/cart", ((RedirectResult)await _controller.Callback("/cart")).Url); - _external.Verify(x => x.Clear(), Times.Once); - _auth.Verify(x => x.SignIn(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + + var result = (RedirectResult)await _controller.Callback("/account"); + + Assert.AreEqual("/account", result.Url); + _sessions.Verify(x => x.Start(_current, "sub-1", "sid-1", "rt-1", "s1"), Times.Once); + _auth.Verify(x => x.SignIn(_current, false, "session-1"), Times.Once); } [TestMethod] @@ -374,7 +394,9 @@ public async Task LinkComplete_TicketIssuedInSameSecondAsLink_Links() var now = DateTimeOffset.FromUnixTimeSeconds(DateTimeOffset.UtcNow.ToUnixTimeSeconds()); Marker("sub-1", now); _ticketIssued = now.AddMilliseconds(800); - Assert.AreEqual("/account", ((RedirectResult)await _controller.LinkComplete("/account")).Url); + var result = (RedirectToRouteResult)await _controller.LinkComplete("/account"); + Assert.AreEqual("OidcLoginLogin", result.RouteName); + Assert.AreEqual("/account", result.RouteValues!["returnUrl"]); } [TestMethod] @@ -421,7 +443,9 @@ public async Task LinkComplete_TicketIssuedAfterLink_Links() _external.Setup(x => x.Complete(_current, "s1")).ReturnsAsync(OidcLinkResult.Linked); Marker("sub-1", DateTimeOffset.UtcNow.AddMinutes(-2)); _ticketIssued = DateTimeOffset.UtcNow.AddMinutes(-1); - Assert.AreEqual("/account", ((RedirectResult)await _controller.LinkComplete("/account")).Url); + var result = (RedirectToRouteResult)await _controller.LinkComplete("/account"); + Assert.AreEqual("OidcLoginLogin", result.RouteName); + Assert.AreEqual("/account", result.RouteValues!["returnUrl"]); _external.Verify(x => x.Complete(_current, "s1"), Times.Once); } @@ -493,7 +517,9 @@ public async Task LinkComplete_ExternalReturnUrl_GoesHome() _external.Setup(x => x.GetPending()).ReturnsAsync(Pending); _external.Setup(x => x.Complete(_current, "s1")).ReturnsAsync(OidcLinkResult.Linked); Marker("sub-1"); - Assert.AreEqual("HomePage", ((RedirectToRouteResult)await _controller.LinkComplete("https://evil.example/")).RouteName); + var result = (RedirectToRouteResult)await _controller.LinkComplete("https://evil.example/"); + Assert.AreEqual("OidcLoginLogin", result.RouteName); + Assert.IsNull(result.RouteValues!["returnUrl"]); } [TestMethod] From ee0a7468704ed53e6c2662e9d4fcf57ce0776df6 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 11:59:32 +0200 Subject: [PATCH 26/28] OpenID Connect login: the incomplete setup message addresses customers too --- src/Plugins/Authentication.OpenIdConnect/OidcLoginPlugin.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Plugins/Authentication.OpenIdConnect/OidcLoginPlugin.cs b/src/Plugins/Authentication.OpenIdConnect/OidcLoginPlugin.cs index a3c47a2ba7..688a559aed 100644 --- a/src/Plugins/Authentication.OpenIdConnect/OidcLoginPlugin.cs +++ b/src/Plugins/Authentication.OpenIdConnect/OidcLoginPlugin.cs @@ -34,7 +34,7 @@ private static readonly (string Key, string Value)[] Resources = [ ("Failed.SignedInAsOtherAccount", "You are already signed in with another account. Sign out first, then sign in with your identity provider."), ("Failed.SessionUnavailable", - "Signing in to the back office with the identity provider is not set up completely. Use your password, or ask the administrator."), + "Signing in with the identity provider is not set up completely. Use another sign-in method, or ask the administrator."), ("DisplayOrder", "Display order"), ("ClientId", "Client ID"), ("ClientSecret", "Client secret"), From bf818d3e6badca37d85c79e91e875a784b507d6f Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Mon, 12 Oct 2026 11:59:38 +0200 Subject: [PATCH 27/28] OpenID Connect login: the token refresher confirms every provider session --- .../Infrastructure/OidcTokenRefresher.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcTokenRefresher.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcTokenRefresher.cs index bece43dde4..49b28c2eaa 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcTokenRefresher.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcTokenRefresher.cs @@ -10,7 +10,7 @@ namespace Authentication.OpenIdConnect.Infrastructure; /// -/// Confirms a back-office session at the identity provider by exchanging its refresh token (RFC 6749, section 6) +/// Confirms a session signed in through the provider at the identity provider by exchanging its refresh token (RFC 6749, section 6) /// public class OidcTokenRefresher( IHttpClientFactory httpClientFactory, @@ -25,7 +25,7 @@ public async Task Refresh(string refreshToken, CancellationTo var options = optionsMonitor.Get(OidcLoginDefaults.AuthenticationScheme); if (options.ConfigurationManager == null) { - logger.LogWarning("OpenID Connect login is not configured: back-office sessions cannot be confirmed"); + logger.LogWarning("OpenID Connect login is not configured: sessions signed in through the provider cannot be confirmed"); return OidcRefreshResult.Refused(); } From 8b0ff4fb4c27830b475a5b071becea29a07730c7 Mon Sep 17 00:00:00 2001 From: KrzysztofPajak Date: Tue, 13 Oct 2026 20:08:22 +0200 Subject: [PATCH 28/28] OpenID Connect login: back-channel logout creates no store or work context, an unconfigured sign-in revokes no access tokens --- .../EndpointProvider.cs | 7 +++++-- .../Infrastructure/OidcTokenRefresher.cs | 2 +- .../OpenIdConnect/OidcTokenRefresherTests.cs | 4 ++-- .../Middleware/ContextMiddlewareTests.cs | 13 +++++++++++-- .../Middleware/ContextMiddleware.cs | 6 ++++++ .../Middleware/NoRequestContextMetadata.cs | 14 ++++++++++++++ 6 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 src/Web/Grand.Web.Common/Middleware/NoRequestContextMetadata.cs diff --git a/src/Plugins/Authentication.OpenIdConnect/EndpointProvider.cs b/src/Plugins/Authentication.OpenIdConnect/EndpointProvider.cs index a1535f9688..165cf45e26 100644 --- a/src/Plugins/Authentication.OpenIdConnect/EndpointProvider.cs +++ b/src/Plugins/Authentication.OpenIdConnect/EndpointProvider.cs @@ -1,5 +1,6 @@ using Authentication.OpenIdConnect.Infrastructure; using Grand.Infrastructure.Endpoints; +using Grand.Web.Common.Middleware; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; @@ -20,9 +21,11 @@ public void RegisterEndpoint(IEndpointRouteBuilder endpointRouteBuilder) new { controller = "OidcLogin", action = "LinkComplete" }); endpointRouteBuilder.MapControllerRoute("OidcLoginFailed", "oidc/failed", new { controller = "OidcLogin", action = "Failed" }); - //called by the identity provider, server to server: no antiforgery, no store context, no MVC filters + //called by the identity provider, server to server: no antiforgery, no store or work context (no guest is + //created for the cookieless call), no MVC filters endpointRouteBuilder.MapPost(OidcLoginDefaults.BackChannelLogoutPath, - (Func>)OidcBackChannelLogoutEndpoint.Handle); + (Func>)OidcBackChannelLogoutEndpoint.Handle) + .WithMetadata(NoRequestContextMetadata.Instance); } public int Priority => 10; diff --git a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcTokenRefresher.cs b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcTokenRefresher.cs index 49b28c2eaa..a7a039b5e5 100644 --- a/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcTokenRefresher.cs +++ b/src/Plugins/Authentication.OpenIdConnect/Infrastructure/OidcTokenRefresher.cs @@ -26,7 +26,7 @@ public async Task Refresh(string refreshToken, CancellationTo if (options.ConfigurationManager == null) { logger.LogWarning("OpenID Connect login is not configured: sessions signed in through the provider cannot be confirmed"); - return OidcRefreshResult.Refused(); + return OidcRefreshResult.NotAsked(); } try diff --git a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcTokenRefresherTests.cs b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcTokenRefresherTests.cs index 6b8bc0a9f4..1d3acc46d2 100644 --- a/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcTokenRefresherTests.cs +++ b/src/Tests/Grand.Modules.Tests/Plugins/OpenIdConnect/OidcTokenRefresherTests.cs @@ -139,10 +139,10 @@ public async Task MetadataUnobtainable_Unavailable() //no sign-in scheme (not configured): nothing can ever confirm the session [TestMethod] - public async Task NotConfigured_Refused() + public async Task NotConfigured_NotAsked() { _options.ConfigurationManager = null; - Assert.AreEqual(OidcRefreshOutcome.Refused, (await Refresher().Refresh("rt-1")).Outcome); + Assert.AreEqual(OidcRefreshOutcome.NotAsked, (await Refresher().Refresh("rt-1")).Outcome); } //a redirect is not an answer of the token endpoint: the token is never re-posted elsewhere diff --git a/src/Tests/Grand.Web.Common.Tests/Middleware/ContextMiddlewareTests.cs b/src/Tests/Grand.Web.Common.Tests/Middleware/ContextMiddlewareTests.cs index dced229a6c..ede80d184e 100644 --- a/src/Tests/Grand.Web.Common.Tests/Middleware/ContextMiddlewareTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Middleware/ContextMiddlewareTests.cs @@ -17,7 +17,7 @@ public class ContextMiddlewareTests private Mock _storeContextSetter; private Mock _workContextSetter; - private async Task Invoke(string routePattern, Action request = null) + private async Task Invoke(string routePattern, Action request = null, params object[] metadata) { _storeContextSetter = new Mock(); var storeContext = new Mock(); @@ -30,7 +30,7 @@ private async Task Invoke(string routePattern, Action request services.AddSingleton(_workContextSetter.Object); var http = new DefaultHttpContext { RequestServices = services.BuildServiceProvider() }; http.SetEndpoint(new RouteEndpoint(_ => Task.CompletedTask, RoutePatternFactory.Parse(routePattern), 0, - EndpointMetadataCollection.Empty, null)); + new EndpointMetadataCollection(metadata), null)); request?.Invoke(http); var nextCalled = false; @@ -64,6 +64,15 @@ public async Task RouteOnlyPrefixedByMetadataPath_InitializesStoreAndWorkContext _workContextSetter.Verify(x => x.InitializeWorkContext("store1"), Times.Once); } + //a server-to-server endpoint (for example the back-channel logout of the login plugin) creates no guest + [TestMethod] + public async Task NoRequestContextMetadata_NoStoreOrWorkContext() + { + Assert.IsTrue(await Invoke("oidc/backchannel-logout", null, NoRequestContextMetadata.Instance)); + _storeContextSetter.Verify(x => x.InitializeStoreContext(It.IsAny()), Times.Never); + _workContextSetter.Verify(x => x.InitializeWorkContext(It.IsAny()), Times.Never); + } + [TestMethod] public async Task OtherRoute_InitializesStoreAndWorkContext() { diff --git a/src/Web/Grand.Web.Common/Middleware/ContextMiddleware.cs b/src/Web/Grand.Web.Common/Middleware/ContextMiddleware.cs index 3de2918910..02721a43df 100644 --- a/src/Web/Grand.Web.Common/Middleware/ContextMiddleware.cs +++ b/src/Web/Grand.Web.Common/Middleware/ContextMiddleware.cs @@ -48,6 +48,12 @@ public async Task InvokeAsync(HttpContext context, IContextAccessor contextAcces var endpoint = context.GetEndpoint(); if (endpoint != null) { + if (endpoint.Metadata.GetMetadata() != null) + { + await _next(context); + return; + } + var routePattern = (endpoint as RouteEndpoint)?.RoutePattern.RawText; if (routePattern != null && (skipRoutePattern.Any(pattern => routePattern.StartsWith(pattern, StringComparison.OrdinalIgnoreCase)) || IsProtectedResourceMetadata(routePattern))) diff --git a/src/Web/Grand.Web.Common/Middleware/NoRequestContextMetadata.cs b/src/Web/Grand.Web.Common/Middleware/NoRequestContextMetadata.cs new file mode 100644 index 0000000000..a3a2c292b4 --- /dev/null +++ b/src/Web/Grand.Web.Common/Middleware/NoRequestContextMetadata.cs @@ -0,0 +1,14 @@ +namespace Grand.Web.Common.Middleware; + +/// +/// Endpoint metadata: ContextMiddleware sets neither store nor work context for the endpoint. For anonymous endpoints +/// called server to server, which need no customer: a cookieless request would otherwise insert a guest +/// +public sealed class NoRequestContextMetadata +{ + public static readonly NoRequestContextMetadata Instance = new(); + + private NoRequestContextMetadata() + { + } +}