Skip to content

Commit 0d70623

Browse files
authored
Only match credential entries with correct namespace in the Windows Credential Manager (#1328)
GCM by default creates entries in the Windows Credential Manager on Windows, and prefixes the 'target name' of the entry with "git:". This 'namespace' prefix is configurable, but is not often changed in practice outside of tests. Visual Studio, when adding GitHub accounts (either natively or by the older GitHub extension for VS), it creates three credential entries: 1. GitHub for Visual Studio - https://github.com 2. git:https://github.com 3. https://github.com Entry 1 is used by VS for it's own purposes. Entry 2 is created for the benefit for GCM, so that we are 'primed'. It is unknown what entry 3 is for at this time. There is an error in our existing logic for enumerating credentials that is also matching entry 3 as well as the expected entry 2. Modify and fix the matching logic to ensure that the namespace prefix matches, rather than just stripping it and matching (even if it doesn't exist!). Fixes #1325 --- **Bug repro instructions:** 1. Open Visual Studio 2. File > Account Settings 3. Add a GitHub account 4. Open a terminal (inside or outside of VS) and attempt to clone/fetch/push to or from a private GitHub repository. At this point a window should appear asking you to select between two "Personal Access Token" accounts. After installing [the bits from this PR build (artifacts > win-x86)](https://github.com/git-ecosystem/git-credential-manager/pull/1328/checks), attempting step 4 should **no longer** result in a prompt to select between two "Personal Access Token" accounts.
2 parents 7ff639d + 88e1297 commit 0d70623

File tree

2 files changed

+74
-2
lines changed

2 files changed

+74
-2
lines changed

src/shared/Core.Tests/Interop/Windows/WindowsCredentialManagerTests.cs

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,70 @@ public void WindowsCredentialManager_IsMatch(
270270
Assert.Equal(expected, actual);
271271
}
272272

273+
[PlatformFact(Platforms.Windows)]
274+
public void WindowsCredentialManager_IsMatch_NoNamespace_NotMatched()
275+
{
276+
var win32Cred = new Win32Credential
277+
{
278+
UserName = "test",
279+
TargetName = $"{WindowsCredentialManager.TargetNameLegacyGenericPrefix}https://example.com"
280+
};
281+
282+
var credManager = new WindowsCredentialManager(TestNamespace);
283+
284+
bool result = credManager.IsMatch("https://example.com", null, win32Cred);
285+
286+
Assert.False(result);
287+
}
288+
289+
[PlatformFact(Platforms.Windows)]
290+
public void WindowsCredentialManager_IsMatch_DifferentNamespace_NotMatched()
291+
{
292+
var win32Cred = new Win32Credential
293+
{
294+
UserName = "test",
295+
TargetName = $"{WindowsCredentialManager.TargetNameLegacyGenericPrefix}:random-namespace:https://example.com"
296+
};
297+
298+
var credManager = new WindowsCredentialManager(TestNamespace);
299+
300+
bool result = credManager.IsMatch("https://example.com", null, win32Cred);
301+
302+
Assert.False(result);
303+
}
304+
305+
[PlatformFact(Platforms.Windows)]
306+
public void WindowsCredentialManager_IsMatch_CaseSensitiveNamespace_NotMatched()
307+
{
308+
var win32Cred = new Win32Credential
309+
{
310+
UserName = "test",
311+
TargetName = $"{WindowsCredentialManager.TargetNameLegacyGenericPrefix}:nAmEsPaCe:https://example.com"
312+
};
313+
314+
var credManager = new WindowsCredentialManager("namespace");
315+
316+
bool result = credManager.IsMatch("https://example.com", null, win32Cred);
317+
318+
Assert.False(result);
319+
}
320+
321+
[PlatformFact(Platforms.Windows)]
322+
public void WindowsCredentialManager_IsMatch_NoNamespaceInQuery_IsMatched()
323+
{
324+
var win32Cred = new Win32Credential
325+
{
326+
UserName = "test",
327+
TargetName = $"{WindowsCredentialManager.TargetNameLegacyGenericPrefix}https://example.com"
328+
};
329+
330+
var credManager = new WindowsCredentialManager();
331+
332+
bool result = credManager.IsMatch("https://example.com", null, win32Cred);
333+
334+
Assert.True(result);
335+
}
336+
273337
[PlatformTheory(Platforms.Windows)]
274338
[InlineData("https://example.com", null, "https://example.com")]
275339
[InlineData("https://example.com", "bob", "https://[email protected]")]

src/shared/Core/Interop/Windows/WindowsCredentialManager.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,11 +274,19 @@ private WindowsCredential CreateCredentialFromStructure(Win32Credential credenti
274274
return false;
275275
}
276276

277-
// Trim the "LegacyGeneric" prefix Windows adds and any namespace we have been filtered with
277+
// Trim the "LegacyGeneric" prefix Windows adds
278278
string targetName = credential.TargetName.TrimUntilIndexOf(TargetNameLegacyGenericPrefix);
279+
280+
// Only match credentials with the namespace we have been configured with (if any)
279281
if (!string.IsNullOrWhiteSpace(_namespace))
280282
{
281-
targetName = targetName.TrimUntilIndexOf($"{_namespace}:");
283+
string nsPrefix = $"{_namespace}:";
284+
if (!targetName.StartsWith(nsPrefix, StringComparison.Ordinal))
285+
{
286+
return false;
287+
}
288+
289+
targetName = targetName.Substring(nsPrefix.Length);
282290
}
283291

284292
// If the target name matches the service name exactly then return 'match'

0 commit comments

Comments
 (0)