From 9736918bdd3ac576c8a58025c2d7496ca8f74081 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Wed, 12 Sep 2018 14:25:26 +0200 Subject: [PATCH] Bug 21336: Search, lock and anonymize methods Content-Type: text/plain; charset=utf-8 Add Koha::Patron->lock and anonymize. Add Koha::Patrons methods search_unsubscribed, search_anonymize_candidates and search_anonymized. And wrappers for lock and anonymize. Add unit tests. Test plan: Run t/db_dependent/Koha/Patrons.t Signed-off-by: Marcel de Rooy --- Koha/Patron.pm | 73 +++++++++++++++++++ Koha/Patrons.pm | 122 +++++++++++++++++++++++++++++++ t/db_dependent/Koha/Patrons.t | 162 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 356 insertions(+), 1 deletion(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index fe110fb..d9ac35e 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -44,9 +44,12 @@ use Koha::Virtualshelves; use Koha::Club::Enrollments; use Koha::Account; use Koha::Subscription::Routinglists; +use Koha::Token; use base qw(Koha::Object); +use constant ADMINISTRATIVE_LOCKOUT => -1; + our $RESULTSET_PATRON_ID_MAPPING = { Accountline => 'borrowernumber', Aqbasketuser => 'borrowernumber', @@ -1139,6 +1142,76 @@ sub account_locked { return 0; } +=head3 lock + + Koha::Patrons->find($id)->lock({ expire => 1, remove => 1 }); + + Lock and optionally expire a patron account. + Remove holds and article requests if remove flag set. + In order to distinguish from locking by entering a wrong password, let's + call this an administrative lockout. + +=cut + +sub lock { + my ( $self, $params ) = @_; + $self->login_attempts( ADMINISTRATIVE_LOCKOUT ); + if( $params->{expire} ) { + $self->dateexpiry( dt_from_string->subtract(days => 1) ); + } + $self->store; + if( $params->{remove} ) { + $self->holds->delete; + $self->article_requests->delete; + } + return $self; +} + +=head3 anonymize + + Koha::Patrons->find($id)->anonymize; + + Anonymize or clear borrower fields. Fields in BorrowerMandatoryField + are randomized, other personal data is cleared too. + Patrons with issues are skipped. + +=cut + +sub anonymize { + my ( $self ) = @_; + if( $self->_result->issues->count ) { + warn "Exiting anonymize: patron ".$self->borrowernumber." still has issues"; + return; + } + my $mandatory = { map { (lc $_, 1); } + split /\s*\|\s*/, C4::Context->preference('BorrowerMandatoryField') }; + $mandatory->{userid} = 1; # needed since sub store does not clear field + my @columns = $self->_result->result_source->columns; + @columns = grep { !/borrowernumber|branchcode|categorycode|^date|password|flags|updated_on|lastseen|lang|login_attempts|flgAnonymized/ } @columns; + push @columns, 'dateofbirth'; # add this date back in + foreach my $col (@columns) { + if( $mandatory->{lc $col} ) { + my $str = $self->_anonymize_column($col); + $self->$col($str); + } else { + $self->$col(undef); + } + } + $self->flgAnonymized(1)->store; +} + +sub _anonymize_column { + my ( $self, $col ) = @_; + my $type = $self->_result->result_source->column_info($col)->{data_type}; + if( $type =~ /char|text/ ) { + return Koha::Token->new->generate({ pattern => '\w{10}' }); + } elsif( $type =~ /integer|int$|float|dec|double/ ) { + return 0; + } elsif( $type =~ /date|time/ ) { + return dt_from_string; + } +} + =head3 can_see_patron_infos my $can_see = $patron->can_see_patron_infos( $patron ); diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index 21061cd..81a85a6 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -236,6 +236,128 @@ sub delete { return $patrons_deleted; } +=head3 search_unsubscribed + + Koha::Patrons->search_unsubscribed; + + Returns a set of Koha patron objects for patrons that recently + unsubscribed and are not locked (candidates for locking). + Depends on UnsubscribeReflectionDelay. + +=cut + +sub search_unsubscribed { + my ( $class ) = @_; + + my $delay = C4::Context->preference('UnsubscribeReflectionDelay'); + if( !defined($delay) || $delay eq q{} ) { + # return empty set + return $class->search({ borrowernumber => undef }); + } + my $parser = Koha::Database->new->schema->storage->datetime_parser; + my $dt = dt_from_string()->subtract( days => $delay ); + my $str = $parser->format_datetime($dt); + my $fails = C4::Context->preference('FailedLoginAttempts') || 0; + my $cond = [ undef, 0, 1..$fails-1 ]; # NULL, 0, 1..fails-1 (if fails>0) + return $class->search( + { + 'patron_consents.refused_on' => { '<=' => $str }, + 'login_attempts' => $cond, + }, + { join => 'patron_consents' }, + ); +} + +=head3 search_anonymize_candidates + + Koha::Patrons->search_anonymize_candidates({ locked => 1 }); + + Returns a set of Koha patron objects for patrons whose account is expired + and locked (if parameter set). These are candidates for anonymizing. + Depends on PatronAnonymizeDelay. + +=cut + +sub search_anonymize_candidates { + my ( $class, $params ) = @_; + + my $delay = C4::Context->preference('PatronAnonymizeDelay'); + if( !defined($delay) || $delay eq q{} ) { + # return empty set + return $class->search({ borrowernumber => undef }); + } + my $cond = {}; + my $parser = Koha::Database->new->schema->storage->datetime_parser; + my $dt = dt_from_string()->subtract( days => $delay ); + my $str = $parser->format_datetime($dt); + $cond->{dateexpiry} = { '<=' => $str }; + $cond->{flgAnonymized} = [ undef, 0 ]; # not yet done + if( $params->{locked} ) { + my $fails = C4::Context->preference('FailedLoginAttempts'); + $cond->{login_attempts} = [ -and => { '!=' => undef }, { -not_in => [0, 1..$fails-1 ] } ]; # -not_in does not like undef + } + return $class->search( $cond ); +} + +=head3 search_anonymized + + Koha::Patrons->search_anonymized; + + Returns a set of Koha patron objects for patron accounts that have been + anonymized before and could be removed. + Depends on PatronRemovalDelay. + +=cut + +sub search_anonymized { + my ( $class ) = @_; + + my $delay = C4::Context->preference('PatronRemovalDelay'); + if( !defined($delay) || $delay eq q{} ) { + # return empty set + return $class->search({ borrowernumber => undef }); + } + my $cond = {}; + my $parser = Koha::Database->new->schema->storage->datetime_parser; + my $dt = dt_from_string()->subtract( days => $delay ); + my $str = $parser->format_datetime($dt); + $cond->{dateexpiry} = { '<=' => $str }; + $cond->{flgAnonymized} = 1; + return $class->search( $cond ); +} + +=head3 lock + + Koha::Patrons->search({ some filters })->lock({ expire => 1, remove => 1 }) + + Lock the passed set of patron objects. Optionally expire and remove holds. + Wrapper around Koha::Patron->lock. + +=cut + +sub lock { + my ( $self, $params ) = @_; + while( my $patron = $self->next ) { + $patron->lock($params); + } +} + +=head3 anonymize + + Koha::Patrons->search({ some filters })->anonymize; + + Anonymize passed set of patron objects. + Wrapper around Koha::Patron->anonymize. + +=cut + +sub anonymize { + my ( $self ) = @_; + while( my $patron = $self->next ) { + $patron->anonymize; + } +} + =head3 _type =cut diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 61bf804..4620abb 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 34; +use Test::More tests => 39; use Test::Warn; use Test::Exception; use Test::MockModule; @@ -1453,6 +1453,166 @@ subtest 'Log cardnumber change' => sub { is( scalar @logs, 2, 'With BorrowerLogs, Change in cardnumber should be logged, as well as general alert of patron mod.' ); }; +subtest 'search_unsubscribed' => sub { + plan tests => 4; + + t::lib::Mocks::mock_preference( 'FailedLoginAttempts', 3 ); + t::lib::Mocks::mock_preference( 'UnsubscribeReflectionDelay', '' ); + is( Koha::Patrons->search_unsubscribed->count, 0, 'Empty delay should return empty set' ); + + my $patron1 = $builder->build_object({ class => 'Koha::Patrons' }); + my $patron2 = $builder->build_object({ class => 'Koha::Patrons' }); + + t::lib::Mocks::mock_preference( 'UnsubscribeReflectionDelay', 0 ); + Koha::Patron::Consents->delete; # for correct counts + Koha::Patron::Consent->new({ borrowernumber => $patron1->borrowernumber, type => 'GDPR_PROCESSING', refused_on => dt_from_string })->store; + is( Koha::Patrons->search_unsubscribed->count, 1, 'Find patron1' ); + + # Add another refusal but shift the period + t::lib::Mocks::mock_preference( 'UnsubscribeReflectionDelay', 2 ); + Koha::Patron::Consent->new({ borrowernumber => $patron2->borrowernumber, type => 'GDPR_PROCESSING', refused_on => dt_from_string->subtract(days=>2) })->store; + is( Koha::Patrons->search_unsubscribed->count, 1, 'Find patron2 only' ); + + # Try another (special) attempts setting + t::lib::Mocks::mock_preference( 'FailedLoginAttempts', 0 ); + # Lockout is now disabled + # Patron2 still matches: refused earlier, not locked + is( Koha::Patrons->search_unsubscribed->count, 1, 'Lockout disabled' ); +}; + +subtest 'search_anonymize_candidates' => sub { + plan tests => 5; + my $patron1 = $builder->build_object({ class => 'Koha::Patrons' }); + my $patron2 = $builder->build_object({ class => 'Koha::Patrons' }); + $patron1->flgAnonymized(0); + $patron1->dateexpiry( dt_from_string->add(days => 1) )->store; + $patron2->flgAnonymized(undef); + $patron2->dateexpiry( dt_from_string->add(days => 1) )->store; + + t::lib::Mocks::mock_preference( 'PatronAnonymizeDelay', q{} ); + is( Koha::Patrons->search_anonymize_candidates->count, 0, 'Empty set' ); + + t::lib::Mocks::mock_preference( 'PatronAnonymizeDelay', 0 ); + my $cnt = Koha::Patrons->search_anonymize_candidates->count; + $patron1->dateexpiry( dt_from_string->subtract(days => 1) )->store; + $patron2->dateexpiry( dt_from_string->subtract(days => 3) )->store; + is( Koha::Patrons->search_anonymize_candidates->count, $cnt+2, 'Delay 0' ); + + t::lib::Mocks::mock_preference( 'PatronAnonymizeDelay', 2 ); + $patron1->dateexpiry( dt_from_string->add(days => 1) )->store; + $patron2->dateexpiry( dt_from_string->add(days => 1) )->store; + $cnt = Koha::Patrons->search_anonymize_candidates->count; + $patron1->dateexpiry( dt_from_string->subtract(days => 1) )->store; + $patron2->dateexpiry( dt_from_string->subtract(days => 3) )->store; + is( Koha::Patrons->search_anonymize_candidates->count, $cnt+1, 'Delay 2' ); + + t::lib::Mocks::mock_preference( 'PatronAnonymizeDelay', 4 ); + $patron1->dateexpiry( dt_from_string->add(days => 1) )->store; + $patron2->dateexpiry( dt_from_string->add(days => 1) )->store; + $cnt = Koha::Patrons->search_anonymize_candidates->count; + $patron1->dateexpiry( dt_from_string->subtract(days => 1) )->store; + $patron2->dateexpiry( dt_from_string->subtract(days => 3) )->store; + is( Koha::Patrons->search_anonymize_candidates->count, $cnt, 'Delay 4' ); + + t::lib::Mocks::mock_preference( 'FailedLoginAttempts', 3 ); + $patron1->dateexpiry( dt_from_string->subtract(days => 5) )->store; + $patron1->login_attempts(0)->store; + $patron2->dateexpiry( dt_from_string->subtract(days => 5) )->store; + $patron2->login_attempts(0)->store; + $cnt = Koha::Patrons->search_anonymize_candidates({locked => 1})->count; + $patron1->login_attempts(3)->store; + is( Koha::Patrons->search_anonymize_candidates({locked => 1})->count, + $cnt+1, 'Locked flag' ); +}; + +subtest 'search_anonymized' => sub { + plan tests => 3; + my $patron1 = $builder->build_object( { class => 'Koha::Patrons' } ); + + t::lib::Mocks::mock_preference( 'PatronRemovalDelay', q{} ); + is( Koha::Patrons->search_anonymized->count, 0, 'Empty set' ); + + t::lib::Mocks::mock_preference( 'PatronRemovalDelay', 1 ); + $patron1->dateexpiry( dt_from_string ); + $patron1->flgAnonymized(0)->store; + my $cnt = Koha::Patrons->search_anonymized->count; + $patron1->flgAnonymized(1)->store; + is( Koha::Patrons->search_anonymized->count, $cnt, 'Number unchanged' ); + $patron1->dateexpiry( dt_from_string->subtract(days => 1) )->store; + is( Koha::Patrons->search_anonymized->count, $cnt+1, 'Found patron1' ); +}; + +subtest 'lock' => sub { + plan tests => 8; + + my $patron1 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $patron2 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $hold = $builder->build_object({ + class => 'Koha::Holds', + value => { borrowernumber => $patron1->borrowernumber }, + }); + + t::lib::Mocks::mock_preference( 'FailedLoginAttempts', 3 ); + my $expiry = dt_from_string->add(days => 1); + $patron1->dateexpiry( $expiry ); + $patron1->lock; + is( $patron1->login_attempts, Koha::Patron::ADMINISTRATIVE_LOCKOUT, 'Check login_attempts' ); + is( $patron1->dateexpiry, $expiry, 'Not expired yet' ); + is( $patron1->holds->count, 1, 'No holds removed' ); + + $patron1->lock({ expire => 1, remove => 1}); + isnt( $patron1->dateexpiry, $expiry, 'Expiry date adjusted' ); + is( $patron1->holds->count, 0, 'Holds removed' ); + + # Disable lockout feature + t::lib::Mocks::mock_preference( 'FailedLoginAttempts', q{} ); + $patron1->login_attempts(0); + $patron1->dateexpiry( $expiry ); + $patron1->store; + $patron1->lock; + is( $patron1->login_attempts, Koha::Patron::ADMINISTRATIVE_LOCKOUT, 'Check login_attempts' ); + + # Trivial wrapper test (Koha::Patrons->lock) + $patron1->login_attempts(0)->store; + Koha::Patrons->search({ borrowernumber => [ $patron1->borrowernumber, $patron2->borrowernumber ] })->lock; + $patron1->discard_changes; # refresh + $patron2->discard_changes; + is( $patron1->login_attempts, Koha::Patron::ADMINISTRATIVE_LOCKOUT, 'Check login_attempts patron 1' ); + is( $patron2->login_attempts, Koha::Patron::ADMINISTRATIVE_LOCKOUT, 'Check login_attempts patron 2' ); +}; + +subtest 'anonymize' => sub { + plan tests => 9; + + my $patron1 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $patron2 = $builder->build_object( { class => 'Koha::Patrons' } ); + + # First try patron with issues + my $issue = $builder->build_object({ class => 'Koha::Checkouts', value => { borrowernumber => $patron2->borrowernumber } }); + warning_like { $patron2->anonymize } qr/still has issues/, 'Skip patron with issues'; + $issue->delete; + + t::lib::Mocks::mock_preference( 'BorrowerMandatoryField', 'surname|email|cardnumber' ); + my $surname = $patron1->surname; # expect change, no clear + my $branchcode = $patron1->branchcode; # expect skip + $patron1->anonymize; + is($patron1->flgAnonymized, 1, 'Check flag' ); + + is( $patron1->dateofbirth, undef, 'Birth date cleared' ); + is( $patron1->firstname, undef, 'First name cleared' ); + isnt( $patron1->surname, $surname, 'Surname changed' ); + ok( $patron1->surname =~ /^\w{10}$/, 'Mandatory surname randomized' ); + is( $patron1->branchcode, $branchcode, 'Branch code skipped' ); + + # Test wrapper in Koha::Patrons + $patron1->surname($surname)->store; # restore + my $rs = Koha::Patrons->search({ borrowernumber => [ $patron1->borrowernumber, $patron2->borrowernumber ] })->anonymize; + $patron1->discard_changes; # refresh + isnt( $patron1->surname, $surname, 'Surname patron1 changed again' ); + $patron2->discard_changes; # refresh + is( $patron2->firstname, undef, 'First name patron2 cleared' ); +}; + $schema->storage->txn_rollback; subtest 'Test Koha::Patrons::merge' => sub { -- 2.1.4