Bug of Users Authentication Ticket Store Feature

[hyzx86]

oc version ：preview-latest

Describe the bug

After enabling the Users Authentication Ticket Store feature,
whenever any other feature is enabled. The user is always redirected to the login page.

[hyzx86]

It looks like here is always null

[hyzx86]

I Debug the code and see that the following method is called after ShellScope.UsingAsync

OrchardCore.Users.Authentication.CacheTicketStore.RetrieveAsync

OrchardCore/src/OrchardCore/OrchardCore.Abstractions/Shell/Scope/ShellScope.cs

Lines 224 to 261 in edccca0

	public async Task UsingAsync(Func<ShellScope, Task> execute, bool activateShell = true)
	{
	if (Current == this)
	{
	await execute(Current);
	return;
	}
	using (this)
	{
	StartAsyncFlow();
	try
	{
	try
	{
	if (activateShell)
	{
	await ActivateShellInternalAsync();
	}
	await execute(this);
	}
	finally
	{
	await TerminateShellInternalAsync();
	}
	}
	catch (Exception e)
	{
	await HandleExceptionAsync(e);
	throw;
	}
	finally
	{
	await BeforeDisposeAsync();
	}
	}
	}

OrchardCore/src/OrchardCore/OrchardCore.Users.Core/Authentication/CacheTicketStore.cs

Lines 57 to 77 in edccca0

	{
	var cacheKey = $"{_keyPrefix}-{key}";
	var cache = _httpContextAccessor.HttpContext.RequestServices.GetService<IDistributedCache>();
	var bytes = await cache.GetAsync(cacheKey);
	if (bytes == null || bytes.Length == 0)
	{
	return null;
	}
	try
	{
	var ticket = DeserializeTicket(DataProtector.Unprotect(bytes));
	return ticket;
	}
	catch (Exception e)
	{
	// Data Protection Error
	Logger.LogError(e, "{methodName} failed for '{key}'.", nameof(RetrieveAsync), cacheKey);
	return null;
	}
	}

[hishamco]

Into your plate @ns8482e :)

[ns8482e]

Correct it's expected behavior.
Bydefault the authentication ticket is stored in cookies. After enabling the Users Authentication Ticket Store feature, the authentication ticket is stored on server in IMemoryCache.

IMemoryCache is destroyed and recreated when tenant is reloaded.

To keep authentication ticket between tenant reload you should also enable and configure redis cache feature.

[hyzx86]

So, should we save the Token to the database without Redis enabled?

At the moment, it seems that this problem will only arise on the site Settings changed

[ns8482e]

Yes you should not rely on IMemoryCache in production if your frequently update the features. In production You should use redid cache

[sebastienros]

We could also have an implementation that is based on SQL. @hyzx86 you can file an issue, and maybe provide a PR for this?

For this current issue maybe we should display a notification/warning when no other specific ticket store is enabled.

[hyzx86]

@sebastienros This is not a very urgent issue in my scenario right now; After all, it's reasonable to tell users that a configuration change caused them to log back in 😁

I actually feel like this part of the code is a little hard for me to change
Maybe we can add some documentation and close it, or someone else can fix the problem

[hyzx86]

We could also have an implementation that is based on SQL

Well, if we start with that, it's really not difficult. I can try it

[hyzx86]

To achieve this, should I re-implement a SqlDistributedCache? It's like

https://learn.microsoft.com/en-us/aspnet/core/performance/caching/memory?view=aspnetcore-6.0

This seems a bit complicated, but for distributed caching, shouldn't we make it a higher priority, such as enabling it only from the default site, and then separate the distributed caching object from the specific tenant?

Just like DatabaseShellsConfiguration

[sebastienros]

Correct, you could create a feature that would register SqlDistributedCache as the IDistributedCache implementation. That would be easier than setting up a Redis instance for lots of deployments since SQL is already available for OC. And a migration would create the required tables.

[Piedone]

#16149 covers SQL Distributed Cache, so we don't need to do anything specific for this module, and rather work on that.