From 9f32de52a6bc2567c821ae206ebf879f92c12178 Mon Sep 17 00:00:00 2001 From: "Marcelo M. Maciel" <4993482+marcelo-maciel@users.noreply.github.com> Date: Mon, 28 Sep 2026 13:58:19 -0300 Subject: [PATCH 1/2] fix(identity): run session cleanup inside each tenant's context SessionCleanupHostedService ran ExecuteDeleteAsync from a plain DI scope. A hosted service has no request, so no tenant is resolved, and the default-on tenant filter on UserSessions threw a NullReferenceException while EF evaluated the query parameters. Every hourly run failed and was logged as "Error during session cleanup"; nothing was ever deleted. The run now enumerates IMultiTenantStore and deletes in a per-tenant scope with the tenant context set, the same shape as RolePermissionSyncHostedService. That also reaches tenants with their own connection string. A failure in one tenant is logged with its id and the remaining tenants are still cleaned. Interval, retention and cancellation are unchanged. The per-run method is internal and Identity exposes internals to Integration.Tests so the regression test can invoke one run directly. --- .../Identity/Modules.Identity/AssemblyInfo.cs | 3 +- .../Services/SessionCleanupHostedService.cs | 51 +++++- .../Tests/Sessions/SessionCleanupTests.cs | 165 ++++++++++++++++++ 3 files changed, 209 insertions(+), 10 deletions(-) create mode 100644 src/Tests/Integration.Tests/Tests/Sessions/SessionCleanupTests.cs diff --git a/src/Modules/Identity/Modules.Identity/AssemblyInfo.cs b/src/Modules/Identity/Modules.Identity/AssemblyInfo.cs index 04a6ca2431..fb4164897e 100644 --- a/src/Modules/Identity/Modules.Identity/AssemblyInfo.cs +++ b/src/Modules/Identity/Modules.Identity/AssemblyInfo.cs @@ -2,4 +2,5 @@ using System.Runtime.CompilerServices; [assembly: FshModule(typeof(FSH.Modules.Identity.IdentityModule), 100)] -[assembly: InternalsVisibleTo("Identity.Tests")] \ No newline at end of file +[assembly: InternalsVisibleTo("Identity.Tests")] +[assembly: InternalsVisibleTo("Integration.Tests")] \ No newline at end of file diff --git a/src/Modules/Identity/Modules.Identity/Services/SessionCleanupHostedService.cs b/src/Modules/Identity/Modules.Identity/Services/SessionCleanupHostedService.cs index d5358f8706..02ec54d36d 100644 --- a/src/Modules/Identity/Modules.Identity/Services/SessionCleanupHostedService.cs +++ b/src/Modules/Identity/Modules.Identity/Services/SessionCleanupHostedService.cs @@ -1,3 +1,6 @@ +using Finbuckle.MultiTenant; +using Finbuckle.MultiTenant.Abstractions; +using FSH.Framework.Shared.Multitenancy; using FSH.Modules.Identity.Data; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; @@ -8,7 +11,7 @@ namespace FSH.Modules.Identity.Services; /// /// Background service that periodically cleans up expired sessions. -/// Runs every hour and removes sessions that have been expired for more than 30 days. +/// Runs every hour and, in every tenant, removes sessions that have been expired for more than 30 days. /// public sealed class SessionCleanupHostedService : BackgroundService { @@ -54,20 +57,50 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) _logger.LogInformation("Session cleanup service stopped"); } - private async Task CleanupExpiredSessionsAsync(CancellationToken cancellationToken) + internal async Task CleanupExpiredSessionsAsync(CancellationToken cancellationToken) { - using var scope = _scopeFactory.CreateScope(); - var db = scope.ServiceProvider.GetRequiredService(); + // A hosted service has no request, so no tenant is resolved and the default-on tenant filter on + // UserSessions has nothing to compare against. Each tenant is cleaned inside its own context, + // which also points IdentityDbContext at a tenant's dedicated database when it has one. + List tenants; + using (var scope = _scopeFactory.CreateScope()) + { + var tenantStore = scope.ServiceProvider.GetRequiredService>(); + tenants = (await tenantStore.GetAllAsync().ConfigureAwait(false)).ToList(); + } // cutoffDate = now - retentionDays, so ExpiresAt < cutoffDate already implies ExpiresAt < now. var cutoffDate = _timeProvider.GetUtcNow().UtcDateTime.AddDays(-_retentionDays); - var deleted = await db.UserSessions - .Where(s => s.ExpiresAt < cutoffDate) - .ExecuteDeleteAsync(cancellationToken); + foreach (var tenant in tenants) + { + cancellationToken.ThrowIfCancellationRequested(); + await CleanupTenantSessionsAsync(tenant, cutoffDate, cancellationToken).ConfigureAwait(false); + } + } + + private async Task CleanupTenantSessionsAsync(AppTenantInfo tenant, DateTime cutoffDate, CancellationToken cancellationToken) + { + try + { + using var scope = _scopeFactory.CreateScope(); + scope.ServiceProvider.GetRequiredService() + .MultiTenantContext = new MultiTenantContext(tenant); + + var db = scope.ServiceProvider.GetRequiredService(); + var deleted = await db.UserSessions + .Where(s => s.ExpiresAt < cutoffDate) + .ExecuteDeleteAsync(cancellationToken) + .ConfigureAwait(false); - if (deleted > 0 && _logger.IsEnabled(LogLevel.Information)) + if (deleted > 0 && _logger.IsEnabled(LogLevel.Information)) + { + _logger.LogInformation("Cleaned up {Count} expired sessions for tenant {TenantId}", deleted, tenant.Id); + } + } + catch (Exception ex) when (ex is not OperationCanceledException) { - _logger.LogInformation("Cleaned up {Count} expired sessions", deleted); + // One tenant's database being unreachable must not keep the other tenants' sessions around. + _logger.LogError(ex, "Session cleanup failed for tenant {TenantId}", tenant.Id); } } } \ No newline at end of file diff --git a/src/Tests/Integration.Tests/Tests/Sessions/SessionCleanupTests.cs b/src/Tests/Integration.Tests/Tests/Sessions/SessionCleanupTests.cs new file mode 100644 index 0000000000..b891572518 --- /dev/null +++ b/src/Tests/Integration.Tests/Tests/Sessions/SessionCleanupTests.cs @@ -0,0 +1,165 @@ +using Finbuckle.MultiTenant; +using Finbuckle.MultiTenant.Abstractions; +using FSH.Framework.Shared.Multitenancy; +using FSH.Modules.Identity.Data; +using FSH.Modules.Identity.Domain; +using FSH.Modules.Identity.Services; +using FSH.Modules.Multitenancy.Contracts.Dtos; +using Integration.Tests.Infrastructure; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Hosting; + +namespace Integration.Tests.Tests.Sessions; + +/// +/// The hourly session cleanup runs outside any request, so there is no ambient tenant. UserSessions +/// carry the default-on tenant filter, which means the cleanup only reaches a tenant's rows when it +/// runs inside that tenant's context. Sessions are seeded in two tenants and the registered hosted +/// service's single run is invoked directly, so the test does not wait for the hourly timer. +/// +[Collection(FshCollectionDefinition.Name)] +public sealed class SessionCleanupTests +{ + // The service keeps sessions for 30 days past expiry; 31 is safely past the cutoff, 1 is inside it. + private const int DaysPastRetention = 31; + private const int DaysInsideRetention = 1; + + private readonly FshWebApplicationFactory _factory; + private readonly AuthHelper _auth; + + public SessionCleanupTests(FshWebApplicationFactory factory) + { + _factory = factory; + _auth = new AuthHelper(factory); + } + + [Fact] + public async Task Cleanup_Should_DeleteSessionsPastRetention_InEveryTenant_And_KeepTheRest() + { + // Arrange + using var rootClient = await _auth.CreateRootAdminClientAsync(); + var uniqueId = Guid.NewGuid().ToString("N")[..8]; + var otherTenantId = $"sess-clean-{uniqueId}"; + var otherAdminEmail = $"sess-clean-admin-{uniqueId}@tenant.com"; + + await CreateTenantAsync(rootClient, otherTenantId, otherAdminEmail); + await WaitForProvisioningAsync(rootClient, otherTenantId); + + var now = DateTime.UtcNow; + var rootExpired = await SeedSessionAsync( + TestConstants.RootTenantId, TestConstants.RootAdminEmail, now.AddDays(-DaysPastRetention)); + var otherExpired = await SeedSessionAsync( + otherTenantId, otherAdminEmail, now.AddDays(-DaysPastRetention)); + var rootInsideRetention = await SeedSessionAsync( + TestConstants.RootTenantId, TestConstants.RootAdminEmail, now.AddDays(-DaysInsideRetention)); + var otherActive = await SeedSessionAsync( + otherTenantId, otherAdminEmail, now.AddDays(7)); + + var cleanup = _factory.Services.GetServices() + .OfType() + .Single(); + + // Act + await cleanup.CleanupExpiredSessionsAsync(CancellationToken.None); + + // Assert + (await SessionExistsAsync(TestConstants.RootTenantId, rootExpired)) + .ShouldBeFalse("a root-tenant session expired past retention must be deleted"); + (await SessionExistsAsync(otherTenantId, otherExpired)) + .ShouldBeFalse("a session expired past retention in a non-root tenant must be deleted"); + (await SessionExistsAsync(TestConstants.RootTenantId, rootInsideRetention)) + .ShouldBeTrue("a session expired inside the retention window must be kept"); + (await SessionExistsAsync(otherTenantId, otherActive)) + .ShouldBeTrue("a session that has not expired must be kept"); + } + + // Tenant context is an AsyncLocal, so it is set in the same method as the DbContext call. + private async Task SeedSessionAsync(string tenantId, string userEmail, DateTime expiresAt) + { + using var scope = _factory.Services.CreateScope(); + var tenant = await scope.ServiceProvider + .GetRequiredService>() + .GetAsync(tenantId); + tenant.ShouldNotBeNull(); + scope.ServiceProvider.GetRequiredService() + .MultiTenantContext = new MultiTenantContext(tenant); + + var user = await scope.ServiceProvider + .GetRequiredService>() + .FindByEmailAsync(userEmail); + user.ShouldNotBeNull(); + + var session = UserSession.Create( + user.Id, + Guid.NewGuid().ToString("N"), + "127.0.0.1", + "session-cleanup-test", + expiresAt); + + var db = scope.ServiceProvider.GetRequiredService(); + db.UserSessions.Add(session); + await db.SaveChangesAsync(); + return session.Id; + } + + private async Task SessionExistsAsync(string tenantId, Guid sessionId) + { + using var scope = _factory.Services.CreateScope(); + var tenant = await scope.ServiceProvider + .GetRequiredService>() + .GetAsync(tenantId); + tenant.ShouldNotBeNull(); + scope.ServiceProvider.GetRequiredService() + .MultiTenantContext = new MultiTenantContext(tenant); + + var db = scope.ServiceProvider.GetRequiredService(); + return await db.UserSessions.AsNoTracking().AnyAsync(s => s.Id == sessionId); + } + + private static async Task CreateTenantAsync(HttpClient rootClient, string tenantId, string adminEmail) + { + var response = await rootClient.PostAsJsonAsync(TestConstants.TenantsBasePath, new + { + id = tenantId, + name = $"Tenant {tenantId}", + connectionString = (string?)null, + adminEmail, + adminPassword = TestConstants.DefaultPassword, + issuer = $"{tenantId}.issuer" + }); + var body = await response.Content.ReadAsStringAsync(); + response.StatusCode.ShouldBe(HttpStatusCode.Created, $"Create tenant failed: {body}"); + } + + // The status body also lists each step, and a finished step reads "Completed" while later steps + // (seeding the tenant admin) are still running, so only the overall Status field is trusted. + private static async Task WaitForProvisioningAsync(HttpClient client, string tenantId) + { + const int maxRetries = 60; + for (int i = 0; i < maxRetries; i++) + { + var statusResponse = await client.GetAsync( + $"{TestConstants.TenantsBasePath}/{tenantId}/provisioning"); + + if (statusResponse.IsSuccessStatusCode) + { + var status = await statusResponse.Content.ReadFromJsonAsync(); + if (string.Equals(status?.Status, "Completed", StringComparison.Ordinal)) + { + return; + } + + if (string.Equals(status?.Status, "Failed", StringComparison.Ordinal)) + { + throw new InvalidOperationException( + $"Tenant {tenantId} provisioning failed at {status?.CurrentStep}: {status?.Error}"); + } + } + + await Task.Delay(1000); + } + + throw new TimeoutException( + $"Tenant {tenantId} provisioning did not complete within {maxRetries} seconds."); + } +} From 621d1b34fb61e3ad8d78e0412ca17b3374908624 Mon Sep 17 00:00:00 2001 From: "Marcelo M. Maciel" <4993482+marcelo-maciel@users.noreply.github.com> Date: Mon, 28 Sep 2026 14:42:08 -0300 Subject: [PATCH 2/2] refactor(identity): ConfigureAwait on the session cleanup loop awaits The rest of the file already uses ConfigureAwait(false), as RolePermissionSyncHostedService does. --- .../Modules.Identity/Services/SessionCleanupHostedService.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Modules/Identity/Modules.Identity/Services/SessionCleanupHostedService.cs b/src/Modules/Identity/Modules.Identity/Services/SessionCleanupHostedService.cs index 02ec54d36d..8fd01b500f 100644 --- a/src/Modules/Identity/Modules.Identity/Services/SessionCleanupHostedService.cs +++ b/src/Modules/Identity/Modules.Identity/Services/SessionCleanupHostedService.cs @@ -39,8 +39,8 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) { try { - await Task.Delay(_cleanupInterval, stoppingToken); - await CleanupExpiredSessionsAsync(stoppingToken); + await Task.Delay(_cleanupInterval, stoppingToken).ConfigureAwait(false); + await CleanupExpiredSessionsAsync(stoppingToken).ConfigureAwait(false); } catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested) {