Skip to content

Commit

Permalink
Client: explicitly set passwordMode boolean flag
Browse files Browse the repository at this point in the history
Instead of checking the value of the password variable to see if it is null vs
empty, especially since we sometimes reassign to it.
  • Loading branch information
mqudsi committed Feb 27, 2024
1 parent b233226 commit ee562b8
Showing 1 changed file with 15 additions and 16 deletions.
31 changes: 15 additions & 16 deletions Client/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -295,11 +295,11 @@ void printVersion()
// It instead requires --password=PASSWORD or --password:PASSWORD or -pPASSWORD
var bareArguments = parseOptions.Parse(args);

if (bareArguments.Count > 0 && password == string.Empty)
var passwordMode = password == string.Empty; // instead of null
if (bareArguments.Count > 0 && passwordMode)
{
// Check if this was the standalone password
var possibles = new[] { "-p", "--password", "/p", "/password" };

for (int i = 0; i < args.Count - 1; ++i)
{
if (!possibles.Contains(args[i]))
Expand Down Expand Up @@ -383,16 +383,15 @@ void printVersion()
Help(Console.Error, "A path to the secrets store is required!", command, parseOptions);
}

if (keyfile == null && password == null)
if (keyfile == null && !passwordMode)
{
// Changed: Default to password mode instead of erroring out
// Help(Console.Error, "Must specify either --password or --keyfile!", command, parseOptions);

// Default to password mode instead of erroring out
password = string.Empty;
passwordMode = true;
}

// We need to differentiate between null (not set) and empty (empty)
if (password == string.Empty && (string.IsNullOrEmpty(keyfile) || !File.Exists(keyfile)))
if (passwordMode)
{
if (command == "create")
{
Expand Down Expand Up @@ -423,13 +422,13 @@ void printVersion()
// Handle parameters specific to certain commands
if (command == "create")
{
if (password is null && string.IsNullOrWhiteSpace(keyfile))
if (!passwordMode && string.IsNullOrWhiteSpace(keyfile))
{
// This block should no longer be reachable!
Console.Error.WriteLine("A newly created store must have one or both of --password and --keyfile specified");
Environment.Exit(1);
}
if (!string.IsNullOrWhiteSpace(password) && File.Exists(keyfile)
&& new FileInfo(keyfile).Length > 0)
if (passwordMode && File.Exists(keyfile) && new FileInfo(keyfile).Length > 0)
{
Confirm($"Overwrite the existing contents of the key file at {keyfile} " +
"with a key derived from the provided password? [yes/no]: ");
Expand All @@ -451,9 +450,9 @@ void printVersion()

using (sman)
{
if (!string.IsNullOrEmpty(password))
if (passwordMode)
{
sman.LoadKeyFromPassword(password);
sman.LoadKeyFromPassword(password!);
try
{
sman.ValidateSentinel();
Expand All @@ -471,7 +470,7 @@ void printVersion()
{
Confirm($"Overwrite existing store at {path}? [yes/no]: ");
}
if (password == null)
if (!passwordMode)
{
ExitIfNullOrEmpty(keyfile, "A keyfile path is required if no password is provided");

Expand All @@ -492,12 +491,12 @@ void printVersion()
keyCreated = keyfile;
}
}
else if (password == null && keyfile != null)
else if (!passwordMode && keyfile != null)
{
sman.LoadKeyFromFile(keyfile);
}

if (password is null && keyfile is not null)
if (!passwordMode && keyfile is not null)
{
try
{
Expand All @@ -512,7 +511,7 @@ void printVersion()
// We store --export-key to its own variable so that it doesn't override our defaulting to password
// mode if no key file was specified. After we've decided on whether or not to use a password above,
// we can now coalesce the two values.
if (keyCreated is null && keyExport is not null && keyExport != keyfile)
if (keyExport is not null && (keyfile is null || keyExport != keyfile))
{
sman.ExportKey(keyExport);
keyCreated = keyExport;
Expand Down

0 comments on commit ee562b8

Please sign in to comment.