Added AuthenticateResultCache type

src/Duende.AccessTokenManagement.OpenIdConnect/AuthenticateResultCache.cs

@@ -0,0 +1,14 @@
+// Copyright (c) Brock Allen & Dominick Baier. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
+
+using Microsoft.AspNetCore.Authentication;
+using System.Collections.Generic;
+
+/// <summary>
+/// Per-request cache so that if SignInAsync is used, we won't re-read the old/cached AuthenticateResult from the handler.
+/// This requires this service to be added as scoped to the DI system.
+/// Be VERY CAREFUL to not accidentally capture this service for longer than the appropriate DI scope - e.g., in an HttpClient.
+/// </summary>
+internal class AuthenticateResultCache: Dictionary<string, AuthenticateResult>
+{
+}

src/Duende.AccessTokenManagement.OpenIdConnect/AuthenticationSessionUserTokenStore.cs

@@ -4,10 +4,10 @@
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Http;
 using System;
-using System.Collections.Generic;
 using System.Security.Claims;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.DependencyInjection;
 
 namespace Duende.AccessTokenManagement.OpenIdConnect
 {
@@ -42,15 +42,14 @@ public async Task<UserToken> GetTokenAsync(
             UserTokenRequestParameters? parameters = null)
         {
             parameters ??= new();
-            var cache = _contextAccessor.HttpContext?.RequestServices.GetService(typeof(Dictionary<string, AuthenticateResult>)) as Dictionary<string, AuthenticateResult>;
-            if (cache == null)
-            {
-                throw new InvalidOperationException("Failed to resolve authenticate result cache");
-            }
-
+            // Resolve the cache here because it needs to have a per-request
+            // lifetime. Sometimes the store itself is captured for longer than
+            // that inside an HttpClient.
+            var cache = _contextAccessor.HttpContext?.RequestServices.GetRequiredService<AuthenticateResultCache>();
+
             // check the cache in case the cookie was re-issued via StoreTokenAsync
             // we use String.Empty as the key for a null SignInScheme
-            if (!cache.TryGetValue(parameters.SignInScheme ?? String.Empty, out var result))
+            if (!cache!.TryGetValue(parameters.SignInScheme ?? String.Empty, out var result))
             {
                 result = await _contextAccessor!.HttpContext!.AuthenticateAsync(parameters.SignInScheme).ConfigureAwait(false);
             }
@@ -88,22 +87,21 @@ public async Task StoreTokenAsync(
         {
             parameters ??= new();
 
-
-            var cache = _contextAccessor.HttpContext?.RequestServices.GetService(typeof(Dictionary<string, AuthenticateResult>)) as Dictionary<string, AuthenticateResult>;
-            if (cache == null)
-            {
-                throw new InvalidOperationException("Failed to resolve authenticate result cache");
-            }
+            // Resolve the cache here because it needs to have a per-request
+            // lifetime. Sometimes the store itself is captured for longer than
+            // that inside an HttpClient.
+            var cache = _contextAccessor.HttpContext?.RequestServices.GetRequiredService<AuthenticateResultCache>();
 
             // check the cache in case the cookie was re-issued via StoreTokenAsync
             // we use String.Empty as the key for a null SignInScheme
-            if (!cache.TryGetValue(parameters.SignInScheme ?? String.Empty, out var result))
+            if (!cache!.TryGetValue(parameters.SignInScheme ?? String.Empty, out var result))
             {
                 result = await _contextAccessor.HttpContext!.AuthenticateAsync(parameters.SignInScheme)!.ConfigureAwait(false);
             }
 
             if (result is not { Succeeded: true })
             {
+                // TODO - Test coverage of this case
                 throw new Exception("Can't store tokens. User is anonymous");
             }
 
@@ -140,4 +138,4 @@ protected virtual Task<ClaimsPrincipal> FilterPrincipalAsync(ClaimsPrincipal pri
             return Task.FromResult(principal);
         }
     }
-}
+}

src/Duende.AccessTokenManagement.OpenIdConnect/OpenIdConnectTokenManagementServiceCollectionExtensions.cs

@@ -51,7 +51,7 @@ public static IServiceCollection AddOpenIdConnectAccessTokenManagement(this ISer
         services.TryAddScoped<IUserTokenStore, AuthenticationSessionUserAccessTokenStore>();
 
         // scoped since it will be caching per-request authentication results
-        services.TryAddScoped<Dictionary<string, AuthenticateResult>>();
+        services.AddScoped<AuthenticateResultCache>();
 
         return services;
     }

test/Tests/UserTokenManagementTests.cs

@@ -400,7 +400,6 @@ public async Task Refresh_responses_without_refresh_token_use_old_refresh_token(
         token.RefreshToken.ShouldBe("initial_refresh_token");
     }
 
-
     [Fact]
     public async Task Multiple_users_have_distinct_tokens_across_refreshes()
     {