Skip to content

Commit

Permalink
input: better input arg checking/handling
Browse files Browse the repository at this point in the history
Improve the handling of input arguments with missing required fields
(e.g., protocol and host).
  • Loading branch information
mjcheetham committed Feb 23, 2021
1 parent ab8d1f9 commit dba2aae
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 5 deletions.
5 changes: 4 additions & 1 deletion src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion src/shared/GitHub/GitHubHostProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>
{
["protocol"] = "http",
["host"] = "example.com",
["username"] = testUserName,
["password"] = testPassword // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>
{
["protocol"] = "http",
["host"] = "example.com",
["username"] = testUserName,
["password"] = testPassword
};
Expand All @@ -29,7 +32,10 @@ public async Task GetCommand_ExecuteAsync_CallsHostProviderAndWritesCredential()
providerMock.Setup(x => x.GetCredentialAsync(It.IsAny<InputArguments>()))
.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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>
{
["protocol"] = "http",
["host"] = "example.com",
["username"] = testUserName,
["password"] = testPassword
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ internal async Task ExecuteAsync()
IDictionary<string, string> 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();

Expand All @@ -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)");
}
}

/// <summary>
/// Execute the command using the given <see cref="InputArguments"/> and <see cref="IHostProvider"/>.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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");
}
}
}
}

0 comments on commit dba2aae

Please sign in to comment.