Fix Issue-15709: Fix NullReferenceException in CacheTicketStore.Renew…

…Async method (#15711)
OrchardCMS · Apr 11, 2024 · 70cc055 · 70cc055
1 parent 112c0d4
commit 70cc055
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 28 deletions.
diff --git a/src/OrchardCore.Modules/OrchardCore.Users/CacheTicketStoreStartup.cs b/src/OrchardCore.Modules/OrchardCore.Users/CacheTicketStoreStartup.cs
@@ -1,6 +1,8 @@
 using Microsoft.AspNetCore.Authentication.Cookies;
-using Microsoft.AspNetCore.Http;
+using Microsoft.Extensions.Caching.Distributed;
+using Microsoft.AspNetCore.DataProtection;
 using Microsoft.AspNetCore.Identity;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Options;
 using OrchardCore.Modules;
@@ -18,18 +20,25 @@ public override void ConfigureServices(IServiceCollection services)
 
     public class CookieAuthenticationOptionsConfigure : IConfigureNamedOptions<CookieAuthenticationOptions>
     {
-        private readonly IHttpContextAccessor _httpContextAccessor;
+        private readonly IDistributedCache _distributedCache;
+        private readonly IDataProtectionProvider _dataProtectionProvider;
+        private readonly ILogger<CacheTicketStore> _logger;
 
-        public CookieAuthenticationOptionsConfigure(IHttpContextAccessor httpContextAccessor)
+        public CookieAuthenticationOptionsConfigure(
+            IDistributedCache distributedCache,
+            IDataProtectionProvider dataProtectionProvider,
+            ILogger<CacheTicketStore> logger)
         {
-            _httpContextAccessor = httpContextAccessor;
+            _distributedCache = distributedCache;
+            _dataProtectionProvider = dataProtectionProvider;
+            _logger = logger;
         }
 
         public void Configure(string name, CookieAuthenticationOptions options)
         {
             if (name == IdentityConstants.ApplicationScheme)
             {
-                options.SessionStore = new CacheTicketStore(_httpContextAccessor);
+                options.SessionStore = new CacheTicketStore(_distributedCache, _dataProtectionProvider, _logger);
             }
         }
 

diff --git a/src/OrchardCore/OrchardCore.Users.Core/Authentication/CacheTicketStore.cs b/src/OrchardCore/OrchardCore.Users.Core/Authentication/CacheTicketStore.cs
@@ -1,12 +1,12 @@
+#nullable enable
+
 using System;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authentication.Cookies;
 using Microsoft.AspNetCore.DataProtection;
-using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Identity;
 using Microsoft.Extensions.Caching.Distributed;
-using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 
 namespace OrchardCore.Users.Authentication;
@@ -15,49 +15,53 @@ public class CacheTicketStore : ITicketStore
 {
     private const string KeyPrefix = "ocauth-ticket";
 
-    private readonly IHttpContextAccessor _httpContextAccessor;
-    private IDataProtector _dataProtector;
-    private ILogger _logger;
+    private IDataProtector? _dataProtector;
+    private readonly ILogger _logger;
+    private readonly IDistributedCache _distributedCache;
+    private readonly IDataProtectionProvider _dataProtectionProvider;
 
-    public CacheTicketStore(IHttpContextAccessor httpContextAccessor)
+    public CacheTicketStore(
+        IDistributedCache distributedCache,
+        IDataProtectionProvider dataProtectionProvider,
+        ILogger<CacheTicketStore> logger
+    )
     {
-        _httpContextAccessor = httpContextAccessor;
+        _distributedCache = distributedCache;
+        _dataProtectionProvider = dataProtectionProvider;
+        _logger = logger;
     }
 
-    public IDataProtector DataProtector => _dataProtector ??= _httpContextAccessor.HttpContext.RequestServices.GetService<IDataProtectionProvider>()
+    private IDataProtector DataProtector => _dataProtector ??= _dataProtectionProvider
         .CreateProtector($"{nameof(CacheTicketStore)}_{IdentityConstants.ApplicationScheme}");
 
-    public ILogger Logger => _logger ??= _httpContextAccessor.HttpContext.RequestServices.GetService<ILogger<CacheTicketStore>>();
-
     public async Task RemoveAsync(string key)
     {
         var cacheKey = $"{KeyPrefix}-{key}";
-        var cache = _httpContextAccessor.HttpContext.RequestServices.GetService<IDistributedCache>();
-        await cache.RemoveAsync(cacheKey);
+        await _distributedCache.RemoveAsync(cacheKey);
     }
 
     public async Task RenewAsync(string key, AuthenticationTicket ticket)
     {
         var cacheKey = $"{KeyPrefix}-{key}";
-        var cache = _httpContextAccessor.HttpContext.RequestServices.GetService<IDistributedCache>();
 
         try
         {
             var protectedBytes = DataProtector.Protect(SerializeTicket(ticket));
-            await cache.SetAsync(cacheKey, protectedBytes, new DistributedCacheEntryOptions() { AbsoluteExpiration = ticket.Properties.ExpiresUtc.Value });
+            await _distributedCache.SetAsync(cacheKey, protectedBytes,
+                new DistributedCacheEntryOptions() { AbsoluteExpiration = ticket.Properties.ExpiresUtc });
         }
         catch (Exception e)
         {
             // Data Protection Error
-            Logger.LogError(e, "{methodName} failed  for '{key}'.", nameof(RenewAsync), cacheKey);
+            _logger.LogError(e, "{methodName} failed  for '{key}'.", nameof(RenewAsync), cacheKey);
         }
     }
 
-    public async Task<AuthenticationTicket> RetrieveAsync(string key)
+    public async Task<AuthenticationTicket?> RetrieveAsync(string key)
     {
         var cacheKey = $"{KeyPrefix}-{key}";
-        var cache = _httpContextAccessor.HttpContext.RequestServices.GetService<IDistributedCache>();
-        var bytes = await cache.GetAsync(cacheKey);
+
+        var bytes = await _distributedCache.GetAsync(cacheKey);
         if (bytes == null || bytes.Length == 0)
         {
             return null;
@@ -71,7 +75,7 @@ public async Task<AuthenticationTicket> RetrieveAsync(string key)
         catch (Exception e)
         {
             // Data Protection Error
-            Logger.LogError(e, "{methodName} failed  for '{key}'.", nameof(RetrieveAsync), cacheKey);
+            _logger.LogError(e, "{methodName} failed  for '{key}'.", nameof(RetrieveAsync), cacheKey);
             return null;
         }
     }
@@ -80,24 +84,27 @@ public async Task<string> StoreAsync(AuthenticationTicket ticket)
     {
         var key = Guid.NewGuid().ToString();
         var cacheKey = $"{KeyPrefix}-{key}";
-        var cache = _httpContextAccessor.HttpContext.RequestServices.GetService<IDistributedCache>();
 
         try
         {
             var protectedBytes = DataProtector.Protect(SerializeTicket(ticket));
-            await cache.SetAsync(cacheKey, protectedBytes, new DistributedCacheEntryOptions() { AbsoluteExpiration = ticket.Properties.ExpiresUtc.Value });
+            await _distributedCache.SetAsync(cacheKey, protectedBytes,
+                new DistributedCacheEntryOptions() { AbsoluteExpiration = ticket.Properties.ExpiresUtc });
+
             return key;
         }
         catch (Exception e)
         {
-            Logger.LogError(e, "{methodName} failed  for '{key}'.", nameof(StoreAsync), cacheKey);
+            _logger.LogError(e, "{methodName} failed  for '{key}'.", nameof(StoreAsync), cacheKey);
+#pragma warning disable CS8603 // Possible null reference return.
             return null;
+#pragma warning restore CS8603 // Possible null reference return.
         }
     }
 
     private static byte[] SerializeTicket(AuthenticationTicket source)
         => TicketSerializer.Default.Serialize(source);
 
-    private static AuthenticationTicket DeserializeTicket(byte[] source)
+    private static AuthenticationTicket? DeserializeTicket(byte[]? source)
         => source == null ? null : TicketSerializer.Default.Deserialize(source);
 }