Enable using a Cookie Session Store from DI

[fredrikhr]

A CookieAuthenticationHandler will use a ITicketStore from DI if and only if the following conditions are met:

1. The UseSessionStoreFromDI property of the handler's CookieAuthenticationOptions is true.
2. The DI container has a registered service for the ITicketStore interface, and the returned instance is non-null.

Since UseSessionStoreFromDI is a new property and new behaviour only is applied when its value is true, these changes remain fully backwards compatible.

Code Changes:

• Adds a new boolean property UseSessionStoreFromDI to Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationOptions.
• Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationHandler requests an ITicketStore in its constructor with an optional argument.
  Because the argument is optional, DI can provide the argument as null if no ITicketStore service is registered. This will ensure backwards-compatability.
• In every case where CookieAuthenticationHandler previously got the ITicketStore from its Options.SessionStore, it now first checks the new Options.UseSessionStoreFromDI property and uses the non-null DI-injected ITicketStore instance if UseSessionStoreFromDI is true.

fixes #581

[Tratcher]

This is going to need tests.

[Tratcher]

No API signature changes are allowed in 2.1. You'll have to add a new constructor.

[Tratcher]

@pakrym does DI support optional parameters like this? We don't usually use them. We tend to use IEnumerable<T> for optional services.

[pakrym]

It does, usually, we use multiple constructors to achieve optional args. Usage of IEnumerable optional services is generally discouraged.

[pakrym]

If this ctor gets injected into cache would never be null.

[pakrym]

Did we ever put ITicketStore to DI? Can't we just enable the feature without the flag? If options.Store == null resolve from DI

[Tratcher]

We never did put it in DI though users might have. There's also the issue of having multiple cookie middleware, they don't all necessarily want the same store. The option may still be redundant though, it may be sufficient to prefer the instance on Options over the instance from DI.
Edit: Wait, that doesn't provide for an opt-out. We still need the option.

[HaoK]

Do we really need this functionality at all? You can already use a custom IConfigureNamedOptions that gets ITicketStore from DI and use that to set the instance...

 public class CookieConfig : IConfigureNamedOptions<CookieAuthenticationOptions> { public CookieConfig(ITicketStore store) => _store = store; public void Configure(string name, CookieAuthenticationOptions o) { o.TicketStore = store; } } services.AddTransient<IConfigureOptions<CookieAuthenticationOptions>, CookieConfig>();

[HaoK]

That seems good enough to me

[Tratcher]

Defining your own class and registering that in DI is more work and less discoverable than just registering the ITokenStore in DI. Though if you also have to set an option then the discoverability suffers there too.

[Tratcher]

File a doc bug to add that example to https://docs.microsoft.com/en-us/aspnet/core/security/authentication/cookie?tabs=aspnetcore2x ?

[HaoK]

That's just the general pattern of how to get things from DI onto options, its not particularly specific to cookies, @scottaddie @Rick-Anderson I think @Tratcher has a good point that we should have an explicit section recommending this pattern of how to configure options instances with things from DI in this doc maybe? https://docs.microsoft.com/en-us/aspnet/core/fundamentals/configuration/options

public class UseSomethingFromDI : IConfigureNamedOptions/IConfigureOptions<YourOptions> { public CookieConfig(ISomethingFromDI thing) => _thing = thing; public void Configure([string name,] YourOptions o) { o.Thing = thing; } } services.AddTransient<IConfigureOptions<YourOptions>, UseSomethingFromDI>();

[HaoK]

Also Its certainly more work in the long run if we adopted something like this everywhere (useXyzFromDI flags on all services?), instead of just always using the options instance, and relying on configuring the options instance appropriately.

[Rick-Anderson]

dotnet/AspNetCore.Docs#5922 Add doc section - Enable using a Cookie Session Store from DI

[kevinchalet]

Instead of a UseSessionStoreFromDI option, why not SessionStoreType, modeled after EventsType? This way, you could register multiple stores in the DI and each handler would pick the one set in the options.

[HaoK]

I still say we should do nothing special here, since this already can be done via options already

[HaoK]

EventsType was added before named options was fully baked, I don't think we'd have added that either since you can accomplish that via a custom named IConfigureOptions as well

[HaoK]

So this is actually even easier in 2.1:

services.AddOptions<CookieAuthenticationOptions>(CookieAuthenticationDefaults.AuthenticationScheme).Configure<ITicketStore>(o,t => o.TicketStore = t);

[kevinchalet]

EventsType was added before named options was fully baked, I don't think we'd have added that either since you can accomplish that via a custom named IConfigureOptions as well

No, it was added because there was a need for scoped "services as options", that you can't do with options, where everything is a singleton.

[HaoK]

Ah right events are recreated per request, but that's not what ticket store needs here

[kevinchalet]

If the ticket stored uses EF Core, it needs to be scoped, since DbContext itself must be scoped.

[HaoK]

You probably can make a particular options type transient for an IOptionsMonitor by plugging in a no-op cache:

https://github.com/aspnet/Options/blob/dev/src/Microsoft.Extensions.Options/IOptionsMonitorCache.cs

services.AddSingleton<IOptionsMonitorCache<CookieAuthenticationOptions>, AlwaysCallFactoryCache>(); 

[kevinchalet]

You probably can make a particular options type scoped for an IOptionsMonitor by plugging in a no-op cache:

Heh, that means the entire options and all the "services" attached are re-created per request, including the IDataFormat and the underlying data protector. Rather inefficient.

The real question is: is using scoped ticket stores common enough to introduce DI support?

[Eilon]

Closing because we are not planning to add direct support for this feature.

To work around this, you can use the service located pattern to get the required services imperatively instead of through typical DI.