From ea581bd044f0c559288fe2d46fddaafbfc96ea5e Mon Sep 17 00:00:00 2001 From: James Britton Date: Wed, 28 Oct 2026 16:12:02 +0000 Subject: [PATCH] fix: set session management remove tokens default backchannel logout to all clients when none specified --- .../DefaultSessionManagementService.cs | 6 +- .../DefaultSessionManagementServiceTests.cs | 72 ++++++++++++++----- 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/Open.IdentityServer/src/Services/Default/DefaultSessionManagementService.cs b/src/Open.IdentityServer/src/Services/Default/DefaultSessionManagementService.cs index c47c2f1b..74fd057e 100644 --- a/src/Open.IdentityServer/src/Services/Default/DefaultSessionManagementService.cs +++ b/src/Open.IdentityServer/src/Services/Default/DefaultSessionManagementService.cs @@ -54,14 +54,14 @@ public async Task RemoveSessionsAsync(RemoveSessionsContext context, Cancellatio if (!sessionClientList.IsNullOrEmpty() && !context.ClientIds.IsNullOrEmpty()) { - clientIds = sessionClientList!.Where(x => context.ClientIds!.Contains(x)).ToArray(); + clientIds = [..sessionClientList!.Where(x => context.ClientIds!.Contains(x))]; } await backChannelLogoutService.SendLogoutNotificationsAsync(new LogoutNotificationContext { SubjectId = session.Session.SubjectId, SessionId = session.Session.SessionId, - ClientIds = clientIds, + ClientIds = context.ClientIds.IsNullOrEmpty() ? sessionClientList : clientIds, }); } } @@ -85,7 +85,7 @@ await persistedGrantStore.RemoveAllAsync(new PersistedGrantFilter SubjectId = context.SubjectId, SessionId = context.SessionId, ClientIds = context.ClientIds?.ToArray() ?? [], - Types = typeFilter.ToArray(), + Types = [..typeFilter], }); } diff --git a/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Services/Default/DefaultSessionManagementServiceTests.cs b/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Services/Default/DefaultSessionManagementServiceTests.cs index e1c7f88f..0e7f4719 100644 --- a/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Services/Default/DefaultSessionManagementServiceTests.cs +++ b/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Services/Default/DefaultSessionManagementServiceTests.cs @@ -127,7 +127,7 @@ public async Task RemoveSessionsAsync_WhenFilterSpecified_ShouldRemoveAllSession List fakeSessions = [ ServerSessionTestGenerators.GenerateAuthenticationTicketFilterResult("key2", "SchemeA", "alice", "session-0002", "Alice"), - ServerSessionTestGenerators.GenerateAuthenticationTicketFilterResult("key4", "SchemeB", "alice", "session-0002", "Alice"), + ServerSessionTestGenerators.GenerateAuthenticationTicketFilterResult("key4", "SchemeB", "alice", "session-0002", "Alice", clientIds: ["fake-client-a", "fake-client-b"]), ServerSessionTestGenerators.GenerateAuthenticationTicketFilterResult("key7", "SchemeC", "alice", "session-0002", "Alice"), ]; @@ -150,20 +150,56 @@ public async Task RemoveSessionsAsync_WhenFilterSpecified_ShouldRemoveAllSession ctx.SessionId == fakeSession.Session.SessionId && ctx.SubjectId == fakeSession.Session.SubjectId && fakeSession.AuthTicket != null && - ctx.ClientIds == fakeSession.AuthTicket.Properties.GetClientList()))); + ctx.ClientIds.SequenceEqual(fakeSession.AuthTicket.Properties.GetClientList())))); } Mock.Get(persistedGrantStore) .Verify(x => x.RemoveAllAsync(It.Is(f => f.SessionId == fakeContext.SessionId && f.SubjectId == fakeContext.SubjectId && - f.Types.AsEnumerable().ToHashSet().SetEquals(AllGrantTypes) && + f.Types.AsEnumerable().SequenceEqual(AllGrantTypes) && f.ClientIds == (fakeContext.ClientIds ?? Array.Empty())))); Mock.Get(serverSessionStore) .Verify(x => x.DeleteSessions(fakeContext.SubjectId, fakeContext.SessionId)); } + [Fact] + public async Task RemoveSessionsAsync_WhenClientIdsNotProvided_ShouldOnlyTriggerBackchannelNotificationsAndRevocationsForAllClients() + { + string[] fakeClientIds = ["client-a", "client-b", "client-c", "client-d"]; + var fakeSession = ServerSessionTestGenerators.GenerateAuthenticationTicketFilterResult("key2", "SchemeA", "alice", "session-0002", "Alice", clientIds: fakeClientIds); + List fakeSessions = [fakeSession]; + + Mock.Get(serverSessionTicketStore) + .Setup(x => x.FilterServerAuthenticationTickets("alice", null)) + .ReturnsAsync(fakeSessions); + + RemoveSessionsContext fakeContext = new RemoveSessionsContext + { + SubjectId = "alice", SessionId = null, + }; + DefaultSessionManagementService sut = CreateSut(); + + await sut.RemoveSessionsAsync(fakeContext, TestContext.Current.CancellationToken); + + Mock.Get(backChannelLogoutService) + .Verify(x => x.SendLogoutNotificationsAsync(It.Is(ctx => + ctx.SessionId == fakeSession.Session.SessionId && + ctx.SubjectId == fakeSession.Session.SubjectId && + ctx.ClientIds.SequenceEqual(fakeClientIds)))); + + Mock.Get(persistedGrantStore) + .Verify(x => x.RemoveAllAsync(It.Is(f => + f.SessionId == fakeContext.SessionId && + f.SubjectId == fakeContext.SubjectId && + f.Types.AsEnumerable().SequenceEqual(AllGrantTypes) && + f.ClientIds.SequenceEqual(fakeContext.ClientIds ?? Array.Empty())))); + + Mock.Get(serverSessionStore) + .Verify(x => x.DeleteSessions(fakeContext.SubjectId, fakeContext.SessionId)); + } + [Fact] public async Task RemoveSessionsAsync_WhenClientIdsProvided_ShouldOnlyTriggerBackchannelNotificationsAndRevocationsForThoseClients() { @@ -188,14 +224,14 @@ public async Task RemoveSessionsAsync_WhenClientIdsProvided_ShouldOnlyTriggerBac .Verify(x => x.SendLogoutNotificationsAsync(It.Is(ctx => ctx.SessionId == fakeSession.Session.SessionId && ctx.SubjectId == fakeSession.Session.SubjectId && - ctx.ClientIds.ToHashSet().SetEquals(expectedClientIds)))); + ctx.ClientIds.SequenceEqual(expectedClientIds)))); Mock.Get(persistedGrantStore) .Verify(x => x.RemoveAllAsync(It.Is(f => f.SessionId == fakeContext.SessionId && f.SubjectId == fakeContext.SubjectId && - f.Types.AsEnumerable().ToHashSet().SetEquals(AllGrantTypes) && - f.ClientIds.ToHashSet().SetEquals(fakeContext.ClientIds ?? Array.Empty())))); + f.Types.AsEnumerable().SequenceEqual(AllGrantTypes) && + f.ClientIds.SequenceEqual(fakeContext.ClientIds ?? Array.Empty())))); Mock.Get(serverSessionStore) .Verify(x => x.DeleteSessions(fakeContext.SubjectId, fakeContext.SessionId)); @@ -231,15 +267,15 @@ public async Task RemoveSessionsAsync_WhenRemoveSessionsSetToFalse_ShouldNotRemo ctx.SessionId == fakeSession.Session.SessionId && ctx.SubjectId == fakeSession.Session.SubjectId && fakeSession.AuthTicket != null && - ctx.ClientIds.ToHashSet().SetEquals(fakeSession.AuthTicket.Properties.GetClientList())))); + ctx.ClientIds.SequenceEqual(fakeSession.AuthTicket.Properties.GetClientList())))); } Mock.Get(persistedGrantStore) .Verify(x => x.RemoveAllAsync(It.Is(f => f.SessionId == fakeContext.SessionId && f.SubjectId == fakeContext.SubjectId && - f.Types.AsEnumerable().ToHashSet().SetEquals(AllGrantTypes) && - f.ClientIds.ToHashSet().SetEquals(fakeContext.ClientIds ?? Array.Empty())))); + f.Types.AsEnumerable().SequenceEqual(AllGrantTypes) && + f.ClientIds.SequenceEqual(fakeContext.ClientIds ?? Array.Empty())))); Mock.Get(serverSessionStore) .Verify(x => x.DeleteSessions(fakeContext.SubjectId, fakeContext.SessionId), Times.Never); @@ -275,8 +311,8 @@ public async Task RemoveSessionsAsync_WhenSendBackchannelFalse_ShouldNotSendBack .Verify(x => x.RemoveAllAsync(It.Is(f => f.SessionId == fakeContext.SessionId && f.SubjectId == fakeContext.SubjectId && - f.Types.AsEnumerable().ToHashSet().SetEquals(AllGrantTypes) && - f.ClientIds.ToHashSet().SetEquals(fakeContext.ClientIds ?? Array.Empty())))); + f.Types.AsEnumerable().SequenceEqual(AllGrantTypes) && + f.ClientIds.SequenceEqual(fakeContext.ClientIds ?? Array.Empty())))); Mock.Get(serverSessionStore) .Verify(x => x.DeleteSessions(fakeContext.SubjectId, fakeContext.SessionId)); @@ -312,15 +348,15 @@ public async Task RemoveSessionsAsync_WhenRevokeTokensFalse_ShouldNotRevokeToken ctx.SessionId == fakeSession.Session.SessionId && ctx.SubjectId == fakeSession.Session.SubjectId && fakeSession.AuthTicket != null && - ctx.ClientIds.ToHashSet().SetEquals(fakeSession.AuthTicket.Properties.GetClientList())))); + ctx.ClientIds.SequenceEqual(fakeSession.AuthTicket.Properties.GetClientList())))); } Mock.Get(persistedGrantStore) .Verify(x => x.RemoveAllAsync(It.Is(f => f.SessionId == fakeContext.SessionId && f.SubjectId == fakeContext.SubjectId && - f.Types.AsEnumerable().ToHashSet().SetEquals(ConsentGrantTypes) && - f.ClientIds.ToHashSet().SetEquals(fakeContext.ClientIds ?? Array.Empty())))); + f.Types.AsEnumerable().SequenceEqual(ConsentGrantTypes) && + f.ClientIds.SequenceEqual(fakeContext.ClientIds ?? Array.Empty())))); Mock.Get(serverSessionStore) .Verify(x => x.DeleteSessions(fakeContext.SubjectId, fakeContext.SessionId)); @@ -356,15 +392,15 @@ public async Task RemoveSessionsAsync_WhenRevokeConsentsFalse_ShouldNotRevokeCon ctx.SessionId == fakeSession.Session.SessionId && ctx.SubjectId == fakeSession.Session.SubjectId && fakeSession.AuthTicket != null && - ctx.ClientIds.ToHashSet().SetEquals(fakeSession.AuthTicket.Properties.GetClientList())))); + ctx.ClientIds.SequenceEqual(fakeSession.AuthTicket.Properties.GetClientList())))); } Mock.Get(persistedGrantStore) .Verify(x => x.RemoveAllAsync(It.Is(f => f.SessionId == fakeContext.SessionId && f.SubjectId == fakeContext.SubjectId && - f.Types.AsEnumerable().ToHashSet().SetEquals(TokenGrantTypes) && - f.ClientIds.ToHashSet().SetEquals(fakeContext.ClientIds ?? Array.Empty())))); + f.Types.AsEnumerable().SequenceEqual(TokenGrantTypes) && + f.ClientIds.SequenceEqual(fakeContext.ClientIds ?? Array.Empty())))); Mock.Get(serverSessionStore) .Verify(x => x.DeleteSessions(fakeContext.SubjectId, fakeContext.SessionId)); @@ -400,7 +436,7 @@ public async Task RemoveSessionsAsync_WhenRevokeTokensAndConsentsFalse_ShouldNot ctx.SessionId == fakeSession.Session.SessionId && ctx.SubjectId == fakeSession.Session.SubjectId && fakeSession.AuthTicket != null && - ctx.ClientIds.ToHashSet().SetEquals(fakeSession.AuthTicket.Properties.GetClientList())))); + ctx.ClientIds.SequenceEqual(fakeSession.AuthTicket.Properties.GetClientList())))); } Mock.Get(persistedGrantStore)