Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 32426: Throw InvalidUserid exception in Patron->store

Browse source 
When creating we still call generate_userid and verify the result.
When modifying we do not accept an invalid userid. When needed,
we recreate the userid; this should be very exceptional.

Test plan:
Run t/db_dependent/Koha/Patrons.t
Go to staff interface. Try changing userid of a patron to an
existing one.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Marcel de Rooy 2022-12-08 16:02:35 +01:00 committed by Tomas Cohen Arazi
parent c0260e4501
commit 6b2e92e576
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
3 changed files with 21 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
Koha/Exceptions/Patron.pm
View file

@ -19,6 +19,11 @@ use Exception::Class (
isa => 'Koha::Exceptions::Patron', isa => 'Koha::Exceptions::Patron',
description => "Deleting patron failed, AnonymousPatron is not deleteable" description => "Deleting patron failed, AnonymousPatron is not deleteable"
}, },
'Koha::Exceptions::Patron::InvalidUserid' => {
isa => 'Koha::Exceptions::Patron',
description => 'Field userid is not valid (probably not unique)',
fields => [ 'userid' ],
},
'Koha::Exceptions::Patron::MissingMandatoryExtendedAttribute' => { 'Koha::Exceptions::Patron::MissingMandatoryExtendedAttribute' => {
isa => 'Koha::Exceptions::Patron', isa => 'Koha::Exceptions::Patron',
description => "Mandatory extended attribute missing", description => "Mandatory extended attribute missing",

16
Koha/Patron.pm
View file

@ -225,8 +225,9 @@ sub store {
unless ( $self->in_storage ) { #AddMember unless ( $self->in_storage ) { #AddMember
# Generate a valid userid/login if needed # Generate a valid userid/login if needed
$self->generate_userid $self->generate_userid unless $self->userid;
if not $self->userid or not $self->has_valid_userid; Koha::Exceptions::Patron::InvalidUserid->throw( userid => $self->userid )
unless $self->has_valid_userid;
# Add expiration date if it isn't already there # Add expiration date if it isn't already there
unless ( $self->dateexpiry ) { unless ( $self->dateexpiry ) {
@ -291,12 +292,11 @@ sub store {
else { #ModMember else { #ModMember
my $self_from_storage = $self->get_from_storage; my $self_from_storage = $self->get_from_storage;
# FIXME We should not deal with that here, callers have to do this job
# Moved from ModMember to prevent regressions # Do not accept invalid userid here
unless ( $self->userid ) { $self->generate_userid unless $self->userid;
my $stored_userid = $self_from_storage->userid; Koha::Exceptions::Patron::InvalidUserid->throw( userid => $self->userid )
$self->userid($stored_userid); unless $self->has_valid_userid;
}
# Password must be updated using $self->set_password # Password must be updated using $self->set_password
$self->password($self_from_storage->password); $self->password($self_from_storage->password);

21
t/db_dependent/Koha/Patrons.t
View file

@ -1810,24 +1810,20 @@ subtest 'Test Koha::Patrons::merge' => sub {
}; };
subtest '->store' => sub { subtest '->store' => sub {
plan tests => 8; plan tests => 9;
my $schema = Koha::Database->new->schema; my $schema = Koha::Database->new->schema;
$schema->storage->txn_begin; $schema->storage->txn_begin;
my $print_error = $schema->storage->dbh->{PrintError};
$schema->storage->dbh->{PrintError} = 0; ; # FIXME This does not longer work - because of the transaction in Koha::Patron->store?
my $patron_1 = $builder->build_object({class=> 'Koha::Patrons'}); my $patron_1 = $builder->build_object({class=> 'Koha::Patrons'});
my $patron_2 = $builder->build_object({class=> 'Koha::Patrons'}); my $patron_2 = $builder->build_object({class=> 'Koha::Patrons'});
{ throws_ok { $patron_2->userid( $patron_1->userid )->store; }
local *STDERR; 'Koha::Exceptions::Patron::InvalidUserid',
open STDERR, '>', '/dev/null'; 'Koha::Patron->store raises an exception on duplicate ID';
throws_ok { $patron_2->userid( $patron_1->userid )->store; }
'Koha::Exceptions::Object::DuplicateID', # Clear userid and check regeneration
'Koha::Patron->store raises an exception on duplicate ID'; $patron_2->userid(undef)->store;
close STDERR; like( $patron_2->userid, qr/\w+\.\w+/, 'Userid regenerated' ); # old school userid
}
# Test password # Test password
t::lib::Mocks::mock_preference( 'RequireStrongPassword', 0 ); t::lib::Mocks::mock_preference( 'RequireStrongPassword', 0 );
@ -1853,7 +1849,6 @@ subtest '->store' => sub {
$patron_1->relationship("")->store; $patron_1->relationship("")->store;
is( $patron_1->relationship, undef, ); is( $patron_1->relationship, undef, );
$schema->storage->dbh->{PrintError} = $print_error;
$schema->storage->txn_rollback; $schema->storage->txn_rollback;
subtest 'skip updated_on for BorrowersLog' => sub { subtest 'skip updated_on for BorrowersLog' => sub {