Skip to content

Commit

Permalink
Only match credential entries with correct namespace in the Windows C…
Browse files Browse the repository at this point in the history
…redential 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.
  • Loading branch information
mjcheetham committed Jul 12, 2023
2 parents 7ff639d + 88e1297 commit 0d70623
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,70 @@ public void WindowsCredentialManager_IsMatch(
Assert.Equal(expected, actual);
}

[PlatformFact(Platforms.Windows)]
public void WindowsCredentialManager_IsMatch_NoNamespace_NotMatched()
{
var win32Cred = new Win32Credential
{
UserName = "test",
TargetName = $"{WindowsCredentialManager.TargetNameLegacyGenericPrefix}https://example.com"
};

var credManager = new WindowsCredentialManager(TestNamespace);

bool result = credManager.IsMatch("https://example.com", null, win32Cred);

Assert.False(result);
}

[PlatformFact(Platforms.Windows)]
public void WindowsCredentialManager_IsMatch_DifferentNamespace_NotMatched()
{
var win32Cred = new Win32Credential
{
UserName = "test",
TargetName = $"{WindowsCredentialManager.TargetNameLegacyGenericPrefix}:random-namespace:https://example.com"
};

var credManager = new WindowsCredentialManager(TestNamespace);

bool result = credManager.IsMatch("https://example.com", null, win32Cred);

Assert.False(result);
}

[PlatformFact(Platforms.Windows)]
public void WindowsCredentialManager_IsMatch_CaseSensitiveNamespace_NotMatched()
{
var win32Cred = new Win32Credential
{
UserName = "test",
TargetName = $"{WindowsCredentialManager.TargetNameLegacyGenericPrefix}:nAmEsPaCe:https://example.com"
};

var credManager = new WindowsCredentialManager("namespace");

bool result = credManager.IsMatch("https://example.com", null, win32Cred);

Assert.False(result);
}

[PlatformFact(Platforms.Windows)]
public void WindowsCredentialManager_IsMatch_NoNamespaceInQuery_IsMatched()
{
var win32Cred = new Win32Credential
{
UserName = "test",
TargetName = $"{WindowsCredentialManager.TargetNameLegacyGenericPrefix}https://example.com"
};

var credManager = new WindowsCredentialManager();

bool result = credManager.IsMatch("https://example.com", null, win32Cred);

Assert.True(result);
}

[PlatformTheory(Platforms.Windows)]
[InlineData("https://example.com", null, "https://example.com")]
[InlineData("https://example.com", "bob", "https://[email protected]")]
Expand Down
12 changes: 10 additions & 2 deletions src/shared/Core/Interop/Windows/WindowsCredentialManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,19 @@ private WindowsCredential CreateCredentialFromStructure(Win32Credential credenti
return false;
}

// Trim the "LegacyGeneric" prefix Windows adds and any namespace we have been filtered with
// Trim the "LegacyGeneric" prefix Windows adds
string targetName = credential.TargetName.TrimUntilIndexOf(TargetNameLegacyGenericPrefix);

// Only match credentials with the namespace we have been configured with (if any)
if (!string.IsNullOrWhiteSpace(_namespace))
{
targetName = targetName.TrimUntilIndexOf($"{_namespace}:");
string nsPrefix = $"{_namespace}:";
if (!targetName.StartsWith(nsPrefix, StringComparison.Ordinal))
{
return false;
}

targetName = targetName.Substring(nsPrefix.Length);
}

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

0 comments on commit 0d70623

Please sign in to comment.