From c6c8b66b74c5bc7ee7e715a55bfa3f451f48c4ea Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 12 Feb 2024 17:36:44 +0100 Subject: [PATCH] Bug 36084: C4::Auth+plack.psgi for svc? Suggestion to move the CSRF check to CGI->new so that we will check it for every request, and it will cover svc scripts as well (they are not using get_template_and_user). The token will be retrieve from the param list *or the csrf_token header* (do we want to name it x-koha-csrf-token instead?). This will be done for *every* request that are not GET: CSRF token is now required everywhere CGI is used (side-effects possible?). Signed-off-by: Jonathan Druart --- C4/Auth.pm | 34 ++++++++-------------------------- debian/templates/plack.psgi | 35 ++++++++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 31 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index a0a93f15c6..156626e488 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -648,20 +648,9 @@ sub get_template_and_user { $template->param( logged_in_user => $patron ); $template->param( sessionID => $sessionID ); - my $op = $in->{query}->param('op'); - if ( defined $op && $op =~ m{^cud-} ) { - unless ( - Koha::Token->new->check_csrf( - { - session_id => scalar $in->{query}->cookie('CGISESSID'), - token => scalar $in->{query}->param('csrf_token'), - } - ) - ) - { - Koha::Logger->get->debug("The form submission failed (Wrong CSRF token)."); - C4::Output::output_and_exit( $in->{query}, $cookie, $template, 'wrong_csrf_token' ); - } + if ( $in->{query}->param('invalid_csrf_token') ) { + Koha::Logger->get->debug("The form submission failed (Wrong CSRF token)."); + C4::Output::output_and_exit( $in->{query}, $cookie, $template, 'wrong_csrf_token' ); } return ( $template, $borrowernumber, $cookie, $flags ); @@ -1362,23 +1351,16 @@ sub checkauth { my $patron = $userid ? Koha::Patrons->find({ userid => $userid }) : undef; $patron->update_lastseen('login') if $patron; + # FIXME This is only needed for scripts not using plack my $op = $query->param('op'); if ( defined $op && $op =~ m{^cud-} ) { die "Cannot use GET for this request" if $request_method eq 'GET'; + } - unless ( - $skip_csrf_check - || Koha::Token->new->check_csrf( - { - session_id => scalar $query->cookie('CGISESSID'), - token => scalar $query->param('csrf_token'), - } - ) - ) - { - Koha::Exceptions::Token::WrongCSRFToken->throw; - } + + if ( !$skip_csrf_check && $query->param('invalid_csrf_token') ) { + Koha::Exceptions::Token::WrongCSRFToken->throw; } # In case, that this request was a login attempt, we want to prevent that users can repost the opac login diff --git a/debian/templates/plack.psgi b/debian/templates/plack.psgi index eeeeca2b08..f651de493a 100644 --- a/debian/templates/plack.psgi +++ b/debian/templates/plack.psgi @@ -65,13 +65,38 @@ use CGI qw(-utf8 ); # we will loose -utf8 under plack, otherwise my $original_op = $q->param('op'); my $request_method = $q->request_method // q{}; if ( $stateless_methods{$request_method} && defined $original_op && $original_op =~ m{^cud-} ) { - warn "Programming error - op '$original_op' must not start with 'cud-' for $request_method"; + Koha::Logger->get->warn("Programming error - op '$original_op' must not start with 'cud-' for $request_method"); $q->param( 'op', '' ); $q->param( 'debug_programming_error', "'$original_op' must not start with 'cud-' for $request_method" ); - } elsif ( $stateful_methods{$request_method} && defined $q->param('op') && $original_op !~ m{^cud-} ) { - warn "Programming error - op '$original_op' must start with 'cud-' for $request_method"; - $q->param( 'op', '' ); - $q->param( 'debug_programming_error', "'$original_op' must start with 'cud-' for $request_method" ); + } elsif ( $stateful_methods{$request_method} ) { + # Get the CSRF token from the param list or the header + my $csrf_token = $q->param('csrf_token') || $q->http('HTTP_CSRF_TOKEN'); + + if ( defined $q->param('op') && $original_op !~ m{^cud-} ) { + Koha::Logger->get->warn("Programming error - op '$original_op' must start with 'cud-' for $request_method"); + $q->param( 'op', '' ); + $q->param( 'debug_programming_error', "'$original_op' must start with 'cud-' for $request_method" ); + } + + if ( $csrf_token ) { + unless ( + Koha::Token->new->check_csrf( + { + session_id => scalar $q->cookie('CGISESSID'), + token => $csrf_token, + } + ) + ) + { + Koha::Logger->get->debug("The form submission failed (Wrong CSRF token)."); + $q->param( 'op', '' ); + $q->param( 'invalid_csrf_token', 1); + } + } else { + Koha::Logger->get->warn("Programming error - No CSRF token passed for $request_method"); + $q->param( 'op', '' ); + $q->param( 'debug_programming_error', "No CSRF token passed for $request_method" ); + } } return $q; -- 2.39.5