From f1a8e146ac582b675a8861a49cf9217b995a21c5 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Tue, 21 Apr 2026 10:57:16 +0100 Subject: [PATCH] Bug 34000: Prevent re-issuance of auto-generated cardnumbers When autoMemberNum is enabled, Koha picks the next cardnumber as MAX(numeric cardnumber in borrowers) + 1. If patron A's auto-generated cardnumber is later changed (e.g. they get a physical card) or the patron is deleted, the original number becomes "free" and the next self-registrant is assigned it -- inheriting anything third-party services (OverDrive, Hoopla, unauthenticated self-check) have keyed on the cardnumber. This patch makes Koha::Patron::fixup_cardnumber use a monotonic counter (the autoMemberNumValue syspref) so that once a number has been auto-generated for any patron, it is never auto-generated again. The syspref row is locked with SELECT ... FOR UPDATE inside the existing txn_do, which serialises concurrent callers via InnoDB row-level locking and addresses the race condition raised against earlier revisions of this patch. The formula is: next = MAX(counter, db_max) + 1. When the counter is 0 (pre-upgrade or missing syspref), this degrades gracefully to db_max + 1 -- the legacy behaviour -- so no feature toggle is needed. The counter is only bumped when Koha actually auto-generates a cardnumber; plugin- or caller-supplied values do not touch it. Test plan: 1. Enable autoMemberNum. 2. Self-register patron A via OPAC, note cardnumber N. 3. Edit patron A in the staff interface, change cardnumber to something non-numeric like "MANUAL-XYZ". 4. Self-register patron B. 5. Confirm B gets N+1, NOT N. 6. Delete patron A. 7. Self-register patron C. 8. Confirm C gets N+2, NOT A's original number. 9. Install a plugin implementing the patron_barcode_transform hook; confirm it still runs and its value is honoured (counter is not bumped when the plugin supplies a cardnumber). Signed-off-by: Andrew Fuerste Henry Signed-off-by: Jacob O'Mara (cherry picked from commit ccfbb35de504c3906c961487422991d226f7f55e) Signed-off-by: Lucas Gass --- Koha/Patron.pm | 44 +++++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index eb637eb2c9..1c4a8c610c 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -121,24 +121,46 @@ sub new { =head3 fixup_cardnumber -Autogenerate next cardnumber from highest value found in database +Autogenerate next cardnumber from highest value found in database. + +Uses the C system preference as a monotonic floor so a +cardnumber is never re-issued after the original holder's cardnumber has been +changed or their account deleted. The syspref row is selected C +inside the enclosing transaction so that concurrent C callers are +serialised. =cut sub fixup_cardnumber { my ($self) = @_; - my $max = $self->cardnumber; - Koha::Plugins->call( 'patron_barcode_transform', \$max ); + my $next = $self->cardnumber; + Koha::Plugins->call( 'patron_barcode_transform', \$next ); - $max ||= Koha::Patrons->search( - { cardnumber => { -regexp => '^-?[0-9]+$' } }, - { - select => \'CAST(cardnumber AS SIGNED)', - as => ['cast_cardnumber'] - } - )->_resultset->get_column('cast_cardnumber')->max; - $self->cardnumber( ( $max || 0 ) + 1 ); + # Only auto-generate when no cardnumber has been supplied (directly or by a plugin). + unless ($next) { + my $schema = $self->_result->result_source->schema; + my $pref_rs = $schema->resultset('Systempreference')->search( + { variable => 'autoMemberNumValue' }, + { for => 'update' }, + ); + my $pref = $pref_rs->first; + my $counter = ( $pref && $pref->value ) || 0; + + my $db_max = Koha::Patrons->search( + { cardnumber => { -regexp => '^-?[0-9]+$' } }, + { + select => \'CAST(cardnumber AS SIGNED)', + as => ['cast_cardnumber'], + } + )->_resultset->get_column('cast_cardnumber')->max + || 0; + + $next = ( $counter > $db_max ? $counter : $db_max ) + 1; + $pref->update( { value => $next } ) if $pref; + } + + $self->cardnumber($next); } =head3 trim_whitespace