Fix Concurrent DbContext Access on Back Channel Logout

identity-server/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs

@@ -91,21 +91,23 @@ public virtual async Task SendLogoutNotificationsAsync(LogoutNotificationContext
     /// </summary>
     /// <param name="requests"></param>
     /// <returns></returns>
-    protected virtual Task SendLogoutNotificationsAsync(IEnumerable<BackChannelLogoutRequest> requests)
+    protected virtual async Task SendLogoutNotificationsAsync(IEnumerable<BackChannelLogoutRequest> requests)
     {
-        requests = requests ?? Enumerable.Empty<BackChannelLogoutRequest>();
-        var tasks = requests.Select(SendLogoutNotificationAsync).ToArray();
-        return Task.WhenAll(tasks);
-    }
+        requests ??= [];
+        var logoutRequestsWithPayload = new List<(BackChannelLogoutRequest, Dictionary<string, string>)>();
+        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));
+        }
 
-    /// <summary>
-    /// Performs the back-channel logout for a single client.
-    /// </summary>
-    /// <param name="request"></param>
-    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);
     }
 
     /// <summary>

identity-server/test/IdentityServer.IntegrationTests/EntityFramework/DatabaseProviderBuilder.cs

@@ -14,7 +14,8 @@ namespace EntityFramework.IntegrationTests;
 public class DatabaseProviderBuilder
 {
     public static DbContextOptions<TDbContext> BuildInMemory<TDbContext, TStoreOptions>(string name,
-        TStoreOptions storeOptions)
+        TStoreOptions storeOptions,
+        TimeSpan? delay = null)
         where TDbContext : DbContext
         where TStoreOptions : class
     {
@@ -23,12 +24,19 @@ public static DbContextOptions<TDbContext> BuildInMemory<TDbContext, TStoreOptio
 
         var builder = new DbContextOptionsBuilder<TDbContext>();
         builder.UseInMemoryDatabase(name);
+
+        if (delay.HasValue)
+        {
+            builder.AddInterceptors(new NetworkDelaySimulationInterceptor(delay.Value));
+        }
+
         builder.UseApplicationServiceProvider(serviceCollection.BuildServiceProvider());
         return builder.Options;
     }
 
     public static DbContextOptions<TDbContext> BuildSqlite<TDbContext, TStoreOptions>(string name,
-        TStoreOptions storeOptions)
+        TStoreOptions storeOptions,
+        TimeSpan? delay = null)
         where TDbContext : DbContext
         where TStoreOptions : class
     {
@@ -41,12 +49,19 @@ public static DbContextOptions<TDbContext> BuildSqlite<TDbContext, TStoreOptions
 
         var builder = new DbContextOptionsBuilder<TDbContext>();
         builder.UseSqlite(connection);
+
+        if (delay.HasValue)
+        {
+            builder.AddInterceptors(new NetworkDelaySimulationInterceptor(delay.Value));
+        }
+
         builder.UseApplicationServiceProvider(serviceCollection.BuildServiceProvider());
         return builder.Options;
     }
 
     public static DbContextOptions<TDbContext> BuildLocalDb<TDbContext, TStoreOptions>(string name,
-        TStoreOptions storeOptions)
+        TStoreOptions storeOptions,
+        TimeSpan? delay = null)
         where TDbContext : DbContext
         where TStoreOptions : class
     {
@@ -56,6 +71,12 @@ public static DbContextOptions<TDbContext> BuildLocalDb<TDbContext, TStoreOption
         var builder = new DbContextOptionsBuilder<TDbContext>();
         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;
     }

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<Client> _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<PersistedGrantDbContext, OperationalStoreOptions>("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<ISigningKeyStore>(new SigningKeyStore(context, new NullLogger<SigningKeyStore>(),
+                new NoneCancellationTokenProvider()));
+            services.Replace(ServiceDescriptor.Singleton<ISigningCredentialStore, AutomaticKeyManagerKeyStore>());
+        };
+        _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<ISigningKeyStoreCache>();
+        await signingKeyStoreCache.StoreKeysAsync([], TimeSpan.Zero);
+
+        await _mockPipeline.LogoutAsync();
+
+        _mockPipeline.ErrorWasCalled.ShouldBeFalse();
+    }
+}

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<InterceptionResult<DbDataReader>> ReaderExecutingAsync(
+        DbCommand command,
+        CommandEventData eventData,
+        InterceptionResult<DbDataReader> result,
+        CancellationToken cancellationToken = default)
+    {
+        await Task.Delay(delay, cancellationToken);
+        return result;
+    }
+
+    public override InterceptionResult<DbDataReader> ReaderExecuting(
+        DbCommand command,
+        CommandEventData eventData,
+        InterceptionResult<DbDataReader> result)
+    {
+        Thread.Sleep(delay);
+        return result;
+    }
+}