From c908d15677b644e13389b155d69a20a8ea3dc174 Mon Sep 17 00:00:00 2001 From: khalidabuhakmeh Date: Wed, 4 Jun 2025 08:34:49 -0400 Subject: [PATCH 1/2] Add null-check for client before coordinating session lifecycle Ensure that the client object is not null before attempting to determine its session coordination behavior. --- .../DefaultSessionCoordinationService.cs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/identity-server/src/IdentityServer/Services/Default/DefaultSessionCoordinationService.cs b/identity-server/src/IdentityServer/Services/Default/DefaultSessionCoordinationService.cs index 99664fb1d..ead7c7253 100644 --- a/identity-server/src/IdentityServer/Services/Default/DefaultSessionCoordinationService.cs +++ b/identity-server/src/IdentityServer/Services/Default/DefaultSessionCoordinationService.cs @@ -137,14 +137,17 @@ public virtual async Task ProcessExpirationAsync(UserSession session) { var client = await ClientStore.FindClientByIdAsync(clientId); // i don't think we care if it's an enabled client at this point - var shouldCoordinate = - client.CoordinateLifetimeWithUserSession == true || - (Options.Authentication.CoordinateClientLifetimesWithUserSession && client.CoordinateLifetimeWithUserSession != false); - - if (shouldCoordinate) + if (client != null) { - // this implies they should also be contacted for backchannel logout below - clientsToCoordinate.Add(clientId); + var shouldCoordinate = + client.CoordinateLifetimeWithUserSession == true || + (Options.Authentication.CoordinateClientLifetimesWithUserSession && client.CoordinateLifetimeWithUserSession != false); + + if (shouldCoordinate) + { + // this implies they should also be contacted for backchannel logout below + clientsToCoordinate.Add(clientId); + } } } From 463518332ed124746b5df8b34cfc7f3e0fbf475f Mon Sep 17 00:00:00 2001 From: khalidabuhakmeh Date: Wed, 4 Jun 2025 11:41:01 -0400 Subject: [PATCH 2/2] Add unit test to verify handling of missing client in session coordination Ensure `DefaultSessionCoordinationService` does not attempt logout for missing or null clients by introducing a dedicated test. --- .../DefaultSessionCoordinationServiceTests.cs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 identity-server/test/IdentityServer.UnitTests/Services/Default/DefaultSessionCoordinationServiceTests.cs diff --git a/identity-server/test/IdentityServer.UnitTests/Services/Default/DefaultSessionCoordinationServiceTests.cs b/identity-server/test/IdentityServer.UnitTests/Services/Default/DefaultSessionCoordinationServiceTests.cs new file mode 100644 index 000000000..1873345fd --- /dev/null +++ b/identity-server/test/IdentityServer.UnitTests/Services/Default/DefaultSessionCoordinationServiceTests.cs @@ -0,0 +1,39 @@ +using System.Threading.Tasks; +using Duende.IdentityServer.Configuration; +using Duende.IdentityServer.Models; +using Duende.IdentityServer.Services; +using Duende.IdentityServer.Stores; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using UnitTests.Endpoints.EndSession; +using Xunit; + +namespace UnitTests.Services.Default; + +public class DefaultSessionCoordinationServiceTests +{ + public DefaultSessionCoordinationService Service; + + [Fact] + public async Task Handles_missing_client_null_reference() + { + var stubBackChannelLogoutClient = new StubBackChannelLogoutClient(); + Service = new DefaultSessionCoordinationService( + new IdentityServerOptions(), + new InMemoryPersistedGrantStore(), + new InMemoryClientStore([]), + stubBackChannelLogoutClient, + new NullLogger()); + + await Service.ProcessExpirationAsync(new UserSession + { + ClientIds = ["not_found"], + SessionId = "1", + SubjectId = "1" + }); + + stubBackChannelLogoutClient + .SendLogoutsWasCalled + .ShouldBeFalse(); + } +} \ No newline at end of file