Fix IsPersistent issue in the session migration samples (v7 and v8)

IdentityServer/v7/SessionMigration/HostingExtensions.cs

@@ -1,7 +1,9 @@
 // Copyright (c) Duende Software. All rights reserved.
 // Licensed under the MIT License. See LICENSE in the project root for license information.
 
+using Microsoft.AspNetCore.Authentication.Cookies;
 using Microsoft.AspNetCore.DataProtection;
+using Microsoft.Extensions.Options;
 
 namespace SessionMigration;
 

IdentityServer/v7/SessionMigration/MigratingTicketDataFormat.cs

@@ -10,74 +10,105 @@
 namespace SessionMigration;
 public class MigratingTicketDataFormat : ISecureDataFormat<AuthenticationTicket>
 {
-    private readonly IHttpContextAccessor httpContextAccessor;
-    private readonly ISecureDataFormat<AuthenticationTicket> inner;
-    private CookieAuthenticationOptions options;
-    private readonly string scheme;
+    private readonly IHttpContextAccessor _httpContextAccessor;
+    private readonly ISecureDataFormat<AuthenticationTicket> _inner;
+    private readonly CookieAuthenticationOptions _options;
+    private readonly string _scheme;
 
-    // Copied from Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationHandler,
-    // unfortunatley it's private and cannot be referenced.
+    // Copied from Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationHandler.
+    // Unfortunately, it's private and cannot be referenced.
     private const string SessionIdClaim = "Microsoft.AspNetCore.Authentication.Cookies-SessionId";
 
     public MigratingTicketDataFormat(
         IHttpContextAccessor httpContextAccessor,
         CookieAuthenticationOptions options,
         string scheme)
     {
-        this.httpContextAccessor = httpContextAccessor;
-        this.options = options;
-        this.scheme = scheme;
+        _httpContextAccessor = httpContextAccessor;
+        _options = options;
+        _scheme = scheme;
 
         // Capture the inner at construction as the value in options will be replaced with
         // a reference to this instance.
-        inner = options.TicketDataFormat;
+        _inner = options.TicketDataFormat;
     }
 
-    public string Protect(AuthenticationTicket data) => inner.Protect(data);
-    public string Protect(AuthenticationTicket data, string purpose) => inner.Protect(data, purpose);
-    public AuthenticationTicket Unprotect(string protectedText) => inner.Unprotect(protectedText);
+    public string Protect(AuthenticationTicket data) => _inner.Protect(data);
+    public string Protect(AuthenticationTicket data, string purpose) => _inner.Protect(data, purpose);
+    public AuthenticationTicket Unprotect(string protectedText) => _inner.Unprotect(protectedText);
     public AuthenticationTicket Unprotect(string protectedText, string purpose)
     {
-        var ticket = inner.Unprotect(protectedText, purpose);
+        var ticket = _inner.Unprotect(protectedText, purpose);
 
         if (ticket.Principal.HasClaim(c => c.Type == SessionIdClaim))
         {
             // The ticket is already a reference ticket into the session store, just return it.
             return ticket;
         }
 
-        var context = httpContextAccessor.HttpContext;
+        var context = _httpContextAccessor.HttpContext;
         var sessionStore = context.RequestServices.GetRequiredService<IServerSideTicketStore>();
 
-        // Unprotect isn't async so we'll have block the thread and wait using .Result when calling an
-        // async methods. This is bad for performance, but this only happens once per logged in user
-        // session so it should be acceptable. Thanks to Asp.Net Core not having any synchronization
-        // context there is no risk for dead locks.
-        var sessionId = sessionStore.StoreAsync(ticket).Result;
-
-        // There's a potential race condition where two requests could migrate the same session. Check
-        // if there's another entry with the same SID and if it is rollback the one we created and
-        // don't alter the cookie.
+        // Unprotect isn't async so we have to block synchronously. ISecureDataFormat<T> is a
+        // synchronous interface defined by ASP.NET Core and cannot be changed. Thanks to ASP.NET
+        // Core not having a synchronization context there is no risk for deadlocks.
+        //
+        // NOTE: In a real implementation with many concurrent users migrating at once (e.g. after
+        // a deployment), blocking here could still cause thread pool starvation. Consider whether your
+        // scale warrants an alternative approach, such as splitting the migration into two phases:
+        //   1. Detect the old cookie-based session in Unprotect and flag the request (e.g. via
+        //      HttpContext.Items) without doing any I/O.
+        //   2. In async middleware further up the pipeline, check the flag, perform the actual
+        //      store write, and rewrite the cookie — all fully async.
+        // This avoids sync-over-async entirely at the cost of more architectural complexity.
+        var sessionId = sessionStore.StoreAsync(ticket).GetAwaiter().GetResult();
+
+        // There's a potential race condition where two requests could migrate the same session.
+        // Check if there's another entry with the same SID and if so, rollback the one we
+        // created and don't alter the cookie.
+        //
+        // NOTE: This check has a TOCTOU (time-of-check-time-of-use) issue. Two concurrent
+        // requests could both store a session, both detect the duplicate, and both roll back,
+        // leaving no migrated session. This is self-healing (the next request will retry), but
+        // in a real implementation you may want to use a distributed lock or compare creation
+        // timestamps to decide which duplicate to keep.
         if (HasDuplicate(sessionStore, ticket))
         {
-            sessionStore.RemoveAsync(sessionId).Wait();
+            sessionStore.RemoveAsync(sessionId).GetAwaiter().GetResult();
         }
         else
         {
+            // Carry over relevant properties from the original ticket so the reference ticket
+            // (and the cookie) accurately reflects the original session state. Without this,
+            // persistent sessions would be downgraded to session cookies, and any downstream
+            // code inspecting properties on this request would see empty defaults.
+            var properties = new AuthenticationProperties
+            {
+                IsPersistent = ticket.Properties.IsPersistent,
+                IssuedUtc = ticket.Properties.IssuedUtc,
+                ExpiresUtc = ticket.Properties.ExpiresUtc,
+                AllowRefresh = ticket.Properties.AllowRefresh,
+                // NOTE: If your application stores custom data in Properties.Items or
+                // Properties.Parameters, you may want to copy those over as well.
+            };
+
             var principal = new ClaimsPrincipal(
                 new ClaimsIdentity(
-                    new[] { new Claim(SessionIdClaim, sessionId, ClaimValueTypes.String, options.ClaimsIssuer) },
-                    options.ClaimsIssuer));
+                    new[] { new Claim(SessionIdClaim, sessionId, ClaimValueTypes.String, _options.ClaimsIssuer) },
+                    _options.ClaimsIssuer));
 
-            ticket = new AuthenticationTicket(principal, null, scheme);
+            ticket = new AuthenticationTicket(principal, properties, _scheme);
 
-            var cookieValue = inner.Protect(ticket, purpose);
+            var cookieValue = _inner.Protect(ticket, purpose);
 
+            // NOTE: AppendResponseCookie will throw if the response has already started
+            // (e.g. if headers have been sent). In a real implementation, guard against this
+            // or ensure migration runs early enough in the pipeline.
             var cookieOptions = CreateCookieOptions(ticket, context);
 
-            options.CookieManager.AppendResponseCookie(
+            _options.CookieManager.AppendResponseCookie(
                 context,
-                options.Cookie.Name!,
+                _options.Cookie.Name!,
                 cookieValue,
                 cookieOptions);
         }
@@ -88,28 +119,13 @@ private CookieOptions CreateCookieOptions(AuthenticationTicket ticket, HttpConte
     {
         // Cookie option generation copied from cookie handler.
 
-        var cookieOptions = options.Cookie.Build(context);
+        var cookieOptions = _options.Cookie.Build(context);
         cookieOptions.Expires = null;
 
         if (ticket.Properties.IsPersistent)
         {
-            DateTimeOffset issuedUtc;
-            if (ticket.Properties.IssuedUtc.HasValue)
-            {
-                issuedUtc = ticket.Properties.IssuedUtc.Value;
-            }
-            else
-            {
-                issuedUtc = DateTime.UtcNow;
-                ticket.Properties.IssuedUtc = issuedUtc;
-            }
-
-            if (!ticket.Properties.ExpiresUtc.HasValue)
-            {
-                ticket.Properties.ExpiresUtc = issuedUtc.Add(options.ExpireTimeSpan);
-            }
-
-            var expiresUtc = ticket.Properties.ExpiresUtc ?? issuedUtc.Add(options.ExpireTimeSpan);
+            var issuedUtc = ticket.Properties.IssuedUtc ?? DateTimeOffset.UtcNow;
+            var expiresUtc = ticket.Properties.ExpiresUtc ?? issuedUtc.Add(_options.ExpireTimeSpan);
             cookieOptions.Expires = expiresUtc.ToUniversalTime();
         }
 
@@ -125,7 +141,9 @@ private bool HasDuplicate(IServerSideTicketStore sessionStore, AuthenticationTic
             SessionId = sid
         };
 
-        var sessions = sessionStore.QuerySessionsAsync(filter).Result;
+        var sessions = sessionStore.QuerySessionsAsync(filter,
+            _httpContextAccessor.HttpContext?.RequestAborted ?? CancellationToken.None)
+            .GetAwaiter().GetResult();
 
         // There should be only one entry, the one we just created.
         return sessions.Results.Count > 1;

IdentityServer/v7/SessionMigration/README.md

@@ -0,0 +1,28 @@
+# Session Migration sample
+
+## Instructions
+
+The purpose of this sample is to show how to migrate existing client side session to server
+side session without the user having to login again. To test it, first run the sample as is
+and log in (user: alice, password: alice) to get a normal cookie based session in your
+browser. Then uncomment the following blocks and run the sample again. The first request
+from your browser to IdentityServer will cause the session to be migrated to be a server
+side session.
+
+Note that further restarts will invalidate your session as the server side session
+store in this sample is in memory only.
+If server side sessions have been enabled and then are removed before another
+test run, you have to manually clear the cookie to be able to log in again.
+
+This is the normal template code to activate server-side sessions:
+```csharp
+var isBuilder = builder.Services.AddIdentityServer(...);
+
+isBuilder.AddServerSideSessions();
+```
+
+And this code adds migration of sessions. Enabling server-side sessions without adding this migration, will
+invalidate all existing sessions.
+```csharp
+builder.Services.AddTransient<IPostConfigureOptions<CookieAuthenticationOptions>, SessionMigrationPostConfigureOptions>();
+```

IdentityServer/v7/SessionMigration/SessionMigrationPostConfigureOptions.cs

@@ -9,11 +9,11 @@ namespace SessionMigration;
 
 public class SessionMigrationPostConfigureOptions : IPostConfigureOptions<CookieAuthenticationOptions>
 {
-    private readonly IHttpContextAccessor httpContextAccessor;
+    private readonly IHttpContextAccessor _httpContextAccessor;
 
     public SessionMigrationPostConfigureOptions(IHttpContextAccessor httpContextAccessor)
     {
-        this.httpContextAccessor = httpContextAccessor;
+        _httpContextAccessor = httpContextAccessor;
     }
 
     public void PostConfigure(string name, CookieAuthenticationOptions options)
@@ -31,6 +31,6 @@ public void PostConfigure(string name, CookieAuthenticationOptions options)
 
         // TicketDataFormat is not injected through DI so we have to supply it with an http context accessor
         // to be able to resolve request services at runtime.
-        options.TicketDataFormat = new MigratingTicketDataFormat(httpContextAccessor, options, name);
+        options.TicketDataFormat = new MigratingTicketDataFormat(_httpContextAccessor, options, name);
     }
 }

IdentityServer/v8/SessionMigration/HostingExtensions.cs

@@ -1,7 +1,9 @@
 // Copyright (c) Duende Software. All rights reserved.
 // Licensed under the MIT License. See LICENSE in the project root for license information.
 
+using Microsoft.AspNetCore.Authentication.Cookies;
 using Microsoft.AspNetCore.DataProtection;
+using Microsoft.Extensions.Options;
 
 namespace SessionMigration;