Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Issue-15709: Fix NullReferenceException in CacheTicketStore.RenewAsync method #15711

Merged
merged 3 commits into from
Apr 11, 2024
Merged

Fix Issue-15709: Fix NullReferenceException in CacheTicketStore.RenewAsync method #15711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fc3ce7f
Issue-15709: Fix CacheTicketStore.RenewAsync method. (throwing NullRe…
M-Lipin Apr 9, 2024
1ac1c34
Issue-15709: Refactoring.
M-Lipin Apr 11, 2024
1cb784b
Issue-15709: Disable Possible null reference return warning.
M-Lipin Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/OrchardCore.Modules/OrchardCore.Users/CacheTicketStoreStartup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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);
}
}

Expand Down
53 changes: 30 additions & 23 deletions src/OrchardCore/OrchardCore.Users.Core/Authentication/CacheTicketStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -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;
}
}
Expand All @@ -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);
}
Loading