Bugzilla – Attachment 133666 Details for
Bug 29894
2FA: Add few validations, clear secret, send register notice
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[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:
Bug 29894: (QA follow-up) Get rid of send_confirm_notice
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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
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