chore(credential): support Windows Credential Manager to store OAuth tokens and API keys - #344
Conversation
…tokens and API keys
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 407a7d6f0c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| Ok(format!("{KEYCHAIN_SERVICE}/{profile_name}") | ||
| .encode_utf16() | ||
| .chain(std::iter::once(0)) | ||
| .collect()) |
There was a problem hiding this comment.
On Windows, Credential Manager matches generic credential target names case-insensitively, while AuthStore and profile lookup treat names as case-sensitive. Creating profiles such as work and Work therefore stores two metadata records but maps both to the same credential: the second login overwrites the first, and deleting or renaming either profile can delete the other's secret. Selecting one profile may then authenticate with the wrong account or fail, so the target must preserve case-sensitive identity or case-only duplicate profile names must be rejected.
Useful? React with 👍 / 👎.
|
Latest downloadable build artifacts for this PR commit
Available artifact names
|
1 participant
The bt used in the screenshot was cedric/windows-credential-manager-support rebased on top of cedric/clarify-credential-metadata-vs-secret-storage.
As shown in the screenshot, the secret storage used is not correctly reported. It's normal since in #341, Windows Credential Manager was not supported yet.
Thus, if #344 is merged before #341, it needs to add support in #341 to show that the Windows Credential Manager is used.
If merged after #341, this PR needs to do it.