diff --git a/src/lib/Hydra/Schema/Users.pm b/src/lib/Hydra/Schema/Users.pm index 55f0f1cb..9f2e1bc0 100644 --- a/src/lib/Hydra/Schema/Users.pm +++ b/src/lib/Hydra/Schema/Users.pm @@ -238,6 +238,12 @@ sub check_password { $self->setPassword($password); } + return 1; + } elsif ($authenticator->verify_password(sha1_hex($password), $self->password)) { + # The user's database record has their old password as sha1, re-hashed as Argon2. + # Store their password hashed only with Argon2. + $self->setPassword($password); + return 1; } else { return 0; @@ -252,4 +258,15 @@ sub setPassword { }); } +sub setPasswordHash { + my ($self, $passwordHash) = @_;; + + if ($passwordHash =~ /^[a-f0-9]{40}$/) { + # This is (probably) a sha1 password, re-hash it and we'll check for a hashed sha1 in Users.pm + $self->setPassword($passwordHash); + } else { + $self->update({ password => $passwordHash }); + } +} + 1; diff --git a/src/script/hydra-create-user b/src/script/hydra-create-user index 6e837270..0ed51bd8 100755 --- a/src/script/hydra-create-user +++ b/src/script/hydra-create-user @@ -109,7 +109,10 @@ $db->txn_do(sub { if (defined $password && !(defined $passwordHash)) { $user->setPassword($password); } - $user->update({ password => $passwordHash }) if defined $passwordHash; + + if (defined $passwordHash) { + $user->setPasswordHash($passwordHash); + } } $user->userroles->delete if $wipeRoles; diff --git a/src/script/hydra-init b/src/script/hydra-init index 74498186..ac9d4afe 100755 --- a/src/script/hydra-init +++ b/src/script/hydra-init @@ -72,3 +72,12 @@ for (my $n = $schemaVersion; $n < $maxSchemaVersion; $n++) { }; die "schema upgrade failed: $@\n" if $@; } + +my @usersWithSha1s = $db->resultset('Users')->search(\['LENGTH(password) = 40 AND password ~ \'^[0-9a-f]{40}$\'']); +if (scalar(@usersWithSha1s) > 0) { + print STDERR "upgrading user passwords from sha1\n"; + for my $user (@usersWithSha1s) { + print STDERR " * " . $user->username . "\n"; + $user->setPassword($user->password); + } +} diff --git a/t/Schema/Users.t b/t/Schema/Users.t index 5f31af76..fd1a66e1 100644 --- a/t/Schema/Users.t +++ b/t/Schema/Users.t @@ -39,4 +39,34 @@ ok($user->check_password("foobar"), "Checking the password, foobar, is right"); isnt($user->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The user has had their password rehashed."); ok($user->check_password("foobar"), "Checking the password, foobar, is still right"); +# All sha1 passwords will be upgraded when `hydra-init` is run, by passing the sha1 through +# Argon2. Verify a rehashed sha1 validates too. This removes very weak password hashes +# from the database without requiring users to log in. +subtest "Hashing their sha1 as Argon2 still lets them log in with their password" => sub { + $user->setPassword("8843d7f92416211de9ebb963ff4ce28125932878"); # SHA1 of "foobar" + my $hashedHashPassword = $user->password; + isnt($user->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The user has had their password's hash rehashed."); + ok($user->check_password("foobar"), "Checking the password, foobar, is still right"); + isnt($user->password, $hashedHashPassword, "The user's hashed hash was replaced with just Argon2."); +}; + + +subtest "Setting the user's passwordHash to a sha1 stores the password as a hashed sha1" => sub { + $user->setPasswordHash("8843d7f92416211de9ebb963ff4ce28125932878"); + isnt($user->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The password was not saved in plain text."); + + my $storedPassword = $user->password; + ok($user->check_password("foobar"), "Their password validates"); + isnt($storedPassword, $user->password, "The password was upgraded."); +}; + +subtest "Setting the user's passwordHash to an argon2 password stores the password as given" => sub { + $user->setPasswordHash('$argon2id$v=19$m=262144,t=3,p=1$tMnV5paYjmIrUIb6hylaNA$M8/e0i3NGrjhOliVLa5LqQ'); + isnt($user->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The password was not saved in plain text."); + is($user->password, '$argon2id$v=19$m=262144,t=3,p=1$tMnV5paYjmIrUIb6hylaNA$M8/e0i3NGrjhOliVLa5LqQ', "The password was saved as-is."); + + my $storedPassword = $user->password; + ok($user->check_password("foobar"), "Their password validates"); + is($storedPassword, $user->password, "The password was not upgraded."); +}; done_testing; diff --git a/t/scripts/hydra-create-user.t b/t/scripts/hydra-create-user.t new file mode 100644 index 00000000..3706d911 --- /dev/null +++ b/t/scripts/hydra-create-user.t @@ -0,0 +1,56 @@ +use feature 'unicode_strings'; +use strict; +use Setup; + +my %ctx = test_init(); + +require Hydra::Schema; +require Hydra::Model::DB; + +use Test2::V0; + +my $db = Hydra::Model::DB->new; +hydra_setup($db); + +subtest "Handling password and password hash creation" => sub { + subtest "Creating a user with a plain text password (insecure) stores the password securely" => sub { + my ($res, $stdout, $stderr) = captureStdoutStderr(5, ("hydra-create-user", "plain-text-user", "--password", "foobar")); + is($res, 0, "hydra-create-user should exit zero"); + + my $user = $db->resultset('Users')->find({ username => "plain-text-user" }); + isnt($user, undef, "The user exists"); + isnt($user->password, "foobar", "The password was not saved in plain text."); + + my $storedPassword = $user->password; + ok($user->check_password("foobar"), "Their password validates"); + is($storedPassword, $user->password, "The password was not upgraded."); + }; + + 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")); + is($res, 0, "hydra-create-user should exit zero"); + + my $user = $db->resultset('Users')->find({ username => "plain-text-user" }); + isnt($user, undef, "The user exists"); + isnt($user->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The password was not saved in plain text."); + + my $storedPassword = $user->password; + ok($user->check_password("foobar"), "Their password validates"); + isnt($storedPassword, $user->password, "The password was upgraded."); + }; + + 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')); + is($res, 0, "hydra-create-user should exit zero"); + + my $user = $db->resultset('Users')->find({ username => "plain-text-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."); + + my $storedPassword = $user->password; + ok($user->check_password("foobar"), "Their password validates"); + is($storedPassword, $user->password, "The password was not upgraded."); + }; +}; + +done_testing; diff --git a/t/scripts/hydra-init.t b/t/scripts/hydra-init.t new file mode 100644 index 00000000..517a14d7 --- /dev/null +++ b/t/scripts/hydra-init.t @@ -0,0 +1,68 @@ +use feature 'unicode_strings'; +use strict; +use Setup; + +my %ctx = test_init(); + +require Hydra::Schema; +require Hydra::Model::DB; + +use Test2::V0; + +my $db = Hydra::Model::DB->new; +hydra_setup($db); + +subtest "hydra-init upgrades user's password hashes from sha1 to sha1 inside Argon2" => sub { + my $alice = $db->resultset('Users')->create({ + "username" => "alice", + "emailaddress" => 'alice@nixos.org', + "password" => "8843d7f92416211de9ebb963ff4ce28125932878" # SHA1 of "foobar" + }); + my $janet = $db->resultset('Users')->create({ + "username" => "janet", + "emailaddress" => 'janet@nixos.org', + "password" => "!" + }); + $janet->setPassword("foobar"); + + is($alice->password, "8843d7f92416211de9ebb963ff4ce28125932878", "Alices's sha1 is stored in the database"); + my ($res, $stdout, $stderr) = captureStdoutStderr(5, ("hydra-init")); + if ($res != 0) { + is($stdout, ""); + is($stderr, ""); + } + is($res, 0, "hydra-init should exit zero"); + + subtest "Alice had their password updated in place" => sub { + my $updatedAlice = $db->resultset('Users')->find({ username => "alice" }); + isnt($updatedAlice, undef); + isnt($updatedAlice->password, "8843d7f92416211de9ebb963ff4ce28125932878", "The password was updated in place."); + + my $storedPassword = $updatedAlice->password; + ok($updatedAlice->check_password("foobar"), "Their password validates"); + isnt($storedPassword, $updatedAlice->password, "The password is upgraded in place."); + }; + + subtest "Janet did not have their password change" => sub { + my $updatedJanet = $db->resultset('Users')->find({ username => "janet" }); + isnt($updatedJanet, undef); + is($updatedJanet->password, $janet->password, "The password was not updated in place."); + + ok($updatedJanet->check_password("foobar"), "Their password validates"); + is($updatedJanet->password, $janet->password, "The password is not upgraded in place."); + }; + + subtest "Running hydra-init don't break Alice or Janet's passwords" => sub { + my ($res, $stdout, $stderr) = captureStdoutStderr(5, ("hydra-init")); + is($res, 0, "hydra-init should exit zero"); + + my $updatedAlice = $db->resultset('Users')->find({ username => "alice" }); + ok($updatedAlice->check_password("foobar"), "Alice's password validates"); + + my $updatedJanet = $db->resultset('Users')->find({ username => "janet" }); + ok($updatedJanet->check_password("foobar"), "Janet's password validates"); + }; + +}; + +done_testing;