From c4b56590c80b61e69f2dfedde4187bf63834e4b3 Mon Sep 17 00:00:00 2001 From: Jacob O'Mara Date: Thu, 14 Aug 2025 13:43:43 +0100 Subject: [PATCH] Bug 40824: Add tests for category-level password history Add comprehensive unit tests for the new password history functionality: - Add effective_password_history_count tests to Category.t - Create PatronPasswordHistory.t - Create PatronPasswordHistories.t - Add password history integration tests to Patron.t --- t/db_dependent/Koha/Patron.t | 197 ++++++++++++- t/db_dependent/Koha/Patron/Category.t | 52 +++- t/db_dependent/Koha/PatronPasswordHistories.t | 265 ++++++++++++++++++ t/db_dependent/Koha/PatronPasswordHistory.t | 219 +++++++++++++++ 4 files changed, 731 insertions(+), 2 deletions(-) create mode 100755 t/db_dependent/Koha/PatronPasswordHistories.t create mode 100755 t/db_dependent/Koha/PatronPasswordHistory.t diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index b9c1eedc66b..2959023f23d 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 42; +use Test::More tests => 44; use Test::NoWarnings; use Test::Exception; use Test::Warn; @@ -1807,6 +1807,201 @@ subtest 'force_password_reset_when_set_by_staff tests' => sub { $schema->storage->txn_rollback; }; +subtest 'password history integration tests' => sub { + plan tests => 7; + + $schema->storage->txn_begin; + + my $category = $builder->build_object( + { + class => 'Koha::Patron::Categories', + value => { + password_history_count => 3, + require_strong_password => 0, + } + } + ); + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $category->categorycode, + password => Koha::AuthUtils::hash_password('initial_password') + } + } + ); + + # Test that password history is created when changing password + my $initial_history_count = + Koha::PatronPasswordHistories->search( { borrowernumber => $patron->borrowernumber } )->count; + + $patron->set_password( { password => 'new_password_1', skip_validation => 1 } ); + + my $new_history_count = + Koha::PatronPasswordHistories->search( { borrowernumber => $patron->borrowernumber } )->count; + + is( $new_history_count, $initial_history_count + 1, 'Password history entry created when password changed' ); + + # Test that UsedBefore exception is thrown for reused password + throws_ok { + $patron->set_password( { password => 'initial_password' } ); + } + 'Koha::Exceptions::Password::UsedBefore', 'UsedBefore exception thrown for reused password'; + + # Test that category-specific history count is respected + $patron->set_password( { password => 'new_password_2', skip_validation => 1 } ); + $patron->set_password( { password => 'new_password_3', skip_validation => 1 } ); + $patron->set_password( { password => 'new_password_4', skip_validation => 1 } ); + + my $final_history_count = + Koha::PatronPasswordHistories->search( { borrowernumber => $patron->borrowernumber } )->count; + + # With password_history_count=3, we should have 2 history entries (3-1 for current) + is( $final_history_count, 2, 'Password history cleanup respects category setting' ); + + # Test that changing category password_history_count affects validation + $category->password_history_count(1)->store; + + lives_ok { + $patron->set_password( { password => 'initial_password', skip_validation => 1 } ); + } + 'With history_count=1, old passwords from history are not checked'; + + throws_ok { + $patron->set_password( { password => 'initial_password' } ); + } + 'Koha::Exceptions::Password::UsedBefore', 'Current password still checked with history_count=1'; + + # Test with history_count=0 (no checking) + $category->password_history_count(0)->store; + + lives_ok { + $patron->set_password( { password => 'initial_password', skip_validation => 1 } ); + } + 'With history_count=0, no password history checking performed'; + + # Test fallback to system preference + $category->password_history_count(undef)->store; + t::lib::Mocks::mock_preference( 'PasswordHistoryCount', 2 ); + + throws_ok { + $patron->set_password( { password => 'initial_password' } ); + } + 'Koha::Exceptions::Password::UsedBefore', 'Falls back to system preference when category setting is null'; + + $schema->storage->txn_rollback; +}; + +subtest 'password history edge cases and overrides' => sub { + plan tests => 4; + + $schema->storage->txn_begin; + + # Test both category and system preference = 0 (should allow password reuse) + t::lib::Mocks::mock_preference( 'PasswordHistoryCount', 0 ); + + my $category_zero = $builder->build_object( + { + class => 'Koha::Patron::Categories', + value => { + password_history_count => 0, + require_strong_password => 0, + } + } + ); + + my $patron_zero = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $category_zero->categorycode, + password => Koha::AuthUtils::hash_password('reusable_password') + } + } + ); + + # Should allow password reuse when both are 0 + lives_ok { + $patron_zero->set_password( { password => 'reusable_password' } ); + } + 'Password reuse allowed when both category and system preference are 0'; + + # Test category overrides system preference (category higher than system) + t::lib::Mocks::mock_preference( 'PasswordHistoryCount', 1 ); + + my $category_high = $builder->build_object( + { + class => 'Koha::Patron::Categories', + value => { + password_history_count => 3, + require_strong_password => 0, + } + } + ); + + my $patron_high = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $category_high->categorycode, + password => Koha::AuthUtils::hash_password('test_password') + } + } + ); + + # Add some password history + $patron_high->set_password( { password => 'password1', skip_validation => 1 } ); + $patron_high->set_password( { password => 'password2', skip_validation => 1 } ); + + # Should prevent reuse of old password because category=3 overrides system=1 + throws_ok { + $patron_high->set_password( { password => 'test_password' } ); + } + 'Koha::Exceptions::Password::UsedBefore', 'Category setting (3) overrides lower system setting (1)'; + + # Test category overrides system preference (category lower than system) + t::lib::Mocks::mock_preference( 'PasswordHistoryCount', 5 ); + + my $category_low = $builder->build_object( + { + class => 'Koha::Patron::Categories', + value => { + password_history_count => 1, + require_strong_password => 0, + } + } + ); + + my $patron_low = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $category_low->categorycode, + password => Koha::AuthUtils::hash_password('another_password') + } + } + ); + + # Add password history that would be caught by system=5 but not category=1 + $patron_low->set_password( { password => 'old_password1', skip_validation => 1 } ); + $patron_low->set_password( { password => 'old_password2', skip_validation => 1 } ); + + # Should allow reuse of original password because category=1 overrides system=5 + lives_ok { + $patron_low->set_password( { password => 'another_password', skip_validation => 1 } ); + } + 'Category setting (1) overrides higher system setting (5) - old password allowed'; + + # But current password should still be blocked + throws_ok { + $patron_low->set_password( { password => 'another_password' } ); + } + 'Koha::Exceptions::Password::UsedBefore', 'Current password still blocked with category=1'; + + $schema->storage->txn_rollback; +}; + subtest 'safe_to_delete() tests' => sub { plan tests => 17; diff --git a/t/db_dependent/Koha/Patron/Category.t b/t/db_dependent/Koha/Patron/Category.t index 14504efccfa..f51cd6a0a50 100755 --- a/t/db_dependent/Koha/Patron/Category.t +++ b/t/db_dependent/Koha/Patron/Category.t @@ -20,7 +20,7 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 9; +use Test::More tests => 10; use t::lib::TestBuilder; use t::lib::Mocks; @@ -310,6 +310,56 @@ subtest 'get_password_expiry_date() tests' => sub { }; +subtest 'effective_password_history_count' => sub { + plan tests => 5; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'PasswordHistoryCount', 3 ); + + my $category = + $builder->build_object( { class => 'Koha::Patron::Categories', value => { password_history_count => undef } } ); + + is( + $category->effective_password_history_count, 3, + 'Category should use password history count from system preference' + ); + + $category->password_history_count(5)->store; + + is( + $category->effective_password_history_count, 5, + 'Category should use password history count from category setting' + ); + + # Test explicit 0 in category (should not fall back to system preference) + $category->password_history_count(0)->store; + + is( + $category->effective_password_history_count, 0, + 'Category setting of 0 should return 0, not fall back to system preference' + ); + + # Test system preference = 0, category = undef (should return 0) + t::lib::Mocks::mock_preference( 'PasswordHistoryCount', 0 ); + $category->password_history_count(undef)->store; + + is( + $category->effective_password_history_count, 0, + 'Should return 0 when system preference is 0 and category is undef' + ); + + # Test both are 0 + $category->password_history_count(0)->store; + + is( + $category->effective_password_history_count, 0, + 'Should return 0 when both category and system preference are 0' + ); + + $schema->storage->txn_rollback; +}; + subtest 'can_make_suggestions' => sub { plan tests => 6; diff --git a/t/db_dependent/Koha/PatronPasswordHistories.t b/t/db_dependent/Koha/PatronPasswordHistories.t new file mode 100755 index 00000000000..ed71b655c23 --- /dev/null +++ b/t/db_dependent/Koha/PatronPasswordHistories.t @@ -0,0 +1,265 @@ +#!/usr/bin/perl + +# Copyright 2025 Koha Development team +# +# This file is part of Koha +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 4; +use Test::NoWarnings; + +use t::lib::TestBuilder; +use t::lib::Mocks; + +use Koha::Database; +use Koha::PatronPasswordHistory; +use Koha::PatronPasswordHistories; +use Koha::AuthUtils; + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +subtest 'cleanup_old_password_history' => sub { + plan tests => 6; + + $schema->storage->txn_begin; + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + + # Create 5 password history entries + my @passwords = (); + for my $i ( 1 .. 5 ) { + my $password_history = Koha::PatronPasswordHistory->new( + { + borrowernumber => $patron->borrowernumber, + password => Koha::AuthUtils::hash_password("password_$i") + } + ); + $password_history->store; + push @passwords, $password_history; + sleep(1); # Ensure different timestamps + } + + my $count = Koha::PatronPasswordHistories->search( { borrowernumber => $patron->borrowernumber } )->count; + is( $count, 5, 'Initially have 5 password history entries' ); + + # Keep 3 most recent + my $deleted = Koha::PatronPasswordHistories->cleanup_old_password_history( $patron->borrowernumber, 3 ); + is( $deleted, 2, 'Deleted 2 old entries' ); + + $count = Koha::PatronPasswordHistories->search( { borrowernumber => $patron->borrowernumber } )->count; + is( $count, 3, 'Now have 3 password history entries' ); + + # Keep 1 most recent + $deleted = Koha::PatronPasswordHistories->cleanup_old_password_history( $patron->borrowernumber, 1 ); + is( $deleted, 2, 'Deleted 2 more entries' ); + + $count = Koha::PatronPasswordHistories->search( { borrowernumber => $patron->borrowernumber } )->count; + is( $count, 1, 'Now have 1 password history entry' ); + + # Delete all (count = 0) + $deleted = Koha::PatronPasswordHistories->cleanup_old_password_history( $patron->borrowernumber, 0 ); + is( $deleted, 1, 'Deleted remaining entry' ); + + $schema->storage->txn_rollback; +}; + +subtest 'has_used_password with category settings' => sub { + plan tests => 10; + + $schema->storage->txn_begin; + + my $category = $builder->build_object( + { + class => 'Koha::Patron::Categories', + value => { password_history_count => 3 } + } + ); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $category->categorycode, + password => Koha::AuthUtils::hash_password('current_password') + } + } + ); + + # Create password history + my $old_password1 = 'old_password_1'; + my $old_password2 = 'old_password_2'; + + my $history1 = Koha::PatronPasswordHistory->new( + { + borrowernumber => $patron->borrowernumber, + password => Koha::AuthUtils::hash_password($old_password1) + } + ); + $history1->store; + sleep(1); + + my $history2 = Koha::PatronPasswordHistory->new( + { + borrowernumber => $patron->borrowernumber, + password => Koha::AuthUtils::hash_password($old_password2) + } + ); + $history2->store; + + # Test checking against current password + my $is_used = Koha::PatronPasswordHistories->has_used_password( + { + borrowernumber => $patron->borrowernumber, + password => 'current_password', + current_password => $patron->password + } + ); + ok( $is_used, 'Current password detected as already used' ); + + # Test checking against history + $is_used = Koha::PatronPasswordHistories->has_used_password( + { + borrowernumber => $patron->borrowernumber, + password => $old_password1, + current_password => $patron->password + } + ); + ok( $is_used, 'Old password from history detected as already used' ); + + $is_used = Koha::PatronPasswordHistories->has_used_password( + { + borrowernumber => $patron->borrowernumber, + password => $old_password2, + current_password => $patron->password + } + ); + ok( $is_used, 'Another old password from history detected as already used' ); + + # Test new password + $is_used = Koha::PatronPasswordHistories->has_used_password( + { + borrowernumber => $patron->borrowernumber, + password => 'brand_new_password', + current_password => $patron->password + } + ); + ok( !$is_used, 'New password not detected as used' ); + + # Test with category history_count = 1 (only checks current) + $category->password_history_count(1)->store; + + $is_used = Koha::PatronPasswordHistories->has_used_password( + { + borrowernumber => $patron->borrowernumber, + password => $old_password1, + current_password => $patron->password + } + ); + ok( !$is_used, 'With history_count=1, old passwords not checked against history' ); + + $is_used = Koha::PatronPasswordHistories->has_used_password( + { + borrowernumber => $patron->borrowernumber, + password => 'current_password', + current_password => $patron->password + } + ); + ok( $is_used, 'With history_count=1, current password still checked' ); + + # Test with category history_count = 0 (no checking) + $category->password_history_count(0)->store; + + $is_used = Koha::PatronPasswordHistories->has_used_password( + { + borrowernumber => $patron->borrowernumber, + password => 'current_password', + current_password => $patron->password + } + ); + ok( !$is_used, 'With history_count=0, no password checking performed' ); + + # Test fallback to system preference + $category->password_history_count(undef)->store; + t::lib::Mocks::mock_preference( 'PasswordHistoryCount', 2 ); + + $is_used = Koha::PatronPasswordHistories->has_used_password( + { + borrowernumber => $patron->borrowernumber, + password => $old_password2, + current_password => $patron->password + } + ); + ok( $is_used, 'Falls back to system preference when category setting is null' ); + + # Test without current password parameter + $is_used = Koha::PatronPasswordHistories->has_used_password( + { + borrowernumber => $patron->borrowernumber, + password => $old_password1 + } + ); + ok( $is_used, 'Can check password history without current_password parameter' ); + + # Test with patron that has no password history + my $new_patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $category->categorycode, + password => Koha::AuthUtils::hash_password('new_patron_password') + } + } + ); + + $is_used = Koha::PatronPasswordHistories->has_used_password( + { + borrowernumber => $new_patron->borrowernumber, + password => 'some_password', + current_password => $new_patron->password + } + ); + ok( !$is_used, 'Returns false for patron with no password history' ); + + $schema->storage->txn_rollback; +}; + +subtest 'edge cases and validation' => sub { + plan tests => 4; + + $schema->storage->txn_begin; + + # Test with missing parameters + my $is_used = Koha::PatronPasswordHistories->has_used_password( {} ); + ok( !$is_used, 'Returns false with no parameters' ); + + $is_used = Koha::PatronPasswordHistories->has_used_password( { borrowernumber => 99999 } ); + ok( !$is_used, 'Returns false with missing password' ); + + $is_used = Koha::PatronPasswordHistories->has_used_password( { password => 'test' } ); + ok( !$is_used, 'Returns false with missing borrowernumber' ); + + # Test with non-existent patron + $is_used = Koha::PatronPasswordHistories->has_used_password( + { + borrowernumber => 99999, + password => 'test_password' + } + ); + ok( !$is_used, 'Returns false for non-existent patron' ); + + $schema->storage->txn_rollback; +}; diff --git a/t/db_dependent/Koha/PatronPasswordHistory.t b/t/db_dependent/Koha/PatronPasswordHistory.t new file mode 100755 index 00000000000..ade4bffb365 --- /dev/null +++ b/t/db_dependent/Koha/PatronPasswordHistory.t @@ -0,0 +1,219 @@ +#!/usr/bin/perl + +# Copyright 2025 Koha Development team +# +# This file is part of Koha +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 4; +use Test::NoWarnings; + +use t::lib::TestBuilder; +use t::lib::Mocks; + +use Koha::Database; +use Koha::PatronPasswordHistory; +use Koha::PatronPasswordHistories; + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +subtest 'basic functionality' => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + my $category = $builder->build_object( + { + class => 'Koha::Patron::Categories', + value => { password_history_count => 3 } + } + ); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { categorycode => $category->categorycode } + } + ); + + my $password_history = Koha::PatronPasswordHistory->new( + { + borrowernumber => $patron->borrowernumber, + password => 'plaintext_password' + } + ); + + ok( $password_history, 'PatronPasswordHistory object created' ); + isa_ok( $password_history, 'Koha::PatronPasswordHistory' ); + + $password_history->store; + ok( $password_history->in_storage, 'PatronPasswordHistory stored successfully' ); + + $schema->storage->txn_rollback; +}; + +subtest 'password hashing on store' => sub { + plan tests => 4; + + $schema->storage->txn_begin; + + my $category = $builder->build_object( + { + class => 'Koha::Patron::Categories', + value => { password_history_count => 3 } + } + ); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { categorycode => $category->categorycode } + } + ); + + my $plain_password = 'test_password_123'; + my $password_history = Koha::PatronPasswordHistory->new( + { + borrowernumber => $patron->borrowernumber, + password => $plain_password + } + ); + + is( $password_history->password, $plain_password, 'Password is plain text before store' ); + + $password_history->store; + + isnt( $password_history->password, $plain_password, 'Password is hashed after store' ); + like( $password_history->password, qr/^\$2a\$/, 'Password is bcrypt hashed' ); + + # Test that already hashed password is not re-hashed + my $hashed_password = $password_history->password; + my $password_history2 = Koha::PatronPasswordHistory->new( + { + borrowernumber => $patron->borrowernumber, + password => $hashed_password + } + ); + + $password_history2->store; + is( $password_history2->password, $hashed_password, 'Already hashed password is not re-hashed' ); + + $schema->storage->txn_rollback; +}; + +subtest 'cleanup of old history entries' => sub { + plan tests => 5; + + $schema->storage->txn_begin; + + my $category = $builder->build_object( + { + class => 'Koha::Patron::Categories', + value => { password_history_count => 3 } + } + ); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { categorycode => $category->categorycode } + } + ); + + # Create multiple password history entries + for my $i ( 1 .. 5 ) { + my $password_history = Koha::PatronPasswordHistory->new( + { + borrowernumber => $patron->borrowernumber, + password => "password_$i" + } + ); + $password_history->store; + sleep(1); # Ensure different created_on timestamps + } + + my $count = Koha::PatronPasswordHistories->search( { borrowernumber => $patron->borrowernumber } )->count; + + # With password_history_count = 3, we should keep 2 historical entries (3 - 1 for current) + is( $count, 2, 'Old password history entries cleaned up correctly (kept 2 for history_count 3)' ); + + # Test with different category setting - add more entries to test the limit + $category->password_history_count(4)->store; + + # Add more password entries to exceed the limit + for my $i ( 6 .. 10 ) { + my $password_history = Koha::PatronPasswordHistory->new( + { + borrowernumber => $patron->borrowernumber, + password => "password_$i" + } + ); + $password_history->store; + sleep(1); + } + + $count = Koha::PatronPasswordHistories->search( { borrowernumber => $patron->borrowernumber } )->count; + + # With password_history_count = 4, we should keep 3 historical entries (4 - 1 for current) + is( $count, 3, 'Cleanup respects updated category password_history_count and does not exceed limit' ); + + # Test with history_count = 1 (should keep 1 historical entry) + $category->password_history_count(1)->store; + + my $password_history2 = Koha::PatronPasswordHistory->new( + { + borrowernumber => $patron->borrowernumber, + password => "password_7" + } + ); + $password_history2->store; + + $count = Koha::PatronPasswordHistories->search( { borrowernumber => $patron->borrowernumber } )->count; + + is( $count, 1, 'With history_count=1, keeps exactly 1 entry' ); + + # Test with history_count = 0 (should delete all) + $category->password_history_count(0)->store; + + my $password_history3 = Koha::PatronPasswordHistory->new( + { + borrowernumber => $patron->borrowernumber, + password => "password_8" + } + ); + $password_history3->store; + + $count = Koha::PatronPasswordHistories->search( { borrowernumber => $patron->borrowernumber } )->count; + + is( $count, 0, 'With history_count=0, deletes all entries' ); + + # Test fallback to system preference + $category->password_history_count(undef)->store; + t::lib::Mocks::mock_preference( 'PasswordHistoryCount', 2 ); + + my $password_history4 = Koha::PatronPasswordHistory->new( + { + borrowernumber => $patron->borrowernumber, + password => "password_9" + } + ); + $password_history4->store; + + $count = Koha::PatronPasswordHistories->search( { borrowernumber => $patron->borrowernumber } )->count; + + is( $count, 1, 'Falls back to system preference when category setting is null' ); + + $schema->storage->txn_rollback; +}; -- 2.39.5