From 5b72a589ee54d11bb7dc292de6e4d7542636ee25 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 23 Feb 2021 11:33:40 +0000 Subject: [PATCH 01/17] app: fix exception handling/printing When we moved to use the System.CommandLine library for command line parsing, we neglected to update the exception handling to match the new model. --- .../Application.cs | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/shared/Microsoft.Git.CredentialManager/Application.cs b/src/shared/Microsoft.Git.CredentialManager/Application.cs index d57aaf8aa..5274c14ad 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Application.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Application.cs @@ -3,6 +3,9 @@ using System; using System.Collections.Generic; using System.CommandLine; +using System.CommandLine.Builder; +using System.CommandLine.Invocation; +using System.CommandLine.Parsing; using System.IO; using System.Linq; using System.Text.RegularExpressions; @@ -84,23 +87,12 @@ protected override async Task RunInternalAsync(string[] args) Context.Trace.WriteLine($"AppPath: {_appPath}"); Context.Trace.WriteLine($"Arguments: {string.Join(" ", args)}"); - try - { - return await rootCommand.InvokeAsync(args); - } - catch (Exception e) - { - if (e is AggregateException ae) - { - ae.Handle(WriteException); - } - else - { - WriteException(e); - } + var parser = new CommandLineBuilder(rootCommand) + .UseDefaults() + .UseExceptionHandler(OnException) + .Build(); - return -1; - } + return await parser.InvokeAsync(args); } protected override void Dispose(bool disposing) @@ -113,7 +105,19 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } - protected bool WriteException(Exception ex) + private void OnException(Exception ex, InvocationContext invocationContext) + { + if (ex is AggregateException aex) + { + aex.Handle(WriteException); + } + else + { + WriteException(ex); + } + } + + private bool WriteException(Exception ex) { // Try and use a nicer format for some well-known exception types switch (ex) From 34ff9df184207bb24e0655114aed344a8c7b5c30 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Thu, 18 Feb 2021 16:21:46 +0000 Subject: [PATCH 02/17] inputargs: fix a bug in remote URI generation Fix a bug in contruction of the remote URI when no username is provided, but the caller wishes to include the username. --- .../InputArgumentsTests.cs | 19 +++++++++++++++++++ .../InputArguments.cs | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/InputArgumentsTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/InputArgumentsTests.cs index 4429b0e58..632bda8c8 100644 --- a/src/shared/Microsoft.Git.CredentialManager.Tests/InputArgumentsTests.cs +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/InputArgumentsTests.cs @@ -140,6 +140,25 @@ public void InputArguments_GetRemoteUri_IncludeUserSpecialCharacters_Authority_R Assert.Equal(expectedUri, actualUri); } + [Fact] + public void InputArguments_GetRemoteUri_IncludeUserNoUser_Authority_ReturnsUriWithAuthority() + { + var expectedUri = new Uri("https://example.com/"); + + var dict = new Dictionary + { + ["protocol"] = "https", + ["host"] = "example.com", + }; + + var inputArgs = new InputArguments(dict); + + Uri actualUri = inputArgs.GetRemoteUri(includeUser: true); + + Assert.NotNull(actualUri); + Assert.Equal(expectedUri, actualUri); + } + [Fact] public void InputArguments_GetRemoteUri_AuthorityAndPort_ReturnsUriWithAuthorityAndPort() { diff --git a/src/shared/Microsoft.Git.CredentialManager/InputArguments.cs b/src/shared/Microsoft.Git.CredentialManager/InputArguments.cs index 1a51c3e2c..5b98910b9 100644 --- a/src/shared/Microsoft.Git.CredentialManager/InputArguments.cs +++ b/src/shared/Microsoft.Git.CredentialManager/InputArguments.cs @@ -106,7 +106,7 @@ public Uri GetRemoteUri(bool includeUser = false) ub.Port = port; } - if (includeUser) + if (includeUser && !string.IsNullOrEmpty(UserName)) { ub.UserName = Uri.EscapeDataString(UserName); } From ba7e9d4e0e0c341f30680aeee03a32ebaf152e09 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 23 Feb 2021 11:34:33 +0000 Subject: [PATCH 03/17] input: better input arg checking/handling Improve the handling of input arguments with missing required fields (e.g., protocol and host). --- .../BitbucketHostProvider.cs | 5 +++- src/shared/GitHub/GitHubHostProvider.cs | 5 +++- .../Commands/EraseCommandTests.cs | 4 ++- .../Commands/GetCommandTests.cs | 8 +++++- .../Commands/StoreCommandTests.cs | 4 ++- .../Commands/GitCommandBase.cs | 26 +++++++++++++++++++ .../Commands/StoreCommand.cs | 17 ++++++++++++ 7 files changed, 64 insertions(+), 5 deletions(-) diff --git a/src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs b/src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs index d8014f0a7..c6da9002d 100644 --- a/src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs +++ b/src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs @@ -46,7 +46,10 @@ public bool IsSupported(InputArguments input) } // Split port number and hostname from host input argument - input.TryGetHostAndPort(out string hostName, out _); + if (!input.TryGetHostAndPort(out string hostName, out _)) + { + return false; + } // We do not support unencrypted HTTP communications to Bitbucket, // but we report `true` here for HTTP so that we can show a helpful diff --git a/src/shared/GitHub/GitHubHostProvider.cs b/src/shared/GitHub/GitHubHostProvider.cs index 2860667a5..606ab878a 100644 --- a/src/shared/GitHub/GitHubHostProvider.cs +++ b/src/shared/GitHub/GitHubHostProvider.cs @@ -63,7 +63,10 @@ public override bool IsSupported(InputArguments input) } // Split port number and hostname from host input argument - input.TryGetHostAndPort(out string hostName, out _); + if (!input.TryGetHostAndPort(out string hostName, out _)) + { + return false; + } if (StringComparer.OrdinalIgnoreCase.Equals(hostName, GitHubConstants.GitHubBaseUrlHost) || StringComparer.OrdinalIgnoreCase.Equals(hostName, GitHubConstants.GistBaseUrlHost)) diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/Commands/EraseCommandTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/Commands/EraseCommandTests.cs index e2ea7e386..58f7f6efb 100644 --- a/src/shared/Microsoft.Git.CredentialManager.Tests/Commands/EraseCommandTests.cs +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/Commands/EraseCommandTests.cs @@ -16,9 +16,11 @@ public async Task EraseCommand_ExecuteAsync_CallsHostProvider() { const string testUserName = "john.doe"; const string testPassword = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")] - var stdin = $"username={testUserName}\npassword={testPassword}\n\n"; + var stdin = $"protocol=http\nhost=example.com\nusername={testUserName}\npassword={testPassword}\n\n"; var expectedInput = new InputArguments(new Dictionary { + ["protocol"] = "http", + ["host"] = "example.com", ["username"] = testUserName, ["password"] = testPassword // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")] }); diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/Commands/GetCommandTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/Commands/GetCommandTests.cs index 0f984e76c..4ab11a8a4 100644 --- a/src/shared/Microsoft.Git.CredentialManager.Tests/Commands/GetCommandTests.cs +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/Commands/GetCommandTests.cs @@ -19,8 +19,11 @@ public async Task GetCommand_ExecuteAsync_CallsHostProviderAndWritesCredential() const string testUserName = "john.doe"; const string testPassword = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")] ICredential testCredential = new GitCredential(testUserName, testPassword); + var stdin = $"protocol=http\nhost=example.com\n\n"; var expectedStdOutDict = new Dictionary { + ["protocol"] = "http", + ["host"] = "example.com", ["username"] = testUserName, ["password"] = testPassword }; @@ -29,7 +32,10 @@ public async Task GetCommand_ExecuteAsync_CallsHostProviderAndWritesCredential() providerMock.Setup(x => x.GetCredentialAsync(It.IsAny())) .ReturnsAsync(testCredential); var providerRegistry = new TestHostProviderRegistry {Provider = providerMock.Object}; - var context = new TestCommandContext(); + var context = new TestCommandContext + { + Streams = {In = stdin} + }; var command = new GetCommand(context, providerRegistry); diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/Commands/StoreCommandTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/Commands/StoreCommandTests.cs index 1bfc42339..7eedd9cf3 100644 --- a/src/shared/Microsoft.Git.CredentialManager.Tests/Commands/StoreCommandTests.cs +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/Commands/StoreCommandTests.cs @@ -15,9 +15,11 @@ public async Task StoreCommand_ExecuteAsync_CallsHostProvider() { const string testUserName = "john.doe"; const string testPassword = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")] - var stdin = $"username={testUserName}\npassword={testPassword}\n\n"; + var stdin = $"protocol=http\nhost=example.com\nusername={testUserName}\npassword={testPassword}\n\n"; var expectedInput = new InputArguments(new Dictionary { + ["protocol"] = "http", + ["host"] = "example.com", ["username"] = testUserName, ["password"] = testPassword }); diff --git a/src/shared/Microsoft.Git.CredentialManager/Commands/GitCommandBase.cs b/src/shared/Microsoft.Git.CredentialManager/Commands/GitCommandBase.cs index 062f3561f..108e3f4d8 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Commands/GitCommandBase.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Commands/GitCommandBase.cs @@ -39,6 +39,9 @@ internal async Task ExecuteAsync() IDictionary inputDict = await Context.Streams.In.ReadDictionaryAsync(StringComparer.Ordinal); var input = new InputArguments(inputDict); + // Validate minimum arguments are present + EnsureMinimumInputArguments(input); + // Set the remote URI to scope settings to throughout the process from now on Context.Settings.RemoteUri = input.GetRemoteUri(); @@ -53,6 +56,29 @@ internal async Task ExecuteAsync() Context.Trace.WriteLine($"End '{Name}' command..."); } + protected virtual void EnsureMinimumInputArguments(InputArguments input) + { + if (input.Protocol is null) + { + throw new InvalidOperationException("Missing 'protocol' input argument"); + } + + if (string.IsNullOrWhiteSpace(input.Protocol)) + { + throw new InvalidOperationException("Invalid 'protocol' input argument (cannot be empty)"); + } + + if (input.Host is null) + { + throw new InvalidOperationException("Missing 'host' input argument"); + } + + if (string.IsNullOrWhiteSpace(input.Host)) + { + throw new InvalidOperationException("Invalid 'host' input argument (cannot be empty)"); + } + } + /// /// Execute the command using the given and . /// diff --git a/src/shared/Microsoft.Git.CredentialManager/Commands/StoreCommand.cs b/src/shared/Microsoft.Git.CredentialManager/Commands/StoreCommand.cs index 4bb580a6e..02454fc29 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Commands/StoreCommand.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Commands/StoreCommand.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. +using System; using System.Threading.Tasks; namespace Microsoft.Git.CredentialManager.Commands @@ -16,5 +17,21 @@ protected override Task ExecuteInternalAsync(InputArguments input, IHostProvider { return provider.StoreCredentialAsync(input); } + + protected override void EnsureMinimumInputArguments(InputArguments input) + { + base.EnsureMinimumInputArguments(input); + + // An empty string username/password are valid inputs, so only check for `null` (not provided) + if (input.UserName is null) + { + throw new InvalidOperationException("Missing 'username' input argument"); + } + + if (input.Password is null) + { + throw new InvalidOperationException("Missing 'password' input argument"); + } + } } } From f31b16feba8c5d9aa7efc5954619020c0bdc8f1e Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Wed, 17 Feb 2021 13:15:14 +0000 Subject: [PATCH 04/17] git: replace progdata/xdg config levels with unknown Replace the unused `ProgramData` and `Xdg` Git configuration level enumeration members with an `Unknown` member. We never directly used any of those, and don't really care(!) --- .../Microsoft.Git.CredentialManager/GitConfiguration.cs | 7 ++----- src/shared/TestInfrastructure/Objects/TestGit.cs | 3 +-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs index e694cb36a..0f8ba28bf 100644 --- a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs +++ b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs @@ -19,11 +19,10 @@ namespace Microsoft.Git.CredentialManager public enum GitConfigurationLevel { All, - ProgramData, System, - Xdg, Global, Local, + Unknown, } public interface IGitConfiguration @@ -410,15 +409,13 @@ private string GetLevelFilterArg() { switch (_filterLevel) { - case GitConfigurationLevel.ProgramData: - case GitConfigurationLevel.Xdg: - return null; case GitConfigurationLevel.System: return "--system"; case GitConfigurationLevel.Global: return "--global"; case GitConfigurationLevel.Local: return "--local"; + case GitConfigurationLevel.Unknown: default: return null; } diff --git a/src/shared/TestInfrastructure/Objects/TestGit.cs b/src/shared/TestInfrastructure/Objects/TestGit.cs index d6129a5a5..a872106ab 100644 --- a/src/shared/TestInfrastructure/Objects/TestGit.cs +++ b/src/shared/TestInfrastructure/Objects/TestGit.cs @@ -26,8 +26,7 @@ IGitConfiguration IGit.GetConfiguration(GitConfigurationLevel level) GlobalConfiguration.Dictionary, LocalConfiguration.Dictionary); return new TestGitConfiguration(mergedConfigDict); - case GitConfigurationLevel.ProgramData: - case GitConfigurationLevel.Xdg: + case GitConfigurationLevel.Unknown: return new TestGitConfiguration(); case GitConfigurationLevel.System: return SystemConfiguration; From dc6a69c6ab6ccb1a4d8b15a8aaeb5547400cab2f Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Thu, 18 Feb 2021 15:26:56 +0000 Subject: [PATCH 05/17] git: drop unused git config extension methods Remove some unused Git configuration extension methods for querying for entries based on 'split' keys (section.scope.property). These are only used in tests! --- .../GitConfigurationTests.cs | 165 ------------------ .../GitConfiguration.cs | 64 ------- .../Objects/TestSettings.cs | 2 +- 3 files changed, 1 insertion(+), 230 deletions(-) diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs index b5cd28024..5bbabbd29 100644 --- a/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs @@ -162,92 +162,6 @@ public void GitConfiguration_TryGet_Name_DoesNotExists_ReturnsFalse() Assert.Null(value); } - [Fact] - public void GitConfiguration_TryGet_SectionProperty_Exists_ReturnsTrueOutString() - { - string repoPath = CreateRepository(out string workDirPath); - Git(repoPath, workDirPath, "config --local user.name john.doe").AssertSuccess(); - - string gitPath = GetGitPath(); - var trace = new NullTrace(); - var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(); - - bool result = config.TryGet("user", "name", out string value); - Assert.True(result); - Assert.NotNull(value); - Assert.Equal("john.doe", value); - } - - [Fact] - public void GitConfiguration_TryGet_SectionProperty_DoesNotExists_ReturnsFalse() - { - string repoPath = CreateRepository(); - - string gitPath = GetGitPath(); - var trace = new NullTrace(); - var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(); - - string randomSection = Guid.NewGuid().ToString("N"); - string randomProperty = Guid.NewGuid().ToString("N"); - bool result = config.TryGet(randomSection, randomProperty, out string value); - Assert.False(result); - Assert.Null(value); - } - - [Fact] - public void GitConfiguration_TryGet_SectionScopeProperty_Exists_ReturnsTrueOutString() - { - string repoPath = CreateRepository(out string workDirPath); - Git(repoPath, workDirPath, "config --local user.example.com.name john.doe").AssertSuccess(); - - string gitPath = GetGitPath(); - var trace = new NullTrace(); - var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(); - - bool result = config.TryGet("user", "example.com", "name", out string value); - Assert.True(result); - Assert.NotNull(value); - Assert.Equal("john.doe", value); - } - - [Fact] - public void GitConfiguration_TryGet_SectionScopeProperty_NullScope_ReturnsTrueOutUnscopedString() - { - string repoPath = CreateRepository(out string workDirPath); - Git(repoPath, workDirPath, "config --local user.name john.doe").AssertSuccess(); - - string gitPath = GetGitPath(); - var trace = new NullTrace(); - var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(); - - bool result = config.TryGet("user", null, "name", out string value); - Assert.True(result); - Assert.NotNull(value); - Assert.Equal("john.doe", value); - } - - [Fact] - public void GitConfiguration_TryGet_SectionScopeProperty_DoesNotExists_ReturnsFalse() - { - string repoPath = CreateRepository(); - - string gitPath = GetGitPath(); - var trace = new NullTrace(); - var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(); - - string randomSection = Guid.NewGuid().ToString("N"); - string randomScope = Guid.NewGuid().ToString("N"); - string randomProperty = Guid.NewGuid().ToString("N"); - bool result = config.TryGet(randomSection, randomScope, randomProperty, out string value); - Assert.False(result); - Assert.Null(value); - } - [Fact] public void GitConfiguration_Get_Name_Exists_ReturnsString() { @@ -278,85 +192,6 @@ public void GitConfiguration_Get_Name_DoesNotExists_ThrowsException() Assert.Throws(() => config.Get(randomName)); } - [Fact] - public void GitConfiguration_Get_SectionProperty_Exists_ReturnsString() - { - string repoPath = CreateRepository(out string workDirPath); - Git(repoPath, workDirPath, "config --local user.name john.doe").AssertSuccess(); - - string gitPath = GetGitPath(); - var trace = new NullTrace(); - var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(); - - string value = config.Get("user", "name"); - Assert.NotNull(value); - Assert.Equal("john.doe", value); - } - - [Fact] - public void GitConfiguration_Get_SectionProperty_DoesNotExists_ThrowsException() - { - string repoPath = CreateRepository(); - - string gitPath = GetGitPath(); - var trace = new NullTrace(); - var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(); - - string randomSection = Guid.NewGuid().ToString("N"); - string randomProperty = Guid.NewGuid().ToString("N"); - Assert.Throws(() => config.Get(randomSection, randomProperty)); - } - - [Fact] - public void GitConfiguration_Get_SectionScopeProperty_Exists_ReturnsString() - { - string repoPath = CreateRepository(out string workDirPath); - Git(repoPath, workDirPath, "config --local user.example.com.name john.doe").AssertSuccess(); - - string gitPath = GetGitPath(); - var trace = new NullTrace(); - var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(); - - string value = config.Get("user", "example.com", "name"); - Assert.NotNull(value); - Assert.Equal("john.doe", value); - } - - [Fact] - public void GitConfiguration_Get_SectionScopeProperty_NullScope_ReturnsUnscopedString() - { - string repoPath = CreateRepository(out string workDirPath); - Git(repoPath, workDirPath, "config --local user.name john.doe").AssertSuccess(); - - string gitPath = GetGitPath(); - var trace = new NullTrace(); - var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(); - - string value = config.Get("user", null, "name"); - Assert.NotNull(value); - Assert.Equal("john.doe", value); - } - - [Fact] - public void GitConfiguration_Get_SectionScopeProperty_DoesNotExists_ThrowsException() - { - string repoPath = CreateRepository(); - - string gitPath = GetGitPath(); - var trace = new NullTrace(); - var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(); - - string randomSection = Guid.NewGuid().ToString("N"); - string randomScope = Guid.NewGuid().ToString("N"); - string randomProperty = Guid.NewGuid().ToString("N"); - Assert.Throws(() => config.Get(randomSection, randomScope, randomProperty)); - } - [Fact] public void GitConfiguration_Set_Local_SetsLocalConfig() { diff --git a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs index 0f8ba28bf..8aca98592 100644 --- a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs +++ b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs @@ -511,69 +511,5 @@ public static string Get(this IGitConfiguration config, string name) return value; } - - /// - /// Get the value of a configuration entry as a string. - /// - /// Configuration object. - /// Configuration section name. - /// Configuration property name. - /// A configuration entry with the specified key was not found. - /// Configuration entry value. - public static string Get(this IGitConfiguration config, string section, string property) - { - return Get(config, $"{section}.{property}"); - } - - /// - /// Get the value of a scoped configuration entry as a string. - /// - /// Configuration object. - /// Configuration section name. - /// Configuration section scope. - /// Configuration property name. - /// A configuration entry with the specified key was not found. - /// Configuration entry value. - public static string Get(this IGitConfiguration config, string section, string scope, string property) - { - if (scope is null) - { - return Get(config, section, property); - } - - return Get(config, $"{section}.{scope}.{property}"); - } - - /// - /// Try and get the value of a configuration entry as a string. - /// - /// Configuration object. - /// Configuration section name. - /// Configuration property name. - /// Configuration entry value. - /// True if the value was found, false otherwise. - public static bool TryGet(this IGitConfiguration config, string section, string property, out string value) - { - return config.TryGet($"{section}.{property}", out value); - } - - /// - /// Try and get the value of a configuration entry as a string. - /// - /// Configuration object. - /// Configuration section name. - /// Configuration section scope. - /// Configuration property name. - /// Configuration entry value. - /// True if the value was found, false otherwise. - public static bool TryGet(this IGitConfiguration config, string section, string scope, string property, out string value) - { - if (scope is null) - { - return TryGet(config, section, property, out value); - } - - return config.TryGet($"{section}.{scope}.{property}", out value); - } } } diff --git a/src/shared/TestInfrastructure/Objects/TestSettings.cs b/src/shared/TestInfrastructure/Objects/TestSettings.cs index f00e09a47..c7f363822 100644 --- a/src/shared/TestInfrastructure/Objects/TestSettings.cs +++ b/src/shared/TestInfrastructure/Objects/TestSettings.cs @@ -50,7 +50,7 @@ public bool TryGetSetting(string envarName, string section, string property, out return true; } - if (GitConfiguration?.TryGet(section, property, out value) ?? false) + if (GitConfiguration?.TryGet($"{section}.{property}", out value) ?? false) { return true; } From 94bfed475bca008935c872fa7a0a6078db35a7fc Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Thu, 18 Feb 2021 15:51:14 +0000 Subject: [PATCH 06/17] git: introduce GitCfgEntry wrapper type for enumeration Introduce a wrapper type representing a single entry in Git's configuration, which is used in the `Enumerate` callback. --- .../GitConfigurationTests.cs | 12 ++++++------ .../GitConfiguration.cs | 7 +++---- .../GitConfigurationEntry.cs | 16 ++++++++++++++++ .../Microsoft.Git.CredentialManager/Settings.cs | 8 ++++---- .../Objects/TestGitConfiguration.cs | 2 +- 5 files changed, 30 insertions(+), 15 deletions(-) create mode 100644 src/shared/Microsoft.Git.CredentialManager/GitConfigurationEntry.cs diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs index 5bbabbd29..bc9f12036 100644 --- a/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs @@ -76,11 +76,11 @@ public void GitConfiguration_Enumerate_CallbackReturnsTrue_InvokesCallbackForEac var actualVisitedEntries = new List<(string name, string value)>(); - bool cb(string name, string value) + bool cb(GitConfigurationEntry entry) { - if (name.StartsWith("foo.")) + if (entry.Key.StartsWith("foo.")) { - actualVisitedEntries.Add((name, value)); + actualVisitedEntries.Add((entry.Key, entry.Value)); } // Continue enumeration @@ -113,11 +113,11 @@ public void GitConfiguration_Enumerate_CallbackReturnsFalse_InvokesCallbackForEa var actualVisitedEntries = new List<(string name, string value)>(); - bool cb(string name, string value) + bool cb(GitConfigurationEntry entry) { - if (name.StartsWith("foo.")) + if (entry.Key.StartsWith("foo.")) { - actualVisitedEntries.Add((name, value)); + actualVisitedEntries.Add((entry.Key, entry.Value)); } // Stop enumeration after 2 'foo' entries diff --git a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs index 8aca98592..4126ac64f 100644 --- a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs +++ b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs @@ -11,10 +11,9 @@ namespace Microsoft.Git.CredentialManager /// /// Invoked for each Git configuration entry during an enumeration (). /// - /// Name of the current configuration entry. - /// Value of the current configuration entry. + /// Current configuration entry. /// True to continue enumeration, false to stop enumeration. - public delegate bool GitConfigurationEnumerationCallback(string name, string value); + public delegate bool GitConfigurationEnumerationCallback(GitConfigurationEntry entry); public enum GitConfigurationLevel { @@ -134,7 +133,7 @@ public void Enumerate(GitConfigurationEnumerationCallback cb) { string[] kvp = entry.Split(new[]{'\n'}, count: 2); - if (kvp.Length == 2 && !cb(kvp[0], kvp[1])) + if (kvp.Length == 2 && !cb(new GitConfigurationEntry(kvp[0], kvp[1]))) { break; } diff --git a/src/shared/Microsoft.Git.CredentialManager/GitConfigurationEntry.cs b/src/shared/Microsoft.Git.CredentialManager/GitConfigurationEntry.cs new file mode 100644 index 000000000..e57c855e4 --- /dev/null +++ b/src/shared/Microsoft.Git.CredentialManager/GitConfigurationEntry.cs @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. +namespace Microsoft.Git.CredentialManager +{ + public class GitConfigurationEntry + { + public GitConfigurationEntry(string key, string value) + { + Key = key; + Value = value; + } + + public string Key { get; } + public string Value { get; } + } +} diff --git a/src/shared/Microsoft.Git.CredentialManager/Settings.cs b/src/shared/Microsoft.Git.CredentialManager/Settings.cs index a71ced4b1..e78756f2f 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Settings.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Settings.cs @@ -251,15 +251,15 @@ public IEnumerable GetSettingValues(string envarName, string section, st // and make a local copy of them here to avoid needing to call `TryGetValue` on the // IGitConfiguration object multiple times in a loop below. var configEntries = new Dictionary(GitConfigurationKeyComparer.Instance); - config.Enumerate((entryName, entryValue) => + config.Enumerate(entry => { - string entrySection = entryName.TruncateFromIndexOf('.'); - string entryProperty = entryName.TrimUntilLastIndexOf('.'); + string entrySection = entry.Key.TruncateFromIndexOf('.'); + string entryProperty = entry.Key.TrimUntilLastIndexOf('.'); if (StringComparer.OrdinalIgnoreCase.Equals(entrySection, section) && StringComparer.OrdinalIgnoreCase.Equals(entryProperty, property)) { - configEntries[entryName] = entryValue; + configEntries[entry.Key] = entry.Value; } // Continue the enumeration diff --git a/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs b/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs index f0df76305..b8a112637 100644 --- a/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs +++ b/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs @@ -37,7 +37,7 @@ public void Enumerate(GitConfigurationEnumerationCallback cb) { foreach (var value in kvp.Value) { - if (!cb(kvp.Key, value)) + if (!cb(new GitConfigurationEntry(kvp.Key, value))) { break; } From b9dfb071c7685a5dd58625b16df725ac9b830cdf Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Thu, 18 Feb 2021 15:56:04 +0000 Subject: [PATCH 07/17] git: include gitcfg scope/level in enumeration results Teach `GitConfiguration::Enumerate` to parse results that include the 'level' of the Git config entry (--show-scope). --- .../GitConfiguration.cs | 61 +++++++++++++++++-- .../GitConfigurationEntry.cs | 4 +- .../Objects/TestGitConfiguration.cs | 2 +- 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs index 4126ac64f..a10aba858 100644 --- a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs +++ b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs @@ -111,7 +111,7 @@ internal GitProcessConfiguration(ITrace trace, GitProcess git, GitConfigurationL public void Enumerate(GitConfigurationEnumerationCallback cb) { string level = GetLevelFilterArg(); - using (Process git = _git.CreateProcess($"config --null {level} --list")) + using (Process git = _git.CreateProcess($"config --null {level} --list --show-scope")) { git.Start(); // To avoid deadlocks, always read the output stream first and then wait @@ -128,12 +128,63 @@ public void Enumerate(GitConfigurationEnumerationCallback cb) throw CreateGitException(git, "Failed to enumerate all Git configuration entries"); } - IEnumerable entries = data.Split('\0').Where(x => !string.IsNullOrWhiteSpace(x)); - foreach (string entry in entries) + var scope = new StringBuilder(); + var name = new StringBuilder(); + var value = new StringBuilder(); + int i = 0; + while (i < data.Length) { - string[] kvp = entry.Split(new[]{'\n'}, count: 2); + scope.Clear(); + name.Clear(); + value.Clear(); + + // Read config scope (null terminated) + while (data[i] != '\0') + { + scope.Append(data[i++]); + } + + // Skip the null terminator + i++; + + // Read key name (LF terminated) + while (data[i] != '\n') + { + name.Append(data[i++]); + } + + // Skip the LF terminator + i++; + + // Read value (null terminated) + while (data[i] != '\0') + { + value.Append(data[i++]); + } + + // Skip the null terminator + i++; + + GitConfigurationLevel entryLevel; + switch (scope.ToString()) + { + case "system": + entryLevel = GitConfigurationLevel.System; + break; + case "global": + entryLevel = GitConfigurationLevel.Global; + break; + case "local": + entryLevel = GitConfigurationLevel.Local; + break; + default: + entryLevel = GitConfigurationLevel.Unknown; + break; + } + + var entry = new GitConfigurationEntry(entryLevel, name.ToString(), value.ToString()); - if (kvp.Length == 2 && !cb(new GitConfigurationEntry(kvp[0], kvp[1]))) + if (!cb(entry)) { break; } diff --git a/src/shared/Microsoft.Git.CredentialManager/GitConfigurationEntry.cs b/src/shared/Microsoft.Git.CredentialManager/GitConfigurationEntry.cs index e57c855e4..0dddf70df 100644 --- a/src/shared/Microsoft.Git.CredentialManager/GitConfigurationEntry.cs +++ b/src/shared/Microsoft.Git.CredentialManager/GitConfigurationEntry.cs @@ -4,12 +4,14 @@ namespace Microsoft.Git.CredentialManager { public class GitConfigurationEntry { - public GitConfigurationEntry(string key, string value) + public GitConfigurationEntry(GitConfigurationLevel level, string key, string value) { + Level = level; Key = key; Value = value; } + public GitConfigurationLevel Level { get; } public string Key { get; } public string Value { get; } } diff --git a/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs b/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs index b8a112637..675f651db 100644 --- a/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs +++ b/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs @@ -37,7 +37,7 @@ public void Enumerate(GitConfigurationEnumerationCallback cb) { foreach (var value in kvp.Value) { - if (!cb(new GitConfigurationEntry(kvp.Key, value))) + if (!cb(new GitConfigurationEntry(GitConfigurationLevel.Unknown, kvp.Key, value))) { break; } From 0bf67cca05e702c49defcf2befc3f6b5cf3e0b89 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Thu, 18 Feb 2021 16:03:28 +0000 Subject: [PATCH 08/17] git: expose TrySplit git cfg key splitting helper Expose the `Split` method from the `GitConfigurationKeyComparer` as a `TrySplit` method and use this implementation for splitting keys in all existing instances. Also introduce a `GitConfiguration::Enumerate` extension method that filters based on section and property name parts of Git config entries. --- .../GitConfiguration.cs | 23 +++++++++++++++++++ .../GitConfigurationKeyComparer.cs | 20 ++++++++++------ .../Settings.cs | 11 ++------- 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs index a10aba858..74ae0c96a 100644 --- a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs +++ b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs @@ -545,6 +545,29 @@ public static string QuoteCmdArg(string str) public static class GitConfigurationExtensions { + /// + /// Enumerate all configuration entries invoking the specified callback for each entry. + /// + /// Configuration object. + /// Optional section name to filter; use null for any. + /// Optional property name to filter; use null for any. + /// Callback to invoke for each matching configuration entry. + public static void Enumerate(this IGitConfiguration config, + string section, string property, GitConfigurationEnumerationCallback cb) + { + config.Enumerate(entry => + { + if (GitConfigurationKeyComparer.TrySplit(entry.Key, out string entrySection, out _, out string entryProperty) && + (section is null || GitConfigurationKeyComparer.SectionComparer.Equals(section, entrySection)) && + (property is null || GitConfigurationKeyComparer.PropertyComparer.Equals(property, entryProperty))) + { + return cb(entry); + } + + return true; + }); + } + /// /// Get the value of a configuration entry as a string. /// diff --git a/src/shared/Microsoft.Git.CredentialManager/GitConfigurationKeyComparer.cs b/src/shared/Microsoft.Git.CredentialManager/GitConfigurationKeyComparer.cs index 599010eff..7f9bf276a 100644 --- a/src/shared/Microsoft.Git.CredentialManager/GitConfigurationKeyComparer.cs +++ b/src/shared/Microsoft.Git.CredentialManager/GitConfigurationKeyComparer.cs @@ -18,12 +18,16 @@ public class GitConfigurationKeyComparer : StringComparer { public static readonly GitConfigurationKeyComparer Instance = new GitConfigurationKeyComparer(); + public static readonly StringComparer SectionComparer = OrdinalIgnoreCase; + public static readonly StringComparer ScopeComparer = Ordinal; + public static readonly StringComparer PropertyComparer = OrdinalIgnoreCase; + private GitConfigurationKeyComparer() { } public override int Compare(string x, string y) { - Split(x, out string xSection, out string xScope, out string xProperty); - Split(y, out string ySection, out string yScope, out string yProperty); + TrySplit(x, out string xSection, out string xScope, out string xProperty); + TrySplit(y, out string ySection, out string yScope, out string yProperty); int cmpSection = OrdinalIgnoreCase.Compare(xSection, ySection); if (cmpSection != 0) return cmpSection; @@ -39,8 +43,8 @@ public override bool Equals(string x, string y) if (ReferenceEquals(x, y)) return true; if (x is null || y is null) return false; - Split(x, out string xSection, out string xScope, out string xProperty); - Split(y, out string ySection, out string yScope, out string yProperty); + TrySplit(x, out string xSection, out string xScope, out string xProperty); + TrySplit(y, out string ySection, out string yScope, out string yProperty); // Section and property names are not case sensitive, but the inner 'scope' IS case sensitive! return OrdinalIgnoreCase.Equals(xSection, ySection) && @@ -50,7 +54,7 @@ public override bool Equals(string x, string y) public override int GetHashCode(string obj) { - Split(obj, out string section, out string scope, out string property); + TrySplit(obj, out string section, out string scope, out string property); int code = OrdinalIgnoreCase.GetHashCode(section) ^ OrdinalIgnoreCase.GetHashCode(property); @@ -60,7 +64,7 @@ public override int GetHashCode(string obj) : code ^ Ordinal.GetHashCode(scope); } - private static void Split(string str, out string section, out string scope, out string property) + public static bool TrySplit(string str, out string section, out string scope, out string property) { section = null; scope = null; @@ -68,13 +72,15 @@ private static void Split(string str, out string section, out string scope, out if (string.IsNullOrWhiteSpace(str)) { - return; + return false; } section = str.TruncateFromIndexOf('.'); property = str.TrimUntilLastIndexOf('.'); int scopeLength = str.Length - (section.Length + property.Length + 2); scope = scopeLength > 0 ? str.Substring(section.Length + 1, scopeLength) : null; + + return true; } } } diff --git a/src/shared/Microsoft.Git.CredentialManager/Settings.cs b/src/shared/Microsoft.Git.CredentialManager/Settings.cs index e78756f2f..d3f7d5499 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Settings.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Settings.cs @@ -251,16 +251,9 @@ public IEnumerable GetSettingValues(string envarName, string section, st // and make a local copy of them here to avoid needing to call `TryGetValue` on the // IGitConfiguration object multiple times in a loop below. var configEntries = new Dictionary(GitConfigurationKeyComparer.Instance); - config.Enumerate(entry => + config.Enumerate(section, property, entry => { - string entrySection = entry.Key.TruncateFromIndexOf('.'); - string entryProperty = entry.Key.TrimUntilLastIndexOf('.'); - - if (StringComparer.OrdinalIgnoreCase.Equals(entrySection, section) && - StringComparer.OrdinalIgnoreCase.Equals(entryProperty, property)) - { - configEntries[entry.Key] = entry.Value; - } + configEntries[entry.Key] = entry.Value; // Continue the enumeration return true; From 5d61bbe0157fac5894ee3ca08fce58d15395220c Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Thu, 18 Feb 2021 16:13:29 +0000 Subject: [PATCH 09/17] git: change git config API to push level filter to methods Change the way we interact with Git configuration so that we specify the level filter in each method call, instead of requesting the `GitConfiguration` object perform the filtering. --- .../AzureReposHostProviderTests.cs | 36 +-- .../AzureReposHostProvider.cs | 8 +- .../ApplicationTests.cs | 68 ++--- .../GitConfigurationTests.cs | 28 +- .../SettingsTests.cs | 76 +++--- .../Application.cs | 22 +- .../Microsoft.Git.CredentialManager/Git.cs | 35 ++- .../GitConfiguration.cs | 243 +++++++++++------- .../Objects/TestCommandContext.cs | 2 +- .../TestInfrastructure/Objects/TestGit.cs | 28 +- .../Objects/TestGitConfiguration.cs | 182 ++++++++----- .../Objects/TestSettings.cs | 9 +- 12 files changed, 418 insertions(+), 319 deletions(-) diff --git a/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs b/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs index ea86fd11a..8ce481aae 100644 --- a/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs +++ b/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs @@ -185,12 +185,12 @@ public async Task AzureReposHostProvider_ConfigureAsync_UseHttpPathSetTrue_DoesN var context = new TestCommandContext(); var provider = new AzureReposHostProvider(context); - context.Git.GlobalConfiguration.Dictionary[AzDevUseHttpPathKey] = new List {"true"}; + context.Git.Configuration.Global[AzDevUseHttpPathKey] = new List {"true"}; await provider.ConfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(AzDevUseHttpPathKey, out IList actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(AzDevUseHttpPathKey, out IList actualValues)); Assert.Single(actualValues); Assert.Equal("true", actualValues[0]); } @@ -201,12 +201,12 @@ public async Task AzureReposHostProvider_ConfigureAsync_UseHttpPathSetFalse_Sets var context = new TestCommandContext(); var provider = new AzureReposHostProvider(context); - context.Git.GlobalConfiguration.Dictionary[AzDevUseHttpPathKey] = new List {"false"}; + context.Git.Configuration.Global[AzDevUseHttpPathKey] = new List {"false"}; await provider.ConfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(AzDevUseHttpPathKey, out IList actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(AzDevUseHttpPathKey, out IList actualValues)); Assert.Single(actualValues); Assert.Equal("true", actualValues[0]); } @@ -219,8 +219,8 @@ public async Task AzureReposHostProvider_ConfigureAsync_UseHttpPathUnset_SetsUse await provider.ConfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(AzDevUseHttpPathKey, out IList actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(AzDevUseHttpPathKey, out IList actualValues)); Assert.Single(actualValues); Assert.Equal("true", actualValues[0]); } @@ -231,11 +231,11 @@ public async Task AzureReposHostProvider_UnconfigureAsync_UseHttpPathSet_Removes var context = new TestCommandContext(); var provider = new AzureReposHostProvider(context); - context.Git.GlobalConfiguration.Dictionary[AzDevUseHttpPathKey] = new List {"true"}; + context.Git.Configuration.Global[AzDevUseHttpPathKey] = new List {"true"}; await provider.UnconfigureAsync(ConfigurationTarget.User); - Assert.Empty(context.Git.GlobalConfiguration.Dictionary); + Assert.Empty(context.Git.Configuration.Global); } [PlatformFact(Platforms.Windows)] @@ -244,12 +244,12 @@ public async Task AzureReposHostProvider_UnconfigureAsync_System_Windows_UseHttp var context = new TestCommandContext(); var provider = new AzureReposHostProvider(context); - context.Git.SystemConfiguration.Dictionary[HelperKey] = new List {"manager-core"}; - context.Git.SystemConfiguration.Dictionary[AzDevUseHttpPathKey] = new List {"true"}; + context.Git.Configuration.System[HelperKey] = new List {"manager-core"}; + context.Git.Configuration.System[AzDevUseHttpPathKey] = new List {"true"}; await provider.UnconfigureAsync(ConfigurationTarget.System); - Assert.True(context.Git.SystemConfiguration.Dictionary.TryGetValue(AzDevUseHttpPathKey, out IList actualValues)); + Assert.True(context.Git.Configuration.System.TryGetValue(AzDevUseHttpPathKey, out IList actualValues)); Assert.Single(actualValues); Assert.Equal("true", actualValues[0]); } @@ -260,11 +260,11 @@ public async Task AzureReposHostProvider_UnconfigureAsync_System_Windows_UseHttp var context = new TestCommandContext(); var provider = new AzureReposHostProvider(context); - context.Git.SystemConfiguration.Dictionary[AzDevUseHttpPathKey] = new List {"true"}; + context.Git.Configuration.System[AzDevUseHttpPathKey] = new List {"true"}; await provider.UnconfigureAsync(ConfigurationTarget.System); - Assert.Empty(context.Git.SystemConfiguration.Dictionary); + Assert.Empty(context.Git.Configuration.System); } [PlatformFact(Platforms.Windows)] @@ -273,12 +273,12 @@ public async Task AzureReposHostProvider_UnconfigureAsync_User_Windows_UseHttpPa var context = new TestCommandContext(); var provider = new AzureReposHostProvider(context); - context.Git.GlobalConfiguration.Dictionary[HelperKey] = new List {"manager-core"}; - context.Git.GlobalConfiguration.Dictionary[AzDevUseHttpPathKey] = new List {"true"}; + context.Git.Configuration.Global[HelperKey] = new List {"manager-core"}; + context.Git.Configuration.Global[AzDevUseHttpPathKey] = new List {"true"}; await provider.UnconfigureAsync(ConfigurationTarget.User); - Assert.False(context.Git.GlobalConfiguration.Dictionary.TryGetValue(AzDevUseHttpPathKey, out _)); + Assert.False(context.Git.Configuration.Global.TryGetValue(AzDevUseHttpPathKey, out _)); } private static IMicrosoftAuthenticationResult CreateAuthResult(string upn, string token) diff --git a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs index 72a42831d..50e8d6921 100644 --- a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs +++ b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs @@ -295,7 +295,7 @@ public Task ConfigureAsync(ConfigurationTarget target) ? GitConfigurationLevel.System : GitConfigurationLevel.Global; - IGitConfiguration targetConfig = _context.Git.GetConfiguration(configurationLevel); + IGitConfiguration targetConfig = _context.Git.GetConfiguration(); if (targetConfig.TryGet(useHttpPathKey, out string currentValue) && currentValue.IsTruthy()) { @@ -304,7 +304,7 @@ public Task ConfigureAsync(ConfigurationTarget target) else { _context.Trace.WriteLine("Setting Git configuration 'credential.useHttpPath' to 'true' for https://dev.azure.com..."); - targetConfig.Set(useHttpPathKey, "true"); + targetConfig.Set(configurationLevel, useHttpPathKey, "true"); } return Task.CompletedTask; @@ -321,14 +321,14 @@ public Task UnconfigureAsync(ConfigurationTarget target) ? GitConfigurationLevel.System : GitConfigurationLevel.Global; - IGitConfiguration targetConfig = _context.Git.GetConfiguration(configurationLevel); + IGitConfiguration targetConfig = _context.Git.GetConfiguration(); // On Windows, if there is a "manager-core" entry remaining in the system config then we must not clear // the useHttpPath option otherwise this would break the bundled version of GCM Core in Git for Windows. if (!PlatformUtils.IsWindows() || target != ConfigurationTarget.System || targetConfig.GetAll(helperKey).All(x => !string.Equals(x, "manager-core"))) { - targetConfig.Unset(useHttpPathKey); + targetConfig.Unset(configurationLevel, useHttpPathKey); } return Task.CompletedTask; diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/ApplicationTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/ApplicationTests.cs index bdb78ad0e..3d8b7bbd3 100644 --- a/src/shared/Microsoft.Git.CredentialManager.Tests/ApplicationTests.cs +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/ApplicationTests.cs @@ -20,8 +20,8 @@ public async Task Application_ConfigureAsync_NoHelpers_AddsEmptyAndGcm() IConfigurableComponent application = new Application(context, executablePath); await application.ConfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(key, out var actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(key, out var actualValues)); Assert.Equal(2, actualValues.Count); Assert.Equal(emptyHelper, actualValues[0]); Assert.Equal(executablePath, actualValues[1]); @@ -37,12 +37,12 @@ public async Task Application_ConfigureAsync_Gcm_AddsEmptyBeforeGcm() var context = new TestCommandContext(); IConfigurableComponent application = new Application(context, executablePath); - context.Git.GlobalConfiguration.Dictionary[key] = new List {executablePath}; + context.Git.Configuration.Global[key] = new List {executablePath}; await application.ConfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(key, out var actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(key, out var actualValues)); Assert.Equal(2, actualValues.Count); Assert.Equal(emptyHelper, actualValues[0]); Assert.Equal(executablePath, actualValues[1]); @@ -58,15 +58,15 @@ public async Task Application_ConfigureAsync_EmptyAndGcm_DoesNothing() var context = new TestCommandContext(); IConfigurableComponent application = new Application(context, executablePath); - context.Git.GlobalConfiguration.Dictionary[key] = new List + context.Git.Configuration.Global[key] = new List { emptyHelper, executablePath }; await application.ConfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(key, out var actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(key, out var actualValues)); Assert.Equal(2, actualValues.Count); Assert.Equal(emptyHelper, actualValues[0]); Assert.Equal(executablePath, actualValues[1]); @@ -83,15 +83,15 @@ public async Task Application_ConfigureAsync_EmptyAndGcmWithOthersBefore_DoesNot var context = new TestCommandContext(); IConfigurableComponent application = new Application(context, executablePath); - context.Git.GlobalConfiguration.Dictionary[key] = new List + context.Git.Configuration.Global[key] = new List { beforeHelper, emptyHelper, executablePath }; await application.ConfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(key, out var actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(key, out var actualValues)); Assert.Equal(3, actualValues.Count); Assert.Equal(beforeHelper, actualValues[0]); Assert.Equal(emptyHelper, actualValues[1]); @@ -109,15 +109,15 @@ public async Task Application_ConfigureAsync_EmptyAndGcmWithOthersAfter_DoesNoth var context = new TestCommandContext(); IConfigurableComponent application = new Application(context, executablePath); - context.Git.GlobalConfiguration.Dictionary[key] = new List + context.Git.Configuration.Global[key] = new List { emptyHelper, executablePath, afterHelper }; await application.ConfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(key, out var actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(key, out var actualValues)); Assert.Equal(3, actualValues.Count); Assert.Equal(emptyHelper, actualValues[0]); Assert.Equal(executablePath, actualValues[1]); @@ -136,15 +136,15 @@ public async Task Application_ConfigureAsync_EmptyAndGcmWithOthersBeforeAndAfter var context = new TestCommandContext(); IConfigurableComponent application = new Application(context, executablePath); - context.Git.GlobalConfiguration.Dictionary[key] = new List + context.Git.Configuration.Global[key] = new List { beforeHelper, emptyHelper, executablePath, afterHelper }; await application.ConfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(key, out var actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(key, out var actualValues)); Assert.Equal(4, actualValues.Count); Assert.Equal(beforeHelper, actualValues[0]); Assert.Equal(emptyHelper, actualValues[1]); @@ -163,15 +163,15 @@ public async Task Application_ConfigureAsync_EmptyAndGcmWithEmptyAfter_RemovesEx var context = new TestCommandContext(); IConfigurableComponent application = new Application(context, executablePath); - context.Git.GlobalConfiguration.Dictionary[key] = new List + context.Git.Configuration.Global[key] = new List { emptyHelper, executablePath, emptyHelper, afterHelper }; await application.ConfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(key, out var actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(key, out var actualValues)); Assert.Equal(5, actualValues.Count); Assert.Equal(emptyHelper, actualValues[0]); Assert.Equal(emptyHelper, actualValues[1]); @@ -190,7 +190,7 @@ public async Task Application_UnconfigureAsync_NoHelpers_DoesNothing() IConfigurableComponent application = new Application(context, executablePath); await application.UnconfigureAsync(ConfigurationTarget.User); - Assert.Empty(context.Git.GlobalConfiguration.Dictionary); + Assert.Empty(context.Git.Configuration.Global); } [Fact] @@ -202,11 +202,11 @@ public async Task Application_UnconfigureAsync_Gcm_RemovesGcm() var context = new TestCommandContext(); IConfigurableComponent application = new Application(context, executablePath); - context.Git.GlobalConfiguration.Dictionary[key] = new List {executablePath}; + context.Git.Configuration.Global[key] = new List {executablePath}; await application.UnconfigureAsync(ConfigurationTarget.User); - Assert.Empty(context.Git.GlobalConfiguration.Dictionary); + Assert.Empty(context.Git.Configuration.Global); } [Fact] @@ -219,11 +219,11 @@ public async Task Application_UnconfigureAsync_EmptyAndGcm_RemovesEmptyAndGcm() var context = new TestCommandContext(); IConfigurableComponent application = new Application(context, executablePath); - context.Git.GlobalConfiguration.Dictionary[key] = new List {emptyHelper, executablePath}; + context.Git.Configuration.Global[key] = new List {emptyHelper, executablePath}; await application.UnconfigureAsync(ConfigurationTarget.User); - Assert.Empty(context.Git.GlobalConfiguration.Dictionary); + Assert.Empty(context.Git.Configuration.Global); } [Fact] @@ -237,15 +237,15 @@ public async Task Application_UnconfigureAsync_EmptyAndGcmWithOthersBefore_Remov var context = new TestCommandContext(); IConfigurableComponent application = new Application(context, executablePath); - context.Git.GlobalConfiguration.Dictionary[key] = new List + context.Git.Configuration.Global[key] = new List { beforeHelper, emptyHelper, executablePath }; await application.UnconfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(key, out var actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(key, out var actualValues)); Assert.Equal(1, actualValues.Count); Assert.Equal(beforeHelper, actualValues[0]); } @@ -261,15 +261,15 @@ public async Task Application_UnconfigureAsync_EmptyAndGcmWithOthersAfterBefore_ var context = new TestCommandContext(); IConfigurableComponent application = new Application(context, executablePath); - context.Git.GlobalConfiguration.Dictionary[key] = new List + context.Git.Configuration.Global[key] = new List { emptyHelper, executablePath, afterHelper }; await application.UnconfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(key, out var actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(key, out var actualValues)); Assert.Equal(2, actualValues.Count); Assert.Equal(emptyHelper, actualValues[0]); Assert.Equal(afterHelper, actualValues[1]); @@ -287,15 +287,15 @@ public async Task Application_UnconfigureAsync_EmptyAndGcmWithOthersBeforeAndAft var context = new TestCommandContext(); IConfigurableComponent application = new Application(context, executablePath); - context.Git.GlobalConfiguration.Dictionary[key] = new List + context.Git.Configuration.Global[key] = new List { beforeHelper, emptyHelper, executablePath, afterHelper }; await application.UnconfigureAsync(ConfigurationTarget.User); - Assert.Single(context.Git.GlobalConfiguration.Dictionary); - Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(key, out var actualValues)); + Assert.Single(context.Git.Configuration.Global); + Assert.True(context.Git.Configuration.Global.TryGetValue(key, out var actualValues)); Assert.Equal(3, actualValues.Count); Assert.Equal(beforeHelper, actualValues[0]); Assert.Equal(emptyHelper, actualValues[1]); diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs index bc9f12036..1d930e979 100644 --- a/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs @@ -200,9 +200,9 @@ public void GitConfiguration_Set_Local_SetsLocalConfig() string gitPath = GetGitPath(); var trace = new NullTrace(); var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(GitConfigurationLevel.Local); + IGitConfiguration config = git.GetConfiguration(); - config.Set("core.foobar", "foo123"); + config.Set(GitConfigurationLevel.Local, "core.foobar", "foo123"); GitResult localResult = Git(repoPath, workDirPath, "config --local core.foobar"); @@ -217,9 +217,9 @@ public void GitConfiguration_Set_All_ThrowsException() string gitPath = GetGitPath(); var trace = new NullTrace(); var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(GitConfigurationLevel.All); + IGitConfiguration config = git.GetConfiguration(); - Assert.Throws(() => config.Set("core.foobar", "test123")); + Assert.Throws(() => config.Set(GitConfigurationLevel.All, "core.foobar", "test123")); } [Fact] @@ -235,9 +235,9 @@ public void GitConfiguration_Unset_Global_UnsetsGlobalConfig() string gitPath = GetGitPath(); var trace = new NullTrace(); var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(GitConfigurationLevel.Global); + IGitConfiguration config = git.GetConfiguration(); - config.Unset("core.foobar"); + config.Unset(GitConfigurationLevel.Global, "core.foobar"); GitResult globalResult = Git(repoPath, workDirPath, "config --global core.foobar"); GitResult localResult = Git(repoPath, workDirPath, "config --local core.foobar"); @@ -265,9 +265,9 @@ public void GitConfiguration_Unset_Local_UnsetsLocalConfig() string gitPath = GetGitPath(); var trace = new NullTrace(); var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(GitConfigurationLevel.Local); + IGitConfiguration config = git.GetConfiguration(); - config.Unset("core.foobar"); + config.Unset(GitConfigurationLevel.Local, "core.foobar"); GitResult globalResult = Git(repoPath, workDirPath, "config --global core.foobar"); GitResult localResult = Git(repoPath, workDirPath, "config --local core.foobar"); @@ -290,9 +290,9 @@ public void GitConfiguration_Unset_All_ThrowsException() string gitPath = GetGitPath(); var trace = new NullTrace(); var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(GitConfigurationLevel.All); + IGitConfiguration config = git.GetConfiguration(); - Assert.Throws(() => config.Unset("core.foobar")); + Assert.Throws(() => config.Unset(GitConfigurationLevel.All, "core.foobar")); } [Fact] @@ -306,9 +306,9 @@ public void GitConfiguration_UnsetAll_UnsetsAllConfig() string gitPath = GetGitPath(); var trace = new NullTrace(); var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(GitConfigurationLevel.Local); + IGitConfiguration config = git.GetConfiguration(); - config.UnsetAll("core.foobar", "foo*"); + config.UnsetAll(GitConfigurationLevel.Local, "core.foobar", "foo*"); GitResult result = Git(repoPath, workDirPath, "config --local --get-all core.foobar"); @@ -323,9 +323,9 @@ public void GitConfiguration_UnsetAll_All_ThrowsException() string gitPath = GetGitPath(); var trace = new NullTrace(); var git = new GitProcess(trace, gitPath, repoPath); - IGitConfiguration config = git.GetConfiguration(GitConfigurationLevel.All); + IGitConfiguration config = git.GetConfiguration(); - Assert.Throws(() => config.UnsetAll("core.foobar", Constants.RegexPatterns.Any)); + Assert.Throws(() => config.UnsetAll(GitConfigurationLevel.All, "core.foobar", Constants.RegexPatterns.Any)); } #region Test helpers diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/SettingsTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/SettingsTests.cs index 027314236..e7a5d9111 100644 --- a/src/shared/Microsoft.Git.CredentialManager.Tests/SettingsTests.cs +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/SettingsTests.cs @@ -135,7 +135,7 @@ public void Settings_IsInteractionAllowed_ConfigAuto_ReturnsTrue() var envars = new TestEnvironment(); var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = "auto"; + git.Configuration.Global[$"{section}.{property}"] = new[] {"auto"}; var settings = new Settings(envars, git); @@ -150,7 +150,7 @@ public void Settings_IsInteractionAllowed_ConfigAlways_ReturnsTrue() var envars = new TestEnvironment(); var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = "always"; + git.Configuration.Global[$"{section}.{property}"] = new[] {"always"}; var settings = new Settings(envars, git); @@ -165,7 +165,7 @@ public void Settings_IsInteractionAllowed_ConfigNever_ReturnsFalse() var envars = new TestEnvironment(); var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = "never"; + git.Configuration.Global[$"{section}.{property}"] = new[] {"never"}; var settings = new Settings(envars, git); @@ -180,7 +180,7 @@ public void Settings_IsInteractionAllowed_ConfigTruthy_ReturnsTrue() var envars = new TestEnvironment(); var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = "1"; + git.Configuration.Global[$"{section}.{property}"] = new[] {"1"}; var settings = new Settings(envars, git); @@ -195,7 +195,7 @@ public void Settings_IsInteractionAllowed_ConfigFalsey_ReturnsFalse() var envars = new TestEnvironment(); var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = "0"; + git.Configuration.Global[$"{section}.{property}"] = new[] {"0"}; var settings = new Settings(envars, git); @@ -210,7 +210,7 @@ public void Settings_IsInteractionAllowed_ConfigNonBooleanyValue_ReturnsTrue() var envars = new TestEnvironment(); var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = Guid.NewGuid().ToString(); + git.Configuration.Global[$"{section}.{property}"] = new[] {Guid.NewGuid().ToString()}; var settings = new Settings(envars, git); @@ -392,7 +392,7 @@ public void Settings_IsWindowsIntegratedAuthenticationEnabled_ConfigTruthy_Retur var envars = new TestEnvironment(); var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = "1"; + git.Configuration.Global[$"{section}.{property}"] = new[] {"1"}; var settings = new Settings(envars, git); @@ -407,7 +407,7 @@ public void Settings_IsWindowsIntegratedAuthenticationEnabled_ConfigFalsey_Retur var envars = new TestEnvironment(); var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = "0"; + git.Configuration.Global[$"{section}.{property}"] = new[] {"0"}; var settings = new Settings(envars, git); @@ -422,7 +422,7 @@ public void Settings_IsWindowsIntegratedAuthenticationEnabled_ConfigNonBooleanyV var envars = new TestEnvironment(); var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = Guid.NewGuid().ToString(); + git.Configuration.Global[$"{section}.{property}"] = new[] {Guid.NewGuid().ToString()}; var settings = new Settings(envars, git); @@ -467,7 +467,7 @@ public void Settings_ProxyConfiguration_GcmHttpConfig_ReturnsValue() Variables = {[Constants.EnvironmentVariables.CurlNoProxy] = string.Join(',', bypassList)} }; var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = settingValue.ToString(); + git.Configuration.Global[$"{section}.{property}"] = new[] {settingValue.ToString()}; var settings = new Settings(envars, git) { @@ -503,7 +503,7 @@ public void Settings_ProxyConfiguration_GcmHttpsConfig_ReturnsValue() Variables = {[Constants.EnvironmentVariables.CurlNoProxy] = string.Join(',', bypassList)} }; var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = settingValue.ToString(); + git.Configuration.Global[$"{section}.{property}"] = new[] {settingValue.ToString()}; var settings = new Settings(envars, git) { @@ -539,7 +539,7 @@ public void Settings_ProxyConfiguration_GitHttpConfig_ReturnsValue() Variables = {[Constants.EnvironmentVariables.CurlNoProxy] = string.Join(',', bypassList)} }; var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = settingValue.ToString(); + git.Configuration.Global[$"{section}.{property}"] = new[] {settingValue.ToString()}; var settings = new Settings(envars, git) { @@ -575,7 +575,7 @@ public void Settings_ProxyConfiguration_NoProxyMixedSplitChar_ReturnsValue() Variables = {[Constants.EnvironmentVariables.CurlNoProxy] = "contoso.com, fabrikam.com example.com,"} }; var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = settingValue.ToString(); + git.Configuration.Global[$"{section}.{property}"] = new[] {settingValue.ToString()}; var settings = new Settings(envars, git) { @@ -782,12 +782,14 @@ void RunTest(Uri expectedValue) RunTest(value2); // Test case 2: http.proxy > cURL environment variables - git.GlobalConfiguration[$"{Constants.GitConfiguration.Http.SectionName}.{Constants.GitConfiguration.Http.Proxy}"] = value3.ToString(); - RunTest(value3); + string httpProxyKey = $"{Constants.GitConfiguration.Http.SectionName}.{Constants.GitConfiguration.Http.Proxy}"; + git.Configuration.Global[httpProxyKey] = new[] {value3.ToString()}; + RunTest(value3); // Test case 3: credential.httpProxy > http.proxy - git.GlobalConfiguration[$"{Constants.GitConfiguration.Credential.SectionName}.{Constants.GitConfiguration.Credential.HttpProxy}"] = value4.ToString(); - RunTest(value4); + string credentialProxyKey = $"{Constants.GitConfiguration.Credential.SectionName}.{Constants.GitConfiguration.Credential.HttpProxy}"; + git.Configuration.Global[credentialProxyKey] = new[] {value4.ToString()}; + RunTest(value4); } [Fact] @@ -843,7 +845,7 @@ public void Settings_ProviderOverride_ConfigSet_ReturnsValue() var envars = new TestEnvironment(); var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = expectedValue; + git.Configuration.Global[$"{section}.{property}"] = new[] {expectedValue}; var settings = new Settings(envars, git) { @@ -870,7 +872,7 @@ public void Settings_ProviderOverride_EnvarAndConfigSet_ReturnsEnvarValue() Variables = {[Constants.EnvironmentVariables.GcmProvider] = expectedValue} }; var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = otherValue; + git.Configuration.Global[$"{section}.{property}"] = new[] {otherValue}; var settings = new Settings(envars, git) { @@ -934,7 +936,7 @@ public void Settings_LegacyAuthorityOverride_ConfigSet_ReturnsTrueOutValue() var envars = new TestEnvironment(); var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = expectedValue; + git.Configuration.Global[$"{section}.{property}"] = new[] {expectedValue}; var settings = new Settings(envars, git) { @@ -961,7 +963,7 @@ public void Settings_LegacyAuthorityOverride_EnvarAndConfigSet_ReturnsEnvarValue Variables = {[Constants.EnvironmentVariables.GcmAuthority] = expectedValue} }; var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = otherValue; + git.Configuration.Global[$"{section}.{property}"] = new[] {otherValue}; var settings = new Settings(envars, git) { @@ -1034,7 +1036,7 @@ public void Settings_TryGetSetting_GlobalConfig_ReturnsTrueAndValue() var envars = new TestEnvironment(); var git = new TestGit(); - git.GlobalConfiguration[$"{section}.{property}"] = expectedValue; + git.Configuration.Global[$"{section}.{property}"] = new[] {expectedValue}; var settings = new Settings(envars, git) { @@ -1059,7 +1061,7 @@ public void Settings_TryGetSetting_RepoConfig_ReturnsTrueAndValue() var envars = new TestEnvironment(); var git = new TestGit(); - git.LocalConfiguration[$"{section}.{property}"] = expectedValue; + git.Configuration.Local[$"{section}.{property}"] = new[] {expectedValue}; var settings = new Settings(envars, git) { @@ -1087,8 +1089,8 @@ public void Settings_TryGetSetting_ScopedConfig() var envars = new TestEnvironment(); var git = new TestGit(); - git.LocalConfiguration[$"{section}.{scope1}.{property}"] = otherValue; - git.LocalConfiguration[$"{section}.{scope2}.{property}"] = expectedValue; + git.Configuration.Local[$"{section}.{scope1}.{property}"] = new []{otherValue}; + git.Configuration.Local[$"{section}.{scope2}.{property}"] = new []{expectedValue}; var settings = new Settings(envars, git) { @@ -1117,7 +1119,7 @@ public void Settings_TryGetSetting_EnvarAndConfig_EnvarTakesPrecedence() Variables = {[envarName] = expectedValue} }; var git = new TestGit(); - git.LocalConfiguration[$"{section}.{property}"] = otherValue; + git.Configuration.Local[$"{section}.{property}"] = new[] {otherValue}; var settings = new Settings(envars, git) { @@ -1152,9 +1154,9 @@ public void Settings_GetSettingValues_EnvarAndMultipleConfig_ReturnsAllWithCorre Variables = {[envarName] = value1} }; var git = new TestGit(); - git.LocalConfiguration[$"{section}.{scope1}.{property}"] = value2; - git.LocalConfiguration[$"{section}.{scope2}.{property}"] = value3; - git.LocalConfiguration[$"{section}.{property}"] = value4; + git.Configuration.Local[$"{section}.{scope1}.{property}"] = new[]{value2}; + git.Configuration.Local[$"{section}.{scope2}.{property}"] = new[]{value3}; + git.Configuration.Local[$"{section}.{property}"] = new[]{value4}; var settings = new Settings(envars, git) { @@ -1193,11 +1195,11 @@ public void Settings_GetSettingValues_ReturnsAllMatchingValues() }; var git = new TestGit(); - git.LocalConfiguration[$"{section}.{property}"] = noScopeValue; - git.LocalConfiguration[$"{section}.{broadScope}.{property}"] = broadScopeValue; - git.LocalConfiguration[$"{section}.{tightScope}.{property}"] = tightScopeValue; - git.LocalConfiguration[$"{section}.{otherScope1}.{property}"] = otherValue1; - git.LocalConfiguration[$"{section}.{otherScope2}.{property}"] = otherValue2; + git.Configuration.Local[$"{section}.{property}"] = new[] {noScopeValue}; + git.Configuration.Local[$"{section}.{broadScope}.{property}"] = new[] {broadScopeValue}; + git.Configuration.Local[$"{section}.{tightScope}.{property}"] = new[] {tightScopeValue}; + git.Configuration.Local[$"{section}.{otherScope1}.{property}"] = new[] {otherValue1}; + git.Configuration.Local[$"{section}.{otherScope2}.{property}"] = new[] {otherValue2}; var settings = new Settings(envars, git) { @@ -1238,9 +1240,9 @@ public void Settings_GetSettingValues_IgnoresSectionAndPropertyCase_ScopeIsCaseS }; var git = new TestGit(); - git.LocalConfiguration[$"{sectionLo}.{propertyHi}"] = noScopeValue; - git.LocalConfiguration[$"{sectionHi}.{scopeLo}.{propertyHi}"] = lowScopeValue; - git.LocalConfiguration[$"{sectionLo}.{scopeHi}.{propertyLo}"] = highScopeValue; + git.Configuration.Local[$"{sectionLo}.{propertyHi}"] = new[] {noScopeValue}; + git.Configuration.Local[$"{sectionHi}.{scopeLo}.{propertyHi}"] = new[] {lowScopeValue}; + git.Configuration.Local[$"{sectionLo}.{scopeHi}.{propertyLo}"] = new[] {highScopeValue}; var settings = new Settings(envars, git) { diff --git a/src/shared/Microsoft.Git.CredentialManager/Application.cs b/src/shared/Microsoft.Git.CredentialManager/Application.cs index 5274c14ad..fbbefe61a 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Application.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Application.cs @@ -122,6 +122,10 @@ private bool WriteException(Exception ex) // Try and use a nicer format for some well-known exception types switch (ex) { + case GitException gitEx: + Context.Streams.Error.WriteLine("fatal: {0} [{1}]", gitEx.Message, gitEx.ExitCode); + Context.Streams.Error.WriteLine(gitEx.GitErrorMessage); + break; case InteropException interopEx: Context.Streams.Error.WriteLine("fatal: {0} [0x{1:x}]", interopEx.Message, interopEx.ErrorCode); break; @@ -154,7 +158,7 @@ Task IConfigurableComponent.ConfigureAsync(ConfigurationTarget target) Context.Trace.WriteLine($"Configuring for config level '{configLevel}'."); - IGitConfiguration config = Context.Git.GetConfiguration(configLevel); + IGitConfiguration config = Context.Git.GetConfiguration(); // We are looking for the following to be set in the config: // @@ -164,7 +168,7 @@ Task IConfigurableComponent.ConfigureAsync(ConfigurationTarget target) // helper = {appPath} # the expected executable value & directly following the empty value // ... # any number of helper entries (possibly none, but not the empty value '') // - string[] currentValues = config.GetAll(helperKey).ToArray(); + string[] currentValues = config.GetAll(configLevel, helperKey).ToArray(); // Try to locate an existing app entry with a blank reset/clear entry immediately preceding, // and no other blank empty/clear entries following (which effectively disable us). @@ -179,12 +183,12 @@ Task IConfigurableComponent.ConfigureAsync(ConfigurationTarget target) Context.Trace.WriteLine("Updating Git credential helper configuration..."); // Clear any existing app entries in the configuration - config.UnsetAll(helperKey, Regex.Escape(appPath)); + config.UnsetAll(configLevel, helperKey, Regex.Escape(appPath)); // Add an empty value for `credential.helper`, which has the effect of clearing any helper value // from any lower-level Git configuration, then add a second value which is the actual executable path. - config.Add(helperKey, string.Empty); - config.Add(helperKey, appPath); + config.Add(configLevel, helperKey, string.Empty); + config.Add(configLevel, helperKey, appPath); } return Task.CompletedTask; @@ -201,7 +205,7 @@ Task IConfigurableComponent.UnconfigureAsync(ConfigurationTarget target) Context.Trace.WriteLine($"Unconfiguring for config level '{configLevel}'."); - IGitConfiguration config = Context.Git.GetConfiguration(configLevel); + IGitConfiguration config = Context.Git.GetConfiguration(); // We are looking for the following to be set in the config: // @@ -215,7 +219,7 @@ Task IConfigurableComponent.UnconfigureAsync(ConfigurationTarget target) // Context.Trace.WriteLine("Removing Git credential helper configuration..."); - string[] currentValues = config.GetAll(helperKey).ToArray(); + string[] currentValues = config.GetAll(configLevel, helperKey).ToArray(); int appIndex = Array.FindIndex(currentValues, x => Context.FileSystem.IsSamePath(x, appPath)); if (appIndex > -1) @@ -225,11 +229,11 @@ Task IConfigurableComponent.UnconfigureAsync(ConfigurationTarget target) string.IsNullOrWhiteSpace(currentValues[appIndex - 1])) { // Clear the blank entry - config.UnsetAll(helperKey, Constants.RegexPatterns.Empty); + config.UnsetAll(configLevel, helperKey, Constants.RegexPatterns.Empty); } // Clear app entry - config.UnsetAll(helperKey, Regex.Escape(appPath)); + config.UnsetAll(configLevel, helperKey, Regex.Escape(appPath)); } return Task.CompletedTask; diff --git a/src/shared/Microsoft.Git.CredentialManager/Git.cs b/src/shared/Microsoft.Git.CredentialManager/Git.cs index 738d7c903..74c316eca 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Git.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Git.cs @@ -10,11 +10,10 @@ namespace Microsoft.Git.CredentialManager public interface IGit { /// - /// Get the configuration for the specific configuration level. + /// Get the configuration object. /// - /// Configuration level filter. /// Git configuration. - IGitConfiguration GetConfiguration(GitConfigurationLevel level); + IGitConfiguration GetConfiguration(); /// /// Run a Git helper process which expects and returns key-value maps @@ -41,9 +40,9 @@ public GitProcess(ITrace trace, string gitPath, string workingDirectory = null) _workingDirectory = workingDirectory; } - public IGitConfiguration GetConfiguration(GitConfigurationLevel level) + public IGitConfiguration GetConfiguration() { - return new GitProcessConfiguration(_trace, this, level); + return new GitProcessConfiguration(_trace, this); } public Process CreateProcess(string args) @@ -107,15 +106,29 @@ public Process CreateProcess(string args) return resultDict; } + + public static GitException CreateGitException(Process git, string message) + { + string gitMessage = git.StandardError.ReadToEnd(); + throw new GitException(message, gitMessage, git.ExitCode); + } + } + + public class GitException : Exception + { + public string GitErrorMessage { get; } + + public int ExitCode { get; } + + public GitException(string message, string gitErrorMessage, int exitCode) + : base(message) + { + GitErrorMessage = gitErrorMessage; + ExitCode = exitCode; + } } public static class GitExtensions { - /// - /// Get the configuration. - /// - /// Git object. - /// Git configuration. - public static IGitConfiguration GetConfiguration(this IGit git) => git.GetConfiguration(GitConfigurationLevel.All); } } diff --git a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs index 74ae0c96a..999bb8f22 100644 --- a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs +++ b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Linq; using System.Text; namespace Microsoft.Git.CredentialManager @@ -29,89 +28,96 @@ public interface IGitConfiguration /// /// Enumerate all configuration entries invoking the specified callback for each entry. /// + /// Filter to the specific configuration level. /// Callback to invoke for each configuration entry. - void Enumerate(GitConfigurationEnumerationCallback cb); + void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCallback cb); /// /// Try and get the value of a configuration entry as a string. /// + /// Filter to the specific configuration level. /// Configuration entry name. /// Configuration entry value. /// True if the value was found, false otherwise. - bool TryGet(string name, out string value); + bool TryGet(GitConfigurationLevel level, string name, out string value); /// /// Set the value of a configuration entry. /// + /// Filter to the specific configuration level. /// Configuration entry name. /// Configuration entry value. - void Set(string name, string value); + void Set(GitConfigurationLevel level, string name, string value); /// /// Add a new value for a configuration entry. /// + /// Filter to the specific configuration level. /// Configuration entry name. /// Configuration entry value. - void Add(string name, string value); + void Add(GitConfigurationLevel level, string name, string value); /// /// Deletes a configuration entry from the highest level. /// + /// Filter to the specific configuration level. /// Configuration entry name. - void Unset(string name); + void Unset(GitConfigurationLevel level, string name); /// /// Get all value of a multivar configuration entry. /// + /// Filter to the specific configuration level. /// Configuration entry name. /// All values of the multivar configuration entry. - IEnumerable GetAll(string name); + IEnumerable GetAll(GitConfigurationLevel level, string name); /// /// Get all values of a multivar configuration entry. /// + /// Filter to the specific configuration level. /// Configuration entry name regular expression. /// Regular expression to filter which variables we're interested in. Use null to indicate all. /// All values of the multivar configuration entry. - IEnumerable GetRegex(string nameRegex, string valueRegex); + IEnumerable GetRegex(GitConfigurationLevel level, string nameRegex, string valueRegex); /// /// Set a multivar configuration entry value. /// + /// Filter to the specific configuration level. /// Configuration entry name regular expression. /// Regular expression to indicate which values to replace. /// Configuration entry value. /// If the regular expression does not match any existing entry, a new entry is created. - void ReplaceAll(string nameRegex, string valueRegex, string value); + void ReplaceAll(GitConfigurationLevel level, string nameRegex, string valueRegex, string value); /// /// Deletes one or several entries from a multivar. /// + /// Filter to the specific configuration level. /// Configuration entry name. /// Regular expression to indicate which values to delete. - void UnsetAll(string name, string valueRegex); + void UnsetAll(GitConfigurationLevel level, string name, string valueRegex); } public class GitProcessConfiguration : IGitConfiguration { private readonly ITrace _trace; private readonly GitProcess _git; - private readonly GitConfigurationLevel? _filterLevel; - internal GitProcessConfiguration(ITrace trace, GitProcess git, GitConfigurationLevel filterLevel = GitConfigurationLevel.All) + internal GitProcessConfiguration(ITrace trace, GitProcess git) { EnsureArgument.NotNull(trace, nameof(trace)); EnsureArgument.NotNull(git, nameof(git)); _trace = trace; _git = git; - _filterLevel = filterLevel; } - public void Enumerate(GitConfigurationEnumerationCallback cb) + public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCallback cb) { - string level = GetLevelFilterArg(); - using (Process git = _git.CreateProcess($"config --null {level} --list --show-scope")) + string levelArg = GetLevelFilterArg(level); + using (Process git = _git.CreateProcess($"config --null {levelArg} --list --show-scope")) { git.Start(); // To avoid deadlocks, always read the output stream first and then wait @@ -124,8 +130,8 @@ public void Enumerate(GitConfigurationEnumerationCallback cb) case 0: // OK break; default: - _trace.WriteLine($"Failed to enumerate config entries (exit={git.ExitCode}, level={_filterLevel})"); - throw CreateGitException(git, "Failed to enumerate all Git configuration entries"); + _trace.WriteLine($"Failed to enumerate config entries (exit={git.ExitCode}, level={level})"); + throw GitProcess.CreateGitException(git, "Failed to enumerate all Git configuration entries"); } var scope = new StringBuilder(); @@ -192,10 +198,10 @@ public void Enumerate(GitConfigurationEnumerationCallback cb) } } - public bool TryGet(string name, out string value) + public bool TryGet(GitConfigurationLevel level, string name, out string value) { - string level = GetLevelFilterArg(); - using (Process git = _git.CreateProcess($"config --null {level} {QuoteCmdArg(name)}")) + string levelArg = GetLevelFilterArg(level); + using (Process git = _git.CreateProcess($"config --null {levelArg} {QuoteCmdArg(name)}")) { git.Start(); git.WaitForExit(); @@ -208,7 +214,7 @@ public bool TryGet(string name, out string value) value = null; return false; default: // Error - _trace.WriteLine($"Failed to read Git configuration entry '{name}'. (exit={git.ExitCode}, level={_filterLevel})"); + _trace.WriteLine($"Failed to read Git configuration entry '{name}'. (exit={git.ExitCode}, level={level})"); value = null; return false; } @@ -226,15 +232,12 @@ public bool TryGet(string name, out string value) } } - public void Set(string name, string value) + public void Set(GitConfigurationLevel level, string name, string value) { - if (_filterLevel == GitConfigurationLevel.All) - { - throw new InvalidOperationException("Must have a specific configuration level filter to modify values."); - } + EnsureSpecificLevel(level); - string level = GetLevelFilterArg(); - using (Process git = _git.CreateProcess($"config {level} {QuoteCmdArg(name)} {QuoteCmdArg(value)}")) + string levelArg = GetLevelFilterArg(level); + using (Process git = _git.CreateProcess($"config {levelArg} {QuoteCmdArg(name)} {QuoteCmdArg(value)}")) { git.Start(); git.WaitForExit(); @@ -244,21 +247,18 @@ public void Set(string name, string value) case 0: // OK break; default: - _trace.WriteLine($"Failed to set config entry '{name}' to value '{value}' (exit={git.ExitCode}, level={_filterLevel})"); - throw CreateGitException(git, $"Failed to set Git configuration entry '{name}'"); + _trace.WriteLine($"Failed to set config entry '{name}' to value '{value}' (exit={git.ExitCode}, level={level})"); + throw GitProcess.CreateGitException(git, $"Failed to set Git configuration entry '{name}'"); } } } - public void Add(string name, string value) + public void Add(GitConfigurationLevel level, string name, string value) { - if (_filterLevel == GitConfigurationLevel.All) - { - throw new InvalidOperationException("Must have a specific configuration level filter to add values."); - } + EnsureSpecificLevel(level); - string level = GetLevelFilterArg(); - using (Process git = _git.CreateProcess($"config {level} --add {QuoteCmdArg(name)} {QuoteCmdArg(value)}")) + string levelArg = GetLevelFilterArg(level); + using (Process git = _git.CreateProcess($"config {levelArg} --add {QuoteCmdArg(name)} {QuoteCmdArg(value)}")) { git.Start(); git.WaitForExit(); @@ -268,21 +268,18 @@ public void Add(string name, string value) case 0: // OK break; default: - _trace.WriteLine($"Failed to add config entry '{name}' with value '{value}' (exit={git.ExitCode}, level={_filterLevel})"); - throw CreateGitException(git, $"Failed to add Git configuration entry '{name}'"); + _trace.WriteLine($"Failed to add config entry '{name}' with value '{value}' (exit={git.ExitCode}, level={level})"); + throw GitProcess.CreateGitException(git, $"Failed to add Git configuration entry '{name}'"); } } } - public void Unset(string name) + public void Unset(GitConfigurationLevel level, string name) { - if (_filterLevel == GitConfigurationLevel.All) - { - throw new InvalidOperationException("Must have a specific configuration level filter to modify values."); - } + EnsureSpecificLevel(level); - string level = GetLevelFilterArg(); - using (Process git = _git.CreateProcess($"config {level} --unset {QuoteCmdArg(name)}")) + string levelArg = GetLevelFilterArg(level); + using (Process git = _git.CreateProcess($"config {levelArg} --unset {QuoteCmdArg(name)}")) { git.Start(); git.WaitForExit(); @@ -293,17 +290,17 @@ public void Unset(string name) case 5: // Trying to unset a value that does not exist break; default: - _trace.WriteLine($"Failed to unset config entry '{name}' (exit={git.ExitCode}, level={_filterLevel})"); - throw CreateGitException(git, $"Failed to unset Git configuration entry '{name}'"); + _trace.WriteLine($"Failed to unset config entry '{name}' (exit={git.ExitCode}, level={level})"); + throw GitProcess.CreateGitException(git, $"Failed to unset Git configuration entry '{name}'"); } } } - public IEnumerable GetAll(string name) + public IEnumerable GetAll(GitConfigurationLevel level, string name) { - string level = GetLevelFilterArg(); + string levelArg = GetLevelFilterArg(level); - var gitArgs = $"config --null {level} --get-all {QuoteCmdArg(name)}"; + var gitArgs = $"config --null {levelArg} --get-all {QuoteCmdArg(name)}"; using (Process git = _git.CreateProcess(gitArgs)) { @@ -330,17 +327,17 @@ public IEnumerable GetAll(string name) break; default: - _trace.WriteLine($"Failed to get all config entries '{name}' (exit={git.ExitCode}, level={_filterLevel})"); - throw CreateGitException(git, $"Failed to get all Git configuration entries '{name}'"); + _trace.WriteLine($"Failed to get all config entries '{name}' (exit={git.ExitCode}, level={level})"); + throw GitProcess.CreateGitException(git, $"Failed to get all Git configuration entries '{name}'"); } } } - public IEnumerable GetRegex(string nameRegex, string valueRegex) + public IEnumerable GetRegex(GitConfigurationLevel level, string nameRegex, string valueRegex) { - string level = GetLevelFilterArg(); + string levelArg = GetLevelFilterArg(level); - var gitArgs = $"config --null {level} --get-regex {QuoteCmdArg(nameRegex)}"; + var gitArgs = $"config --null {levelArg} --get-regex {QuoteCmdArg(nameRegex)}"; if (valueRegex != null) { gitArgs += $" {QuoteCmdArg(valueRegex)}"; @@ -360,8 +357,8 @@ public IEnumerable GetRegex(string nameRegex, string valueRegex) case 1: // No results break; default: - _trace.WriteLine($"Failed to get all multivar regex '{nameRegex}' and value regex '{valueRegex}' (exit={git.ExitCode}, level={_filterLevel})"); - throw CreateGitException(git, $"Failed to get Git configuration multi-valued entries with name regex '{nameRegex}'"); + _trace.WriteLine($"Failed to get all multivar regex '{nameRegex}' and value regex '{valueRegex}' (exit={git.ExitCode}, level={level})"); + throw GitProcess.CreateGitException(git, $"Failed to get Git configuration multi-valued entries with name regex '{nameRegex}'"); } string[] entries = data.Split('\0'); @@ -377,15 +374,12 @@ public IEnumerable GetRegex(string nameRegex, string valueRegex) } } - public void ReplaceAll(string name, string valueRegex, string value) + public void ReplaceAll(GitConfigurationLevel level, string name, string valueRegex, string value) { - if (_filterLevel == GitConfigurationLevel.All) - { - throw new InvalidOperationException("Must have a specific configuration level filter to modify values."); - } + EnsureSpecificLevel(level); - string level = GetLevelFilterArg(); - var gitArgs = $"config {level} --replace-all {QuoteCmdArg(name)} {QuoteCmdArg(value)}"; + string levelArg = GetLevelFilterArg(level); + var gitArgs = $"config {levelArg} --replace-all {QuoteCmdArg(name)} {QuoteCmdArg(value)}"; if (valueRegex != null) { gitArgs += $" {QuoteCmdArg(valueRegex)}"; @@ -401,21 +395,18 @@ public void ReplaceAll(string name, string valueRegex, string value) case 0: // OK break; default: - _trace.WriteLine($"Failed to replace all multivar '{name}' and value regex '{valueRegex}' with new value '{value}' (exit={git.ExitCode}, level={_filterLevel})"); - throw CreateGitException(git, $"Failed to replace all Git configuration multi-valued entries '{name}'"); + _trace.WriteLine($"Failed to replace all multivar '{name}' and value regex '{valueRegex}' with new value '{value}' (exit={git.ExitCode}, level={level})"); + throw GitProcess.CreateGitException(git, $"Failed to replace all Git configuration multi-valued entries '{name}'"); } } } - public void UnsetAll(string name, string valueRegex) + public void UnsetAll(GitConfigurationLevel level, string name, string valueRegex) { - if (_filterLevel == GitConfigurationLevel.All) - { - throw new InvalidOperationException("Must have a specific configuration level filter to modify values."); - } + EnsureSpecificLevel(level); - string level = GetLevelFilterArg(); - var gitArgs = $"config {level} --unset-all {QuoteCmdArg(name)}"; + string levelArg = GetLevelFilterArg(level); + var gitArgs = $"config {levelArg} --unset-all {QuoteCmdArg(name)}"; if (valueRegex != null) { gitArgs += $" {QuoteCmdArg(valueRegex)}"; @@ -432,32 +423,23 @@ public void UnsetAll(string name, string valueRegex) case 5: // Trying to unset a value that does not exist break; default: - _trace.WriteLine($"Failed to unset all multivar '{name}' with value regex '{valueRegex}' (exit={git.ExitCode}, level={_filterLevel})"); - throw CreateGitException(git, $"Failed to unset all Git configuration multi-valued entries '{name}'"); + _trace.WriteLine($"Failed to unset all multivar '{name}' with value regex '{valueRegex}' (exit={git.ExitCode}, level={level})"); + throw GitProcess.CreateGitException(git, $"Failed to unset all Git configuration multi-valued entries '{name}'"); } } } - private Exception CreateGitException(Process git, string message) + private static void EnsureSpecificLevel(GitConfigurationLevel level) { - var exceptionMessage = new StringBuilder(); - string gitMessage = git.StandardError.ReadToEnd(); - - if (!string.IsNullOrWhiteSpace(gitMessage)) + if (level == GitConfigurationLevel.All) { - exceptionMessage.AppendLine(gitMessage); + throw new InvalidOperationException("Must have a specific configuration level filter to modify values."); } - - exceptionMessage.AppendLine(message); - exceptionMessage.AppendLine($"Exit code: '{git.ExitCode}'"); - exceptionMessage.AppendLine($"Configuration level: {_filterLevel}"); - - throw new Exception(exceptionMessage.ToString()); } - private string GetLevelFilterArg() + private static string GetLevelFilterArg(GitConfigurationLevel level) { - switch (_filterLevel) + switch (level) { case GitConfigurationLevel.System: return "--system"; @@ -549,13 +531,24 @@ public static class GitConfigurationExtensions /// Enumerate all configuration entries invoking the specified callback for each entry. /// /// Configuration object. + /// Callback to invoke for each matching configuration entry. + public static void Enumerate(this IGitConfiguration config, GitConfigurationEnumerationCallback cb) + { + config.Enumerate(GitConfigurationLevel.All, cb); + } + + /// + /// Enumerate all configuration entries invoking the specified callback for each entry. + /// + /// Configuration object. + /// Filter to the specific configuration level. /// Optional section name to filter; use null for any. /// Optional property name to filter; use null for any. /// Callback to invoke for each matching configuration entry. public static void Enumerate(this IGitConfiguration config, - string section, string property, GitConfigurationEnumerationCallback cb) + GitConfigurationLevel level, string section, string property, GitConfigurationEnumerationCallback cb) { - config.Enumerate(entry => + config.Enumerate(level, entry => { if (GitConfigurationKeyComparer.TrySplit(entry.Key, out string entrySection, out _, out string entryProperty) && (section is null || GitConfigurationKeyComparer.SectionComparer.Equals(section, entrySection)) && @@ -568,21 +561,81 @@ public static class GitConfigurationExtensions }); } + /// + /// Enumerate all configuration entries invoking the specified callback for each entry. + /// + /// Configuration object. + /// Optional section name to filter; use null for any. + /// Optional property name to filter; use null for any. + /// Callback to invoke for each matching configuration entry. + public static void Enumerate(this IGitConfiguration config, string section, string property, GitConfigurationEnumerationCallback cb) + { + Enumerate(config, GitConfigurationLevel.All, section, property, cb); + } + /// /// Get the value of a configuration entry as a string. /// /// A configuration entry with the specified key was not found. /// Configuration object. + /// Filter to the specific configuration level. /// Configuration entry name. /// Configuration entry value. - public static string Get(this IGitConfiguration config, string name) + public static string Get(this IGitConfiguration config, GitConfigurationLevel level, string name) { - if (!config.TryGet(name, out string value)) + if (!config.TryGet(level, name, out string value)) { throw new KeyNotFoundException($"Git configuration entry with the name '{name}' was not found."); } return value; } + + /// + /// Get the value of a configuration entry as a string. + /// + /// A configuration entry with the specified key was not found. + /// Configuration object. + /// Configuration entry name. + /// Configuration entry value. + public static string Get(this IGitConfiguration config, string name) + { + return Get(config, GitConfigurationLevel.All, name); + } + + /// + /// Try and get the value of a configuration entry as a string. + /// + /// Configuration object. + /// Configuration entry name. + /// Configuration entry value. + /// True if the value was found, false otherwise. + public static bool TryGet(this IGitConfiguration config, string name, out string value) + { + return config.TryGet(GitConfigurationLevel.All, name, out value); + } + + /// + /// Get all value of a multivar configuration entry. + /// + /// Configuration object. + /// Configuration entry name. + /// All values of the multivar configuration entry. + public static IEnumerable GetAll(this IGitConfiguration config, string name) + { + return config.GetAll(GitConfigurationLevel.All, name); + } + + /// + /// Get all values of a multivar configuration entry. + /// + /// Configuration object. + /// Configuration entry name regular expression. + /// Regular expression to filter which variables we're interested in. Use null to indicate all. + /// All values of the multivar configuration entry. + public static IEnumerable GetRegex(this IGitConfiguration config, string nameRegex, string valueRegex) + { + return config.GetRegex(GitConfigurationLevel.All, nameRegex, valueRegex); + } } } diff --git a/src/shared/TestInfrastructure/Objects/TestCommandContext.cs b/src/shared/TestInfrastructure/Objects/TestCommandContext.cs index bf5b59e2e..6947e70ea 100644 --- a/src/shared/TestInfrastructure/Objects/TestCommandContext.cs +++ b/src/shared/TestInfrastructure/Objects/TestCommandContext.cs @@ -23,7 +23,7 @@ public TestCommandContext() Environment = new TestEnvironment(FileSystem); SystemPrompts = new TestSystemPrompts(); - Settings = new TestSettings {Environment = Environment, GitConfiguration = Git.GlobalConfiguration}; + Settings = new TestSettings {Environment = Environment, GitConfiguration = Git.Configuration}; } public TestSettings Settings { get; set; } diff --git a/src/shared/TestInfrastructure/Objects/TestGit.cs b/src/shared/TestInfrastructure/Objects/TestGit.cs index a872106ab..862ac9fdb 100644 --- a/src/shared/TestInfrastructure/Objects/TestGit.cs +++ b/src/shared/TestInfrastructure/Objects/TestGit.cs @@ -9,35 +9,11 @@ namespace Microsoft.Git.CredentialManager.Tests.Objects { public class TestGit : IGit { - public TestGitConfiguration SystemConfiguration { get; } = new TestGitConfiguration(); - public TestGitConfiguration GlobalConfiguration { get; } = new TestGitConfiguration(); - public TestGitConfiguration LocalConfiguration { get; } = new TestGitConfiguration(); + public readonly TestGitConfiguration Configuration = new TestGitConfiguration(); #region IGit - IGitConfiguration IGit.GetConfiguration(GitConfigurationLevel level) - { - switch (level) - { - case GitConfigurationLevel.All: - IDictionary> mergedConfigDict = - MergeDictionaries( - SystemConfiguration.Dictionary, - GlobalConfiguration.Dictionary, - LocalConfiguration.Dictionary); - return new TestGitConfiguration(mergedConfigDict); - case GitConfigurationLevel.Unknown: - return new TestGitConfiguration(); - case GitConfigurationLevel.System: - return SystemConfiguration; - case GitConfigurationLevel.Global: - return GlobalConfiguration; - case GitConfigurationLevel.Local: - return LocalConfiguration; - default: - throw new ArgumentOutOfRangeException(nameof(level), level, $"Unknown {nameof(GitConfigurationLevel)}"); - } - } + IGitConfiguration IGit.GetConfiguration() => Configuration; Task> IGit.InvokeHelperAsync(string args, IDictionary standardInput) { diff --git a/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs b/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs index 675f651db..af9dee1d5 100644 --- a/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs +++ b/src/shared/TestInfrastructure/Objects/TestGitConfiguration.cs @@ -9,72 +9,68 @@ namespace Microsoft.Git.CredentialManager.Tests.Objects { public class TestGitConfiguration : IGitConfiguration { - public TestGitConfiguration(IDictionary> config = null) - { - Dictionary = config ?? new Dictionary>(GitConfigurationKeyComparer.Instance); - } - - /// - /// Backing dictionary for the test configuration entries. - /// - public IDictionary> Dictionary { get; } - - /// - /// Convenience accessor for the backing of configuration entries. - /// - /// - public string this[string key] - { - get => TryGet(key, out string value) ? value : null; - set => Set(key, value); - } + public IDictionary> System { get; set; } = + new Dictionary>(GitConfigurationKeyComparer.Instance); + public IDictionary> Global { get; set; } = + new Dictionary>(GitConfigurationKeyComparer.Instance); + public IDictionary> Local { get; set; } = + new Dictionary>(GitConfigurationKeyComparer.Instance); #region IGitConfiguration - public void Enumerate(GitConfigurationEnumerationCallback cb) + public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCallback cb) { - foreach (var kvp in Dictionary) + foreach (var (dictLevel, dict) in GetDictionaries(level)) { - foreach (var value in kvp.Value) + foreach (var kvp in dict) { - if (!cb(new GitConfigurationEntry(GitConfigurationLevel.Unknown, kvp.Key, value))) + foreach (var value in kvp.Value) { - break; + var entry = new GitConfigurationEntry(dictLevel, kvp.Key, value); + if (!cb(entry)) + { + break; + } } } } } - public bool TryGet(string name, out string value) + public bool TryGet(GitConfigurationLevel level, string name, out string value) { - if (Dictionary.TryGetValue(name, out var values)) + value = null; + + // Proceed in order from least to most specific level and read the entry value + foreach (var (_, dict) in GetDictionaries(level)) { - // TODO: simulate git - if (values.Count > 1) + if (dict.TryGetValue(name, out var values)) { - throw new Exception("Configuration entry is a multivar"); - } + if (values.Count > 1) + { + throw new Exception("Configuration entry is a multivar"); + } - if (values.Count == 1) - { - value = values[0]; - return true; + if (values.Count == 1) + { + value = values[0]; + } } } - value = null; - return false; + return value != null; } - public void Set(string name, string value) + public void Set(GitConfigurationLevel level, string name, string value) { - if (!Dictionary.TryGetValue(name, out IList values)) + IDictionary> dict = GetDictionary(level); + + if (!dict.TryGetValue(name, out var values)) { values = new List(); - Dictionary[name] = values; + dict[name] = values; } - // TODO: simulate git + // Simulate git if (values.Count > 1) { throw new Exception("Configuration entry is a multivar"); @@ -90,57 +86,72 @@ public void Set(string name, string value) } } - public void Add(string name, string value) + public void Add(GitConfigurationLevel level, string name, string value) { - if (!Dictionary.TryGetValue(name, out IList values)) + IDictionary> dict = GetDictionary(level); + + if (!dict.TryGetValue(name, out IList values)) { values = new List(); - Dictionary[name] = values; + dict[name] = values; } values.Add(value); } - public void Unset(string name) + public void Unset(GitConfigurationLevel level, string name) { - // TODO: simulate git - if (Dictionary.TryGetValue(name, out var values) && values.Count > 1) + IDictionary> dict = GetDictionary(level); + + // Simulate git + if (dict.TryGetValue(name, out var values) && values.Count > 1) { throw new Exception("Configuration entry is a multivar"); } - Dictionary.Remove(name); + dict.Remove(name); } - public IEnumerable GetAll(string name) + public IEnumerable GetAll(GitConfigurationLevel level, string name) { - if (Dictionary.TryGetValue(name, out IList values)) + foreach (var (_, dict) in GetDictionaries(level)) { - return values; + if (dict.TryGetValue(name, out IList values)) + { + foreach (string value in values) + { + yield return value; + } + } } - - return Enumerable.Empty(); } - public IEnumerable GetRegex(string nameRegex, string valueRegex) + public IEnumerable GetRegex(GitConfigurationLevel level, string nameRegex, string valueRegex) { - foreach (string key in Dictionary.Keys) + foreach (var (_, dict) in GetDictionaries(level)) { - if (Regex.IsMatch(key, nameRegex)) + foreach (string key in dict.Keys) { - return Dictionary[key].Where(x => Regex.IsMatch(x, valueRegex)); + if (Regex.IsMatch(key, nameRegex)) + { + var values = dict[key].Where(x => Regex.IsMatch(x, valueRegex)); + foreach (string value in values) + { + yield return value; + } + } } } - - return Enumerable.Empty(); } - public void ReplaceAll(string nameRegex, string valueRegex, string value) + public void ReplaceAll(GitConfigurationLevel level, string nameRegex, string valueRegex, string value) { - if (!Dictionary.TryGetValue(nameRegex, out IList values)) + IDictionary> dict = GetDictionary(level); + + if (!dict.TryGetValue(nameRegex, out IList values)) { values = new List(); - Dictionary[nameRegex] = values; + dict[nameRegex] = values; } bool updated = false; @@ -161,9 +172,11 @@ public void ReplaceAll(string nameRegex, string valueRegex, string value) } } - public void UnsetAll(string name, string valueRegex) + public void UnsetAll(GitConfigurationLevel level, string name, string valueRegex) { - if (Dictionary.TryGetValue(name, out IList values)) + IDictionary> dict = GetDictionary(level); + + if (dict.TryGetValue(name, out IList values)) { for (int i = 0; i < values.Count;) { @@ -182,11 +195,52 @@ public void UnsetAll(string name, string valueRegex) // If we've removed all values, remove the top-level list from the multivar dictionary if (values.Count == 0) { - Dictionary.Remove(name); + dict.Remove(name); } } } #endregion + + private IDictionary> GetDictionary(GitConfigurationLevel level) + { + switch (level) + { + case GitConfigurationLevel.System: + return System; + case GitConfigurationLevel.Global: + return Global; + case GitConfigurationLevel.Local: + return Local; + case GitConfigurationLevel.All: + throw new ArgumentException("Must specify a specific level"); + default: + throw new ArgumentOutOfRangeException(nameof(level), level, "Unsupported level"); + } + } + + private IEnumerable<(GitConfigurationLevel level, IDictionary> dict)> GetDictionaries( + GitConfigurationLevel level) + { + switch (level) + { + case GitConfigurationLevel.System: + yield return (GitConfigurationLevel.System, System); + break; + case GitConfigurationLevel.Global: + yield return (GitConfigurationLevel.Global, Global); + break; + case GitConfigurationLevel.Local: + yield return (GitConfigurationLevel.Local, Local); + break; + case GitConfigurationLevel.All: + yield return (GitConfigurationLevel.System, System); + yield return (GitConfigurationLevel.Global, Global); + yield return (GitConfigurationLevel.Local, Local); + break; + default: + throw new ArgumentOutOfRangeException(nameof(level), level, "Unsupported level"); + } + } } } diff --git a/src/shared/TestInfrastructure/Objects/TestSettings.cs b/src/shared/TestInfrastructure/Objects/TestSettings.cs index c7f363822..7e0cd8cb4 100644 --- a/src/shared/TestInfrastructure/Objects/TestSettings.cs +++ b/src/shared/TestInfrastructure/Objects/TestSettings.cs @@ -70,13 +70,10 @@ public IEnumerable GetSettingValues(string envarName, string section, st { string key = $"{section}.{scope}.{property}"; - IList configValues = null; - if (GitConfiguration?.Dictionary.TryGetValue(key, out configValues) ?? false) + IEnumerable configValues = GitConfiguration.GetAll(key); + foreach (string value in configValues) { - if (configValues.Count > 0) - { - yield return configValues[0]; - } + yield return value; } } } From 798785004353c85adb19cd89eca148c2d7fb6a8e Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Fri, 19 Feb 2021 11:30:02 +0000 Subject: [PATCH 10/17] git: add ability to list remotes and get current repo dir Add the ability to list the Git remotes for the current repository, as well as resolve the current repository path. --- .../GitConfigurationTests.cs | 155 +++----------- .../GitTests.cs | 191 ++++++++++++++++++ .../Constants.cs | 7 + .../Microsoft.Git.CredentialManager/Git.cs | 100 ++++++++- .../TestInfrastructure/GitTestUtilities.cs | 114 +++++++++++ .../TestInfrastructure/Objects/TestGit.cs | 25 ++- 6 files changed, 456 insertions(+), 136 deletions(-) create mode 100644 src/shared/Microsoft.Git.CredentialManager.Tests/GitTests.cs create mode 100644 src/shared/TestInfrastructure/GitTestUtilities.cs diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs index 1d930e979..a274bbfee 100644 --- a/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs @@ -2,10 +2,9 @@ // Licensed under the MIT license. using System; using System.Collections.Generic; -using System.Diagnostics; -using System.IO; using Microsoft.Git.CredentialManager.Tests.Objects; using Xunit; +using static Microsoft.Git.CredentialManager.Tests.GitTestUtilities; namespace Microsoft.Git.CredentialManager.Tests { @@ -58,9 +57,9 @@ public void GitProcess_GetConfiguration_ReturnsConfiguration() public void GitConfiguration_Enumerate_CallbackReturnsTrue_InvokesCallbackForEachEntry() { string repoPath = CreateRepository(out string workDirPath); - Git(repoPath, workDirPath, "config --local foo.name lancelot").AssertSuccess(); - Git(repoPath, workDirPath, "config --local foo.quest seek-holy-grail").AssertSuccess(); - Git(repoPath, workDirPath, "config --local foo.favcolor blue").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local foo.name lancelot").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local foo.quest seek-holy-grail").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local foo.favcolor blue").AssertSuccess(); var expectedVisitedEntries = new List<(string name, string value)> { @@ -96,9 +95,9 @@ bool cb(GitConfigurationEntry entry) public void GitConfiguration_Enumerate_CallbackReturnsFalse_InvokesCallbackForEachEntryUntilReturnsFalse() { string repoPath = CreateRepository(out string workDirPath); - Git(repoPath, workDirPath, "config --local foo.name lancelot").AssertSuccess(); - Git(repoPath, workDirPath, "config --local foo.quest seek-holy-grail").AssertSuccess(); - Git(repoPath, workDirPath, "config --local foo.favcolor blue").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local foo.name lancelot").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local foo.quest seek-holy-grail").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local foo.favcolor blue").AssertSuccess(); var expectedVisitedEntries = new List<(string name, string value)> { @@ -133,7 +132,7 @@ bool cb(GitConfigurationEntry entry) public void GitConfiguration_TryGet_Name_Exists_ReturnsTrueOutString() { string repoPath = CreateRepository(out string workDirPath); - Git(repoPath, workDirPath, "config --local user.name john.doe").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local user.name john.doe").AssertSuccess(); string gitPath = GetGitPath(); var trace = new NullTrace(); @@ -166,7 +165,7 @@ public void GitConfiguration_TryGet_Name_DoesNotExists_ReturnsFalse() public void GitConfiguration_Get_Name_Exists_ReturnsString() { string repoPath = CreateRepository(out string workDirPath); - Git(repoPath, workDirPath, "config --local user.name john.doe").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local user.name john.doe").AssertSuccess(); string gitPath = GetGitPath(); var trace = new NullTrace(); @@ -204,7 +203,7 @@ public void GitConfiguration_Set_Local_SetsLocalConfig() config.Set(GitConfigurationLevel.Local, "core.foobar", "foo123"); - GitResult localResult = Git(repoPath, workDirPath, "config --local core.foobar"); + GitResult localResult = ExecGit(repoPath, workDirPath, "config --local core.foobar"); Assert.Equal("foo123", localResult.StandardOutput.Trim()); } @@ -229,8 +228,8 @@ public void GitConfiguration_Unset_Global_UnsetsGlobalConfig() try { - Git(repoPath, workDirPath, "config --global core.foobar alice").AssertSuccess(); - Git(repoPath, workDirPath, "config --local core.foobar bob").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --global core.foobar alice").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local core.foobar bob").AssertSuccess(); string gitPath = GetGitPath(); var trace = new NullTrace(); @@ -239,8 +238,8 @@ public void GitConfiguration_Unset_Global_UnsetsGlobalConfig() config.Unset(GitConfigurationLevel.Global, "core.foobar"); - GitResult globalResult = Git(repoPath, workDirPath, "config --global core.foobar"); - GitResult localResult = Git(repoPath, workDirPath, "config --local core.foobar"); + GitResult globalResult = ExecGit(repoPath, workDirPath, "config --global core.foobar"); + GitResult localResult = ExecGit(repoPath, workDirPath, "config --local core.foobar"); Assert.Equal(string.Empty, globalResult.StandardOutput.Trim()); Assert.Equal("bob", localResult.StandardOutput.Trim()); @@ -248,7 +247,7 @@ public void GitConfiguration_Unset_Global_UnsetsGlobalConfig() finally { // Cleanup global config changes - Git(repoPath, workDirPath, "config --global --unset core.foobar"); + ExecGit(repoPath, workDirPath, "config --global --unset core.foobar"); } } @@ -259,8 +258,8 @@ public void GitConfiguration_Unset_Local_UnsetsLocalConfig() try { - Git(repoPath, workDirPath, "config --global core.foobar alice").AssertSuccess(); - Git(repoPath, workDirPath, "config --local core.foobar bob").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --global core.foobar alice").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local core.foobar bob").AssertSuccess(); string gitPath = GetGitPath(); var trace = new NullTrace(); @@ -269,8 +268,8 @@ public void GitConfiguration_Unset_Local_UnsetsLocalConfig() config.Unset(GitConfigurationLevel.Local, "core.foobar"); - GitResult globalResult = Git(repoPath, workDirPath, "config --global core.foobar"); - GitResult localResult = Git(repoPath, workDirPath, "config --local core.foobar"); + GitResult globalResult = ExecGit(repoPath, workDirPath, "config --global core.foobar"); + GitResult localResult = ExecGit(repoPath, workDirPath, "config --local core.foobar"); Assert.Equal("alice", globalResult.StandardOutput.Trim()); Assert.Equal(string.Empty, localResult.StandardOutput.Trim()); @@ -278,7 +277,7 @@ public void GitConfiguration_Unset_Local_UnsetsLocalConfig() finally { // Cleanup global config changes - Git(repoPath, workDirPath, "config --global --unset core.foobar"); + ExecGit(repoPath, workDirPath, "config --global --unset core.foobar"); } } @@ -299,9 +298,9 @@ public void GitConfiguration_Unset_All_ThrowsException() public void GitConfiguration_UnsetAll_UnsetsAllConfig() { string repoPath = CreateRepository(out string workDirPath); - Git(repoPath, workDirPath, "config --local --add core.foobar foo1").AssertSuccess(); - Git(repoPath, workDirPath, "config --local --add core.foobar foo2").AssertSuccess(); - Git(repoPath, workDirPath, "config --local --add core.foobar bar1").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local --add core.foobar foo1").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local --add core.foobar foo2").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local --add core.foobar bar1").AssertSuccess(); string gitPath = GetGitPath(); var trace = new NullTrace(); @@ -310,7 +309,7 @@ public void GitConfiguration_UnsetAll_UnsetsAllConfig() config.UnsetAll(GitConfigurationLevel.Local, "core.foobar", "foo*"); - GitResult result = Git(repoPath, workDirPath, "config --local --get-all core.foobar"); + GitResult result = ExecGit(repoPath, workDirPath, "config --local --get-all core.foobar"); Assert.Equal("bar1", result.StandardOutput.Trim()); } @@ -327,111 +326,5 @@ public void GitConfiguration_UnsetAll_All_ThrowsException() Assert.Throws(() => config.UnsetAll(GitConfigurationLevel.All, "core.foobar", Constants.RegexPatterns.Any)); } - - #region Test helpers - - private static string GetGitPath() - { - ProcessStartInfo psi; - if (PlatformUtils.IsWindows()) - { - psi = new ProcessStartInfo( - Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.System), - "where.exe"), - "git.exe" - ); - } - else - { - psi = new ProcessStartInfo("/usr/bin/which", "git"); - } - - psi.RedirectStandardOutput = true; - - using (var which = new Process {StartInfo = psi}) - { - which.Start(); - which.WaitForExit(); - - if (which.ExitCode != 0) - { - throw new Exception("Failed to locate Git"); - } - - string data = which.StandardOutput.ReadLine(); - - if (string.IsNullOrWhiteSpace(data)) - { - throw new Exception("Failed to locate Git on the PATH"); - } - - return data; - } - } - - private static string CreateRepository() => CreateRepository(out _); - - private static string CreateRepository(out string workDirPath) - { - string tempDirectory = Path.GetTempPath(); - string repoName = $"repo-{Guid.NewGuid().ToString("N").Substring(0, 8)}"; - workDirPath = Path.Combine(tempDirectory, repoName); - string gitDirPath = Path.Combine(workDirPath, ".git"); - - if (Directory.Exists(workDirPath)) - { - Directory.Delete(workDirPath); - } - - Directory.CreateDirectory(workDirPath); - - Git(gitDirPath, workDirPath, "init").AssertSuccess(); - - return gitDirPath; - } - - private static GitResult Git(string repositoryPath, string workingDirectory, string command) - { - var procInfo = new ProcessStartInfo("git", command) - { - RedirectStandardOutput = true, - RedirectStandardError = true, - WorkingDirectory = workingDirectory - }; - - procInfo.Environment["GIT_DIR"] = repositoryPath; - - Process proc = Process.Start(procInfo); - if (proc is null) - { - throw new Exception("Failed to start Git process"); - } - - proc.WaitForExit(); - - var result = new GitResult - { - ExitCode = proc.ExitCode, - StandardOutput = proc.StandardOutput.ReadToEnd(), - StandardError = proc.StandardError.ReadToEnd() - }; - - return result; - } - - private struct GitResult - { - public int ExitCode; - public string StandardOutput; - public string StandardError; - - public void AssertSuccess() - { - Assert.Equal(0, ExitCode); - } - } - - #endregion } } diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/GitTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/GitTests.cs new file mode 100644 index 000000000..ac84aa834 --- /dev/null +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/GitTests.cs @@ -0,0 +1,191 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. +using System.IO; +using System.Linq; +using Microsoft.Git.CredentialManager.Tests.Objects; +using Xunit; +using static Microsoft.Git.CredentialManager.Tests.GitTestUtilities; + +namespace Microsoft.Git.CredentialManager.Tests +{ + public class GitTests + { + [Fact] + public void Git_GetCurrentRepository_NoLocalRepo_ReturnsNull() + { + string gitPath = GetGitPath(); + var trace = new NullTrace(); + var git = new GitProcess(trace, gitPath, Path.GetTempPath()); + + string actual = git.GetCurrentRepository(); + + Assert.Null(actual); + } + + [Fact] + public void Git_GetCurrentRepository_LocalRepo_ReturnsNotNull() + { + CreateRepository(out string workDirPath); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + var git = new GitProcess(trace, gitPath, workDirPath); + + string actual = git.GetCurrentRepository(); + + Assert.NotNull(actual); + } + + [Fact] + public void Git_GetRemotes_NoLocalRepo_ReturnsEmpty() + { + string gitPath = GetGitPath(); + var trace = new NullTrace(); + var git = new GitProcess(trace, gitPath, Path.GetTempPath()); + + GitRemote[] remotes = git.GetRemotes().ToArray(); + + Assert.Empty(remotes); + } + + [Fact] + public void Git_GetRemotes_NoRemotes_ReturnsEmpty() + { + CreateRepository(out string workDirPath); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + var git = new GitProcess(trace, gitPath, workDirPath); + + GitRemote[] remotes = git.GetRemotes().ToArray(); + + Assert.Empty(remotes); + } + + [Fact] + public void Git_GetRemotes_OneRemote_ReturnsRemote() + { + string name = "origin"; + string url = "https://example.com"; + string repoPath = CreateRepository(out string workDirPath); + ExecGit(repoPath, workDirPath, $"remote add {name} {url}"); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + + var git = new GitProcess(trace, gitPath, workDirPath); + GitRemote[] remotes = git.GetRemotes().ToArray(); + + Assert.Single(remotes); + AssertRemote(name, url, remotes[0]); + } + + [Fact] + public void Git_GetRemotes_OneRemoteFetchAndPull_ReturnsRemote() + { + string name = "origin"; + string fetchUrl = "https://fetch.example.com"; + string pushUrl = "https://push.example.com"; + string repoPath = CreateRepository(out string workDirPath); + ExecGit(repoPath, workDirPath, $"remote add {name} {fetchUrl}"); + ExecGit(repoPath, workDirPath, $"remote set-url --push {name} {pushUrl}"); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + + var git = new GitProcess(trace, gitPath, workDirPath); + GitRemote[] remotes = git.GetRemotes().ToArray(); + + Assert.Single(remotes); + AssertRemote(name, fetchUrl, pushUrl, remotes[0]); + } + + [Theory] + [InlineData("ssh://user@example.com/account/repo.git")] + [InlineData("user@example.com:account/repo.git")] + [InlineData("git://example.com/path/to/repo.git")] + [InlineData("file:///path/to/repo.git")] + [InlineData("/path/to/repo.git")] + public void Git_GetRemotes_NonHttpRemote_ReturnsRemote(string url) + { + string name = "origin"; + string repoPath = CreateRepository(out string workDirPath); + ExecGit(repoPath, workDirPath, $"remote add {name} {url}"); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + + var git = new GitProcess(trace, gitPath, workDirPath); + GitRemote[] remotes = git.GetRemotes().ToArray(); + + Assert.Single(remotes); + AssertRemote(name, url, remotes[0]); + } + + [Fact] + public void Git_GetRemotes_MultipleRemotes_ReturnsAllRemotes() + { + string name1 = "origin"; + string name2 = "test"; + string name3 = "upstream"; + string url1 = "https://example.com/origin"; + string url2 = "https://example.com/test"; + string url3 = "https://example.com/upstream"; + string repoPath = CreateRepository(out string workDirPath); + ExecGit(repoPath, workDirPath, $"remote add {name1} {url1}"); + ExecGit(repoPath, workDirPath, $"remote add {name2} {url2}"); + ExecGit(repoPath, workDirPath, $"remote add {name3} {url3}"); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + + var git = new GitProcess(trace, gitPath, workDirPath); + GitRemote[] remotes = git.GetRemotes().ToArray(); + + Assert.Equal(3, remotes.Length); + + AssertRemote(name1, url1, remotes[0]); + AssertRemote(name2, url2, remotes[1]); + AssertRemote(name3, url3, remotes[2]); + } + + [Fact] + public void Git_GetRemotes_RemoteNoFetchOnlyPull_ReturnsRemote() + { + string name = "origin"; + string pushUrl = "https://example.com"; + string repoPath = CreateRepository(out string workDirPath); + ExecGit(repoPath, workDirPath, $"remote add {name} {pushUrl}"); + ExecGit(repoPath, workDirPath, $"remote set-url --push {name} {pushUrl}"); + ExecGit(repoPath, workDirPath, $"config --unset --local remote.{name}.url"); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + + var git = new GitProcess(trace, gitPath, workDirPath); + GitRemote[] remotes = git.GetRemotes().ToArray(); + + Assert.Single(remotes); + + AssertRemote(name, null, pushUrl, remotes[0]); + } + + #region Test Helpers + + private static void AssertRemote(string expectedName, string expectedUrl, GitRemote remote) + { + Assert.Equal(expectedName, remote.Name); + Assert.Equal(expectedUrl, remote.FetchUrl); + Assert.Equal(expectedUrl, remote.PushUrl); + } + + private static void AssertRemote(string expectedName, string expectedFetchUrl, string expectedPushUrl, GitRemote remote) + { + Assert.Equal(expectedName, remote.Name); + Assert.Equal(expectedFetchUrl, remote.FetchUrl); + Assert.Equal(expectedPushUrl, remote.PushUrl); + } + + #endregion + } +} diff --git a/src/shared/Microsoft.Git.CredentialManager/Constants.cs b/src/shared/Microsoft.Git.CredentialManager/Constants.cs index cc9c8d684..3b436f657 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Constants.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Constants.cs @@ -95,6 +95,13 @@ public static class Http public const string Proxy = "proxy"; public const string SslVerify = "sslVerify"; } + + public static class Remote + { + public const string SectionName = "remote"; + public const string FetchUrl = "url"; + public const string PushUrl = "pushUrl"; + } } public static class HelpUrls diff --git a/src/shared/Microsoft.Git.CredentialManager/Git.cs b/src/shared/Microsoft.Git.CredentialManager/Git.cs index 74c316eca..42edb3e44 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Git.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Git.cs @@ -3,12 +3,26 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Text; using System.Threading.Tasks; namespace Microsoft.Git.CredentialManager { public interface IGit { + /// + /// Return the path to the current repository, or null if this instance is not + /// scoped to a Git repository. + /// + /// Absolute path to the current Git repository, or null. + string GetCurrentRepository(); + + /// + /// Get all remotes for the current repository. + /// + /// Names of all remotes in the current repository. + IEnumerable GetRemotes(); + /// /// Get the configuration object. /// @@ -24,6 +38,20 @@ public interface IGit Task> InvokeHelperAsync(string args, IDictionary standardInput); } + public class GitRemote + { + public GitRemote(string name, string fetchUrl, string pushUrl) + { + Name = name; + FetchUrl = fetchUrl; + PushUrl = pushUrl; + } + + public string Name { get; } + public string FetchUrl { get; } + public string PushUrl { get; } + } + public class GitProcess : IGit { private readonly ITrace _trace; @@ -45,6 +73,74 @@ public IGitConfiguration GetConfiguration() return new GitProcessConfiguration(_trace, this); } + public string GetCurrentRepository() + { + using (var git = CreateProcess("rev-parse --absolute-git-dir")) + { + git.Start(); + // To avoid deadlocks, always read the output stream first and then wait + // TODO: don't read in all the data at once; stream it + string data = git.StandardOutput.ReadToEnd(); + git.WaitForExit(); + + switch (git.ExitCode) + { + case 0: // OK + return data.TrimEnd(); + case 128: // Not inside a Git repository + return null; + default: + _trace.WriteLine($"Failed to get current Git repository (exit={git.ExitCode})"); + throw CreateGitException(git, "Failed to get current Git repository"); + } + } + } + + public IEnumerable GetRemotes() + { + using (var git = CreateProcess("remote -v show")) + { + git.Start(); + // To avoid deadlocks, always read the output stream first and then wait + // TODO: don't read in all the data at once; stream it + string data = git.StandardOutput.ReadToEnd(); + string stderr = git.StandardError.ReadToEnd(); + git.WaitForExit(); + + switch (git.ExitCode) + { + case 0: // OK + break; + case 128 when stderr.Contains("not a git repository"): // Not inside a Git repository + yield break; + default: + _trace.WriteLine($"Failed to enumerate Git remotes (exit={git.ExitCode})"); + throw CreateGitException(git, "Failed to enumerate Git remotes"); + } + + string[] lines = data.Split('\n'); + + // Remotes are always output in groups of two (fetch and push) + for (int i = 0; i + 1 < lines.Length; i += 2) + { + // The fetch URL is written first, followed by the push URL + string[] fetchLine = lines[i].Split(); + string[] pushLine = lines[i + 1].Split(); + + // Remote name is always first (and should match between fetch/push) + string remoteName = fetchLine[0]; + + // The next part, if present, is the URL + string fetchUrl = null; + string pushUrl = null; + if (fetchLine.Length > 1 && !string.IsNullOrWhiteSpace(fetchLine[1])) fetchUrl = fetchLine[1].TrimEnd(); + if (pushLine.Length > 1 && !string.IsNullOrWhiteSpace(pushLine[1])) pushUrl = pushLine[1].TrimEnd(); + + yield return new GitRemote(remoteName, fetchUrl, pushUrl); + } + } + } + public Process CreateProcess(string args) { var psi = new ProcessStartInfo(_gitPath, args) @@ -127,8 +223,4 @@ public GitException(string message, string gitErrorMessage, int exitCode) ExitCode = exitCode; } } - - public static class GitExtensions - { - } } diff --git a/src/shared/TestInfrastructure/GitTestUtilities.cs b/src/shared/TestInfrastructure/GitTestUtilities.cs new file mode 100644 index 000000000..eae54d592 --- /dev/null +++ b/src/shared/TestInfrastructure/GitTestUtilities.cs @@ -0,0 +1,114 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. +using System; +using System.Diagnostics; +using System.IO; +using Xunit; + +namespace Microsoft.Git.CredentialManager.Tests +{ + public static class GitTestUtilities + { + public static string GetGitPath() + { + ProcessStartInfo psi; + if (PlatformUtils.IsWindows()) + { + psi = new ProcessStartInfo( + Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.System), + "where.exe"), + "git.exe" + ); + } + else + { + psi = new ProcessStartInfo("/usr/bin/which", "git"); + } + + psi.RedirectStandardOutput = true; + + using (var which = new Process {StartInfo = psi}) + { + which.Start(); + which.WaitForExit(); + + if (which.ExitCode != 0) + { + throw new Exception("Failed to locate Git"); + } + + string data = which.StandardOutput.ReadLine(); + + if (string.IsNullOrWhiteSpace(data)) + { + throw new Exception("Failed to locate Git on the PATH"); + } + + return data; + } + } + + public static string CreateRepository() => CreateRepository(out _); + + public static string CreateRepository(out string workDirPath) + { + string tempDirectory = Path.GetTempPath(); + string repoName = $"repo-{Guid.NewGuid().ToString("N").Substring(0, 8)}"; + workDirPath = Path.Combine(tempDirectory, repoName); + string gitDirPath = Path.Combine(workDirPath, ".git"); + + if (Directory.Exists(workDirPath)) + { + Directory.Delete(workDirPath); + } + + Directory.CreateDirectory(workDirPath); + + ExecGit(gitDirPath, workDirPath, "init").AssertSuccess(); + + return gitDirPath; + } + + public static GitResult ExecGit(string repositoryPath, string workingDirectory, string command) + { + var procInfo = new ProcessStartInfo("git", command) + { + RedirectStandardOutput = true, + RedirectStandardError = true, + WorkingDirectory = workingDirectory + }; + + procInfo.Environment["GIT_DIR"] = repositoryPath; + + Process proc = Process.Start(procInfo); + if (proc is null) + { + throw new Exception("Failed to start Git process"); + } + + proc.WaitForExit(); + + var result = new GitResult + { + ExitCode = proc.ExitCode, + StandardOutput = proc.StandardOutput.ReadToEnd(), + StandardError = proc.StandardError.ReadToEnd() + }; + + return result; + } + + public struct GitResult + { + public int ExitCode; + public string StandardOutput; + public string StandardError; + + public void AssertSuccess() + { + Assert.Equal(0, ExitCode); + } + } + } +} diff --git a/src/shared/TestInfrastructure/Objects/TestGit.cs b/src/shared/TestInfrastructure/Objects/TestGit.cs index 862ac9fdb..14c4e8fe9 100644 --- a/src/shared/TestInfrastructure/Objects/TestGit.cs +++ b/src/shared/TestInfrastructure/Objects/TestGit.cs @@ -2,17 +2,33 @@ // Licensed under the MIT license. using System; using System.Collections.Generic; -using System.Diagnostics; +using System.IO; using System.Threading.Tasks; namespace Microsoft.Git.CredentialManager.Tests.Objects { public class TestGit : IGit { + public string CurrentRepository { get; set; } + + public IList Remotes { get; set; } = new List(); + public readonly TestGitConfiguration Configuration = new TestGitConfiguration(); + public TestGit(bool insideRepo = true) + { + if (insideRepo) + { + CurrentRepository = GetFakeRepositoryPath(); + } + } + #region IGit + string IGit.GetCurrentRepository() => CurrentRepository; + + IEnumerable IGit.GetRemotes() => Remotes; + IGitConfiguration IGit.GetConfiguration() => Configuration; Task> IGit.InvokeHelperAsync(string args, IDictionary standardInput) @@ -36,5 +52,12 @@ public class TestGit : IGit return result; } + + public static string GetFakeRepositoryPath(string name = null) + { + name ??= Guid.NewGuid().ToString("N").Substring(8); + var basePath = Path.GetTempPath(); + return Path.Combine(basePath, "fake-repo", name); + } } } From ee6456534cafe1e27d363e8b511d8ca617cbcdd8 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Fri, 19 Feb 2021 11:35:20 +0000 Subject: [PATCH 11/17] azrepos: add utility to extract AzDevOps org name Add a utility to extract the Azure DevOps organisation name from a remote URL. --- .../UriHelpersTests.cs | 119 +++++------------- .../AzureReposHostProvider.cs | 27 ++-- src/shared/Microsoft.AzureRepos/UriHelpers.cs | 61 ++++----- 3 files changed, 73 insertions(+), 134 deletions(-) diff --git a/src/shared/Microsoft.AzureRepos.Tests/UriHelpersTests.cs b/src/shared/Microsoft.AzureRepos.Tests/UriHelpersTests.cs index 7fb2839e2..eac9112de 100644 --- a/src/shared/Microsoft.AzureRepos.Tests/UriHelpersTests.cs +++ b/src/shared/Microsoft.AzureRepos.Tests/UriHelpersTests.cs @@ -76,159 +76,96 @@ public void UriHelpers_IsVisualStudioComHost(string host, bool expected) [Fact] public void UriHelpers_CreateOrganizationUri_Null_ThrowsException() { - Assert.Throws(() => UriHelpers.CreateOrganizationUri(null)); - } - - [Fact] - public void UriHelpers_CreateOrganizationUri_InputArgsMissingProtocol_ThrowsException() - { - var input = new InputArguments(new Dictionary - { - ["host"] = "dev.azure.com" - }); - - Assert.Throws(() => UriHelpers.CreateOrganizationUri(input)); - } - - [Fact] - public void UriHelpers_CreateOrganizationUri_InputArgsMissingHost_ThrowsException() - { - var input = new InputArguments(new Dictionary - { - ["protocol"] = "https" - }); - - Assert.Throws(() => UriHelpers.CreateOrganizationUri(input)); + Assert.Throws(() => UriHelpers.CreateOrganizationUri(null, out _)); } [Fact] public void UriHelpers_CreateOrganizationUri_AzureHost_ReturnsCorrectUri() { var expected = new Uri("https://dev.azure.com/myorg"); - var input = new InputArguments(new Dictionary - { - ["protocol"] = "https", - ["host"] = "dev.azure.com", - ["path"] = "myorg/myproject/_git/myrepo" - }); + var input = new Uri("https://dev.azure.com/myorg/myproject/_git/myrepo"); + const string expectedOrg = "myorg"; - Uri actual = UriHelpers.CreateOrganizationUri(input); + Uri actual = UriHelpers.CreateOrganizationUri(input, out string actualOrg); Assert.Equal(expected, actual); + Assert.Equal(expectedOrg, actualOrg); } [Fact] public void UriHelpers_CreateOrganizationUri_AzureHost_WithPort_ReturnsCorrectUri() { var expected = new Uri("https://dev.azure.com:456/myorg"); - var input = new InputArguments(new Dictionary - { - ["protocol"] = "https", - ["host"] = "dev.azure.com:456", - ["path"] = "myorg/myproject/_git/myrepo" - }); + var input = new Uri("https://dev.azure.com:456/myorg/myproject/_git/myrepo"); + const string expectedOrg = "myorg"; - Uri actual = UriHelpers.CreateOrganizationUri(input); + Uri actual = UriHelpers.CreateOrganizationUri(input, out string actualOrg); Assert.Equal(expected, actual); - } - - [Fact] - public void UriHelpers_CreateOrganizationUri_AzureHost_WithBadPort_ThrowsException() - { - var expected = new Uri("https://dev.azure.com:456/myorg"); - var input = new InputArguments(new Dictionary - { - ["protocol"] = "https", - ["host"] = "dev.azure.com:not-a-port", - ["path"] = "myorg/myproject/_git/myrepo" - }); - - Assert.Throws(() => UriHelpers.CreateOrganizationUri(input)); + Assert.Equal(expectedOrg, actualOrg); } [Fact] public void UriHelpers_CreateOrganizationUri_AzureHost_OrgAlsoInUser_PrefersPathOrg() { var expected = new Uri("https://dev.azure.com/myorg-path"); - var input = new InputArguments(new Dictionary - { - ["protocol"] = "https", - ["host"] = "dev.azure.com", - ["path"] = "myorg-path", - ["username"] = "myorg-user" - }); + var input = new Uri("https://myorg-user@dev.azure.com/myorg-path"); + const string expectedOrg = "myorg-path"; - Uri actual = UriHelpers.CreateOrganizationUri(input); + Uri actual = UriHelpers.CreateOrganizationUri(input, out string actualOrg); Assert.Equal(expected, actual); + Assert.Equal(expectedOrg, actualOrg); } [Fact] public void UriHelpers_CreateOrganizationUri_AzureHost_InputArgsMissingPath_HasUser_UsesUserOrg() { var expected = new Uri("https://dev.azure.com/myorg-user"); - var input = new InputArguments(new Dictionary - { - ["protocol"] = "https", - ["host"] = "dev.azure.com", - ["username"] = "myorg-user" - }); + var input = new Uri("https://myorg-user@dev.azure.com"); + const string expectedOrg = "myorg-user"; - Uri actual = UriHelpers.CreateOrganizationUri(input); + Uri actual = UriHelpers.CreateOrganizationUri(input, out string actualOrg); Assert.Equal(expected, actual); + Assert.Equal(expectedOrg, actualOrg); } [Fact] public void UriHelpers_CreateOrganizationUri_AzureHost_InputArgsMissingPathAndUser_ThrowsException() { - var input = new InputArguments(new Dictionary - { - ["protocol"] = "https", - ["host"] = "dev.azure.com" - }); + var input = new Uri("https://dev.azure.com"); - Assert.Throws(() => UriHelpers.CreateOrganizationUri(input)); + Assert.Throws(() => UriHelpers.CreateOrganizationUri(input, out _)); } [Fact] public void UriHelpers_CreateOrganizationUri_VisualStudioHost_ReturnsCorrectUri() { - var expected = new Uri("https://myorg.visualstudio.com/"); - var input = new InputArguments(new Dictionary - { - ["protocol"] = "https", - ["host"] = "myorg.visualstudio.com", - }); + var expected = new Uri("https://myorg.visualstudio.com"); + var input = new Uri("https://myorg.visualstudio.com"); + const string expectedOrg = "myorg"; - Uri actual = UriHelpers.CreateOrganizationUri(input); + Uri actual = UriHelpers.CreateOrganizationUri(input, out string actualOrg); Assert.Equal(expected, actual); + Assert.Equal(expectedOrg, actualOrg); } [Fact] public void UriHelpers_CreateOrganizationUri_VisualStudioHost_MissingOrgInHost_ThrowsException() { - var input = new InputArguments(new Dictionary - { - ["protocol"] = "https", - ["host"] = "visualstudio.com", - }); + var input = new Uri("https://visualstudio.com"); - Assert.Throws(() => UriHelpers.CreateOrganizationUri(input)); + Assert.Throws(() => UriHelpers.CreateOrganizationUri(input, out _)); } [Fact] public void UriHelpers_CreateOrganizationUri_NonAzureDevOpsHost_ThrowsException() { - var input = new InputArguments(new Dictionary - { - ["protocol"] = "https", - ["host"] = "example.com", - }); + var input = new Uri("https://example.com"); - Assert.Throws(() => UriHelpers.CreateOrganizationUri(input)); + Assert.Throws(() => UriHelpers.CreateOrganizationUri(input, out _)); } } } diff --git a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs index 50e8d6921..c887009a1 100644 --- a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs +++ b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs @@ -67,7 +67,8 @@ public bool IsSupported(HttpResponseMessage response) public async Task GetCredentialAsync(InputArguments input) { - string service = GetServiceName(input); + Uri remoteUri = input.GetRemoteUri(); + string service = GetServiceName(remoteUri); string account = GetAccountNameForCredentialQuery(input); _context.Trace.WriteLine($"Looking for existing credential in store with service={service} account={account}..."); @@ -92,7 +93,8 @@ public async Task GetCredentialAsync(InputArguments input) public Task StoreCredentialAsync(InputArguments input) { - string service = GetServiceName(input); + Uri remoteUri = input.GetRemoteUri(); + string service = GetServiceName(remoteUri); // We always store credentials against the given username argument for // both vs.com and dev.azure.com-style URLs. @@ -108,7 +110,8 @@ public Task StoreCredentialAsync(InputArguments input) public Task EraseCredentialAsync(InputArguments input) { - string service = GetServiceName(input); + Uri remoteUri = input.GetRemoteUri(); + string service = GetServiceName(remoteUri); string account = GetAccountNameForCredentialQuery(input); // Try to locate an existing credential @@ -141,8 +144,8 @@ private async Task GenerateCredentialAsync(InputArguments input) throw new Exception("Unencrypted HTTP is not supported for Azure Repos. Ensure the repository remote URL is using HTTPS."); } - Uri orgUri = UriHelpers.CreateOrganizationUri(input); Uri remoteUri = input.GetRemoteUri(); + Uri orgUri = UriHelpers.CreateOrganizationUri(remoteUri, out _); // Determine the MS authentication authority for this organization _context.Trace.WriteLine("Determining Microsoft Authentication Authority..."); @@ -224,28 +227,22 @@ private Uri GetRedirectUri() /// Users that need to clone a repository from Azure Repos against the full path therefore must /// use the vs.com-style remote URL and not the dev.azure.com one. /// - private static string GetServiceName(InputArguments input) + private static string GetServiceName(Uri remoteUri) { - if (!input.TryGetHostAndPort(out string hostName, out _)) - { - throw new InvalidOperationException("Failed to parse host name and/or port"); - } - - // dev.azure.com - if (UriHelpers.IsDevAzureComHost(hostName)) + if (UriHelpers.IsDevAzureComHost(remoteUri.Host)) { // We can never store the new dev.azure.com-style URLs against the full path because // we have forced the useHttpPath option to true to in order to retrieve the AzDevOps // organization name from Git. - return UriHelpers.CreateOrganizationUri(input).AbsoluteUri.TrimEnd('/'); + return UriHelpers.CreateOrganizationUri(remoteUri, out _).AbsoluteUri.TrimEnd('/'); } // *.visualstudio.com - if (UriHelpers.IsVisualStudioComHost(hostName)) + if (UriHelpers.IsVisualStudioComHost(remoteUri.Host)) { // If we're given the full path for an older *.visualstudio.com-style URL then we should // respect that in the service name. - return input.GetRemoteUri().AbsoluteUri.TrimEnd('/'); + return remoteUri.AbsoluteUri.TrimEnd('/'); } throw new InvalidOperationException("Host is not Azure DevOps."); diff --git a/src/shared/Microsoft.AzureRepos/UriHelpers.cs b/src/shared/Microsoft.AzureRepos/UriHelpers.cs index 453a7b49c..9aed0fc0d 100644 --- a/src/shared/Microsoft.AzureRepos/UriHelpers.cs +++ b/src/shared/Microsoft.AzureRepos/UriHelpers.cs @@ -100,10 +100,17 @@ public static bool IsAzureDevOpsHost(string host) return IsVisualStudioComHost(host) || IsDevAzureComHost(host); } + public static string GetOrganizationName(Uri remoteUri) + { + CreateOrganizationUri(remoteUri, out string orgName); + return orgName; + } + /// - /// Create a URI for the Azure DevOps organization from the give Git input query arguments. + /// Create a URI for the Azure DevOps organization from the Git remote URI. /// - /// Git query arguments. + /// Git remote URI arguments. + /// Azure DevOps organization name. /// Azure DevOps organization URI /// /// Thrown if is null or white space. @@ -116,54 +123,44 @@ public static bool IsAzureDevOpsHost(string host) /// are null or white space when is an Azure-style URL /// ('dev.azure.com' rather than '*.visualstudio.com'). /// - public static Uri CreateOrganizationUri(InputArguments input) + public static Uri CreateOrganizationUri(Uri remoteUri, out string orgName) { - EnsureArgument.NotNull(input, nameof(input)); - - if (string.IsNullOrWhiteSpace(input.Protocol)) - { - throw new InvalidOperationException("Input arguments must include protocol."); - } - - if (string.IsNullOrWhiteSpace(input.Host)) - { - throw new InvalidOperationException("Input arguments must include host."); - } + EnsureArgument.AbsoluteUri(remoteUri, nameof(remoteUri)); - if (!input.TryGetHostAndPort(out string hostName, out int? port)) - { - throw new InvalidOperationException("Host name and/or port is invalid."); - } + orgName = null; - if (!IsAzureDevOpsHost(hostName)) + if (!IsAzureDevOpsHost(remoteUri.Host)) { throw new InvalidOperationException("Host is not Azure DevOps."); } var ub = new UriBuilder { - Scheme = input.Protocol, - Host = hostName, + Scheme = remoteUri.Scheme, + Host = remoteUri.Host, }; - if (port.HasValue) + if (!remoteUri.IsDefaultPort) { - ub.Port = port.Value; + ub.Port = remoteUri.Port; } // Extract the organization name for Azure ('dev.azure.com') style URLs. // The older *.visualstudio.com URLs contained the organization name in the host already. - if (IsDevAzureComHost(hostName)) + if (IsDevAzureComHost(remoteUri.Host)) { + string firstPathComponent = GetFirstPathComponent(remoteUri.AbsolutePath); + string remoteUriUserName = remoteUri.GetUserName(); + // Prefer getting the org name from the path: dev.azure.com/{org} - if (GetFirstPathComponent(input.Path) is string orgName) + if (!string.IsNullOrWhiteSpace(firstPathComponent)) { - ub.Path = orgName; + orgName = firstPathComponent; } // Failing that try using the username: {org}@dev.azure.com - else if (!string.IsNullOrWhiteSpace(input.UserName)) + else if (!string.IsNullOrWhiteSpace(remoteUriUserName)) { - ub.Path = input.UserName; + orgName = remoteUriUserName; } else { @@ -173,6 +170,14 @@ public static Uri CreateOrganizationUri(InputArguments input) "name as the user in the remote URL '{org}@dev.azure.com'." ); } + + ub.Path = orgName; + } + else if (IsVisualStudioComHost(remoteUri.Host)) + { + // {org}.visualstudio.com + int orgNameLength = remoteUri.Host.Length - AzureDevOpsConstants.VstsHostSuffix.Length; + orgName = remoteUri.Host.Substring(0, orgNameLength); } return ub.Uri; From 7e493d74800aae374c57bc45aa5034d3293d1538 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Mon, 8 Mar 2021 11:43:54 +0000 Subject: [PATCH 12/17] azrepos: add non-PAT based auth method to AzRepos Add a new settings to the Azure Repos provider that instructs GCM to return the Azure access token directly, rather than use that token to generate a new Azure DevOps Personal Access Token (PAT). At the moment the only indication as to what user account a user wants to use is via the userinfo parts of the remote URL. This must be set manually. The default configuration is to continue to use PATs, for now. --- docs/configuration.md | 23 ++ docs/environment.md | 29 +++ .../AzureReposHostProviderTests.cs | 217 +++++++++++++++++- .../AzureDevOpsConstants.cs | 5 + .../AzureReposHostProvider.cs | 171 +++++++++++--- 5 files changed, 404 insertions(+), 41 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index ef2b12dec..4362b747b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -333,3 +333,26 @@ Credential: "git:https://bob@github.com/example/myrepo" (user = bob) https://bob@github.com/example/myrepo ``` + +--- + +### credential.azreposCredentialType _(experimental)_ + +Specify the type of credential the Azure Repos host provider should return. + +Defaults to the value `pat`. + +Value|Description +-|- +`pat` _(default)_|Azure DevOps personal access tokens +`oauth`|Microsoft identity OAuth tokens (AAD or MSA tokens) + +More information about Azure Access tokens can be found [here](azrepos-azuretokens.md). + +#### Example + +```shell +git config --global credential.azreposCredentialType oauth +``` + +**Also see: [GCM_AZREPOS_CREDENTIALTYPE](environment.md#GCM_AZREPOS_CREDENTIALTYPE-experimental)** diff --git a/docs/environment.md b/docs/environment.md index 0177935d0..5b953a860 100644 --- a/docs/environment.md +++ b/docs/environment.md @@ -424,3 +424,32 @@ export GCM_MSAUTH_FLOW="devicecode" ``` **Also see: [credential.msauthFlow](configuration.md#credentialmsauthflow)** + +--- + +### GCM_AZREPOS_CREDENTIALTYPE _(experimental)_ + +Specify the type of credential the Azure Repos host provider should return. + +Defaults to the value `pat`. + +Value|Description +-|- +`pat` _(default)_|Azure DevOps personal access tokens +`oauth`|Microsoft identity OAuth tokens (AAD or MSA tokens) + +More information about Azure Access tokens can be found [here](azrepos-azuretokens.md). + +##### Windows + +```batch +SET GCM_AZREPOS_CREDENTIALTYPE="oauth" +``` + +##### macOS/Linux + +```bash +export GCM_AZREPOS_CREDENTIALTYPE="oauth" +``` + +**Also see: [credential.azreposCredentialType](configuration.md#azreposcredentialtype-experimental)** diff --git a/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs b/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs index 8ce481aae..678e9c2de 100644 --- a/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs +++ b/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs @@ -139,7 +139,179 @@ public async Task AzureReposProvider_GetCredentialAsync_UnencryptedHttp_ThrowsEx } [Fact] - public async Task AzureReposProvider_GetCredentialAsync_ReturnsCredential() + public async Task AzureReposProvider_GetCredentialAsync_JwtMode_VsComUrlUser_ReturnsCredential() + { + var urlAccount = "jane.doe"; + + var input = new InputArguments(new Dictionary + { + ["protocol"] = "https", + ["host"] = "org.visualstudio.com", + ["username"] = urlAccount + }); + + var expectedOrgUri = new Uri("https://org.visualstudio.com"); + var remoteUri = new Uri("https://org.visualstudio.com/"); + var authorityUrl = "https://login.microsoftonline.com/common"; + var expectedClientId = AzureDevOpsConstants.AadClientId; + var expectedRedirectUri = AzureDevOpsConstants.AadRedirectUri; + var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes; + var accessToken = "ACCESS-TOKEN"; + var authResult = CreateAuthResult(urlAccount, accessToken); + + var context = new TestCommandContext(); + + // Use OAuth Access Tokens + context.Environment.Variables[AzureDevOpsConstants.EnvironmentVariables.CredentialType] = + AzureDevOpsConstants.OAuthCredentialType; + + var azDevOpsMock = new Mock(MockBehavior.Strict); + azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl); + + var msAuthMock = new Mock(MockBehavior.Strict); + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, urlAccount)) + .ReturnsAsync(authResult); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object); + + ICredential credential = await provider.GetCredentialAsync(input); + + Assert.NotNull(credential); + Assert.Equal(urlAccount, credential.Account); + Assert.Equal(accessToken, credential.Password); + } + + [Fact] + public async Task AzureReposProvider_GetCredentialAsync_JwtMode_DevAzureUrlUser_ReturnsCredential() + { + var urlAccount = "jane.doe"; + + var input = new InputArguments(new Dictionary + { + ["protocol"] = "https", + ["host"] = "dev.azure.com", + ["path"] = "org/project/_git/repo", + ["username"] = urlAccount + }); + + var expectedOrgUri = new Uri("https://dev.azure.com/org"); + var remoteUri = new Uri("https://dev.azure.com/org/project/_git/repo"); + var authorityUrl = "https://login.microsoftonline.com/common"; + var expectedClientId = AzureDevOpsConstants.AadClientId; + var expectedRedirectUri = AzureDevOpsConstants.AadRedirectUri; + var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes; + var accessToken = "ACCESS-TOKEN"; + var authResult = CreateAuthResult(urlAccount, accessToken); + + var context = new TestCommandContext(); + + // Use OAuth Access Tokens + context.Environment.Variables[AzureDevOpsConstants.EnvironmentVariables.CredentialType] = + AzureDevOpsConstants.OAuthCredentialType; + + var azDevOpsMock = new Mock(MockBehavior.Strict); + azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl); + + var msAuthMock = new Mock(MockBehavior.Strict); + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, urlAccount)) + .ReturnsAsync(authResult); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object); + + ICredential credential = await provider.GetCredentialAsync(input); + + Assert.NotNull(credential); + Assert.Equal(urlAccount, credential.Account); + Assert.Equal(accessToken, credential.Password); + } + + [Fact] + public async Task AzureReposProvider_GetCredentialAsync_JwtMode_DevAzureUrlOrgName_ReturnsCredential() + { + var input = new InputArguments(new Dictionary + { + ["protocol"] = "https", + ["host"] = "dev.azure.com", + ["path"] = "org/project/_git/repo", + ["username"] = "org" + }); + + var expectedOrgUri = new Uri("https://dev.azure.com/org"); + var remoteUri = new Uri("https://dev.azure.com/org/project/_git/repo"); + var authorityUrl = "https://login.microsoftonline.com/common"; + var expectedClientId = AzureDevOpsConstants.AadClientId; + var expectedRedirectUri = AzureDevOpsConstants.AadRedirectUri; + var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes; + var accessToken = "ACCESS-TOKEN"; + var account = "jane.doe"; + var authResult = CreateAuthResult(account, accessToken); + + var context = new TestCommandContext(); + + // Use OAuth Access Tokens + context.Environment.Variables[AzureDevOpsConstants.EnvironmentVariables.CredentialType] = + AzureDevOpsConstants.OAuthCredentialType; + + var azDevOpsMock = new Mock(MockBehavior.Strict); + azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl); + + var msAuthMock = new Mock(MockBehavior.Strict); + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null)) + .ReturnsAsync(authResult); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object); + + ICredential credential = await provider.GetCredentialAsync(input); + + Assert.NotNull(credential); + Assert.Equal(account, credential.Account); + Assert.Equal(accessToken, credential.Password); + } + + [Fact] + public async Task AzureReposProvider_GetCredentialAsync_JwtMode_NoUser_ReturnsCredential() + { + var input = new InputArguments(new Dictionary + { + ["protocol"] = "https", + ["host"] = "dev.azure.com", + ["path"] = "org/proj/_git/repo" + }); + + var expectedOrgUri = new Uri("https://dev.azure.com/org"); + var remoteUri = new Uri("https://dev.azure.com/org/proj/_git/repo"); + var authorityUrl = "https://login.microsoftonline.com/common"; + var expectedClientId = AzureDevOpsConstants.AadClientId; + var expectedRedirectUri = AzureDevOpsConstants.AadRedirectUri; + var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes; + var accessToken = "ACCESS-TOKEN"; + var account = "john.doe"; + var authResult = CreateAuthResult(account, accessToken); + + var context = new TestCommandContext(); + + // Use OAuth Access Tokens + context.Environment.Variables[AzureDevOpsConstants.EnvironmentVariables.CredentialType] = + AzureDevOpsConstants.OAuthCredentialType; + + var azDevOpsMock = new Mock(MockBehavior.Strict); + azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl); + + var msAuthMock = new Mock(MockBehavior.Strict); + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null)) + .ReturnsAsync(authResult); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object); + + ICredential credential = await provider.GetCredentialAsync(input); + + Assert.NotNull(credential); + Assert.Equal(account, credential.Account); + Assert.Equal(accessToken, credential.Password); + } + + [Fact] + public async Task AzureReposProvider_GetCredentialAsync_PatMode_NoExistingPat_GeneratesCredential() { var input = new InputArguments(new Dictionary { @@ -156,17 +328,17 @@ public async Task AzureReposProvider_GetCredentialAsync_ReturnsCredential() var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes; var accessToken = "ACCESS-TOKEN"; var personalAccessToken = "PERSONAL-ACCESS-TOKEN"; - var authResult = CreateAuthResult("john.doe", accessToken); + var account = "john.doe"; + var authResult = CreateAuthResult(account, accessToken); var context = new TestCommandContext(); - var azDevOpsMock = new Mock(); - azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)) - .ReturnsAsync(authorityUrl); + var azDevOpsMock = new Mock(MockBehavior.Strict); + azDevOpsMock.Setup(x => x.GetAuthorityAsync(expectedOrgUri)).ReturnsAsync(authorityUrl); azDevOpsMock.Setup(x => x.CreatePersonalAccessTokenAsync(expectedOrgUri, accessToken, It.IsAny>())) .ReturnsAsync(personalAccessToken); - var msAuthMock = new Mock(); + var msAuthMock = new Mock(MockBehavior.Strict); msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null)) .ReturnsAsync(authResult); @@ -175,8 +347,39 @@ public async Task AzureReposProvider_GetCredentialAsync_ReturnsCredential() ICredential credential = await provider.GetCredentialAsync(input); Assert.NotNull(credential); + Assert.Equal(account, credential.Account); + Assert.Equal(personalAccessToken, credential.Password); + } + + [Fact] + public async Task AzureReposProvider_GetCredentialAsync_PatMode_ExistingPat_ReturnsExistingCredential() + { + var input = new InputArguments(new Dictionary + { + ["protocol"] = "https", + ["host"] = "dev.azure.com", + ["path"] = "org/proj/_git/repo" + }); + + var remoteUri = new Uri("https://dev.azure.com/org/proj/_git/repo"); + var personalAccessToken = "PERSONAL-ACCESS-TOKEN"; + const string service = "https://dev.azure.com/org"; + const string account = "john.doe"; + + var context = new TestCommandContext(); + + context.CredentialStore.Add(service, account, personalAccessToken); + + var azDevOps = Mock.Of(); + var msAuth = Mock.Of(); + + var provider = new AzureReposHostProvider(context, azDevOps, msAuth); + + ICredential credential = await provider.GetCredentialAsync(input); + + Assert.NotNull(credential); + Assert.Equal(account, credential.Account); Assert.Equal(personalAccessToken, credential.Password); - // We don't care about the username value } [Fact] diff --git a/src/shared/Microsoft.AzureRepos/AzureDevOpsConstants.cs b/src/shared/Microsoft.AzureRepos/AzureDevOpsConstants.cs index 37de175bc..3eea95449 100644 --- a/src/shared/Microsoft.AzureRepos/AzureDevOpsConstants.cs +++ b/src/shared/Microsoft.AzureRepos/AzureDevOpsConstants.cs @@ -24,6 +24,9 @@ internal static class AzureDevOpsConstants public const string VssResourceTenantHeader = "X-VSS-ResourceTenant"; + public const string PatCredentialType = "pat"; + public const string OAuthCredentialType = "oauth"; + public static class PersonalAccessTokenScopes { public const string ReposWrite = "vso.code_write"; @@ -35,6 +38,7 @@ public static class EnvironmentVariables public const string DevAadClientId = "GCM_DEV_AZREPOS_CLIENTID"; public const string DevAadRedirectUri = "GCM_DEV_AZREPOS_REDIRECTURI"; public const string DevAadAuthorityBaseUri = "GCM_DEV_AZREPOS_AUTHORITYBASEURI"; + public const string CredentialType = "GCM_AZREPOS_CREDENTIALTYPE"; } public static class GitConfiguration @@ -44,6 +48,7 @@ public static class Credential public const string DevAadClientId = "azreposDevClientId"; public const string DevAadRedirectUri = "azreposDevRedirectUri"; public const string DevAadAuthorityBaseUri = "azreposDevAuthorityBaseUri"; + public const string CredentialType = "azreposCredentialType"; } } } diff --git a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs index c887009a1..51cf2257b 100644 --- a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs +++ b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs @@ -68,42 +68,57 @@ public bool IsSupported(HttpResponseMessage response) public async Task GetCredentialAsync(InputArguments input) { Uri remoteUri = input.GetRemoteUri(); - string service = GetServiceName(remoteUri); - string account = GetAccountNameForCredentialQuery(input); - _context.Trace.WriteLine($"Looking for existing credential in store with service={service} account={account}..."); - - ICredential credential = _context.CredentialStore.Get(service, account); - if (credential == null) + if (UsePersonalAccessTokens()) { - _context.Trace.WriteLine("No existing credentials found."); - - // No existing credential was found, create a new one - _context.Trace.WriteLine("Creating new credential..."); - credential = await GenerateCredentialAsync(input); - _context.Trace.WriteLine("Credential created."); + string service = GetServiceName(remoteUri); + string account = GetAccountNameForCredentialQuery(input); + + _context.Trace.WriteLine($"Looking for existing credential in store with service={service} account={account}..."); + + ICredential credential = _context.CredentialStore.Get(service, account); + if (credential == null) + { + _context.Trace.WriteLine("No existing credentials found."); + + // No existing credential was found, create a new one + _context.Trace.WriteLine("Creating new credential..."); + credential = await GeneratePersonalAccessTokenAsync(input); + _context.Trace.WriteLine("Credential created."); + } + else + { + _context.Trace.WriteLine("Existing credential found."); + } + + return credential; } else { - _context.Trace.WriteLine("Existing credential found."); + // Include the username request here so that we may use it as an override + // for user account lookups when getting Azure Access Tokens. + var azureResult = await GetAzureAccessTokenAsync(remoteUri, input.UserName); + return new GitCredential(azureResult.AccountUpn, azureResult.AccessToken); } - - return credential; } public Task StoreCredentialAsync(InputArguments input) { Uri remoteUri = input.GetRemoteUri(); - string service = GetServiceName(remoteUri); - // We always store credentials against the given username argument for - // both vs.com and dev.azure.com-style URLs. - string account = input.UserName; + if (UsePersonalAccessTokens()) + { + string service = GetServiceName(remoteUri); - // Add or update the credential in the store. - _context.Trace.WriteLine($"Storing credential with service={service} account={account}..."); - _context.CredentialStore.AddOrUpdate(service, account, input.Password); - _context.Trace.WriteLine("Credential was successfully stored."); + // We always store credentials against the given username argument for + // both vs.com and dev.azure.com-style URLs. + string account = input.UserName; + + // Add or update the credential in the store. + _context.Trace.WriteLine($"Storing credential with service={service} account={account}..."); + _context.CredentialStore.AddOrUpdate(service, account, input.Password); + _context.Trace.WriteLine("Credential was successfully stored."); + } return Task.CompletedTask; } @@ -111,18 +126,22 @@ public Task StoreCredentialAsync(InputArguments input) public Task EraseCredentialAsync(InputArguments input) { Uri remoteUri = input.GetRemoteUri(); - string service = GetServiceName(remoteUri); - string account = GetAccountNameForCredentialQuery(input); - // Try to locate an existing credential - _context.Trace.WriteLine($"Erasing stored credential in store with service={service} account={account}..."); - if (_context.CredentialStore.Remove(service, account)) + if (UsePersonalAccessTokens()) { - _context.Trace.WriteLine("Credential was successfully erased."); - } - else - { - _context.Trace.WriteLine("No credential was erased."); + string service = GetServiceName(remoteUri); + string account = GetAccountNameForCredentialQuery(input); + + // Try to locate an existing credential + _context.Trace.WriteLine($"Erasing stored credential in store with service={service} account={account}..."); + if (_context.CredentialStore.Remove(service, account)) + { + _context.Trace.WriteLine("Credential was successfully erased."); + } + else + { + _context.Trace.WriteLine("No credential was erased."); + } } return Task.CompletedTask; @@ -134,7 +153,7 @@ protected override void ReleaseManagedResources() base.ReleaseManagedResources(); } - private async Task GenerateCredentialAsync(InputArguments input) + private async Task GeneratePersonalAccessTokenAsync(InputArguments input) { ThrowIfDisposed(); @@ -179,6 +198,54 @@ private async Task GenerateCredentialAsync(InputArguments input) return new GitCredential(result.AccountUpn, pat); } + private async Task GetAzureAccessTokenAsync(Uri remoteUri, string userName) + { + // We should not allow unencrypted communication and should inform the user + if (StringComparer.OrdinalIgnoreCase.Equals(remoteUri.Scheme, "http")) + { + throw new Exception("Unencrypted HTTP is not supported for Azure Repos. Ensure the repository remote URL is using HTTPS."); + } + + Uri orgUri = UriHelpers.CreateOrganizationUri(remoteUri, out string orgName); + + _context.Trace.WriteLine($"Determining Microsoft Authentication authority for Azure DevOps organization '{orgName}'..."); + string authAuthority = await _azDevOps.GetAuthorityAsync(orgUri); + _context.Trace.WriteLine($"Authority is '{authAuthority}'."); + + // + // If the remote URI is a classic "*.visualstudio.com" host name and we have a user specified from the + // remote then take that as the current AAD/MSA user in the first instance. + // + // For "dev.azure.com" host names we only use the user info part of the remote when this doesn't + // match the Azure DevOps organization name. Our friends in Azure DevOps decided "borrow" the username + // part of the remote URL to include the organization name (not an actual username). + // + if (UriHelpers.IsAzureDevOpsHost(remoteUri.Host) && StringComparer.OrdinalIgnoreCase.Equals(orgName, userName)) + { + _context.Trace.WriteLine("Cannot use username from dev.azure.com remote URL because this is the Azure DevOps organization name."); + userName = null; + } + else if (!string.IsNullOrWhiteSpace(userName)) + { + _context.Trace.WriteLine("Using username as specified in remote."); + } + + _context.Trace.WriteLine(string.IsNullOrWhiteSpace(userName) ? "No user found." : $"User is '{userName}'."); + + // Get an AAD access token for the Azure DevOps SPS + _context.Trace.WriteLine("Getting Azure AD access token..."); + IMicrosoftAuthenticationResult result = await _msAuth.GetTokenAsync( + authAuthority, + GetClientId(), + GetRedirectUri(), + AzureDevOpsConstants.AzureDevOpsDefaultScopes, + userName); + _context.Trace.WriteLineSecrets( + $"Acquired Azure access token. Account='{result.AccountUpn}' Token='{{0}}'", new object[] {result.AccessToken}); + + return result; + } + private string GetClientId() { // Check for developer override value @@ -229,6 +296,7 @@ private Uri GetRedirectUri() /// private static string GetServiceName(Uri remoteUri) { + // dev.azure.com if (UriHelpers.IsDevAzureComHost(remoteUri.Host)) { // We can never store the new dev.azure.com-style URLs against the full path because @@ -278,6 +346,41 @@ private static string GetAccountNameForCredentialQuery(InputArguments input) throw new InvalidOperationException("Host is not Azure DevOps."); } + /// + /// Check if Azure DevOps Personal Access Tokens should be used or not. + /// + /// True if Personal Access Tokens should be used, false otherwise. + private bool UsePersonalAccessTokens() + { + // Default to using PATs whilst the Azure AT functionality is being tested + const bool defaultValue = true; + + if (_context.Settings.TryGetSetting( + AzureDevOpsConstants.EnvironmentVariables.CredentialType, + KnownGitCfg.Credential.SectionName, + AzureDevOpsConstants.GitConfiguration.Credential.CredentialType, + out string valueStr)) + { + _context.Trace.WriteLine($"Azure Repos credential type override set to '{valueStr}'"); + + switch (valueStr.ToLowerInvariant()) + { + case AzureDevOpsConstants.PatCredentialType: + return true; + + case AzureDevOpsConstants.OAuthCredentialType: + return false; + + default: + _context.Streams.Error.WriteLine( + $"warning: unknown Azure Repos credential type '{valueStr}' - using default option"); + return defaultValue; + } + } + + return defaultValue; + } + #endregion #region IConfigurationComponent From 8e351df699e49f23d724674ee362be2953857ea7 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Mon, 8 Mar 2021 11:58:02 +0000 Subject: [PATCH 13/17] azrepos: add Azure authority cache Add a cache of the Azure backing authority for Azure DevOps orgs. This cache is only consulted when the credential type is "oauth" and not "pat". We use Git's configuration as the persistence mechanism. --- .../AzureReposAuthorityCacheTests.cs | 189 ++++++++++++++++++ .../AzureReposHostProviderTests.cs | 47 ++++- .../AzureDevOpsAuthorityCache.cs | 127 ++++++++++++ .../AzureDevOpsConstants.cs | 4 + .../AzureReposHostProvider.cs | 23 ++- 5 files changed, 376 insertions(+), 14 deletions(-) create mode 100644 src/shared/Microsoft.AzureRepos.Tests/AzureReposAuthorityCacheTests.cs create mode 100644 src/shared/Microsoft.AzureRepos/AzureDevOpsAuthorityCache.cs diff --git a/src/shared/Microsoft.AzureRepos.Tests/AzureReposAuthorityCacheTests.cs b/src/shared/Microsoft.AzureRepos.Tests/AzureReposAuthorityCacheTests.cs new file mode 100644 index 000000000..18e406322 --- /dev/null +++ b/src/shared/Microsoft.AzureRepos.Tests/AzureReposAuthorityCacheTests.cs @@ -0,0 +1,189 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. +using System; +using System.Collections.Generic; +using System.Globalization; +using Microsoft.Git.CredentialManager; +using Microsoft.Git.CredentialManager.Tests.Objects; +using Xunit; + +namespace Microsoft.AzureRepos.Tests +{ + public class AzureReposAuthorityCacheTests + { + [Fact] + public void AzureReposAuthorityCache_GetAuthority_Null_ThrowException() + { + var trace = new NullTrace(); + var git = new TestGit(); + var cache = new AzureDevOpsAuthorityCache(trace, git); + + Assert.Throws(() => cache.GetAuthority(null)); + } + + [Fact] + public void AzureReposAuthorityCache_GetAuthority_NoCachedAuthority_ReturnsNull() + { + string key = CreateKey("contoso"); + + var trace = new NullTrace(); + var git = new TestGit(); + var cache = new AzureDevOpsAuthorityCache(trace, git); + + string authority = cache.GetAuthority(key); + + Assert.Null(authority); + } + + [Fact] + public void AzureReposAuthorityCache_GetAuthority_CachedAuthority_ReturnsAuthority() + { + const string orgName = "contoso"; + string key = CreateKey(orgName); + const string expectedAuthority = "https://login.contoso.com"; + + var git = new TestGit + { + Configuration = + { + Global = + { + [key] = new[] {expectedAuthority} + } + } + }; + + var trace = new NullTrace(); + var cache = new AzureDevOpsAuthorityCache(trace, git); + + string actualAuthority = cache.GetAuthority(orgName); + + Assert.Equal(expectedAuthority, actualAuthority); + } + + [Fact] + public void AzureReposAuthorityCache_UpdateAuthority_NoCachedAuthority_SetsAuthority() + { + const string orgName = "contoso"; + string key = CreateKey(orgName); + const string expectedAuthority = "https://login.contoso.com"; + + var trace = new NullTrace(); + var git = new TestGit(); + var cache = new AzureDevOpsAuthorityCache(trace, git); + + cache.UpdateAuthority(orgName, expectedAuthority); + + Assert.True(git.Configuration.Global.TryGetValue(key, out IList values)); + Assert.Single(values); + string actualAuthority = values[0]; + Assert.Equal(expectedAuthority, actualAuthority); + } + + [Fact] + public void AzureReposAuthorityCache_UpdateAuthority_CachedAuthority_UpdatesAuthority() + { + const string orgName = "contoso"; + string key = CreateKey(orgName); + const string oldAuthority = "https://old-login.contoso.com"; + const string expectedAuthority = "https://login.contoso.com"; + + var git = new TestGit + { + Configuration = + { + Global = + { + [key] = new[] {oldAuthority} + } + } + }; + + var trace = new NullTrace(); + var cache = new AzureDevOpsAuthorityCache(trace, git); + + cache.UpdateAuthority(orgName, expectedAuthority); + + Assert.True(git.Configuration.Global.TryGetValue(key, out IList values)); + Assert.Single(values); + string actualAuthority = values[0]; + Assert.Equal(expectedAuthority, actualAuthority); + } + + [Fact] + public void AzureReposAuthorityCache_EraseAuthority_NoCachedAuthority_DoesNothing() + { + const string orgName = "contoso"; + string key = CreateKey(orgName); + string otherKey = CreateKey("org.fabrikam.authority"); + const string otherAuthority = "https://fabrikam.com/login"; + + var git = new TestGit + { + Configuration = + { + Global = + { + [otherKey] = new[] {otherAuthority} + } + } + }; + + var trace = new NullTrace(); + var cache = new AzureDevOpsAuthorityCache(trace, git); + + cache.EraseAuthority(orgName); + + // Other entries should remain + Assert.False(git.Configuration.Global.ContainsKey(key)); + Assert.Single(git.Configuration.Global); + Assert.True(git.Configuration.Global.TryGetValue(otherKey, out IList values)); + Assert.Single(values); + string actualOtherAuthority = values[0]; + Assert.Equal(otherAuthority, actualOtherAuthority); + } + + [Fact] + public void AzureReposAuthorityCache_EraseAuthority_CachedAuthority_RemovesAuthority() + { + const string orgName = "contoso"; + string key = CreateKey(orgName); + const string authority = "https://login.contoso.com"; + string otherKey = CreateKey("fabrikam"); + const string otherAuthority = "https://fabrikam.com/login"; + + var git = new TestGit + { + Configuration = + { + Global = + { + [key] = new[] {authority}, + [otherKey] = new[] {otherAuthority} + } + } + }; + + var trace = new NullTrace(); + var cache = new AzureDevOpsAuthorityCache(trace, git); + + cache.EraseAuthority(orgName); + + // Only the other entries should remain + Assert.False(git.Configuration.Global.ContainsKey(key)); + Assert.Single(git.Configuration.Global); + Assert.True(git.Configuration.Global.TryGetValue(otherKey, out IList values)); + Assert.Single(values); + string actualOtherAuthority = values[0]; + Assert.Equal(otherAuthority, actualOtherAuthority); + } + + private static string CreateKey(string orgName) + { + return string.Format(CultureInfo.InvariantCulture, "{0}.{1}:{2}/{3}.{4}", + Constants.GitConfiguration.Credential.SectionName, + AzureDevOpsConstants.UrnScheme, AzureDevOpsConstants.UrnOrgPrefix, orgName, + AzureDevOpsConstants.GitConfiguration.Credential.AzureAuthority); + } + } +} diff --git a/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs b/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs index 678e9c2de..e317b4253 100644 --- a/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs +++ b/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs @@ -132,15 +132,17 @@ public async Task AzureReposProvider_GetCredentialAsync_UnencryptedHttp_ThrowsEx var context = new TestCommandContext(); var azDevOps = Mock.Of(); var msAuth = Mock.Of(); + var authorityCache = Mock.Of(); - var provider = new AzureReposHostProvider(context, azDevOps, msAuth); + var provider = new AzureReposHostProvider(context, azDevOps, msAuth, authorityCache); await Assert.ThrowsAsync(() => provider.GetCredentialAsync(input)); } [Fact] - public async Task AzureReposProvider_GetCredentialAsync_JwtMode_VsComUrlUser_ReturnsCredential() + public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_VsComUrlUser_ReturnsCredential() { + var orgName = "org"; var urlAccount = "jane.doe"; var input = new InputArguments(new Dictionary @@ -172,7 +174,10 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_VsComUrlUser_Ret msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, urlAccount)) .ReturnsAsync(authResult); - var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object); + var authorityCacheMock = new Mock(MockBehavior.Strict); + authorityCacheMock.Setup(x => x.GetAuthority(orgName)).Returns(authorityUrl); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object); ICredential credential = await provider.GetCredentialAsync(input); @@ -182,8 +187,9 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_VsComUrlUser_Ret } [Fact] - public async Task AzureReposProvider_GetCredentialAsync_JwtMode_DevAzureUrlUser_ReturnsCredential() + public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_DevAzureUrlUser_ReturnsCredential() { + var orgName = "org"; var urlAccount = "jane.doe"; var input = new InputArguments(new Dictionary @@ -216,7 +222,10 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_DevAzureUrlUser_ msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, urlAccount)) .ReturnsAsync(authResult); - var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object); + var authorityCacheMock = new Mock(MockBehavior.Strict); + authorityCacheMock.Setup(x => x.GetAuthority(orgName)).Returns(authorityUrl); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object); ICredential credential = await provider.GetCredentialAsync(input); @@ -226,8 +235,10 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_DevAzureUrlUser_ } [Fact] - public async Task AzureReposProvider_GetCredentialAsync_JwtMode_DevAzureUrlOrgName_ReturnsCredential() + public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_DevAzureUrlOrgName_ReturnsCredential() { + var orgName = "org"; + var input = new InputArguments(new Dictionary { ["protocol"] = "https", @@ -259,7 +270,10 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_DevAzureUrlOrgNa msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null)) .ReturnsAsync(authResult); - var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object); + var authorityCacheMock = new Mock(MockBehavior.Strict); + authorityCacheMock.Setup(x => x.GetAuthority(orgName)).Returns(authorityUrl); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object); ICredential credential = await provider.GetCredentialAsync(input); @@ -269,8 +283,10 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_DevAzureUrlOrgNa } [Fact] - public async Task AzureReposProvider_GetCredentialAsync_JwtMode_NoUser_ReturnsCredential() + public async Task AzureReposProvider_GetCredentialAsync_JwtMode_NoCachedAuthority_ReturnsCredential() { + var orgName = "org"; + var input = new InputArguments(new Dictionary { ["protocol"] = "https", @@ -301,7 +317,11 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_NoUser_ReturnsCr msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null)) .ReturnsAsync(authResult); - var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object); + var authorityCacheMock = new Mock(MockBehavior.Strict); + authorityCacheMock.Setup(x => x.GetAuthority(It.IsAny())).Returns((string)null); + authorityCacheMock.Setup(x => x.UpdateAuthority(orgName, authorityUrl)); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object); ICredential credential = await provider.GetCredentialAsync(input); @@ -313,6 +333,8 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_NoUser_ReturnsCr [Fact] public async Task AzureReposProvider_GetCredentialAsync_PatMode_NoExistingPat_GeneratesCredential() { + var orgName = "org"; + var input = new InputArguments(new Dictionary { ["protocol"] = "https", @@ -342,7 +364,9 @@ public async Task AzureReposProvider_GetCredentialAsync_PatMode_NoExistingPat_Ge msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null)) .ReturnsAsync(authResult); - var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object); + var authorityCacheMock = new Mock(MockBehavior.Strict); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object); ICredential credential = await provider.GetCredentialAsync(input); @@ -372,8 +396,9 @@ public async Task AzureReposProvider_GetCredentialAsync_PatMode_ExistingPat_Retu var azDevOps = Mock.Of(); var msAuth = Mock.Of(); + var authorityCache = Mock.Of(); - var provider = new AzureReposHostProvider(context, azDevOps, msAuth); + var provider = new AzureReposHostProvider(context, azDevOps, msAuth, authorityCache); ICredential credential = await provider.GetCredentialAsync(input); diff --git a/src/shared/Microsoft.AzureRepos/AzureDevOpsAuthorityCache.cs b/src/shared/Microsoft.AzureRepos/AzureDevOpsAuthorityCache.cs new file mode 100644 index 000000000..a379e4d61 --- /dev/null +++ b/src/shared/Microsoft.AzureRepos/AzureDevOpsAuthorityCache.cs @@ -0,0 +1,127 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. +using System; +using System.Collections.Generic; +using System.Globalization; +using Microsoft.Git.CredentialManager; + +namespace Microsoft.AzureRepos +{ + public interface IAzureDevOpsAuthorityCache + { + /// + /// Lookup the cached authority for the specified Azure DevOps organization. + /// + /// Azure DevOps organization name. + /// Authority for the organization, or null if not found. + string GetAuthority(string orgName); + + /// + /// Updates the cached authority for the specified Azure DevOps organization. + /// + /// Azure DevOps organization name. + /// New authority value. + void UpdateAuthority(string orgName, string authority); + + /// + /// Erase the cached authority for the specified Azure DevOps organization. + /// + /// Azure DevOps organization name. + void EraseAuthority(string orgName); + + /// + /// Clear all cached authorities. + /// + void Clear(); + } + + public class AzureDevOpsAuthorityCache : IAzureDevOpsAuthorityCache + { + private readonly ITrace _trace; + private readonly IGit _git; + + public AzureDevOpsAuthorityCache(ICommandContext context) + : this(context.Trace, context.Git) { } + + public AzureDevOpsAuthorityCache(ITrace trace, IGit git) + { + EnsureArgument.NotNull(trace, nameof(trace)); + EnsureArgument.NotNull(git, nameof(git)); + + _trace = trace; + _git = git; + } + + public string GetAuthority(string orgName) + { + EnsureArgument.NotNullOrWhiteSpace(orgName, nameof(orgName)); + + _trace.WriteLine($"Looking up cached authority for organization '{orgName}'..."); + + IGitConfiguration config = _git.GetConfiguration(); + + if (config.TryGet(GitConfigurationLevel.Global, GetAuthorityKey(orgName), out string authority)) + { + return authority; + } + + return null; + } + + public void UpdateAuthority(string orgName, string authority) + { + EnsureArgument.NotNullOrWhiteSpace(orgName, nameof(orgName)); + + _trace.WriteLine($"Updating cached authority for '{orgName}' to '{authority}'..."); + + IGitConfiguration config = _git.GetConfiguration(); + config.Set(GitConfigurationLevel.Global, GetAuthorityKey(orgName), authority); + } + + public void EraseAuthority(string orgName) + { + EnsureArgument.NotNullOrWhiteSpace(orgName, nameof(orgName)); + + _trace.WriteLine($"Removing cached authority for '{orgName}'..."); + IGitConfiguration config = _git.GetConfiguration(); + config.Unset(GitConfigurationLevel.Global, GetAuthorityKey(orgName)); + } + + public void Clear() + { + _trace.WriteLine("Clearing all cached authorities..."); + + IGitConfiguration config = _git.GetConfiguration(); + + var orgKeys = new HashSet(GitConfigurationKeyComparer.Instance); + config.Enumerate( + GitConfigurationLevel.Global, + Constants.GitConfiguration.Credential.SectionName, + AzureDevOpsConstants.GitConfiguration.Credential.AzureAuthority, + entry => + { + if (GitConfigurationKeyComparer.TrySplit(entry.Key, out _, out string scope, out _) && + Uri.TryCreate(scope, UriKind.Absolute, out Uri orgUrn) && + orgUrn.Scheme == AzureDevOpsConstants.UrnScheme) + { + orgKeys.Add(entry.Key); + } + + return true; + }); + + foreach (string orgKey in orgKeys) + { + config.Unset(GitConfigurationLevel.Global, orgKey); + } + } + + private static string GetAuthorityKey(string orgName) + { + return string.Format(CultureInfo.InvariantCulture, "{0}.{1}:{2}/{3}.{4}", + Constants.GitConfiguration.Credential.SectionName, + AzureDevOpsConstants.UrnScheme, AzureDevOpsConstants.UrnOrgPrefix, orgName, + AzureDevOpsConstants.GitConfiguration.Credential.AzureAuthority); + } + } +} diff --git a/src/shared/Microsoft.AzureRepos/AzureDevOpsConstants.cs b/src/shared/Microsoft.AzureRepos/AzureDevOpsConstants.cs index 3eea95449..2d6ecb195 100644 --- a/src/shared/Microsoft.AzureRepos/AzureDevOpsConstants.cs +++ b/src/shared/Microsoft.AzureRepos/AzureDevOpsConstants.cs @@ -27,6 +27,9 @@ internal static class AzureDevOpsConstants public const string PatCredentialType = "pat"; public const string OAuthCredentialType = "oauth"; + public const string UrnScheme = "azrepos"; + public const string UrnOrgPrefix = "org"; + public static class PersonalAccessTokenScopes { public const string ReposWrite = "vso.code_write"; @@ -49,6 +52,7 @@ public static class Credential public const string DevAadRedirectUri = "azreposDevRedirectUri"; public const string DevAadAuthorityBaseUri = "azreposDevAuthorityBaseUri"; public const string CredentialType = "azreposCredentialType"; + public const string AzureAuthority = "azureAuthority"; } } } diff --git a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs index 51cf2257b..cfc8e5607 100644 --- a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs +++ b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs @@ -17,22 +17,26 @@ public class AzureReposHostProvider : DisposableObject, IHostProvider, IConfigur private readonly ICommandContext _context; private readonly IAzureDevOpsRestApi _azDevOps; private readonly IMicrosoftAuthentication _msAuth; + private readonly IAzureDevOpsAuthorityCache _authorityCache; public AzureReposHostProvider(ICommandContext context) - : this(context, new AzureDevOpsRestApi(context), new MicrosoftAuthentication(context)) + : this(context, new AzureDevOpsRestApi(context), new MicrosoftAuthentication(context), + new AzureDevOpsAuthorityCache(context)) { } public AzureReposHostProvider(ICommandContext context, IAzureDevOpsRestApi azDevOps, - IMicrosoftAuthentication msAuth) + IMicrosoftAuthentication msAuth, IAzureDevOpsAuthorityCache authorityCache) { EnsureArgument.NotNull(context, nameof(context)); EnsureArgument.NotNull(azDevOps, nameof(azDevOps)); EnsureArgument.NotNull(msAuth, nameof(msAuth)); + EnsureArgument.NotNull(authorityCache, nameof(authorityCache)); _context = context; _azDevOps = azDevOps; _msAuth = msAuth; + _authorityCache = authorityCache; } #region IHostProvider @@ -143,6 +147,12 @@ public Task EraseCredentialAsync(InputArguments input) _context.Trace.WriteLine("No credential was erased."); } } + else + { + // Clear the authority cache in case this was the reason for failure + string orgName = UriHelpers.GetOrganizationName(remoteUri); + _authorityCache.EraseAuthority(orgName); + } return Task.CompletedTask; } @@ -209,7 +219,14 @@ private async Task GetAzureAccessTokenAsync(Uri Uri orgUri = UriHelpers.CreateOrganizationUri(remoteUri, out string orgName); _context.Trace.WriteLine($"Determining Microsoft Authentication authority for Azure DevOps organization '{orgName}'..."); - string authAuthority = await _azDevOps.GetAuthorityAsync(orgUri); + string authAuthority = _authorityCache.GetAuthority(orgName); + if (authAuthority is null) + { + // If there is no cached value we must query for it and cache it for future use + _context.Trace.WriteLine($"No cached authority value - querying {orgUri} for authority..."); + authAuthority = await _azDevOps.GetAuthorityAsync(orgUri); + _authorityCache.UpdateAuthority(orgName, authAuthority); + } _context.Trace.WriteLine($"Authority is '{authAuthority}'."); // From 242073a1293874a7afed681ac2386c1b7a4083ca Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Mon, 8 Mar 2021 12:02:30 +0000 Subject: [PATCH 14/17] azrepos: add cmd to clear the authority cache Add a command to enable clearing of the Azure authority cache manually. --- docs/usage.md | 5 ++++ .../AzureReposHostProvider.cs | 28 ++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/docs/usage.md b/docs/usage.md index 567457b4e..699aeb481 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -30,3 +30,8 @@ Read the [Git manual](https://git-scm.com/docs/gitcredentials#_custom_helpers) a Set your user-level Git configuration (`~/.gitconfig`) to use GCM Core. If you pass `--system` to these commands, they act on the system-level Git configuration (`/etc/gitconfig`) instead. + +### azure-repos (experimental) + +Interact with the Azure Repos host provider to manage the authentication +authority cache. diff --git a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs index cfc8e5607..5b8a6f409 100644 --- a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs +++ b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs @@ -2,6 +2,8 @@ // Licensed under the MIT license. using System; using System.Collections.Generic; +using System.CommandLine; +using System.CommandLine.Invocation; using System.Linq; using System.Net.Http; using System.Threading.Tasks; @@ -12,7 +14,7 @@ namespace Microsoft.AzureRepos { - public class AzureReposHostProvider : DisposableObject, IHostProvider, IConfigurableComponent + public class AzureReposHostProvider : DisposableObject, IHostProvider, IConfigurableComponent, ICommandProvider { private readonly ICommandContext _context; private readonly IAzureDevOpsRestApi _azDevOps; @@ -452,5 +454,29 @@ public Task UnconfigureAsync(ConfigurationTarget target) } #endregion + + #region ICommandProvider + + ProviderCommand ICommandProvider.CreateCommand() + { + var clearCacheCmd = new Command("clear-cache") + { + Description = "Clear the Azure authority cache", + Handler = CommandHandler.Create(ClearCacheCmd), + }; + + var rootCmd = new ProviderCommand(this); + rootCmd.AddCommand(clearCacheCmd); + return rootCmd; + } + + private int ClearCacheCmd() + { + _authorityCache.Clear(); + _context.Streams.Out.WriteLine("Authority cache cleared"); + return 0; + } + + #endregion } } From 606a4751d1bd64654a5fc6e18b3fabc44734319b Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Mon, 8 Mar 2021 13:31:47 +0000 Subject: [PATCH 15/17] azrepos: add a binding manager to store org-user maps Add a new binding manager component that can be used to 'bind' user accounts to Azure DevOps organisations. This enables the Azure Repos host provider to attempt silent authentication requests via MSAL - to attempt to use an existing access token from the cache. We only allow binding at the organisation level (and not any other level) as this is the most common scenario: one user for an entire Azure DevOps organisation. If the user wishes to override the chosen user for a particular clone they can do so by binding the user to the local repository configuration, rather than the default global configuration. Furthermore, if the user wishes to use a different user account for a particular remote within a repository, they can set the username in the remote URL. --- .../AzureReposBindingManagerTests.cs | 780 ++++++++++++++++++ .../AzureReposHostProviderTests.cs | 128 ++- .../AzureReposBindingManager.cs | 325 ++++++++ .../AzureReposHostProvider.cs | 36 +- .../Constants.cs | 1 + .../Microsoft.Git.CredentialManager/Git.cs | 15 +- 6 files changed, 1268 insertions(+), 17 deletions(-) create mode 100644 src/shared/Microsoft.AzureRepos.Tests/AzureReposBindingManagerTests.cs create mode 100644 src/shared/Microsoft.AzureRepos/AzureReposBindingManager.cs diff --git a/src/shared/Microsoft.AzureRepos.Tests/AzureReposBindingManagerTests.cs b/src/shared/Microsoft.AzureRepos.Tests/AzureReposBindingManagerTests.cs new file mode 100644 index 000000000..98ee4fe40 --- /dev/null +++ b/src/shared/Microsoft.AzureRepos.Tests/AzureReposBindingManagerTests.cs @@ -0,0 +1,780 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using Microsoft.Git.CredentialManager; +using Microsoft.Git.CredentialManager.Tests.Objects; +using Xunit; + +namespace Microsoft.AzureRepos.Tests +{ + public class AzureReposBindingManagerTests + { + #region Bind + + [Fact] + public void AzureReposBindingManager_Bind_NullOrganization_ThrowException() + { + var trace = new NullTrace(); + var git = new TestGit(); + var manager = new AzureReposBindingManager(trace, git); + + Assert.Throws(() => manager.Bind(null, "user", false)); + } + + [Fact] + public void AzureReposBindingManager_Bind_NoUser_SetsOrgKey() + { + const string expectedUser = "user1"; + const string orgName = "org"; + + var trace = new NullTrace(); + var git = new TestGit(); + var manager = new AzureReposBindingManager(trace, git); + + manager.Bind(orgName, expectedUser, false); + + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var users)); + Assert.Single(users); + string actualUser = users[0]; + Assert.Equal(expectedUser, actualUser); + } + + [Fact] + public void AzureReposBindingManager_BindLocal_NoUser_SetsOrgKey() + { + const string expectedUser = "user1"; + const string orgName = "org"; + + var trace = new NullTrace(); + var git = new TestGit(); + var manager = new AzureReposBindingManager(trace, git); + + manager.Bind(orgName, expectedUser, true); + + Assert.True(git.Configuration.Local.TryGetValue(CreateKey(orgName), out var users)); + Assert.Single(users); + string actualUser = users[0]; + Assert.Equal(expectedUser, actualUser); + } + + [Fact] + public void AzureReposBindingManager_Bind_ExistingUser_SetsOrgKey() + { + const string expectedUser = "user1"; + const string orgName = "org"; + + var trace = new NullTrace(); + var git = new TestGit(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new[] {"org-user"}; + + manager.Bind(orgName, expectedUser, false); + + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var users)); + Assert.Single(users); + string actualUser = users[0]; + Assert.Equal(expectedUser, actualUser); + } + + [Fact] + public void AzureReposBindingManager_BindLocal_ExistingUser_SetsOrgKey() + { + const string expectedUser = "user1"; + const string orgName = "org"; + + var trace = new NullTrace(); + var git = new TestGit(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Local[CreateKey(orgName)] = new[] {"org-user"}; + + manager.Bind(orgName, expectedUser, true); + + Assert.True(git.Configuration.Local.TryGetValue(CreateKey(orgName), out var users)); + Assert.Single(users); + string actualUser = users[0]; + Assert.Equal(expectedUser, actualUser); + } + + #endregion + + #region Unbind + + [Fact] + public void AzureReposBindingManager_Unbind_NullOrganization_ThrowException() + { + var trace = new NullTrace(); + var git = new TestGit(); + var manager = new AzureReposBindingManager(trace, git); + + Assert.Throws(() => manager.Unbind(null, false)); + } + + [Fact] + public void AzureReposBindingManager_Unbind_NoUser_DoesNothing() + { + const string orgName = "org"; + + var trace = new NullTrace(); + var git = new TestGit(); + var manager = new AzureReposBindingManager(trace, git); + + manager.Unbind(orgName, false); + + Assert.Empty(git.Configuration.Global); + } + + [Fact] + public void AzureReposBindingManager_UnbindLocal_NoUser_DoesNothing() + { + const string orgName = "org"; + + var trace = new NullTrace(); + var git = new TestGit(); + var manager = new AzureReposBindingManager(trace, git); + + manager.Unbind(orgName, true); + + Assert.Empty(git.Configuration.Local); + } + + [Fact] + public void AzureReposBindingManager_Unbind_ExistingUser_RemovesKey() + { + const string orgName = "org"; + + var trace = new NullTrace(); + var git = new TestGit(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new[] {"org-user"}; + + manager.Unbind(orgName, false); + + Assert.Empty(git.Configuration.Global); + } + + [Fact] + public void AzureReposBindingManager_UnbindLocal_ExistingUser_RemovesKey() + { + const string orgName = "org"; + + var trace = new NullTrace(); + var git = new TestGit(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Local[CreateKey(orgName)] = new[] {"org-user"}; + + manager.Unbind(orgName, true); + + Assert.Empty(git.Configuration.Local); + } + + #endregion + + #region GetBinding + + [Fact] + public void AzureReposBindingManager_GetBinding_Null_ThrowException() + { + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + Assert.Throws(() => manager.GetBinding(null)); + } + + [Fact] + public void AzureReposBindingManager_GetBinding_NoUser_ReturnsNull() + { + const string orgName = "org"; + + var trace = new NullTrace(); + var git = new TestGit(); + var manager = new AzureReposBindingManager(trace, git); + + AzureReposBinding binding = manager.GetBinding(orgName); + + Assert.Null(binding); + } + + [Fact] + public void AzureReposBindingManager_GetBinding_GlobalUser_ReturnsBinding() + { + const string orgUser = "john.doe"; + const string orgName = "org"; + string orgKey = CreateKey(orgName); + + var git = new TestGit + { + Configuration = + { + Global = + { + [orgKey] = new[] {orgUser} + } + } + }; + + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + AzureReposBinding binding = manager.GetBinding(orgName); + + Assert.Equal(orgName, binding.Organization); + Assert.Equal(orgUser, binding.GlobalUserName); + Assert.Null(binding.LocalUserName); + } + + [Fact] + public void AzureReposBindingManager_GetBinding_LocalUser_ReturnsBinding() + { + const string orgUser = "john.doe"; + const string orgName = "org"; + string orgKey = CreateKey(orgName); + + var git = new TestGit + { + Configuration = + { + Local = + { + [orgKey] = new[] {orgUser} + } + } + }; + + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + AzureReposBinding binding = manager.GetBinding(orgName); + + Assert.Equal(orgName, binding.Organization); + Assert.Null(binding.GlobalUserName); + Assert.Equal(orgUser, binding.LocalUserName); + } + + [Fact] + public void AzureReposBindingManager_GetBinding_LocalAndGlobalUsers_ReturnsBinding() + { + const string orgUserLocal = "john.doe"; + const string orgUserGlobal = "jane.doe"; + const string orgName = "org"; + string orgKey = CreateKey(orgName); + + var git = new TestGit + { + Configuration = + { + Global = { [orgKey] = new[] {orgUserGlobal} }, + Local = { [orgKey] = new[] {orgUserLocal} } + } + }; + + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + AzureReposBinding binding = manager.GetBinding(orgName); + + Assert.Equal(orgName, binding.Organization); + Assert.Equal(orgUserGlobal, binding.GlobalUserName); + Assert.Equal(orgUserLocal, binding.LocalUserName); + } + + #endregion + + #region GetBindings + + [Fact] + public void AzureReposBindingManager_GetBindings_NoUsers_ReturnsEmpty() + { + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + IList actual = manager.GetBindings().ToList(); + + Assert.Empty(actual); + } + + [Fact] + public void AzureReposBindingManager_GetBindings_Users_ReturnsUsers() + { + const string org1 = "org1"; + const string org2 = "org2"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(org1)] = new[] {"user1"}; + git.Configuration.Global[CreateKey(org2)] = new[] {"user2"}; + + AzureReposBinding[] bindings = manager.GetBindings().ToArray(); + + static void AssertBinding( + string expectedOrg, string expectedGlobalUser, string expectedLocalUser, AzureReposBinding binding) + { + Assert.Equal(expectedOrg, binding.Organization); + Assert.Equal(expectedGlobalUser, binding.GlobalUserName); + Assert.Equal(expectedLocalUser, binding.LocalUserName); + } + + Assert.Equal(2, bindings.Length); + AssertBinding(org1, "user1", null, bindings[0]); + AssertBinding(org2, "user2", null, bindings[1]); + } + + #endregion + + #region GetUser + + [Fact] + public void AzureReposBindingManager_GetUser_NullOrg_ThrowsException() + { + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + Assert.Throws(() => manager.GetUser(null)); + } + + [Fact] + public void AzureReposBindingManager_GetUser_NoUser_ReturnsNull() + { + const string orgName = "org"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + string actualUser = manager.GetUser(orgName); + + Assert.Null(actualUser); + } + + [Fact] + public void AzureReposBindingManager_GetUser_GlobalUser_ReturnsGlobalUser() + { + const string orgName = "org"; + const string user1 = "user1"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new[] {user1}; + + string actualUser = manager.GetUser(orgName); + + Assert.Equal(user1, actualUser); + } + + [Fact] + public void AzureReposBindingManager_GetUser_LocalUser_ReturnsLocalUser() + { + const string orgName = "org"; + const string user1 = "user1"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Local[CreateKey(orgName)] = new[] {user1}; + + string actualUser = manager.GetUser(orgName); + + Assert.Equal(user1, actualUser); + } + + [Fact] + public void AzureReposBindingManager_GetUser_GlobalAndLocalUsers_ReturnsLocalUser() + { + const string orgName = "org"; + const string user1 = "user1"; + const string user2 = "user2"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new[] {user1}; + git.Configuration.Local[CreateKey(orgName)] = new[] {user2}; + + string actualUser = manager.GetUser(orgName); + + Assert.Equal(user2, actualUser); + } + + #endregion + + #region SignIn + + [Fact] + public void AzureReposBindingManager_SignIn_NullOrg_ThrowsException() + { + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + Assert.Throws(() => manager.SignIn(null, "user1")); + } + + [Fact] + public void AzureReposBindingManager_SignIn_NullUser_ThrowsException() + { + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + Assert.Throws(() => manager.SignIn("org", null)); + } + + [Fact] + public void AzureReposBindingManager_SignIn_NoGlobalNoLocal_BindsGlobal() + { + const string orgName = "org"; + const string user1 = "user1"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + manager.SignIn(orgName, user1); + + Assert.Empty(git.Configuration.Local); + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var globalUsers)); + string actualUser = globalUsers[0]; + Assert.Equal(user1, actualUser); + } + + [Fact] + public void AzureReposBindingManager_SignIn_NoGlobalSameLocal_BindsGlobalUnbindLocal() + { + const string orgName = "org"; + const string user1 = "user1"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Local[CreateKey(orgName)] = new []{user1}; + + manager.SignIn(orgName, user1); + + Assert.Empty(git.Configuration.Local); + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var globalUsers)); + string actualUser = globalUsers[0]; + Assert.Equal(user1, actualUser); + } + + [Fact] + public void AzureReposBindingManager_SignIn_NoGlobalOtherLocal_BindsGlobalUnbindLocal() + { + const string orgName = "org"; + const string user1 = "user1"; + const string user2 = "user2"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Local[CreateKey(orgName)] = new []{user2}; + + manager.SignIn(orgName, user1); + + Assert.Empty(git.Configuration.Local); + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var globalUsers)); + string actualUser = globalUsers[0]; + Assert.Equal(user1, actualUser); + } + + [Fact] + public void AzureReposBindingManager_SignIn_SameGlobalNoLocal_DoesNothing() + { + const string orgName = "org"; + const string user1 = "user1"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new []{user1}; + + manager.SignIn(orgName, user1); + + Assert.Empty(git.Configuration.Local); + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var globalUsers)); + string actualUser = globalUsers[0]; + Assert.Equal(user1, actualUser); + } + + [Fact] + public void AzureReposBindingManager_SignIn_SameGlobalSameLocal_UnbindLocal() + { + const string orgName = "org"; + const string user1 = "user1"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new []{user1}; + git.Configuration.Local[CreateKey(orgName)] = new []{user1}; + + manager.SignIn(orgName, user1); + + Assert.Empty(git.Configuration.Local); + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var globalUsers)); + string actualUser = globalUsers[0]; + Assert.Equal(user1, actualUser); + } + + [Fact] + public void AzureReposBindingManager_SignIn_SameGlobalOtherLocal_UnbindLocal() + { + const string orgName = "org"; + const string user1 = "user1"; + const string user2 = "user2"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new []{user1}; + git.Configuration.Local[CreateKey(orgName)] = new []{user2}; + + manager.SignIn(orgName, user1); + + Assert.Empty(git.Configuration.Local); + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var globalUsers)); + string actualUser = globalUsers[0]; + Assert.Equal(user1, actualUser); + } + + [Fact] + public void AzureReposBindingManager_SignIn_OtherGlobalNoLocal_BindsLocal() + { + const string orgName = "org"; + const string user1 = "user1"; + const string user2 = "user2"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new []{user2}; + + manager.SignIn(orgName, user1); + + Assert.True(git.Configuration.Local.TryGetValue(CreateKey(orgName), out var localUsers)); + string actualLocalUser = localUsers[0]; + Assert.Equal(user1, actualLocalUser); + + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var globalUsers)); + string actualGlobalUser = globalUsers[0]; + Assert.Equal(user2, actualGlobalUser); + } + + [Fact] + public void AzureReposBindingManager_SignIn_OtherGlobalSameLocal_DoesNothing() + { + const string orgName = "org"; + const string user1 = "user1"; + const string user2 = "user2"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new []{user2}; + git.Configuration.Local[CreateKey(orgName)] = new []{user1}; + + manager.SignIn(orgName, user1); + + Assert.True(git.Configuration.Local.TryGetValue(CreateKey(orgName), out var localUsers)); + string actualLocalUser = localUsers[0]; + Assert.Equal(user1, actualLocalUser); + + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var globalUsers)); + string actualGlobalUser = globalUsers[0]; + Assert.Equal(user2, actualGlobalUser); + } + + [Fact] + public void AzureReposBindingManager_SignIn_OtherGlobalOtherLocal_BindsLocal() + { + const string orgName = "org"; + const string user1 = "user1"; + const string user2 = "user2"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new []{user2}; + git.Configuration.Local[CreateKey(orgName)] = new []{user2}; + + manager.SignIn(orgName, user1); + + Assert.True(git.Configuration.Local.TryGetValue(CreateKey(orgName), out var localUsers)); + string actualLocalUser = localUsers[0]; + Assert.Equal(user1, actualLocalUser); + + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var globalUsers)); + string actualGlobalUser = globalUsers[0]; + Assert.Equal(user2, actualGlobalUser); + } + + #endregion + + #region SignOut + + [Fact] + public void AzureReposBindingManager_SignOut_NoGlobalNoLocal_DoesNothing() + { + const string orgName = "org"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + manager.SignOut(orgName); + + Assert.Empty(git.Configuration.Local); + Assert.Empty(git.Configuration.Global); + } + + [Fact] + public void AzureReposBindingManager_SignOut_NoGlobalUserLocal_UnbindsLocal() + { + const string orgName = "org"; + const string user1 = "user1"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Local[CreateKey(orgName)] = new[] { user1 }; + + manager.SignOut(orgName); + + Assert.Empty(git.Configuration.Local); + Assert.Empty(git.Configuration.Global); + } + + [Fact] + public void AzureReposBindingManager_SignOut_NoGlobalNoInheritLocal_UnbindsLocal() + { + const string orgName = "org"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Local[CreateKey(orgName)] = new[] { AzureReposBinding.NoInherit }; + + manager.SignOut(orgName); + + Assert.Empty(git.Configuration.Global); + Assert.Empty(git.Configuration.Local); + } + + [Fact] + public void AzureReposBindingManager_SignOut_UserGlobalNoLocal_BindLocalNoInherit() + { + const string orgName = "org"; + const string user1 = "user1"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new[] { user1 }; + + manager.SignOut(orgName); + + Assert.True(git.Configuration.Local.TryGetValue(CreateKey(orgName), out var localUsers)); + Assert.Single(localUsers); + string actualLocalUser = localUsers[0]; + Assert.Equal(AzureReposBinding.NoInherit, actualLocalUser); + + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var globalUsers)); + Assert.Single(globalUsers); + string actualGlobalUser = globalUsers[0]; + Assert.Equal(user1, actualGlobalUser); + } + + [Fact] + public void AzureReposBindingManager_SignOut_UserGlobalNoInheritLocal_DoesNothing() + { + const string orgName = "org"; + const string user1 = "user1"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new[] { user1 }; + git.Configuration.Local[CreateKey(orgName)] = new[] { AzureReposBinding.NoInherit }; + + manager.SignOut(orgName); + + Assert.True(git.Configuration.Local.TryGetValue(CreateKey(orgName), out var localUsers)); + Assert.Single(localUsers); + string actualLocalUser = localUsers[0]; + Assert.Equal(AzureReposBinding.NoInherit, actualLocalUser); + + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var globalUsers)); + Assert.Single(globalUsers); + string actualGlobalUser = globalUsers[0]; + Assert.Equal(user1, actualGlobalUser); + } + + [Fact] + public void AzureReposBindingManager_SignOut_UserGlobalUserLocal_BindLocalNoInherit() + { + const string orgName = "org"; + const string user1 = "user1"; + const string user2 = "user2"; + + var git = new TestGit(); + var trace = new NullTrace(); + var manager = new AzureReposBindingManager(trace, git); + + git.Configuration.Global[CreateKey(orgName)] = new[] { user1 }; + git.Configuration.Local[CreateKey(orgName)] = new[] { user2 }; + + manager.SignOut(orgName); + + Assert.True(git.Configuration.Local.TryGetValue(CreateKey(orgName), out var localUsers)); + Assert.Single(localUsers); + string actualLocalUser = localUsers[0]; + Assert.Equal(AzureReposBinding.NoInherit, actualLocalUser); + + Assert.True(git.Configuration.Global.TryGetValue(CreateKey(orgName), out var globalUsers)); + Assert.Single(globalUsers); + string actualGlobalUser = globalUsers[0]; + Assert.Equal(user1, actualGlobalUser); + } + + + #endregion + + #region Helpers + + private static string CreateKey(string orgName) + { + return string.Format(CultureInfo.InvariantCulture, "{0}.{1}:{2}/{3}.{4}", + Constants.GitConfiguration.Credential.SectionName, + AzureDevOpsConstants.UrnScheme, AzureDevOpsConstants.UrnOrgPrefix, orgName, + Constants.GitConfiguration.Credential.UserName); + } + + #endregion + } +} diff --git a/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs b/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs index e317b4253..468aad804 100644 --- a/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs +++ b/src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs @@ -133,8 +133,9 @@ public async Task AzureReposProvider_GetCredentialAsync_UnencryptedHttp_ThrowsEx var azDevOps = Mock.Of(); var msAuth = Mock.Of(); var authorityCache = Mock.Of(); + var userMgr = Mock.Of(); - var provider = new AzureReposHostProvider(context, azDevOps, msAuth, authorityCache); + var provider = new AzureReposHostProvider(context, azDevOps, msAuth, authorityCache, userMgr); await Assert.ThrowsAsync(() => provider.GetCredentialAsync(input)); } @@ -177,7 +178,9 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_ var authorityCacheMock = new Mock(MockBehavior.Strict); authorityCacheMock.Setup(x => x.GetAuthority(orgName)).Returns(authorityUrl); - var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object); + var userMgrMock = new Mock(MockBehavior.Strict); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object, userMgrMock.Object); ICredential credential = await provider.GetCredentialAsync(input); @@ -225,7 +228,9 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_ var authorityCacheMock = new Mock(MockBehavior.Strict); authorityCacheMock.Setup(x => x.GetAuthority(orgName)).Returns(authorityUrl); - var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object); + var userMgrMock = new Mock(MockBehavior.Strict); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object, userMgrMock.Object); ICredential credential = await provider.GetCredentialAsync(input); @@ -273,7 +278,58 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_ var authorityCacheMock = new Mock(MockBehavior.Strict); authorityCacheMock.Setup(x => x.GetAuthority(orgName)).Returns(authorityUrl); - var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object); + var userMgrMock = new Mock(MockBehavior.Strict); + userMgrMock.Setup(x => x.GetBinding(orgName)).Returns((AzureReposBinding)null); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object, userMgrMock.Object); + + ICredential credential = await provider.GetCredentialAsync(input); + + Assert.NotNull(credential); + Assert.Equal(account, credential.Account); + Assert.Equal(accessToken, credential.Password); + } + + [Fact] + public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_NoUser_ReturnsCredential() + { + var input = new InputArguments(new Dictionary + { + ["protocol"] = "https", + ["host"] = "dev.azure.com", + ["path"] = "org/proj/_git/repo" + }); + + var expectedOrgUri = new Uri("https://dev.azure.com/org"); + var remoteUri = new Uri("https://dev.azure.com/org/proj/_git/repo"); + var orgName = "org"; + var authorityUrl = "https://login.microsoftonline.com/common"; + var expectedClientId = AzureDevOpsConstants.AadClientId; + var expectedRedirectUri = AzureDevOpsConstants.AadRedirectUri; + var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes; + var accessToken = "ACCESS-TOKEN"; + var account = "john.doe"; + var authResult = CreateAuthResult(account, accessToken); + + var context = new TestCommandContext(); + + // Use OAuth Access Tokens + context.Environment.Variables[AzureDevOpsConstants.EnvironmentVariables.CredentialType] = + AzureDevOpsConstants.OAuthCredentialType; + + var azDevOpsMock = new Mock(MockBehavior.Strict); + + var msAuthMock = new Mock(MockBehavior.Strict); + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, null)) + .ReturnsAsync(authResult); + + var authorityCacheMock = new Mock(MockBehavior.Strict); + authorityCacheMock.Setup(x => x.GetAuthority(orgName)).Returns(authorityUrl); + + var userMgrMock = new Mock(MockBehavior.Strict); + userMgrMock.Setup(x => x.GetBinding(orgName)).Returns((AzureReposBinding)null); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object, userMgrMock.Object); ICredential credential = await provider.GetCredentialAsync(input); @@ -283,7 +339,57 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_ } [Fact] - public async Task AzureReposProvider_GetCredentialAsync_JwtMode_NoCachedAuthority_ReturnsCredential() + public async Task AzureReposProvider_GetCredentialAsync_JwtMode_CachedAuthority_BoundUser_ReturnsCredential() + { + var orgName = "org"; + + var input = new InputArguments(new Dictionary + { + ["protocol"] = "https", + ["host"] = "dev.azure.com", + ["path"] = "org/proj/_git/repo" + }); + + var expectedOrgUri = new Uri("https://dev.azure.com/org"); + var remoteUri = new Uri("https://dev.azure.com/org/proj/_git/repo"); + var authorityUrl = "https://login.microsoftonline.com/common"; + var expectedClientId = AzureDevOpsConstants.AadClientId; + var expectedRedirectUri = AzureDevOpsConstants.AadRedirectUri; + var expectedScopes = AzureDevOpsConstants.AzureDevOpsDefaultScopes; + var accessToken = "ACCESS-TOKEN"; + var account = "john.doe"; + var authResult = CreateAuthResult(account, accessToken); + + var context = new TestCommandContext(); + + // Use OAuth Access Tokens + context.Environment.Variables[AzureDevOpsConstants.EnvironmentVariables.CredentialType] = + AzureDevOpsConstants.OAuthCredentialType; + + var azDevOpsMock = new Mock(MockBehavior.Strict); + + var msAuthMock = new Mock(MockBehavior.Strict); + msAuthMock.Setup(x => x.GetTokenAsync(authorityUrl, expectedClientId, expectedRedirectUri, expectedScopes, account)) + .ReturnsAsync(authResult); + + var authorityCacheMock = new Mock(MockBehavior.Strict); + authorityCacheMock.Setup(x => x.GetAuthority(orgName)).Returns(authorityUrl); + + var userMgrMock = new Mock(MockBehavior.Strict); + userMgrMock.Setup(x => x.GetBinding(orgName)) + .Returns(new AzureReposBinding(orgName, account, null)); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object, userMgrMock.Object); + + ICredential credential = await provider.GetCredentialAsync(input); + + Assert.NotNull(credential); + Assert.Equal(account, credential.Account); + Assert.Equal(accessToken, credential.Password); + } + + [Fact] + public async Task AzureReposProvider_GetCredentialAsync_JwtMode_NoCachedAuthority_NoUser_ReturnsCredential() { var orgName = "org"; @@ -321,7 +427,10 @@ public async Task AzureReposProvider_GetCredentialAsync_JwtMode_NoCachedAuthorit authorityCacheMock.Setup(x => x.GetAuthority(It.IsAny())).Returns((string)null); authorityCacheMock.Setup(x => x.UpdateAuthority(orgName, authorityUrl)); - var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object); + var userMgrMock = new Mock(MockBehavior.Strict); + userMgrMock.Setup(x => x.GetBinding(orgName)).Returns((AzureReposBinding)null); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object, userMgrMock.Object); ICredential credential = await provider.GetCredentialAsync(input); @@ -366,7 +475,9 @@ public async Task AzureReposProvider_GetCredentialAsync_PatMode_NoExistingPat_Ge var authorityCacheMock = new Mock(MockBehavior.Strict); - var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object); + var userMgrMock = new Mock(MockBehavior.Strict); + + var provider = new AzureReposHostProvider(context, azDevOpsMock.Object, msAuthMock.Object, authorityCacheMock.Object, userMgrMock.Object); ICredential credential = await provider.GetCredentialAsync(input); @@ -397,8 +508,9 @@ public async Task AzureReposProvider_GetCredentialAsync_PatMode_ExistingPat_Retu var azDevOps = Mock.Of(); var msAuth = Mock.Of(); var authorityCache = Mock.Of(); + var userMgr = Mock.Of(); - var provider = new AzureReposHostProvider(context, azDevOps, msAuth, authorityCache); + var provider = new AzureReposHostProvider(context, azDevOps, msAuth, authorityCache, userMgr); ICredential credential = await provider.GetCredentialAsync(input); diff --git a/src/shared/Microsoft.AzureRepos/AzureReposBindingManager.cs b/src/shared/Microsoft.AzureRepos/AzureReposBindingManager.cs new file mode 100644 index 000000000..4219eb11d --- /dev/null +++ b/src/shared/Microsoft.AzureRepos/AzureReposBindingManager.cs @@ -0,0 +1,325 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using Microsoft.Git.CredentialManager; + +namespace Microsoft.AzureRepos +{ + /// + /// Manages bindings of users and organizations for Azure Repos. + /// + public interface IAzureReposBindingManager + { + /// + /// Get the binding for the given Azure DevOps organization. + /// + /// Organization name. + /// Binding for the organization, or null if no binding exists. + AzureReposBinding GetBinding(string orgName); + + /// + /// Bind a user to the given organization. + /// + /// Organization to bind the user to. + /// User identifier to bind. + /// If true then bind local configuration, otherwise unbind global configuration. + /// + /// To prevent inheritance of a user binding at the global level, you can "bind" an organization + /// to a special value . + /// + /// The special value can be used as the + /// only when is true. + /// + void Bind(string orgName, string userName, bool local); + + /// + /// Unbind the given organization. + /// + /// Organization to unbind. + /// If true then unbind local configuration, otherwise unbind global configuration. + void Unbind(string orgName, bool local); + + /// + /// Get all bindings to Azure DevOps organizations. + /// + /// Optional organization filter. + /// All organization bindings. + IEnumerable GetBindings(string orgName = null); + } + + public class AzureReposBinding + { + /// + /// Do not inherit any higher-level binding. + /// + public const string NoInherit = ""; + + public AzureReposBinding(string organization, string globalUserName, string localUserName) + { + Organization = organization; + GlobalUserName = globalUserName; + LocalUserName = localUserName; + } + + public string Organization { get; } + public string GlobalUserName { get; } + public string LocalUserName { get; } + } + + public class AzureReposBindingManager : IAzureReposBindingManager + { + private readonly ITrace _trace; + private readonly IGit _git; + + public AzureReposBindingManager(ICommandContext context) : this(context.Trace, context.Git) { } + + public AzureReposBindingManager(ITrace trace, IGit git) + { + EnsureArgument.NotNull(trace, nameof(trace)); + EnsureArgument.NotNull(git, nameof(git)); + + _trace = trace; + _git = git; + } + + public AzureReposBinding GetBinding(string orgName) + { + EnsureArgument.NotNullOrWhiteSpace(orgName, nameof(orgName)); + + string orgKey = GetOrgUserKey(orgName); + + IGitConfiguration config = _git.GetConfiguration(); + + _trace.WriteLine($"Looking up organization binding for '{orgName}'..."); + + // NOT using the short-circuiting OR operator here on purpose - we need both branches to be evaluated + if (config.TryGet(GitConfigurationLevel.Local, orgKey, out string localUser) | + config.TryGet(GitConfigurationLevel.Global, orgKey, out string globalUser)) + { + return new AzureReposBinding(orgName, globalUser, localUser); + } + + // No bound user + return null; + } + + public void Bind(string orgName, string userName, bool local) + { + EnsureArgument.NotNullOrWhiteSpace(orgName, nameof(orgName)); + + IGitConfiguration config = _git.GetConfiguration(); + + string key = GetOrgUserKey(orgName); + + if (local) + { + _trace.WriteLine(userName == AzureReposBinding.NoInherit + ? $"Setting binding to 'do not inherit' for organization '{orgName}' in local repository..." + : $"Binding user '{userName}' to organization '{orgName}' in local repository..."); + + if (_git.IsInsideRepository()) + { + config.Set(GitConfigurationLevel.Local, key, userName); + } + else + { + _trace.WriteLine("Cannot set local configuration binding - not inside a repository!"); + } + } + else + { + EnsureArgument.NotNullOrWhiteSpace(userName, nameof(userName)); + + _trace.WriteLine($"Binding user '{userName}' to organization '{orgName}' in global configuration..."); + config.Set(GitConfigurationLevel.Global, key, userName); + } + } + + public void Unbind(string orgName, bool local) + { + EnsureArgument.NotNullOrWhiteSpace(orgName, nameof(orgName)); + + IGitConfiguration config = _git.GetConfiguration(); + + string key = GetOrgUserKey(orgName); + + if (local) + { + _trace.WriteLine($"Unbinding organization '{orgName}' in local repository..."); + if (_git.IsInsideRepository()) + { + config.Unset(GitConfigurationLevel.Local, key); + } + else + { + _trace.WriteLine("Cannot set local configuration binding - not inside a repository!"); + } + } + else + { + _trace.WriteLine($"Unbinding organization '{orgName}' in global configuration..."); + config.Unset(GitConfigurationLevel.Global, key); + } + } + + public IEnumerable GetBindings(string orgName = null) + { + var globalUsers = new Dictionary(StringComparer.OrdinalIgnoreCase); + var localUsers = new Dictionary(StringComparer.OrdinalIgnoreCase); + + IGitConfiguration config = _git.GetConfiguration(); + + string orgPrefix = $"{AzureDevOpsConstants.UrnOrgPrefix}/"; + + config.Enumerate( + Constants.GitConfiguration.Credential.SectionName, + Constants.GitConfiguration.Credential.UserName, + entry => + { + if (GitConfigurationKeyComparer.TrySplit(entry.Key, out _, out string scope, out _) && + Uri.TryCreate(scope, UriKind.Absolute, out Uri uri) && + uri.Scheme == AzureDevOpsConstants.UrnScheme && uri.AbsolutePath.StartsWith(orgPrefix)) + { + string entryOrgName = uri.AbsolutePath.Substring(orgPrefix.Length); + if (orgName is null || StringComparer.OrdinalIgnoreCase.Equals(entryOrgName, orgName)) + { + if (entry.Level == GitConfigurationLevel.Local) + { + localUsers[entryOrgName] = entry.Value; + } + else + { + globalUsers[entryOrgName] = entry.Value; + } + } + } + + return true; + }); + + foreach (string org in globalUsers.Keys.Union(localUsers.Keys)) + { + // NOT using the short-circuiting OR operator here on purpose - we need both branches to be evaluated + if (globalUsers.TryGetValue(org, out string globalUser) | localUsers.TryGetValue(org, out string localUser)) + { + yield return new AzureReposBinding(org, globalUser, localUser); + } + } + } + + private static string GetOrgUserKey(string orgName) + { + return string.Format(CultureInfo.InvariantCulture, "{0}.{1}:{2}/{3}.{4}", + Constants.GitConfiguration.Credential.SectionName, + AzureDevOpsConstants.UrnScheme, AzureDevOpsConstants.UrnOrgPrefix, orgName, + "username" + ); + } + } + + public static class AzureReposUserManagerExtensions + { + /// + /// Get the user that is bound to the specified Azure DevOps organization. + /// + /// Binding manager. + /// Organization name. + /// User identifier bound to the organization, or null if no such bound user exists. + public static string GetUser(this IAzureReposBindingManager bindingManager, string orgName) + { + AzureReposBinding binding = bindingManager.GetBinding(orgName); + if (binding is null || binding.LocalUserName == AzureReposBinding.NoInherit) + { + return null; + } + + return binding.LocalUserName ?? binding.GlobalUserName; + } + + /// + /// Marks a user as 'signed in' to an Azure DevOps organization. + /// + /// Binding manager. + /// Organization name. + /// User identifier to bind to this organization. + public static void SignIn(this IAzureReposBindingManager bindingManager, string orgName, string userName) + { + EnsureArgument.NotNullOrWhiteSpace(orgName, nameof(orgName)); + EnsureArgument.NotNullOrWhiteSpace(userName, nameof(userName)); + + // + // Try to bind the user to the organization. + // + // A = User to sign-in + // B = Another user + // - = No user + // + // Global | Local | -> | Global | Local + // --------|-------|----|--------|------- + // - | - | -> | A | - + // - | A | -> | A | - + // - | B | -> | A | - + // A | - | -> | A | - + // A | A | -> | A | - + // A | B | -> | A | - + // B | - | -> | B | A + // B | A | -> | B | A + // B | B | -> | B | A + // + AzureReposBinding existingBinding = bindingManager.GetBinding(orgName); + if (existingBinding?.GlobalUserName != null && + !StringComparer.OrdinalIgnoreCase.Equals(existingBinding.GlobalUserName, userName)) + { + bindingManager.Bind(orgName, userName, local: true); + } + else + { + bindingManager.Bind(orgName, userName, local: false); + bindingManager.Unbind(orgName, local: true); + } + } + + /// + /// Marks a user as 'signed out' of an Azure DevOps organization. + /// + /// Binding manager. + /// Organization name. + public static void SignOut(this IAzureReposBindingManager bindingManager, string orgName) + { + EnsureArgument.NotNullOrWhiteSpace(orgName, nameof(orgName)); + + // + // Unbind the organization so we prompt the user to select a user on the next attempt. + // + // U = User + // X = Do not inherit (valid in local only) + // - = No user + // + // Global | Local | -> | Global | Local + // --------|-------|----|--------|------- + // - | - | -> | - | - + // - | U | -> | - | - + // - | X | -> | - | - + // U | - | -> | U | X + // U | X | -> | U | X + // U | U | -> | U | X + // + AzureReposBinding existingBinding = bindingManager.GetBinding(orgName); + if (existingBinding is null) + { + // Nothing to do! + } + else if (existingBinding.GlobalUserName is null) + { + bindingManager.Unbind(orgName, local: true); + } + else + { + bindingManager.Bind(orgName, AzureReposBinding.NoInherit, local: true); + } + } + } +} diff --git a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs index 5b8a6f409..7ec43057e 100644 --- a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs +++ b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs @@ -20,25 +20,29 @@ public class AzureReposHostProvider : DisposableObject, IHostProvider, IConfigur private readonly IAzureDevOpsRestApi _azDevOps; private readonly IMicrosoftAuthentication _msAuth; private readonly IAzureDevOpsAuthorityCache _authorityCache; + private readonly IAzureReposBindingManager _bindingManager; public AzureReposHostProvider(ICommandContext context) : this(context, new AzureDevOpsRestApi(context), new MicrosoftAuthentication(context), - new AzureDevOpsAuthorityCache(context)) + new AzureDevOpsAuthorityCache(context), new AzureReposBindingManager(context)) { } public AzureReposHostProvider(ICommandContext context, IAzureDevOpsRestApi azDevOps, - IMicrosoftAuthentication msAuth, IAzureDevOpsAuthorityCache authorityCache) + IMicrosoftAuthentication msAuth, IAzureDevOpsAuthorityCache authorityCache, + IAzureReposBindingManager bindingManager) { EnsureArgument.NotNull(context, nameof(context)); EnsureArgument.NotNull(azDevOps, nameof(azDevOps)); EnsureArgument.NotNull(msAuth, nameof(msAuth)); EnsureArgument.NotNull(authorityCache, nameof(authorityCache)); + EnsureArgument.NotNull(bindingManager, nameof(bindingManager)); _context = context; _azDevOps = azDevOps; _msAuth = msAuth; _authorityCache = authorityCache; + _bindingManager = bindingManager; } #region IHostProvider @@ -125,6 +129,12 @@ public Task StoreCredentialAsync(InputArguments input) _context.CredentialStore.AddOrUpdate(service, account, input.Password); _context.Trace.WriteLine("Credential was successfully stored."); } + else + { + string orgName = UriHelpers.GetOrganizationName(remoteUri); + _context.Trace.WriteLine($"Signing user {input.UserName} in to organization '{orgName}'..."); + _bindingManager.SignIn(orgName, input.UserName); + } return Task.CompletedTask; } @@ -151,8 +161,12 @@ public Task EraseCredentialAsync(InputArguments input) } else { - // Clear the authority cache in case this was the reason for failure string orgName = UriHelpers.GetOrganizationName(remoteUri); + + _context.Trace.WriteLine($"Signing out of organization '{orgName}'..."); + _bindingManager.SignOut(orgName); + + // Clear the authority cache in case this was the reason for failure _authorityCache.EraseAuthority(orgName); } @@ -239,14 +253,20 @@ private async Task GetAzureAccessTokenAsync(Uri // match the Azure DevOps organization name. Our friends in Azure DevOps decided "borrow" the username // part of the remote URL to include the organization name (not an actual username). // - if (UriHelpers.IsAzureDevOpsHost(remoteUri.Host) && StringComparer.OrdinalIgnoreCase.Equals(orgName, userName)) + // If we have no specified user from the remote (or this is org@dev.azure.com/org/..) then query the + // user manager for a bound user for this organization, if one exists... + // + var icmp = StringComparer.OrdinalIgnoreCase; + if (!string.IsNullOrWhiteSpace(userName) && + (UriHelpers.IsVisualStudioComHost(remoteUri.Host) || + (UriHelpers.IsAzureDevOpsHost(remoteUri.Host) && !icmp.Equals(orgName, userName)))) { - _context.Trace.WriteLine("Cannot use username from dev.azure.com remote URL because this is the Azure DevOps organization name."); - userName = null; + _context.Trace.WriteLine("Using username as specified in remote."); } - else if (!string.IsNullOrWhiteSpace(userName)) + else { - _context.Trace.WriteLine("Using username as specified in remote."); + _context.Trace.WriteLine($"Looking up user for organization '{orgName}'..."); + userName = _bindingManager.GetUser(orgName); } _context.Trace.WriteLine(string.IsNullOrWhiteSpace(userName) ? "No user found." : $"User is '{userName}'."); diff --git a/src/shared/Microsoft.Git.CredentialManager/Constants.cs b/src/shared/Microsoft.Git.CredentialManager/Constants.cs index 3b436f657..e6ae24610 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Constants.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Constants.cs @@ -87,6 +87,7 @@ public static class Credential public const string CredentialStore = "credentialStore"; public const string CredCacheOptions = "cacheOptions"; public const string PlaintextStorePath = "plaintextStorePath"; + public const string UserName = "username"; } public static class Http diff --git a/src/shared/Microsoft.Git.CredentialManager/Git.cs b/src/shared/Microsoft.Git.CredentialManager/Git.cs index 42edb3e44..899947c12 100644 --- a/src/shared/Microsoft.Git.CredentialManager/Git.cs +++ b/src/shared/Microsoft.Git.CredentialManager/Git.cs @@ -154,7 +154,7 @@ public Process CreateProcess(string args) return new Process {StartInfo = psi}; } - // This code was originally copied from + // This code was originally copied from // src/shared/Microsoft.Git.CredentialManager/Authentication/AuthenticationBase.cs // That code is for GUI helpers in this codebase, while the below is for // communicating over Git's stdin/stdout helper protocol. The GUI helper @@ -223,4 +223,17 @@ public GitException(string message, string gitErrorMessage, int exitCode) ExitCode = exitCode; } } + + public static class GitExtensions + { + /// + /// Returns true if the current Git instance is scoped to a local repository. + /// + /// Git object. + /// True if inside a local Git repository, false otherwise. + public static bool IsInsideRepository(this IGit git) + { + return !string.IsNullOrWhiteSpace(git.GetCurrentRepository()); + } + } } From e2b7debdee1421ac0e8c4ed8e144034dc2bac4a3 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Mon, 8 Mar 2021 13:34:48 +0000 Subject: [PATCH 16/17] azrepos: add commands to manager user bindings Add commands to manually manager the user/org bindings for the Azure Repos host provider. --- docs/azrepos-users-and-tokens.md | 215 ++++++++++++++++++ docs/usage.md | 6 +- .../AzureReposHostProvider.cs | 195 ++++++++++++++++ .../UriExtensionsTests.cs | 17 ++ .../DictionaryExtensions.cs | 27 +++ .../UriExtensions.cs | 5 + 6 files changed, 463 insertions(+), 2 deletions(-) create mode 100644 docs/azrepos-users-and-tokens.md diff --git a/docs/azrepos-users-and-tokens.md b/docs/azrepos-users-and-tokens.md new file mode 100644 index 000000000..7caf63156 --- /dev/null +++ b/docs/azrepos-users-and-tokens.md @@ -0,0 +1,215 @@ +# Azure Repos: Access tokens and Accounts + +## Different credential types + +The Azure Repos host provider supports creating multiple types of credential: + +- Azure DevOps personal access tokens +- Microsoft identity OAuth tokens (experimental) + +### Azure DevOps personal access tokens + +Historically, the only option supported by the Azure Repos host provider was +Azure DevOps Personal Access Tokens (PATs). + +These PATs are only used by Azure DevOps, and must be [managed through the Azure +DevOps user settings page](https://docs.microsoft.com/en-us/azure/devops/organizations/accounts/use-personal-access-tokens-to-authenticate?view=azure-devops&tabs=preview-page). + +PATs have a limited lifetime and new tokens must be created once they expire. In +Git Credential Manager, when a PAT expired (or was manually revoked) this +resulted in a new authentication prompt. + +### Microsoft identity OAuth tokens (experimental) + +"Microsoft identity OAuth token" is the generic term for OAuth-based access +tokens issued by Azure Active Directory for either Work and School Accounts +(AAD tokens) or Personal Accounts (Microsoft Account/MSA tokens). + +Azure DevOps supports Git authentication using Microsoft identity OAuth tokens +as well as PATs. Microsoft identity OAuth tokens created by Git Credential +Manager are scoped to Azure DevOps only. + +Unlike PATs, Microsoft identity OAuth tokens get automatically refreshed and +renewed as long as you are actively using them to perform Git operations. + +These tokens are also securely shared with other Microsoft developer tools +including the Visual Studio IDE and Azure CLI. This means that as long as you're +using Git or one of these tools with the same account, you'll never need to +re-authenticate due to expired tokens! + +#### User accounts + +In versions of Git Credential Manager that support Microsoft identity OAuth +tokens, the user account used to authenticate for a particular Azure DevOps +organization will now be remembered. + +The first time you clone, fetch or push from/to an Azure DevOps organization you +will be prompted to sign-in and select a user account. Git Credential Manager +will remember which account you used and continue to use that for all future +remote Git operations (clone/fetch/push). An account is said to be "bound" to +an Azure DevOps organization. + +--- + +**Note:** If GCM is set to use PAT credentials, this account will **NOT** be +used and you will continue to be prompted to select a user account to renew the +credential. This may change in the future. + +--- + +Normally you won't need to worry about managing which user accounts Git +Credential Manager is using as this is configured automatically when you first +authenticate for a particular Azure DevOps organziation. + +In advanced scenarios (such as using multiple accounts) you can interact with +and manage remembered user accounts using the 'azure-repos' provider command: + +```shell +git-credential-manager-core azure-repos [ list | bind | unbind | ... ] +``` + +##### Listing remembered accounts + +You can list all bound user accounts by Git Credential Manager for each Azure +DevOps organization using the `list` command: + +```shell +$ git-credential-manager-core azure-repos list +contoso: + (global) -> alice@contoso.com +fabrikam: + (global) -> user42@fabrikam.com +``` + +In the above example, the `contoso` Azure DevOps organization is associated with +the `alice@contoso.com` user account, while the `fabrikam` organization is +associated to the `user42@fabrikam.com` user account. + +Global "bindings" apply to all remote Git operations for the current computer +user profile and are stored in `~/.gitconfig` or `%USERPROFILE%\.gitconfig`. + +##### Using different accounts within a repository + +If you generally use one account for an Azure DevOps organization, the default +global bindings will be sufficient. However, if you wish to use a different +user account for an organization in a particular repository you can use a local +binding. + +Local account bindings only apply within a single repository and are stored in +the `.git/config` file. If there are local bindings in a repository you can show +them with the `list` command: + +```shell +~/myrepo$ git-credential-manager-core azure-repos list +contoso: + (global) -> alice@contoso.com + (local) -> alice-alt@contoso.com +``` + +Within the `~/myrepo` repository, the `alice-alt@contoso.com` account will be +used by Git and GCM for the `contoso` Azure DevOps organization. + +To create a local binding, use the `bind` command with the `--local` option when +inside a repository: + +```shell +~/myrepo$ git-credential-manager-core azure-repos bind --local contoso alice-alt@contso.com +``` + +```diff + contoso: + (global) -> alice@contoso.com ++ (local) -> alice-alt@contoso.com +``` + +##### Forget an account + +To have Git Credential Manager forget a user account, use the `unbind` command: + +```shell +git-credential-manager-core azure-repos unbind fabrikam +``` + +```diff + contoso: + (global) -> alice@contoso.com +- fabrikam: +- (global) -> user42@fabrikam.com +``` + +In the above example, and global account binding for the `fabrikam` organization +will be forgotten. The next time you need to renew a PAT (if using PATs) or +perform any remote Git operation (is using Azure tokens) you will be prompted +to authenticate again. + +To forget or remove a local binding, within the repository run the `unbind` +command with the `--local` option: + +```shell +~/myrepo$ git-credential-manager-core azure-repos unbind --local contoso +``` + +```diff + contoso: + (global) -> alice@contoso.com +- (local) -> alice-alt@contoso.com +``` + +##### Using different accounts for specific Git remotes + +As well as global and local user account bindings, you can instruct Git +Credential Manager to use a specific user account for an individual Git remotes +within the same local repository. + +To show which accounts are being used for each Git remote in a repository use +the `list` command with the `--show-remotes` option: + +```shell +~/myrepo$ git-credential-manager-core azure-repos list --show-remotes +contoso: + (global) -> alice@contoso.com + origin: + (fetch) -> (inherit) + (push) -> (inherit) +fabrikam: + (global) -> alice@fabrikam.com +``` + +In the above example, the `~/myrepo` repository has a single Git remote named +`origin` that points to the `contoso` Azure DevOps organziation. There is no +user account specifically associated with the `origin` remote, so the global +user account binding for `contoso` will be used (the global binding is +inherited). + +To associate a user account with a particular Git remote you must manually edit +the remote URL using `git config` commands to include the username in the +[user information](https://tools.ietf.org/html/rfc3986#section-3.2.1) part of +the URL. + +```shell +git config --local remote.origin.url https://alice-alt%40contoso.com@contoso.visualstudio.com/project/_git/repo +``` + +In the above example the `alice-alt@contoso.com` account is being set as the +account to use for the `origin` Git remote. + +--- + +**Note:** All special characters must be URL encoded/escaped, for example `@` +becomes `%40`. + +--- + +The `list --show-remotes` command will show the user account specified in the +remote URL: + +```shell +~/myrepo$ git-credential-manager-core azure-repos list --show-remotes +contoso: + (global) -> alice@contoso.com + origin: + (fetch) -> alice-alt@contoso.com + (push) -> alice-alt@contoso.com +fabrikam: + (global) -> alice@fabrikam.com +``` diff --git a/docs/usage.md b/docs/usage.md index 699aeb481..701163f96 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -33,5 +33,7 @@ Set your user-level Git configuration (`~/.gitconfig`) to use GCM Core. If you p ### azure-repos (experimental) -Interact with the Azure Repos host provider to manage the authentication -authority cache. +Interact with the Azure Repos host provider to bind/unbind user accounts to Azure DevOps +organizations or specific remote URLs, and manage the authentication authority cache. + +For more information about managing user account bindings see [here](azrepos-users-and-tokens.md#useraccounts). diff --git a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs index 7ec43057e..39c1f60ce 100644 --- a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs +++ b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs @@ -485,7 +485,59 @@ ProviderCommand ICommandProvider.CreateCommand() Handler = CommandHandler.Create(ClearCacheCmd), }; + var orgArg = new Argument("organization") + { + Arity = ArgumentArity.ExactlyOne, + Description = "Azure DevOps organization name" + }; + var localOpt = new Option("--local") + { + Description = "Target the local repository Git configuration" + }; + + var listCmd = new Command("list", "List all user account bindings") + { + Handler = CommandHandler.Create(ListCmd) + }; + listCmd.AddArgument(new Argument("organization") + { + Arity = ArgumentArity.ZeroOrOne, + Description = "(optional) Filter results by Azure DevOps organization name" + }); + listCmd.AddOption(new Option("--show-remotes") + { + Description = "Also show Azure DevOps remote user bindings for the current repository" + }); + listCmd.AddOption(new Option(new[] {"--verbose", "-v"}) + { + Description = "Verbose output - show remote URLs" + }); + + var bindCmd = new Command("bind") + { + Description = "Bind a user account to an Azure DevOps organization", + Handler = CommandHandler.Create(BindCmd), + }; + bindCmd.AddArgument(orgArg); + bindCmd.AddArgument(new Argument("username") + { + Arity = ArgumentArity.ExactlyOne, + Description = "Username or email (e.g.: alice@example.com)" + }); + bindCmd.AddOption(localOpt); + + var unbindCmd = new Command("unbind") + { + Description = "Remove user account binding for an Azure DevOps organization", + Handler = CommandHandler.Create(UnbindCmd), + }; + unbindCmd.AddArgument(orgArg); + unbindCmd.AddOption(localOpt); + var rootCmd = new ProviderCommand(this); + rootCmd.AddCommand(listCmd); + rootCmd.AddCommand(bindCmd); + rootCmd.AddCommand(unbindCmd); rootCmd.AddCommand(clearCacheCmd); return rootCmd; } @@ -497,6 +549,149 @@ private int ClearCacheCmd() return 0; } + private class RemoteBinding + { + public string Remote { get; set; } + public bool IsPush { get; set; } + public Uri Uri { get; set; } + } + + private int ListCmd(string organization, bool showRemotes, bool verbose) + { + // Get all organization bindings from the user manager + IList bindings = _bindingManager.GetBindings(organization).ToList(); + IDictionary> orgBindingMap = + bindings.GroupBy(x => x.Organization).ToDictionary(); + + // If we are asked to also show remotes we build the remote binding map + var orgRemotesMap = new Dictionary>(); + if (showRemotes) + { + if (!_context.Git.IsInsideRepository()) + { + _context.Streams.Error.WriteLine("warning: not inside a git repository (--show-remotes has no effect)"); + } + + static bool IsAzureDevOpsHttpRemote(string url, out Uri uri) + { + return Uri.TryCreate(url, UriKind.Absolute, out uri) && + (StringComparer.OrdinalIgnoreCase.Equals(Uri.UriSchemeHttp, uri.Scheme) || + StringComparer.OrdinalIgnoreCase.Equals(Uri.UriSchemeHttps, uri.Scheme)) && + UriHelpers.IsAzureDevOpsHost(uri.Host); + } + + foreach (GitRemote remote in _context.Git.GetRemotes()) + { + if (IsAzureDevOpsHttpRemote(remote.FetchUrl, out Uri fetchUri)) + { + string fetchOrg = UriHelpers.GetOrganizationName(fetchUri); + var binding = new RemoteBinding {IsPush = false, Remote = remote.Name, Uri = fetchUri}; + orgRemotesMap.Append(fetchOrg, binding); + } + + if (IsAzureDevOpsHttpRemote(remote.PushUrl, out Uri pushUri)) + { + string pushOrg = UriHelpers.GetOrganizationName(pushUri); + var binding = new RemoteBinding {IsPush = true, Remote = remote.Name, Uri = pushUri}; + orgRemotesMap.Append(pushOrg, binding); + } + } + } + + bool isFiltered = organization != null; + string indent = isFiltered ? string.Empty : " "; + + // Get the set of all organization names (organization names are not case sensitive) + ISet orgNames = new HashSet(orgBindingMap.Keys, StringComparer.OrdinalIgnoreCase); + orgNames.UnionWith(orgRemotesMap.Keys); + + var icmp = StringComparer.OrdinalIgnoreCase; + + foreach (string orgName in orgNames) + { + if (!isFiltered) + { + _context.Streams.Out.WriteLine($"{orgName}:"); + } + + // Print organization bindings + foreach (AzureReposBinding binding in orgBindingMap.GetValues(orgName)) + { + if (binding.GlobalUserName != null) + { + _context.Streams.Out.WriteLine($"{indent}(global) -> {binding.GlobalUserName}"); + } + + if (binding.LocalUserName != null) + { + string value = string.IsNullOrEmpty(binding.LocalUserName) + ? "(no inherit)" + : binding.LocalUserName; + _context.Streams.Out.WriteLine($"{indent}(local) -> {value}"); + } + } + + // Print remote bindings + IEnumerable> remoteBindingMap = + orgRemotesMap.GetValues(orgName).GroupBy(x => x.Remote); + + foreach (var remoteBinding in remoteBindingMap) + { + _context.Streams.Out.WriteLine($"{indent}{remoteBinding.Key}:"); + foreach (RemoteBinding binding in remoteBinding) + { + // User names in dev.azure.com URLs cannot always be used as *actual user names* + // because of the unfortunate decision to use this field to get the Azure DevOps + // organization name to be sent by Git to credential helpers. + // + // We show dev.azure.com URLs as "inherit", if there is a username that matches + // the organization name. + if (!binding.Uri.TryGetUserInfo(out string userName, out _) || + UriHelpers.IsDevAzureComHost(binding.Uri.Host) && icmp.Equals(userName, orgName)) + { + userName = "(inherit)"; + } + + string url = null; + if (verbose) + { + url = $"{binding.Uri.WithoutUserInfo()} "; + } + + _context.Streams.Out.WriteLine(binding.IsPush + ? $"{indent} {url}(push) -> {userName}" + : $"{indent} {url}(fetch) -> {userName}"); + } + } + } + + return 0; + } + + private int BindCmd(string organization, string userName, bool local) + { + if (local && !_context.Git.IsInsideRepository()) + { + _context.Streams.Error.WriteLine("error: not inside a git repository (cannot use --local)"); + return -1; + } + + _bindingManager.Bind(organization, userName, local); + return 0; + } + + private int UnbindCmd(string organization, bool local) + { + if (local && !_context.Git.IsInsideRepository()) + { + _context.Streams.Error.WriteLine("error: not inside a git repository (cannot use --local)"); + return -1; + } + + _bindingManager.Unbind(organization, local); + return 0; + } + #endregion } } diff --git a/src/shared/Microsoft.Git.CredentialManager.Tests/UriExtensionsTests.cs b/src/shared/Microsoft.Git.CredentialManager.Tests/UriExtensionsTests.cs index c57b7c617..764d6304a 100644 --- a/src/shared/Microsoft.Git.CredentialManager.Tests/UriExtensionsTests.cs +++ b/src/shared/Microsoft.Git.CredentialManager.Tests/UriExtensionsTests.cs @@ -79,5 +79,22 @@ public void UriExtensions_GetUserInfo(string input, bool expectedResult, string Assert.Equal(expectedUser, actualUser); Assert.Equal(expectedPass, actualPass); } + + [Theory] + [InlineData("http://example.com", "http://example.com")] + [InlineData("http://john.doe:password123@example.com", "http://example.com")] + [InlineData("http://john.doe@example.com", "http://example.com")] + [InlineData("http://john.doe:@example.com", "http://example.com")] + [InlineData("http://:password123@example.com", "http://example.com")] + [InlineData("http://john.doe:::password123@example.com", "http://example.com")] + [InlineData("http://john%20doe:password%20123@example.com", "http://example.com")] + public void UriExtensions_WithoutUserInfo(string input, string expected) + { + var uri = new Uri(input); + + Uri result = UriExtensions.WithoutUserInfo(uri); + + Assert.Equal(expected, result.ToString().TrimEnd('/')); + } } } diff --git a/src/shared/Microsoft.Git.CredentialManager/DictionaryExtensions.cs b/src/shared/Microsoft.Git.CredentialManager/DictionaryExtensions.cs index b7d4f0695..853296d1b 100644 --- a/src/shared/Microsoft.Git.CredentialManager/DictionaryExtensions.cs +++ b/src/shared/Microsoft.Git.CredentialManager/DictionaryExtensions.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. using System.Collections.Generic; +using System.Linq; using System.Text; using System.Web; @@ -50,5 +51,31 @@ public static string ToQueryString(this IDictionary dict) return sb.ToString(); } + + public static void Append(this IDictionary> dict, TKey key, TValue value) + { + if (!dict.TryGetValue(key, out var values)) + { + values = new List(); + dict[key] = values; + } + + values.Add(value); + } + + public static IEnumerable GetValues(this IDictionary> dict, TKey key) + { + return dict.TryGetValue(key, out var values) ? values : Enumerable.Empty(); + } + + public static IEnumerable GetValues(this IDictionary> dict, TKey key) + { + return dict.TryGetValue(key, out var values) ? values : Enumerable.Empty(); + } + + public static IDictionary> ToDictionary(this IEnumerable> grouping) + { + return grouping.ToDictionary(x => x.Key, x => (IEnumerable) x); + } } } diff --git a/src/shared/Microsoft.Git.CredentialManager/UriExtensions.cs b/src/shared/Microsoft.Git.CredentialManager/UriExtensions.cs index 23e41198a..1f3e7311d 100644 --- a/src/shared/Microsoft.Git.CredentialManager/UriExtensions.cs +++ b/src/shared/Microsoft.Git.CredentialManager/UriExtensions.cs @@ -74,6 +74,11 @@ public static string GetUserName(this Uri uri) return TryGetUserInfo(uri, out string userName, out _) ? userName : null; } + public static Uri WithoutUserInfo(this Uri uri) + { + return new UriBuilder(uri) {UserName = string.Empty, Password = string.Empty}.Uri; + } + public static IEnumerable GetGitConfigurationScopes(this Uri uri) { EnsureArgument.NotNull(uri, nameof(uri)); From c8a7b2b74af4ca83cbe9ed61a7e3f7ba23fbf225 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 9 Mar 2021 15:19:13 +0000 Subject: [PATCH 17/17] git: add guards against malformed Git config data Add some checks/guards against malformed data output from Git configuration when enumerating all entries. If we hit the unexpected end of the data stream we trace and stop parsing. --- .../GitConfiguration.cs | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs index 999bb8f22..3fef4f9fe 100644 --- a/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs +++ b/src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs @@ -145,29 +145,47 @@ public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCa value.Clear(); // Read config scope (null terminated) - while (data[i] != '\0') + while (i < data.Length && data[i] != '\0') { scope.Append(data[i++]); } + if (i >= data.Length) + { + _trace.WriteLine("Invalid Git configuration output. Expected null terminator (\\0) after scope."); + break; + } + // Skip the null terminator i++; // Read key name (LF terminated) - while (data[i] != '\n') + while (i < data.Length && data[i] != '\n') { name.Append(data[i++]); } + if (i >= data.Length) + { + _trace.WriteLine("Invalid Git configuration output. Expected newline terminator (\\n) after key."); + break; + } + // Skip the LF terminator i++; // Read value (null terminated) - while (data[i] != '\0') + while (i < data.Length && data[i] != '\0') { value.Append(data[i++]); } + if (i >= data.Length) + { + _trace.WriteLine("Invalid Git configuration output. Expected null terminator (\\0) after value."); + break; + } + // Skip the null terminator i++;