Merge pull request #1129 from DeterminateSystems/fixup-argon2

Fixup argon2 instructions in hydra-create-user

src/script/hydra-create-user

@@ -14,8 +14,8 @@ Usage: hydra-create-user NAME
   [--type hydra|google|github]
   [--full-name FULLNAME]
   [--email-address EMAIL-ADDRESS]
-  [--password PASSWORD]
   [--password-hash HASH]
+  [--password PASSWORD (dangerous)]
   [--wipe-roles]
   [--role ROLE]...
 
@@ -25,27 +25,37 @@ exists, roles are added to the existing roles unless --wipe-roles is
 specified.  If --rename-from is given, the specified account is
 renamed.
 
-* PASSWORD HASH
+* Specifying Passwords
-The password hash should be an Argon2id hash, which can be generated
+
-via:
+** Specifying a Hash
+
+You can generate a password hash and provide the hash as well. This
+is useful so a user can send the administrator their password pre-hashed,
+allowing the user to get their preferred password without exposing it
+to the administrator.
+
+Hydra uses Argon2id hashes, which can be generated like so:
 
     $ nix-shell -p libargon2
-    [nix-shell]$ argon2 "$(LC_ALL=C tr -dc '[:alnum:]' < /dev/urandom | head -c16)" -id -t 3 -k 262144 -p 1 -l 16 -e
+    [nix-shell]$ tr -d \\\\n | argon2 "$(LC_ALL=C tr -dc '[:alnum:]' < /dev/urandom | head -c16)" -id -t 3 -k 262144 -p 1 -l 16 -e
     foobar
     Ctrl^D
     $argon2id$v=19$m=262144,t=3,p=1$NFU1QXJRNnc4V1BhQ0NJQg$6GHqjqv5cNDDwZqrqUD0zQ
 
-SHA1 is also accepted, but SHA1 support is deprecated and the user's
+Example:
-password will be upgraded to Argon2id on first login.
-
-
-Examples:
-
-Create a user with an argon2 password:
 
   $ hydra-create-user alice --password-hash '$argon2id$v=19$m=262144,t=3,p=1$NFU1QXJRNnc4V1BhQ0NJQg$6GHqjqv5cNDDwZqrqUD0zQ' --role admin
 
-Create a user with a password insecurely provided on the commandline:
+SHA1 is also accepted, but SHA1 support is deprecated and the user's
+password will be upgraded to Argon2id on first login.
+
+** Specifying a plain-text password as an argument (dangerous)
+
+This option is dangerous and should not be used: it exposes passwords to
+other users on the system. This option only exists for backwards
+compatibility.
+
+Example:
 
   $ hydra-create-user alice --password foobar --role admin
 
@@ -71,6 +81,11 @@ GetOptions("rename-from=s" => \$renameFrom,
 die "$0: one user name required\n" if scalar @ARGV != 1;
 my $userName = $ARGV[0];
 
+my $chosenPasswordOptions = grep { defined($_) }  ($passwordHash, $password);
+if ($chosenPasswordOptions > 1) {
+    die "$0: please specify only one --password* option. See --help for more information.\n";
+}
+
 die "$0: type must be `hydra', `google' or `github'\n"
     if defined $type && $type ne "hydra" && $type ne "google" && $type ne "github";
 

t/scripts/hydra-create-user.t

@@ -1,17 +1,10 @@
-use feature 'unicode_strings';
 use strict;
 use warnings;
 use Setup;
-
-my %ctx = test_init();
-
-require Hydra::Schema;
-require Hydra::Model::DB;
-
 use Test2::V0;
 
-my $db = Hydra::Model::DB->new;
+my $ctx = test_context();
-hydra_setup($db);
+my $db = $ctx->db();
 
 subtest "Handling password and password hash creation" => sub {
     subtest "Creating a user with a plain text password (insecure) stores the password securely" => sub {
@@ -28,10 +21,10 @@ subtest "Handling password and password hash creation" => sub {
     };
 
     subtest "Creating a user with a sha1 password (still insecure) stores the password as a hashed sha1" => sub {
-        my ($res, $stdout, $stderr) = captureStdoutStderr(5, ("hydra-create-user", "plain-text-user", "--password-hash", "8843d7f92416211de9ebb963ff4ce28125932878"));
+        my ($res, $stdout, $stderr) = captureStdoutStderr(5, ("hydra-create-user", "old-password-hash-user", "--password-hash", "8843d7f92416211de9ebb963ff4ce28125932878"));
         is($res, 0, "hydra-create-user should exit zero");
 
-        my $user = $db->resultset('Users')->find({ username => "plain-text-user" });
+        my $user = $db->resultset('Users')->find({ username => "old-password-hash-user" });
         isnt($user, undef, "The user exists");
         isnt($user->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The password was not saved in plain text.");
 
@@ -41,10 +34,10 @@ subtest "Handling password and password hash creation" => sub {
     };
 
     subtest "Creating a user with an argon2 password stores the password as given" => sub {
-        my ($res, $stdout, $stderr) = captureStdoutStderr(5, ("hydra-create-user", "plain-text-user", "--password-hash", '$argon2id$v=19$m=262144,t=3,p=1$tMnV5paYjmIrUIb6hylaNA$M8/e0i3NGrjhOliVLa5LqQ'));
+        my ($res, $stdout, $stderr) = captureStdoutStderr(5, ("hydra-create-user", "argon2-hash-user", "--password-hash", '$argon2id$v=19$m=262144,t=3,p=1$tMnV5paYjmIrUIb6hylaNA$M8/e0i3NGrjhOliVLa5LqQ'));
         is($res, 0, "hydra-create-user should exit zero");
 
-        my $user = $db->resultset('Users')->find({ username => "plain-text-user" });
+        my $user = $db->resultset('Users')->find({ username => "argon2-hash-user" });
         isnt($user, undef, "The user exists");
         is($user->password, '$argon2id$v=19$m=262144,t=3,p=1$tMnV5paYjmIrUIb6hylaNA$M8/e0i3NGrjhOliVLa5LqQ', "The password was saved as-is.");
 
@@ -52,6 +45,25 @@ subtest "Handling password and password hash creation" => sub {
         ok($user->check_password("foobar"), "Their password validates");
         is($storedPassword, $user->password, "The password was not upgraded.");
     };
+
+    subtest "Specifying conflicting password options fails" => sub {
+        my @cases = (
+            [ "--password=foo", "--password-hash=8843d7f92416211de9ebb963ff4ce28125932878" ],
+        );
+
+        for my $case (@cases) {
+            my ($res, $stdout, $stderr) = captureStdoutStderr(5, (
+                "hydra-create-user", "bogus-password-options", @{$case}));
+            like($stderr, qr/please specify only one --password\* option/, "We get an error about specifying the password");
+            isnt($res, 0, "hydra-create-user should exit non-zero with conflicting " . join(" ", @{$case}));
+        }
+    };
+
+    subtest "A password is not required for creating a Google-based account" => sub {
+        my ($res, $stdout, $stderr) = captureStdoutStderr(5, (
+            "hydra-create-user", "google-account", "--type", "google"));
+        is($res, 0, "hydra-create-user should exit zero");
+    };
 };
 
 done_testing;