Added Identity Token to TokenRefreshEventArgs #367
Conversation
This change allows updating the identity token cached at the client with the latest identity token coming from token renewal
|
Thanks will have a look after holidays. |
Thank you for taking this into consideration @leastprivilege Enjoy your holidays ✌️ |
|
Hey @leastprivilege, this is just a friendly reminder for you to check this PR. I wanted to avoid having to fork the repo for such a small tweak. Thank you in advance for your consideration! |
| @@ -17,10 +17,11 @@ public class TokenRefreshedEventArgs : EventArgs | |||
| /// <param name="accessToken">The access token.</param> | |||
| /// <param name="refreshToken">The refresh token.</param> | |||
| /// <param name="expiresIn">The expires in.</param> | |||
| public TokenRefreshedEventArgs(string accessToken, string refreshToken, int expiresIn) | |||
| public TokenRefreshedEventArgs(string accessToken, string refreshToken, string identityToken, int expiresIn) | |||
There was a problem hiding this comment.
The problem is that this is introducing a breaking change because you changed the signature.
I would prefer to move identityToken to the end of the param list and make it optional.
There was a problem hiding this comment.
Understood. New changes pushed
fmgracias
requested review from
leastprivilege
and removed request for
brockallen
|
Didn't mean to remove @brockallen from the reviewers. Not sure how that happened and I don't have permission to add him back |
|
@leastprivilege thanks for the review. |
This change allows updating the identity token cached at the client with the latest identity token coming from token renewal.
The motivation for this change is that claims can change during the runtime of the identity client (in my case, it's user claims related to permissions).
The alternative approach would be to call
/userinfoendpoint, which we consider an avoidable overhead.