-
Notifications
You must be signed in to change notification settings - Fork 251
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
rename AccessToken to Secret and expand it's usage #1472
Conversation
This does 3 things: 1. Renames `AccessToken` to `Secret` 2. Prevents `Debug` of the `AccessToken` from actually showing the secret 3. Starts expanding the use of `Secret` to other areas, such as client certificates
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'm open to this. Is there precedence in any other Azure SDKs or libraries? .NET uses Azure.Core.AccessToken.
https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/core/Azure.Core/src/AccessToken.cs
|
||
impl Debug for Secret { | ||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
f.debug_tuple("Secret").field(&"<REDACTED>").finish() |
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.
Nice
I don't know about wrapping secrets in a fashion like we've done. As it relates to your link, the AccessToken implementation used in DotNet is what we call TokenResponse, as it includes both the token and an expiration date. It's my intention to rename TokenResponse to AccessToken once this PR lands, as that should impact different places. |
This does 3 things:
AccessToken
toSecret
Debug
of theAccessToken
from actually showing the secretSecret
to other areas, such as client certificates