Authorization : AddAuthorization() method do not use the configureOptions parameter

[ycrumeyrolle]

https://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNet.Authorization/ServiceCollectionExtensions.cs#L22

[kevinchalet]

This overload shouldn't even exist, actually 😄

[HaoK]

Why shouldn't this overload exist? Its just sugar to add/configure in one line (assuming it was implemented :)

[kevinchalet]

Because it's actually the only Add method in aspnet/Security that accepts a delegate to configure the options 😄

This pattern is fine, but it should be used everywhere or nowhere, for consistency 😄

[HaoK]

If you are talking about AddAuthentication(), that's because its fake, and is only adding other things like DataProtection. All of the other AddXyz follow this pattern. The difference its there is no master AuthenticationOptions to configure.

I'm probably going to prototype nuking using IOptions from all of the auth middlewares soon, and having them go back to taking concrete instances in UseGoogle etc...

[kevinchalet]

I guess it could/should be updated to configure SharedAuthenticationOptions, which must currently be configured with the hardly-discoverable generic services.Configure() 😄

There are a few places where Add doesn't take a delegate: https://github.com/aspnet/Session/blob/dev/src/Microsoft.AspNet.Session/SessionServiceCollectionExtensions.cs

https://github.com/aspnet/Antiforgery/blob/dev/src/Microsoft.AspNet.Antiforgery/ServiceCollectionExtensions.cs

https://github.com/aspnet/CORS/blob/dev/src/Microsoft.AspNet.Cors.Core/CorsServiceCollectionExtensions.cs

[kevinchalet]

I'm probably going to prototype nuking using IOptions from all of the auth middlewares soon, and having them go back to taking concrete instances in UseGoogle etc...

Does that mean we won't be able to use Configure*Authentication anymore?

[HaoK]

Yeah those are probably bugs in those repos, I made a pass through to fix these once

Yeah AuthOptions will basically no longer go in DI, and only can be configured via UseXyz.

[kevinchalet]

I protest then, because it's a great way to configure per-environment options: https://gist.github.com/PinpointTownes/c2096cd2cbf812f8d500

Le 20 juil. 2015 à 23:30, Hao Kung notifications@github.com a écrit :

Yeah those are probably bugs in those repos, I made a pass through to fix these once

Yeah AuthOptions will basically no longer go in DI, and only can be configured via UseXyz.

—
Reply to this email directly or view it on GitHub.

[HaoK]

I don't think that argument holds much water, since you can just switch to using the ConfigureXyz methods instead and still have something similar

[HaoK]

Both ConfigureXyzServices and ConfigureXyz can be per environment today

[ycrumeyrolle]

@HaoK I asked the question around the differents ways to "configure" the Options on stackoverflow:
http://stackoverflow.com/questions/31485311/configure-a-component-in-asp-net-5
If you have the answer of what should be done and what should not, it would be great!

[kevinchalet]

Both ConfigureXyzServices and ConfigureXyz can be per environment today

Using IHostingEnvironment.EnvironmentName? It seems less handy than the Configure*Services approach.

I'm probably going to prototype nuking using IOptions from all of the auth middlewares soon, and having them go back to taking concrete instances in UseGoogle etc...

You would remove it from the security middleware, but not from the other ones?

[HaoK]

Yes, the motivation is mostly around nuking named options. Security middleware (unlike frameworks) doesn't lend itself to the singleton style of options that frameworks use. Many of the other middlewares today aren't actually using IOptions for their options anyways, so this would make everything consistent, and clean up options as well by nuking the whole named complexity that I had to create just to get Security/Identity/Cookies all playing well together...

[kevinchalet]

Understood 😄

Related question: do you have plans for multi-tenant scenarios? (#35). IOptions<> seemed to be a nice first step in that direction.

[HaoK]

For once I'll reply with a @blowdart -ism: no plans

[CoskunSunali]

@HaoK, that is sad.

[HaoK]

3294de1