View | Details | Raw Unified | Return to bug 40824
Collapse All | Expand All

(-)a/Koha/Patron.pm (-1 / +1 lines)
Lines 1160-1166 sub set_password { Link Here
1160
    if ($self->password && $self->in_storage) {
1160
    if ($self->password && $self->in_storage) {
1161
        # Get the old password before it's replaced
1161
        # Get the old password before it's replaced
1162
        my $old_password = $self->get_from_storage->password;
1162
        my $old_password = $self->get_from_storage->password;
1163
        my $history_count = C4::Context->preference('PasswordHistoryCount') || 0;
1163
        my $history_count = $self->category->effective_password_history_count || 0;
1164
        
1164
        
1165
        # Always store the old password, regardless of current preference
1165
        # Always store the old password, regardless of current preference
1166
        # This ensures we maintain history if the preference is increased later
1166
        # This ensures we maintain history if the preference is increased later
(-)a/Koha/PatronPasswordHistories.pm (-42 / +61 lines)
Lines 23-28 use Koha::Database; Link Here
23
use Koha::PatronPasswordHistory;
23
use Koha::PatronPasswordHistory;
24
use C4::Context;
24
use C4::Context;
25
use Koha::AuthUtils;
25
use Koha::AuthUtils;
26
use Koha::Patrons;
26
27
27
use base qw(Koha::Objects);
28
use base qw(Koha::Objects);
28
29
Lines 44-80 If count is 0 or negative, all password history for the patron will be deleted. Link Here
44
=cut
45
=cut
45
46
46
sub cleanup_old_password_history {
47
sub cleanup_old_password_history {
47
    my ($class, $borrowernumber, $count) = @_;
48
    my ( $class, $borrowernumber, $count ) = @_;
48
    
49
    
49
    return unless $borrowernumber;
50
    return unless $borrowernumber;
50
    
51
51
    # If count is 0 or negative, delete all entries
52
    # If count is 0 or negative, delete all entries
52
    if ($count <= 0) {
53
    if ( $count <= 0 ) {
53
        my $deleted = $class->search({
54
        my $deleted = $class->search(
54
            borrowernumber => $borrowernumber
55
            {
55
        })->delete;
56
                borrowernumber => $borrowernumber
57
            }
58
        )->delete;
56
        return $deleted;
59
        return $deleted;
57
    }
60
    }
58
    
61
59
    # Get IDs of entries to keep (the most recent ones)
62
    # Get IDs of entries to keep (the most recent ones)
60
    my @keep_ids = $class->search(
63
    my @keep_ids = $class->search(
61
        { borrowernumber => $borrowernumber },
64
        { borrowernumber => $borrowernumber },
62
        { 
65
        {
63
            order_by => { -desc => 'created_on' },
66
            order_by => { -desc => 'created_on' },
64
            rows => $count,
67
            rows     => $count,
65
            columns => ['id']
68
            columns  => ['id']
66
        }
69
        }
67
    )->get_column('id');
70
    )->get_column('id');
68
    
71
69
    # Delete all entries for this borrower except the ones we're keeping
72
    # Delete all entries for this borrower except the ones we're keeping
70
    my $deleted = 0;
73
    my $deleted = 0;
71
    if (@keep_ids) {
74
    if (@keep_ids) {
72
        $deleted = $class->search({
75
        $deleted = $class->search(
73
            borrowernumber => $borrowernumber,
76
            {
74
            id => { -not_in => \@keep_ids }
77
                borrowernumber => $borrowernumber,
75
        })->delete;
78
                id             => { -not_in => \@keep_ids }
79
            }
80
        )->delete;
76
    }
81
    }
77
    
82
78
    return $deleted;
83
    return $deleted;
79
}
84
}
80
85
Lines 85-90 Return the DBIC resultset type for this class Link Here
85
=cut
90
=cut
86
91
87
sub _type {
92
sub _type {
93
88
    # The table name in DB is borrower_password_history
94
    # The table name in DB is borrower_password_history
89
    return 'BorrowerPasswordHistory';
95
    return 'BorrowerPasswordHistory';
90
}
96
}
Lines 114-171 Returns true if the password has been used before, false otherwise. Link Here
114
=cut
120
=cut
115
121
116
sub has_used_password {
122
sub has_used_password {
117
    my ($class, $params) = @_;
123
    my ( $class, $params ) = @_;
118
    
124
    
119
    my $borrowernumber = $params->{borrowernumber};
125
    my $borrowernumber   = $params->{borrowernumber};
120
    my $password = $params->{password};
126
    my $password         = $params->{password};
121
    my $current_password = $params->{current_password};
127
    my $current_password = $params->{current_password};
122
    
128
123
    unless ($borrowernumber && $password) {
129
    unless ( $borrowernumber && $password ) {
124
        return 0;
130
        return 0;
125
    }
131
    }
126
    
132
127
    my $history_count = C4::Context->preference('PasswordHistoryCount') || 0;
133
    # Get the patron and their category
134
    my $patron = Koha::Patrons->find($borrowernumber);
135
    return 0 unless $patron;
136
137
    my $history_count =
138
      $patron->category->effective_password_history_count || 0;
128
    
139
    
129
    # If history count is 0, don't check history at all
140
    # If history count is 0, don't check history at all
130
    if ($history_count == 0) {
141
    if ( $history_count == 0 ) {
131
        return 0;
142
        return 0;
132
    }
143
    }
133
    
144
134
    # If history count is 1, only check current password
145
    # If history count is 1, only check current password
135
    if ($history_count == 1) {
146
    if ( $history_count == 1 ) {
136
        my $matches_current = $current_password && C4::Auth::checkpw_hash($password, $current_password);
147
        my $matches_current = $current_password
148
          && C4::Auth::checkpw_hash( $password, $current_password );
137
        return $matches_current ? 1 : 0;
149
        return $matches_current ? 1 : 0;
138
    }
150
    }
139
    
151
    
140
    # First check if password matches current one, if provided
152
    # First check if password matches current one, if provided
141
    if ($current_password && C4::Auth::checkpw_hash($password, $current_password)) {
153
    if ( $current_password
142
        return 1; # Current password counts as a used password
154
        && C4::Auth::checkpw_hash( $password, $current_password ) )
155
    {
156
        return 1;    # Current password counts as a used password
143
    }
157
    }
144
    
158
145
    # Adjust history count to account for current password
159
    # Adjust history count to account for current password
146
    my $history_entries_to_check = $current_password ? $history_count - 1 : $history_count;
160
    my $history_entries_to_check =
147
    
161
      $current_password ? $history_count - 1 : $history_count;
148
    return 0 if $history_entries_to_check <= 0; # No need to check history if only checking current
162
163
    return 0
164
      if $history_entries_to_check <=
165
      0;    # No need to check history if only checking current
149
    
166
    
150
    # Get the most recent password history entries
167
    # Get the most recent password history entries
151
    my $password_history = $class->search(
168
    my $password_history = $class->search(
152
        { borrowernumber => $borrowernumber },
169
        { borrowernumber => $borrowernumber },
153
        { 
170
        {
154
            order_by => { -desc => 'created_on' },
171
            order_by => { -desc => 'created_on' },
155
            rows => $history_entries_to_check
172
            rows     => $history_entries_to_check
156
        }
173
        }
157
    );
174
    );
158
    
175
159
    # Check each password history entry
176
    # Check each password history entry
160
    while (my $history_entry = $password_history->next) {
177
    while ( my $history_entry = $password_history->next ) {
161
        # Use checkpw_hash to properly compare the plaintext password with stored hash
178
162
        my $matches = C4::Auth::checkpw_hash($password, $history_entry->password);
179
  # Use checkpw_hash to properly compare the plaintext password with stored hash
180
        my $matches =
181
          C4::Auth::checkpw_hash( $password, $history_entry->password );
163
        if ($matches) {
182
        if ($matches) {
164
            return 1; # Password found in history
183
            return 1;    # Password found in history
165
        }
184
        }
166
    }
185
    }
167
    
186
168
    return 0; # Password not found in history
187
    return 0;            # Password not found in history
169
}
188
}
170
189
171
=head1 AUTHOR
190
=head1 AUTHOR
(-)a/Koha/PatronPasswordHistory.pm (-22 / +25 lines)
Lines 42-47 Return the DBIC resultset type for this object Link Here
42
=cut
42
=cut
43
43
44
sub _type {
44
sub _type {
45
45
    # The table name in DB is borrower_password_history
46
    # The table name in DB is borrower_password_history
46
    return 'BorrowerPasswordHistory';
47
    return 'BorrowerPasswordHistory';
47
}
48
}
Lines 65-108 Also handles cleanup of old password history entries. Link Here
65
66
66
sub store {
67
sub store {
67
    my ($self) = @_;
68
    my ($self) = @_;
68
    
69
69
    my $borrowernumber = $self->borrowernumber;
70
    my $borrowernumber = $self->borrowernumber;
70
    my $password = $self->password;
71
    my $password       = $self->password;
71
    
72
72
    unless ($password) {
73
    unless ($password) {
73
        return;
74
        return;
74
    }
75
    }
75
    
76
76
    # Note: Don't use string comparison for password hashes - this is a common pattern
77
# Note: Don't use string comparison for password hashes - this is a common pattern
77
    # where we're intentionally storing the old password in history
78
# where we're intentionally storing the old password in history
78
    # The old check would fail when the patron's current hashed password was exactly
79
# The old check would fail when the patron's current hashed password was exactly
79
    # what we were trying to store (which is the expected case when changing passwords)
80
# what we were trying to store (which is the expected case when changing passwords)
80
    
81
81
    # Hash the password if it hasn't been hashed already
82
    # Hash the password if it hasn't been hashed already
82
    unless ( $self->in_storage ) {
83
    unless ( $self->in_storage ) {
83
        if ( $password && $password !~ /\$2a\$/ ) {
84
        if ( $password && $password !~ /\$2a\$/ ) {
84
            $self->password( Koha::AuthUtils::hash_password($password) );
85
            $self->password( Koha::AuthUtils::hash_password($password) );
85
        }
86
        }
86
    }
87
    }
87
    
88
88
    # Store in database
89
    # Store in database
89
    $self = $self->SUPER::store();
90
    $self = $self->SUPER::store();
90
    
91
92
    # Get the patron and their category
93
    my $patron = Koha::Patrons->find($borrowernumber);
94
    return $self unless $patron;
95
91
    # Get the history count preference
96
    # Get the history count preference
92
    my $history_count = C4::Context->preference('PasswordHistoryCount') || 0;
97
    my $history_count =
93
    
98
      $patron->category->effective_password_history_count || 0;
99
94
    # Clean up old history entries
100
    # Clean up old history entries
95
    # Adjust history count to account for current password
101
    # Adjust history count to account for current password
96
    my $history_entries_to_keep = $history_count <= 1 
102
    my $history_entries_to_keep = $history_count <= 1
97
        ? $history_count  # If 0 or 1, keep that many
103
      ? $history_count         # If 0 or 1, keep that many
98
        : $history_count - 1;  # Otherwise keep historical entries minus current
104
      : $history_count - 1;    # Otherwise keep historical entries minus current
99
    
105
100
    # Clean up old entries
106
    # Clean up old entries
101
    Koha::PatronPasswordHistories->cleanup_old_password_history(
107
    Koha::PatronPasswordHistories->cleanup_old_password_history(
102
        $borrowernumber,
108
        $borrowernumber, $history_entries_to_keep );
103
        $history_entries_to_keep
109
104
    );
105
    
106
    return $self;
110
    return $self;
107
}
111
}
108
112
109
- 

Return to bug 40824