Skip to content
This repository has been archived by the owner on Nov 7, 2018. It is now read-only.

IConfigureOptions isn't called for authentication (named) options #225

Closed
khellang opened this issue Aug 17, 2017 · 7 comments
Closed

IConfigureOptions isn't called for authentication (named) options #225

khellang opened this issue Aug 17, 2017 · 7 comments
Milestone

Comments

@khellang
Copy link

khellang commented Aug 17, 2017

After a Twitter discussion with @mwadams, we discovered this weird thing...

The OptionsFactory.Create method seems to only call registered IConfigureOptions<T> when name == Options.DefaultName:

public TOptions Create(string name)
{
var options = new TOptions();
foreach (var setup in _setups)
{
if (setup is IConfigureNamedOptions<TOptions> namedSetup)
{
namedSetup.Configure(name, options);
}
else if (name == Options.DefaultName)
{
setup.Configure(options);
}
}
foreach (var post in _postConfigures)
{
post.PostConfigure(name, options);
}
return options;
}

This results in IConfigureOptions<T> not being called when options are requested from authentication handlers (or any other named options):

https://github.com/aspnet/Security/blob/488eb44467eb677eab62bdc49aa6255cc1be3119/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs#L89

Why is this? Was this intended or a regression? It seems like a weird, artificial restriction to have. Not to mention the inconsistently configured named/default options you'd have around.

A workaround is to just use the exact same class, but implement IPostConfigureOptions<T> instead.

// @HaoK (from commit 52ab5f1)

@khellang khellang changed the title IConfigureOptions for authentication options isn't called IConfigureOptions isn't called for authentication (named) options Aug 17, 2017
@mwadams
Copy link

mwadams commented Aug 17, 2017

The 1.1 behaviour was for this to be called for all handlers.

@khellang
Copy link
Author

khellang commented Aug 17, 2017

The 1.1 behaviour was for this to be called for all handlers.

Yes, judging by commit 52ab5f1, it was introduced in 2.0.0-preview2.

@HaoK
Copy link
Member

HaoK commented Aug 18, 2017

Yes Auth 2.0 uses named options, IConfigureOptions are equivalent to IConfigureNamed options with name = Options.DefaultName

@HaoK HaoK closed this as completed Aug 18, 2017
@HaoK HaoK reopened this Aug 18, 2017
@HaoK
Copy link
Member

HaoK commented Aug 18, 2017

IPostConfigureOptions works because it supports names since its new in 2.0, we wanted to preserve existing behavior for existing users of IOptions/IConfigureOptions so we left those alone and added IConfigureNamedOptions.

@HaoK
Copy link
Member

HaoK commented Aug 18, 2017

You need to switch to IConfigureNamedOptions to be able to target the correct authentication scheme in 2.0, this breaking change was intentional

@HaoK HaoK added this to the Discussions milestone Aug 18, 2017
@HaoK
Copy link
Member

HaoK commented Aug 18, 2017

I added this to the known issues/breaking changes section at the bottom of the Auth 2.0 announcement: aspnet/Announcements#262

@khellang
Copy link
Author

Alright. I think we're done here 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants