From 219a52dfddf4f1bae45ded0c800d2d24eac05dce Mon Sep 17 00:00:00 2001 From: Brett Hazen <2651260+bhazen@users.noreply.github.com> Date: Wed, 14 May 2025 11:47:32 -0500 Subject: [PATCH 1/3] Added way to introduce simulated network delay as needed for integration tests against a dbcontext --- .../DatabaseProviderBuilder.cs | 27 +++++++++++++++-- .../NetworkDelaySimulationInterceptor.cs | 29 +++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 identity-server/test/IdentityServer.IntegrationTests/EntityFramework/NetworkDelaySimulationInterceptor.cs diff --git a/identity-server/test/IdentityServer.IntegrationTests/EntityFramework/DatabaseProviderBuilder.cs b/identity-server/test/IdentityServer.IntegrationTests/EntityFramework/DatabaseProviderBuilder.cs index 0c6d61bb0..606b6487d 100644 --- a/identity-server/test/IdentityServer.IntegrationTests/EntityFramework/DatabaseProviderBuilder.cs +++ b/identity-server/test/IdentityServer.IntegrationTests/EntityFramework/DatabaseProviderBuilder.cs @@ -14,7 +14,8 @@ namespace EntityFramework.IntegrationTests; public class DatabaseProviderBuilder { public static DbContextOptions BuildInMemory(string name, - TStoreOptions storeOptions) + TStoreOptions storeOptions, + TimeSpan? delay = null) where TDbContext : DbContext where TStoreOptions : class { @@ -23,12 +24,19 @@ public static DbContextOptions BuildInMemory(); builder.UseInMemoryDatabase(name); + + if (delay.HasValue) + { + builder.AddInterceptors(new NetworkDelaySimulationInterceptor(delay.Value)); + } + builder.UseApplicationServiceProvider(serviceCollection.BuildServiceProvider()); return builder.Options; } public static DbContextOptions BuildSqlite(string name, - TStoreOptions storeOptions) + TStoreOptions storeOptions, + TimeSpan? delay = null) where TDbContext : DbContext where TStoreOptions : class { @@ -41,12 +49,19 @@ public static DbContextOptions BuildSqlite(); builder.UseSqlite(connection); + + if (delay.HasValue) + { + builder.AddInterceptors(new NetworkDelaySimulationInterceptor(delay.Value)); + } + builder.UseApplicationServiceProvider(serviceCollection.BuildServiceProvider()); return builder.Options; } public static DbContextOptions BuildLocalDb(string name, - TStoreOptions storeOptions) + TStoreOptions storeOptions, + TimeSpan? delay = null) where TDbContext : DbContext where TStoreOptions : class { @@ -56,6 +71,12 @@ public static DbContextOptions BuildLocalDb(); builder.UseSqlServer( $@"Data Source=(LocalDb)\MSSQLLocalDB;database=Test.DuendeIdentityServer.EntityFramework.{name};trusted_connection=yes;"); + + if (delay.HasValue) + { + builder.AddInterceptors(new NetworkDelaySimulationInterceptor(delay.Value)); + } + builder.UseApplicationServiceProvider(serviceCollection.BuildServiceProvider()); return builder.Options; } diff --git a/identity-server/test/IdentityServer.IntegrationTests/EntityFramework/NetworkDelaySimulationInterceptor.cs b/identity-server/test/IdentityServer.IntegrationTests/EntityFramework/NetworkDelaySimulationInterceptor.cs new file mode 100644 index 000000000..631cd1f14 --- /dev/null +++ b/identity-server/test/IdentityServer.IntegrationTests/EntityFramework/NetworkDelaySimulationInterceptor.cs @@ -0,0 +1,29 @@ +// Copyright (c) Duende Software. All rights reserved. +// See LICENSE in the project root for license information. + +using System.Data.Common; +using Microsoft.EntityFrameworkCore.Diagnostics; + +namespace EntityFramework.IntegrationTests; + +public class NetworkDelaySimulationInterceptor(TimeSpan delay) : DbCommandInterceptor +{ + public override async ValueTask> ReaderExecutingAsync( + DbCommand command, + CommandEventData eventData, + InterceptionResult result, + CancellationToken cancellationToken = default) + { + await Task.Delay(delay, cancellationToken); + return result; + } + + public override InterceptionResult ReaderExecuting( + DbCommand command, + CommandEventData eventData, + InterceptionResult result) + { + Thread.Sleep(delay); + return result; + } +} From 722621a2359900e1a9a76425336a99d632f0f751 Mon Sep 17 00:00:00 2001 From: Brett Hazen <2651260+bhazen@users.noreply.github.com> Date: Wed, 14 May 2025 12:27:24 -0500 Subject: [PATCH 2/3] Creat payloads for outgoing backchannel logout requests sequentially to avoid concurrent acces to a DbContext --- .../DefaultBackChannelLogoutService.cs | 23 ++-- .../EntityFrameworkBasedLogoutTests.cs | 112 ++++++++++++++++++ 2 files changed, 122 insertions(+), 13 deletions(-) create mode 100644 identity-server/test/IdentityServer.IntegrationTests/EntityFramework/EntityFrameworkBasedLogoutTests.cs diff --git a/identity-server/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs b/identity-server/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs index 10d859ff7..9de12d938 100644 --- a/identity-server/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs +++ b/identity-server/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs @@ -91,21 +91,18 @@ public virtual async Task SendLogoutNotificationsAsync(LogoutNotificationContext /// /// /// - protected virtual Task SendLogoutNotificationsAsync(IEnumerable requests) + protected virtual async Task SendLogoutNotificationsAsync(IEnumerable requests) { - requests = requests ?? Enumerable.Empty(); - var tasks = requests.Select(SendLogoutNotificationAsync).ToArray(); - return Task.WhenAll(tasks); - } + requests ??= []; + var logoutRequestsWithPayload = new List<(BackChannelLogoutRequest, Dictionary)>(); + foreach (var backChannelLogoutRequest in requests) + { + var payload = await CreateFormPostPayloadAsync(backChannelLogoutRequest); + logoutRequestsWithPayload.Add((backChannelLogoutRequest, payload)); + } - /// - /// Performs the back-channel logout for a single client. - /// - /// - protected virtual async Task SendLogoutNotificationAsync(BackChannelLogoutRequest request) - { - var data = await CreateFormPostPayloadAsync(request); - await PostLogoutJwt(request, data); + var logoutRequests = logoutRequestsWithPayload.Select(request => PostLogoutJwt(request.Item1, request.Item2)).ToArray(); + await Task.WhenAll(logoutRequests); } /// diff --git a/identity-server/test/IdentityServer.IntegrationTests/EntityFramework/EntityFrameworkBasedLogoutTests.cs b/identity-server/test/IdentityServer.IntegrationTests/EntityFramework/EntityFrameworkBasedLogoutTests.cs new file mode 100644 index 000000000..ca7c6a902 --- /dev/null +++ b/identity-server/test/IdentityServer.IntegrationTests/EntityFramework/EntityFrameworkBasedLogoutTests.cs @@ -0,0 +1,112 @@ +// Copyright (c) Duende Software. All rights reserved. +// See LICENSE in the project root for license information. + +using Duende.IdentityModel.Client; +using Duende.IdentityServer.EntityFramework.DbContexts; +using Duende.IdentityServer.EntityFramework.Options; +using Duende.IdentityServer.EntityFramework.Stores; +using Duende.IdentityServer.Models; +using Duende.IdentityServer.Services; +using Duende.IdentityServer.Services.KeyManagement; +using Duende.IdentityServer.Stores; +using Duende.IdentityServer.Test; +using IntegrationTests.Common; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Logging.Abstractions; +using Client = Duende.IdentityServer.Models.Client; + +namespace EntityFramework.IntegrationTests; + +public class EntityFrameworkBasedLogoutTests +{ + private readonly IdentityServerPipeline _mockPipeline = new(); + + private static readonly ICollection _clients = + [ + new() + { + ClientId = "client_one", + ClientName = "Client One", + AllowedGrantTypes = GrantTypes.Code, + RequireClientSecret = false, + RequireConsent = false, + RequirePkce = false, + AllowedScopes = { "openid", "api" }, + AllowOfflineAccess = true, + CoordinateLifetimeWithUserSession = true, + BackChannelLogoutUri = "https://client_one/logout", + RedirectUris = ["https://client_one/redirect"] + }, + new() + { + ClientId = "client_two", + ClientName = "Client Two", + AllowedGrantTypes = GrantTypes.Code, + RequireClientSecret = false, + RequireConsent = false, + RequirePkce = false, + AllowedScopes = { "openid", "api" }, + AllowOfflineAccess = true, + CoordinateLifetimeWithUserSession = true, + BackChannelLogoutUri = "https://client_two/logout", + RedirectUris = ["https://client_two/redirect"] + } + ]; + + public EntityFrameworkBasedLogoutTests() + { + _mockPipeline.Clients.AddRange(_clients); + _mockPipeline.IdentityScopes.Add(new IdentityResources.OpenId()); + _mockPipeline.ApiScopes.Add(new ApiScope("api")); + + _mockPipeline.Users.Add(new TestUser + { + SubjectId = "alice", + Username = "alice", + }); + } + + [Fact] + public async Task LogoutWithMultipleClientsInSession_WhenUsingEntityFrameworkBackedKeyStore_Succeeds() + { + //Setup db context with simulated network delay to cause concurrent db access + var options = DatabaseProviderBuilder.BuildSqlite("NotUsed", new OperationalStoreOptions(), + TimeSpan.FromMilliseconds(1)); + await using var context = new PersistedGrantDbContext(options); + await context.Database.EnsureCreatedAsync(); + + _mockPipeline.OnPostConfigureServices += services => + { + //Override the default developer signing key store and signing credential store with the EF based implementations to repo bug specific to concurrent access to an EF db context + services.AddSingleton(new SigningKeyStore(context, new NullLogger(), + new NoneCancellationTokenProvider())); + services.Replace(ServiceDescriptor.Singleton()); + }; + _mockPipeline.Initialize(); + _mockPipeline.Options.KeyManagement.Enabled = true; + + await _mockPipeline.LoginAsync("alice"); + + //Ensure user session is tied to multiple clients so back channel logout will execute against multiple clients + foreach (var client in _clients) + { + var authzResponse = await _mockPipeline.RequestAuthorizationEndpointAsync(client.ClientId, "code", "openid api offline_access", client.RedirectUris.First()); + _ = await _mockPipeline.BackChannelClient.RequestAuthorizationCodeTokenAsync(new AuthorizationCodeTokenRequest + { + Address = IdentityServerPipeline.TokenEndpoint, + ClientId = client.ClientId, + Code = authzResponse.Code, + RedirectUri = client.RedirectUris.First() + }); + } + + //Clear cache to simulate needing to load from db when creating logout notifications to send + var signingKeyStoreCache = _mockPipeline.Resolve(); + await signingKeyStoreCache.StoreKeysAsync([], TimeSpan.Zero); + + await _mockPipeline.LogoutAsync(); + + _mockPipeline.ErrorWasCalled.ShouldBeFalse(); + } +} From 0c4db92504a0bdb1ee7b43086947bd0f387482c9 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 15 May 2025 09:41:07 -0500 Subject: [PATCH 3/3] Added an explanatory comment --- .../Services/Default/DefaultBackChannelLogoutService.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/identity-server/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs b/identity-server/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs index 9de12d938..0bf5f6cea 100644 --- a/identity-server/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs +++ b/identity-server/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs @@ -97,6 +97,11 @@ protected virtual async Task SendLogoutNotificationsAsync(IEnumerable)>(); foreach (var backChannelLogoutRequest in requests) { + // Creation of the payload can cause database access to retrieve the + // signing key. That needs to be done in serial so that our EF store + // implementation doesn't make parallel use of a single DB context. + // Since the signing key material should be cached, only the + // first serial operation will call the db. var payload = await CreateFormPostPayloadAsync(backChannelLogoutRequest); logoutRequestsWithPayload.Add((backChannelLogoutRequest, payload)); }