Skip to content

Two different AuthenticationProperties types #2681

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.

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

Closed
aspnet-hello opened this issue Jan 2, 2018 · 9 comments
Closed

Two different AuthenticationProperties types #2681

aspnet-hello opened this issue Jan 2, 2018 · 9 comments
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-http-abstractions

Comments

@aspnet-hello
Copy link

From @khellang on Monday, November 13, 2017 8:26:26 AM

Why is there two different AuthenticationProperties types in this repository?

Why doesn't Microsoft.AspNetCore.Authentication.Abstractions use the one from Microsoft.AspNetCore.Http.Abstractions?

Copied from original issue: aspnet/HttpAbstractions#966

@aspnet-hello
Copy link
Author

From @davidfowl on Monday, November 13, 2017 9:07:01 AM

My guess is that the type in Microsoft.AspNetCore.Http.Authentication is for the 1.0 auth stack.

@HaoK ?

@aspnet-hello
Copy link
Author

From @Tratcher on Monday, November 13, 2017 9:11:32 AM

Everything in Http.Authentication was depreciated and/or moved in 2.0.

@aspnet-hello
Copy link
Author

From @khellang on Monday, November 13, 2017 9:44:25 AM

Everything in Http.Authentication was depreciated and/or moved in 2.0.

Except they're not. Only AuthenticationManager is deprecated.

@aspnet-hello
Copy link
Author

From @davidfowl on Monday, November 13, 2017 9:49:49 AM

@khellang Only AuthenticationManager has the Obsolete attribute. Those types are considered deprecated. I'm not sure why we didn't mark them as such.

@aspnet-hello
Copy link
Author

From @khellang on Monday, November 13, 2017 10:03:38 AM

Those types are considered deprecated. I'm not sure why we didn't mark them as such.

Right, but unfortunately, there's no tooling (yet?) that can read the ASP.NET team's minds 😉

It's kinda confusing when you end up importing the wrong namespace/type and there's nothing telling you that it's obsolete 😝

@aspnet-hello
Copy link
Author

From @HaoK on Monday, November 13, 2017 11:31:50 AM

We settled on obsoleting all the main entry points: (aspnet/HttpAbstractions#863 (comment)).

So there should be no way to actually use the old AuthenticationProperties, but yeah there would be no warning if you were only using the properties type. But you would get a compile warning/error as soon as you tried to actually do anything with it.

@aspnet-hello
Copy link
Author

From @poke on Tuesday, November 14, 2017 4:12:18 AM

I originally pointed this out on chat; thanks for opening an issue about it @khellang.

The reason why I encountered this is that while migrating to 2.0, I still had a using on Microsoft.AspNetCore.Http.Authentication inside my code. So when I created AuthenticationProperties somewhere, I got an error about the ambiguous type which I was not able to resolve at that time without actually knowing which type is used and which isn’t (I only was able to do so later when that object was passed to SignOutAsync).

If we still want to keep the obsolete AuthenticationManager around for now, maybe we could at least change the AuthenticationProperties type it uses to the new type, so that we simply no longer have two identical types in the framework?

@muratg
Copy link
Contributor

muratg commented Jan 31, 2018

@poke not sure if you're still following this bug, but that also would be a breaking change, so we have no plans as such. Closing this bug.

@muratg muratg closed this as completed Jan 31, 2018
@poke
Copy link
Contributor

poke commented Jan 31, 2018

Yeah, I’m still following this. Too bad that we cannot do something about this because when I was upgrading from 1.0 to this, the type existing twice was the thing that broke it for me :/

But I understand your reasoning. Hope we can get rid of the old stuff soon. – Anyway, thanks for the reply!

@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-http-abstractions
Projects
None yet
Development

No branches or pull requests

5 participants