From 6a07aae9cec7fb5141283cabba640fdbfb63e65c Mon Sep 17 00:00:00 2001 From: James Britton Date: Tue, 27 Oct 2026 14:17:03 +0000 Subject: [PATCH] bug: fix session id being regenerated on new logins when session already exists --- .../AuthenticationPropertiesExtensions.cs | 19 ++++++++++ .../Services/Default/DefaultUserSession.cs | 18 +++++++--- .../Default/DefaultUserSessionTests.cs | 35 +++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/Open.IdentityServer/src/Extensions/AuthenticationPropertiesExtensions.cs b/src/Open.IdentityServer/src/Extensions/AuthenticationPropertiesExtensions.cs index 8e0f106d8..06198098e 100644 --- a/src/Open.IdentityServer/src/Extensions/AuthenticationPropertiesExtensions.cs +++ b/src/Open.IdentityServer/src/Extensions/AuthenticationPropertiesExtensions.cs @@ -60,6 +60,25 @@ public static IEnumerable GetClientList(this AuthenticationProperties pr return Enumerable.Empty(); } + /// + /// Sets the list of client ids, removes if null + /// + /// The authentication properties from which to remove the client list. + /// List of client ids to set + public static void SetClientList(this AuthenticationProperties properties, IEnumerable clientIds) + { + var values = EncodeList(clientIds); + + if (string.IsNullOrWhiteSpace(values)) + { + properties.RemoveClientList(); + } + else + { + properties.Items[ClientListKey] = values; + } + } + /// /// Removes the list of client ids. /// diff --git a/src/Open.IdentityServer/src/Services/Default/DefaultUserSession.cs b/src/Open.IdentityServer/src/Services/Default/DefaultUserSession.cs index d31704648..4cdd253b0 100644 --- a/src/Open.IdentityServer/src/Services/Default/DefaultUserSession.cs +++ b/src/Open.IdentityServer/src/Services/Default/DefaultUserSession.cs @@ -158,12 +158,22 @@ protected virtual async Task AuthenticateAsync() /// public virtual async Task CreateSessionIdAsync(ClaimsPrincipal principal, AuthenticationProperties properties) { - if (principal == null) throw new ArgumentNullException(nameof(principal)); - if (properties == null) throw new ArgumentNullException(nameof(properties)); + ArgumentNullException.ThrowIfNull(principal); + ArgumentNullException.ThrowIfNull(properties); - var currentSubjectId = (await GetUserAsync())?.GetSubjectId(); + await AuthenticateAsync(); + + var currentSessionId = Properties?.GetSessionId(); + var currentSubjectId = Principal?.GetSubjectId(); var newSubjectId = principal.GetSubjectId(); - + + if (properties.GetSessionId() == null && currentSessionId != null) + { + properties.SetSessionId(currentSessionId); + var clients = Properties.GetClientList().ToList(); + properties.SetClientList(clients); + } + if (properties.GetSessionId() == null || currentSubjectId != newSubjectId) { properties.SetSessionId(CryptoRandom.CreateUniqueId(16, CryptoRandom.OutputFormat.Hex)); diff --git a/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Services/Default/DefaultUserSessionTests.cs b/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Services/Default/DefaultUserSessionTests.cs index 324872ba2..7eb492529 100644 --- a/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Services/Default/DefaultUserSessionTests.cs +++ b/src/Open.IdentityServer/test/Open.IdentityServer.UnitTests/Services/Default/DefaultUserSessionTests.cs @@ -82,6 +82,41 @@ public async Task CreateSessionId_when_props_does_not_contain_key_should_generat _props.GetSessionId().Should().NotBeNull(); } + [Fact] + public async Task CreateSessionId_WhenPropsArgDoesNotContainKey_ShouldPreserveSid() + { + string existingSid = "999"; + + var propsAuthResult = _props.Clone(); + propsAuthResult.SetSessionId(existingSid); + _mockAuthenticationHandler.Result = AuthenticateResult.Success(new AuthenticationTicket(_user, propsAuthResult, "scheme")); + + _props.GetSessionId().Should().BeNull(); + + await _subject.CreateSessionIdAsync(_user, _props); + + _props.GetSessionId().Should().Be(existingSid); + } + + [Fact] + public async Task CreateSessionId_WhenPropsArgDoesNotContainKey_ShouldPreserveClientList() + { + string existingSid = "999"; + string[] existingClientList = ["clientA", "clientB"]; + + var propsAuthResult = _props.Clone(); + propsAuthResult.SetSessionId(existingSid); + propsAuthResult.SetClientList(existingClientList); + + _mockAuthenticationHandler.Result = AuthenticateResult.Success(new AuthenticationTicket(_user, propsAuthResult, "scheme")); + + _props.GetSessionId().Should().BeNull(); + + await _subject.CreateSessionIdAsync(_user, _props); + + _props.GetClientList().Should().BeEquivalentTo(existingClientList); + } + [Fact] public async Task CreateSessionId_when_user_is_authenticated_but_different_sub_should_create_new_sid() {