-
Notifications
You must be signed in to change notification settings - Fork 191
Consolidate auth APIs #282
Conversation
Update to include #273 - POCO context objects. |
HttpResponseFeature.StatusCode = 401; | ||
var handler = HttpAuthenticationFeature.Handler; | ||
|
||
var challengeContext = new ChallengeContext(authenticationScheme, properties == null ? null : properties.Items); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
properties?.Items
Updated and rebased. |
Updated with related changes for WebSockets. #267 |
{ | ||
get | ||
{ | ||
var webSocketFeature = WebSocketFeature; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use the property directly? Do we need to assign it to a variable?
Code changes look good to me and reflect the discussions from the API review. @davidfowl does new API surface look good to you? |
Challenge(properties: null, authenticationScheme: authenticationScheme); | ||
} | ||
|
||
public abstract void Challenge(AuthenticationProperties properties, string authenticationScheme); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with Brent's issue, we should probably switch scheme to be the first parameter here to be consistent with sign in/signout
#266 - Consolidate all the auth APIs to HttpContext.Authentication
#270 - Naming
@davidfowl @muratg
/cc @HaoK