Fix bug in Windows credential manager credential matching on port numbers #748
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mjcheetham
commented
Jun 16, 2022
ldennington
changed the title
Fix a bug whereby we would not correctly match existing credentials when port numbers were used in the search. Additionally, actually now store the non-default port number in the target URI if there is one given. We also now always trim any trailing '/' from the end of the path of a service/target URI. Previously we erronously only trimmed the '/' from a 'pathless' URI, i.e.: https://example.com/ => https://example.com https://example.com/path/ => https://example.com/path/ This is a safe change since the `Enumerate` method that is used by both `Get` and `Remove` already matches _both_ trailing and non-trailing slashes by virtue of creating and comparing components of a `System.Uri` which does normalisation of the path. Both `Get` and `Remove` would act (eventually) to remove any bad credentials stored with a trailing slash, even if new credentials are only written out without the slash.
ldennington
approved these changes
Jun 16, 2022
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix a bug whereby we would not correctly match existing credentials when port numbers were used in the search. Additionally, actually now store the non-default port number in the target URI if there is one given.
Fixes #747