Bugzilla – Attachment 189213 Details for
Bug 40824
Add option to prevent re-use of passwords
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40824: Use category-specific password history count
Bug-40824-Use-category-specific-password-history-c.patch (text/plain), 8.37 KB, created by
Jacob O'Mara
on 2025-11-06 19:34:29 UTC
(
hide
)
Description:
Bug 40824: Use category-specific password history count
Filename:
MIME Type:
Creator:
Jacob O'Mara
Created:
2025-11-06 19:34:29 UTC
Size:
8.37 KB
patch
obsolete
>From a1614e92ab4b20511821522e80848a90f6870215 Mon Sep 17 00:00:00 2001 >From: Jacob O'Mara <Jacob.omara@openfifth.co.uk> >Date: Thu, 14 Aug 2025 11:49:27 +0100 >Subject: [PATCH] Bug 40824: Use category-specific password history count > >Update password validation logic to use the category-specific >effective_password_history_count method instead of the global >system preference: > >- Update Patron.pm to use category method >- Update PatronPasswordHistories.pm validation logic >- Update PatronPasswordHistory.pm storage cleanup logic >--- > Koha/Patron.pm | 2 +- > Koha/PatronPasswordHistories.pm | 77 ++++++++++++++++++++------------- > Koha/PatronPasswordHistory.pm | 18 +++++--- > 3 files changed, 60 insertions(+), 37 deletions(-) > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 6af253a13b1..b47defb8aa5 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -1162,7 +1162,7 @@ sub set_password { > if ($self->password && $self->in_storage) { > # Get the old password before it's replaced > my $old_password = $self->get_from_storage->password; >- my $history_count = C4::Context->preference('PasswordHistoryCount') || 0; >+ my $history_count = $self->category->effective_password_history_count || 0; > > # Always store the old password, regardless of current preference > # This ensures we maintain history if the preference is increased later >diff --git a/Koha/PatronPasswordHistories.pm b/Koha/PatronPasswordHistories.pm >index 82609e4d016..876ff2cb73e 100644 >--- a/Koha/PatronPasswordHistories.pm >+++ b/Koha/PatronPasswordHistories.pm >@@ -21,6 +21,7 @@ use Koha::Database; > use Koha::PatronPasswordHistory; > use C4::Context; > use Koha::AuthUtils; >+use Koha::Patrons; > > use base qw(Koha::Objects); > >@@ -42,15 +43,17 @@ If count is 0 or negative, all password history for the patron will be deleted. > =cut > > sub cleanup_old_password_history { >- my ($class, $borrowernumber, $count) = @_; >+ my ( $class, $borrowernumber, $count ) = @_; > > return unless $borrowernumber; > > # If count is 0 or negative, delete all entries >- if ($count <= 0) { >- my $deleted = $class->search({ >- borrowernumber => $borrowernumber >- })->delete; >+ if ( $count <= 0 ) { >+ my $deleted = $class->search( >+ { >+ borrowernumber => $borrowernumber >+ } >+ )->delete; > return $deleted; > } > >@@ -59,18 +62,20 @@ sub cleanup_old_password_history { > { borrowernumber => $borrowernumber }, > { > order_by => { -desc => 'created_on' }, >- rows => $count, >- columns => ['id'] >+ rows => $count, >+ columns => ['id'] > } > )->get_column('id'); > > # Delete all entries for this borrower except the ones we're keeping > my $deleted = 0; > if (@keep_ids) { >- $deleted = $class->search({ >- borrowernumber => $borrowernumber, >- id => { -not_in => \@keep_ids } >- })->delete; >+ $deleted = $class->search( >+ { >+ borrowernumber => $borrowernumber, >+ id => { -not_in => \@keep_ids } >+ } >+ )->delete; > } > > return $deleted; >@@ -83,6 +88,7 @@ Return the DBIC resultset type for this class > =cut > > sub _type { >+ > # The table name in DB is borrower_password_history > return 'BorrowerPasswordHistory'; > } >@@ -112,58 +118,71 @@ Returns true if the password has been used before, false otherwise. > =cut > > sub has_used_password { >- my ($class, $params) = @_; >+ my ( $class, $params ) = @_; > >- my $borrowernumber = $params->{borrowernumber}; >- my $password = $params->{password}; >+ my $borrowernumber = $params->{borrowernumber}; >+ my $password = $params->{password}; > my $current_password = $params->{current_password}; > >- unless ($borrowernumber && $password) { >+ unless ( $borrowernumber && $password ) { > return 0; > } > >- my $history_count = C4::Context->preference('PasswordHistoryCount') || 0; >+ # Get the patron and their category >+ my $patron = Koha::Patrons->find($borrowernumber); >+ return 0 unless $patron; >+ >+ my $history_count = >+ $patron->category->effective_password_history_count || 0; > > # If history count is 0, don't check history at all >- if ($history_count == 0) { >+ if ( $history_count == 0 ) { > return 0; > } > > # If history count is 1, only check current password >- if ($history_count == 1) { >- my $matches_current = $current_password && C4::Auth::checkpw_hash($password, $current_password); >+ if ( $history_count == 1 ) { >+ my $matches_current = $current_password >+ && C4::Auth::checkpw_hash( $password, $current_password ); > return $matches_current ? 1 : 0; > } > > # First check if password matches current one, if provided >- if ($current_password && C4::Auth::checkpw_hash($password, $current_password)) { >- return 1; # Current password counts as a used password >+ if ( $current_password >+ && C4::Auth::checkpw_hash( $password, $current_password ) ) >+ { >+ return 1; # Current password counts as a used password > } > > # Adjust history count to account for current password >- my $history_entries_to_check = $current_password ? $history_count - 1 : $history_count; >+ my $history_entries_to_check = >+ $current_password ? $history_count - 1 : $history_count; > >- return 0 if $history_entries_to_check <= 0; # No need to check history if only checking current >+ return 0 >+ if $history_entries_to_check <= >+ 0; # No need to check history if only checking current > > # Get the most recent password history entries > my $password_history = $class->search( > { borrowernumber => $borrowernumber }, > { > order_by => { -desc => 'created_on' }, >- rows => $history_entries_to_check >+ rows => $history_entries_to_check > } > ); > > # Check each password history entry >- while (my $history_entry = $password_history->next) { >- # Use checkpw_hash to properly compare the plaintext password with stored hash >- my $matches = C4::Auth::checkpw_hash($password, $history_entry->password); >+ while ( my $history_entry = $password_history->next ) { >+ >+ # Use checkpw_hash to properly compare the plaintext password with stored hash >+ my $matches = >+ C4::Auth::checkpw_hash( $password, $history_entry->password ); > if ($matches) { >- return 1; # Password found in history >+ return 1; # Password found in history > } > } > >- return 0; # Password not found in history >+ return 0; # Password not found in history > } > > =head1 AUTHOR >diff --git a/Koha/PatronPasswordHistory.pm b/Koha/PatronPasswordHistory.pm >index 816b5737bf3..5d3f0bb8213 100644 >--- a/Koha/PatronPasswordHistory.pm >+++ b/Koha/PatronPasswordHistory.pm >@@ -40,6 +40,7 @@ Return the DBIC resultset type for this object > =cut > > sub _type { >+ > # The table name in DB is borrower_password_history > return 'BorrowerPasswordHistory'; > } >@@ -65,7 +66,7 @@ sub store { > my ($self) = @_; > > my $borrowernumber = $self->borrowernumber; >- my $password = $self->password; >+ my $password = $self->password; > > unless ($password) { > return; >@@ -78,20 +79,23 @@ sub store { > # Store in database > $self = $self->SUPER::store(); > >+ # Get the patron and their category >+ my $patron = Koha::Patrons->find($borrowernumber); >+ return $self unless $patron; >+ > # Get the history count preference >- my $history_count = C4::Context->preference('PasswordHistoryCount') || 0; >+ my $history_count = >+ $patron->category->effective_password_history_count || 0; > > # Clean up old history entries > # Adjust history count to account for current password > my $history_entries_to_keep = $history_count <= 1 >- ? $history_count # If 0 or 1, keep that many >- : $history_count - 1; # Otherwise keep historical entries minus current >+ ? $history_count # If 0 or 1, keep that many >+ : $history_count - 1; # Otherwise keep historical entries minus current > > # Clean up old entries > Koha::PatronPasswordHistories->cleanup_old_password_history( >- $borrowernumber, >- $history_entries_to_keep >- ); >+ $borrowernumber, $history_entries_to_keep ); > > return $self; > } >-- >2.39.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40824
:
186511
|
186512
|
186513
|
186514
|
186515
|
186516
|
186517
|
186518
|
186519
|
186520
|
186521
|
186522
|
186523
|
186524
|
186525
|
186526
|
186527
|
189201
|
189202
|
189203
|
189204
|
189205
|
189206
|
189207
|
189208
|
189209
|
189210
|
189211
|
189212
| 189213 |
189214
|
189215
|
189216
|
189217