-
Notifications
You must be signed in to change notification settings - Fork 864
Add OnSecurityStampReplacePrincipal event #977
Conversation
This is why there should be a security stamp options (maybe in 2.0.0 when you can add a breaking change). |
@Tratcher mind reviewing as well since you are the only other one with some context :) |
After a quick glance, it looks ok to me (FWIW) |
What's the other event where the user can copy claims from the external cookie to the identity cookie? |
CreatingTicket I believe is the other OAuth event that can be hooked |
? Wasn't there an event at the identity layer? |
Nope, shuttling claims around was always done directly via cookie events. We added a method which took care of some of the token refreshing, but identity doesn't expose any cookie related events, it just exposes the cookie options directly (until this PR anyways...) |
/// <summary> | ||
/// The principal contained in the current cookie. | ||
/// </summary> | ||
public ClaimsPrincipal Current { get; set; } |
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.
CurrentPrincipal and NewPrincipal?
/// <summary> | ||
/// Invoked when the default security stamp validator replaces the user's ClaimsPrincipal in the cookie. | ||
/// </summary> | ||
public Func<SecurityStampReplacingPrincipalContext, Task> OnSecurityStampReplacingPrincipal { get; set; } |
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.
Refreshing?
Sure, updated to suggested names, New/Current Principal + Replacing => Refreshing |
Fixes #958
Adds a new event called from the default security stamp validator which allows tweaking the user principal regeneration using the old principal.
Not super happy with the name (or the fact that the event lives at the top level of IdentityOptions)
cc @divega @blowdart