From 0988807436bb31cdbc56f3143cabdcf839908c2f Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 22 Apr 2022 06:39:32 +0000 Subject: [PATCH] Bug 29894: (QA follow-up) Get rid of send_confirm_notice Chose here to fall back to $patron->queue_notice. Which is tested already, so removing the additional test code. Signed-off-by: Marcel de Rooy Signed-off-by: Jonathan Druart Signed-off-by: Fridolin Somers --- Koha/Auth/TwoFactorAuth.pm | 42 ------------------------ members/two_factor_auth.pl | 18 ++++++++-- t/db_dependent/Koha/Auth/TwoFactorAuth.t | 32 +----------------- 3 files changed, 17 insertions(+), 75 deletions(-) diff --git a/Koha/Auth/TwoFactorAuth.pm b/Koha/Auth/TwoFactorAuth.pm index f2cce95872..aa4f5aca52 100644 --- a/Koha/Auth/TwoFactorAuth.pm +++ b/Koha/Auth/TwoFactorAuth.pm @@ -25,9 +25,6 @@ use Koha::Exceptions::Patron; use base qw( Auth::GoogleAuth ); -use constant CONFIRM_NOTICE_REG => '2FA_ENABLE'; -use constant CONFIRM_NOTICE_DEREG => '2FA_DISABLE'; - =head1 NAME Koha::Auth::TwoFactorAuth- Koha class deal with Two factor authentication @@ -40,7 +37,6 @@ my $secret = Koha::AuthUtils::generate_salt( 'weak', 16 ); my $auth = Koha::Auth::TwoFactorAuth->new({ patron => $patron, secret => $secret }); my $image_src = $auth->qr_code; my $ok = $auth->verify( $pin_code, 1 ); -$auth->send_confirm_notice({ patron => $patron }); It's based on Auth::GoogleAuth @@ -108,42 +104,4 @@ sub qr_code { return "data:image/png;base64,". encode_base64( $data, q{} ); # does not contain newlines } -=head3 send_confirm_notice - - $auth->send_confirm_notice({ patron => $p, deregister => 1 }); - - Send a notice to confirm (de)registering 2FA. - Parameter patron is mandatory. - If there is no deregister param, a register notice is sent. - If the patron has no email address, we throw an exception. - -=cut - -sub send_confirm_notice { - my ( $self, $params ) = @_; - my $patron = $params->{patron}; - my $deregister = $params->{deregister}; - - Koha::Exceptions::MissingParameter->throw("Mandatory patron parameter missing") - unless $patron && ref($patron) eq 'Koha::Patron'; - Koha::Exceptions::Patron::MissingEmailAddress->throw - if !$patron->notice_email_address; - - my $letter = C4::Letters::GetPreparedLetter ( - module => 'members', # called patrons on interface - letter_code => $deregister ? CONFIRM_NOTICE_DEREG : CONFIRM_NOTICE_REG, - branchcode => $patron->branchcode, - lang => $patron->lang, - tables => { - 'branches' => $patron->branchcode, - 'borrowers' => $patron->id, - }, - ); - C4::Letters::EnqueueLetter({ - letter => $letter, - borrowernumber => $patron->id, - message_transport_type => 'email', - }) or warn "Couldnt enqueue 2FA notice for patron ". $patron->id; -} - 1; diff --git a/members/two_factor_auth.pl b/members/two_factor_auth.pl index 6d5847e0ac..0a5278c8e6 100755 --- a/members/two_factor_auth.pl +++ b/members/two_factor_auth.pl @@ -73,7 +73,14 @@ if ( $op eq 'register-2FA' ) { $logged_in_user->auth_method('two-factor')->store; $op = 'registered'; if( $logged_in_user->notice_email_address ) { - $auth->send_confirm_notice({ patron => $logged_in_user }); + $logged_in_user->queue_notice({ + letter_params => { + module => 'members', letter_code => '2FA_ENABLE', + branchcode => $logged_in_user->branchcode, lang => $logged_in_user->lang, + tables => { branches => $logged_in_user->branchcode, borrowers => $logged_in_user->id }, + }, + message_transports => [ 'email' ], + }); } } else { @@ -104,7 +111,14 @@ elsif ( $op eq 'disable-2FA' ) { $logged_in_user->secret(undef); $logged_in_user->auth_method('password')->store; if( $logged_in_user->notice_email_address ) { - $auth->send_confirm_notice({ patron => $logged_in_user, deregister => 1 }); + $logged_in_user->queue_notice({ + letter_params => { + module => 'members', letter_code => '2FA_DISABLE', + branchcode => $logged_in_user->branchcode, lang => $logged_in_user->lang, + tables => { branches => $logged_in_user->branchcode, borrowers => $logged_in_user->id }, + }, + message_transports => [ 'email' ], + }); } } diff --git a/t/db_dependent/Koha/Auth/TwoFactorAuth.t b/t/db_dependent/Koha/Auth/TwoFactorAuth.t index 4185e5cde0..39e8a1d3fa 100755 --- a/t/db_dependent/Koha/Auth/TwoFactorAuth.t +++ b/t/db_dependent/Koha/Auth/TwoFactorAuth.t @@ -1,5 +1,5 @@ use Modern::Perl; -use Test::More tests => 3; +use Test::More tests => 2; use Test::Exception; use Test::MockModule; @@ -85,33 +85,3 @@ subtest 'qr_code' => sub { $schema->storage->txn_rollback; }; - -subtest 'send_confirm_notice' => sub { - plan tests => 4; - $schema->storage->txn_begin; - - t::lib::Mocks::mock_preference('TwoFactorAuthentication', 1); - my $patron = $builder->build_object({ class => 'Koha::Patrons' }); - $patron->secret('you2wont2guess2it'); # this is base32 btw - $patron->auth_method('two-factor'); - $patron->store; - my $auth = Koha::Auth::TwoFactorAuth->new({ patron => $patron }); - - # Trivial tests: no patron, no email - throws_ok { $auth->send_confirm_notice; } - 'Koha::Exceptions::MissingParameter', - 'Croaked on missing patron'; - $patron->set({ email => undef, emailpro => undef, B_email => undef }); - throws_ok { $auth->send_confirm_notice({ patron => $patron }) } - 'Koha::Exceptions::Patron::MissingEmailAddress', - 'Croaked on missing email'; - - $patron->email('noreply@doof.nl')->store; - $auth->send_confirm_notice({ patron => $patron }); - is( Koha::Notice::Messages->search({ borrowernumber => $patron->id, letter_code => '2FA_REGISTER' })->count, 1, 'Found message' ); - $auth->send_confirm_notice({ patron => $patron, deregister => 1 }); - is( Koha::Notice::Messages->search({ borrowernumber => $patron->id, letter_code => '2FA_DEREGISTER' })->count, 1, 'Found message' ); - - $schema->storage->txn_rollback; - $mocked_stuffer->unmock; -}; -- 2.39.5