From 5b82d6147729daa4f729667908d34a06e1a64cdf Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 20 Feb 2024 11:03:37 +0100 Subject: [PATCH] Bug 36102: (follow-up) Add cud-login to the login form Hum this didn't make sense. We are not checking credentials after checkauth. This patch is suggesting to rename "userid" and "password" parameters from login forms to "login_userid" and "login_password" to not interfere with other parameters with the same name. This looks quite correct, however I am seeing "The form submission failed (Wrong CSRF token)." in the log after a successful login. Which feels wrong, what's happening? Signed-off-by: Jonathan Druart --- C4/Auth.pm | 34 +++++++++---------- C4/InstallAuth.pm | 23 +++++-------- .../intranet-tmpl/prog/en/modules/auth.tt | 4 +-- .../prog/en/modules/installer/auth.tt | 4 +-- .../bootstrap/en/includes/masthead.inc | 4 +-- .../bootstrap/en/modules/opac-auth.tt | 4 +-- .../bootstrap/en/modules/opac-main.tt | 4 +-- 7 files changed, 36 insertions(+), 41 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 5ab3249900..a0a93f15c6 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -183,6 +183,14 @@ sub get_template_and_user { $in->{'query'}, ); + my $request_method = $in->{query}->request_method // q{}; + unless ( $request_method eq 'POST' && $in->{query}->param('op') eq 'cud-login' ) { + for my $v ( qw( login_userid login_password ) ) { + $in->{query}->param($v, '') + if $in->{query}->param($v); + } + } + if ( $in->{'template_name'} !~ m/maintenance/ ) { ( $user, $cookie, $sessionID, $flags ) = checkauth( $in->{'query'}, @@ -194,16 +202,8 @@ sub get_template_and_user { { skip_csrf_check => 1 }, ); } - my $session = get_session($sessionID); - # We have just logged in - # If we are not coming from the login form we empty the credential to reject the access - if ( !$session && $user ) { - if ( $in->{query}->param('op') ne 'cud-login' ) { - $in->{query}->param('userid', ''); - $in->{query}->param('password', ''); - } - } + my $session = get_session($sessionID); # If we enforce GDPR and the user did not consent, redirect # Exceptions for consent page itself and SCI/SCO system @@ -861,7 +861,7 @@ sub checkauth { # This parameter is the name of the CAS server we want to authenticate against, # when using authentication against multiple CAS servers, as configured in Auth_cas_servers.yaml my $casparam = $query->param('cas'); - my $q_userid = $query->param('userid') // ''; + my $q_userid = $query->param('login_userid') // ''; my $session; my $invalid_otp_token; @@ -1047,7 +1047,7 @@ sub checkauth { || $pki_field ne 'None' || $emailaddress ) { - my $password = $query->param('password'); + my $password = $query->param('login_password'); my $shibSuccess = 0; my ( $return, $cardnumber ); @@ -1384,10 +1384,10 @@ sub checkauth { # In case, that this request was a login attempt, we want to prevent that users can repost the opac login # request. We therefore redirect the user to the requested page again without the login parameters. # See Post/Redirect/Get (PRG) design pattern: https://en.wikipedia.org/wiki/Post/Redirect/Get - if ( $type eq "opac" && $query->param('koha_login_context') && $query->param('koha_login_context') ne 'sco' && $query->param('password') && $query->param('userid') ) { + if ( $type eq "opac" && $query->param('koha_login_context') && $query->param('koha_login_context') ne 'sco' && $query->param('login_password') && $query->param('login_userid') ) { my $uri = URI->new($query->url(-relative=>1, -query_string=>1)); - $uri->query_param_delete('userid'); - $uri->query_param_delete('password'); + $uri->query_param_delete('login_userid'); + $uri->query_param_delete('login_password'); $uri->query_param_delete('koha_login_context'); $uri->query_param_delete('op'); $uri->query_param_delete('csrf_token'); @@ -1410,7 +1410,7 @@ sub checkauth { # get the inputs from the incoming query my @inputs = (); - my @inputs_to_clean = qw( userid password ticket logout.x otp_token ); + my @inputs_to_clean = qw( login_userid login_password ticket logout.x otp_token ); foreach my $name ( param $query) { next if grep { $name eq $_ } @inputs_to_clean; my @value = $query->multi_param($name); @@ -1644,8 +1644,8 @@ sub check_api_auth { } else { # new login - my $userid = $query->param('userid'); - my $password = $query->param('password'); + my $userid = $query->param('login_userid'); + my $password = $query->param('login_password'); my ( $return, $cardnumber, $cas_ticket ); # Proxy CAS auth diff --git a/C4/InstallAuth.pm b/C4/InstallAuth.pm index 9a92b3837e..6730e94383 100644 --- a/C4/InstallAuth.pm +++ b/C4/InstallAuth.pm @@ -111,7 +111,13 @@ sub get_template_and_user { my $filename = "$path/modules/" . $tmplbase; my $interface = 'intranet'; my $template = C4::Templates->new( $interface, $filename, $tmplbase, $query); - + + my $request_method = $in->{query}->request_method // q{}; + unless ( $request_method eq 'POST' && $in->{query}->param('op') eq 'cud-login' ) { + $in->{query}->param('login_userid', ''); + $in->{query}->param('login_password', '') + } + my ( $user, $cookie, $sessionID, $flags ) = checkauth( $in->{'query'}, $in->{'authnotrequired'}, @@ -121,17 +127,6 @@ sub get_template_and_user { my $session = Koha::Session->get_session( { sessionID => $sessionID, storage_method => 'file' } ); - # We have just logged in - # If we are not coming from the login form we empty the credential to reject the access - if ( !$session && $user ) { - if ( $in->{query}->param('op') ne 'cud-login' ) { - $in->{query}->param('userid', ''); - $in->{query}->param('password', ''); - } - } - - # use Data::Dumper;warn "utilisateur $user cookie : ".Dumper($cookie); - my $borrowernumber; if ($user) { $template->param( loggedinusername => $user ); @@ -295,9 +290,9 @@ sub checkauth { unless ($userid) { my $session = Koha::Session->get_session( { sessionID => $sessionID, storage_method => 'file' } ); $sessionID = $session->id; - $userid = $query->param('userid'); + $userid = $query->param('login_userid'); C4::Context->_new_userenv($sessionID); - my $password = $query->param('password'); + my $password = $query->param('login_password'); C4::Context->_new_userenv($sessionID); my ( $return, $cardnumber ) = checkpw( $userid, $password ); if ($return) { diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt index 1d72bbda88..9f83efe32c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt @@ -132,10 +132,10 @@ [% END %]

- +

- +

[% UNLESS IndependentBranches %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/installer/auth.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/installer/auth.tt index d99550848b..6674b06f21 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/installer/auth.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/installer/auth.tt @@ -63,11 +63,11 @@
- +
- +
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc index 3ee5421e32..a9d0f1a4e9 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc @@ -431,8 +431,8 @@