Attachment 133666 Details for Bug 29894 – Bug 29894: (QA follow-up) Get rid of send_confirm_notice

[patch] Bug 29894: (QA follow-up) Get rid of send_confirm_notice

Bug-29894-QA-follow-up-Get-rid-of-sendconfirmnotic.patch (text/plain), 6.31 KB, created by Jonathan Druart on 2022-04-22 10:30:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2022-04-22 10:30:45 UTC

Size: 6.31 KB

patch

obsolete

>From 3d567fdba1d2d313dec2af388a015897a03e93ac Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>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 <m.de.rooy@rijksmuseum.nl>
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>---
> 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 f2cce958723..aa4f5aca523 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 6d5847e0ac1..0a5278c8e68 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 4185e5cde09..39e8a1d3fa7 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.25.1

Actions: View | Diff | Splinter Review

Attachments on bug 29894: 129538 | 129539 | 129669 | 129670 | 129689 | 129690 | 129691 | 129692 | 133286 | 133287 | 133288 | 133289 | 133562 | 133563 | 133564 | 133565 | 133566 | 133618 | 133661 | 133662 | 133663 | 133664 | 133665 | 133666 | 133667