The web installer login template has markup for handling the same kinds of errors the regular login template does (nopermission, timed_out, different_ip, wrongip, invalid_username_or_password) but the script doesn't seem to be written to handle those errors. For instance: Submitting the wrong username and password doesn't result in an error message. The page simply reloads.
Created attachment 61768 [details] [review] Bug 12930 - Implemented error message display for invalid username or password on web installer login Now InstallAuth.pm hands the invalid_username_or_password variable to the template file auth.tt if the user entered an invalid username or password and this is displayed to the user
The patch I have attached displays an error message if the user writes in an invalid username and/or password. The InstallAuth.pm module does not contain subroutines to check for permissions, session timeout, or if the ip address is different even though the markup checking for these in the auth.tt has been implemented. Regarding the nopermission check, as I understand it the only password that is valid to be written into this installer login is the database admin credentials and so I don't see why we need the nopermission check in the markup (as is required for the staff interface login). Test plan: 1. Restart memcached: sudo service memcached restart 2. Drop and recreate the database 3. Login with these pretend credentials: username: fred password: password 4. Notice the page refreshes and you get no "invalid username or password" error displayed 5. Apply the patch attached to this bug 6. Try logging in with the same username and password specified in step 3 7. Notice the page refreshes and a "invalid username or password' error message is displayed 8. Now login with the Koha database administrator account and notice you are logged in successfully
Hi Alex I do not understand the empty else branch in https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61768&action=diff#a/C4/InstallAuth.pm_sec5 Marc
Created attachment 61777 [details] [review] Bug 12930 - Followup removing empty else branch
Thanks for noticing that Marc, I forgot to remove it. The followup patch I have just attached removes it
Created attachment 61807 [details] [review] Bug 12930 - Implemented error message display for invalid username or password on web installer login Now InstallAuth.pm hands the invalid_username_or_password variable to the template file auth.tt if the user entered an invalid username or password and this is displayed to the user I tested both patches together by direkctly calling [MY SERVER]/cgi-bin/koha/installer/install.pl?step=1&op=updatestructure Works as expected Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 61808 [details] [review] Bug 12930 - Followup removing empty else branch Signed-off-by: Marc Véron <veron@veron.ch>
Alex, it seems that the only changes you made are: + if ($info{'invalid_username_or_password'} == 1) { + $template->param( 'invalid_username_or_password' => $info{'invalid_username_or_password'}); + } If it is right I would prefer a single patch with only the needed changes instead of indentation changes and modification of the return value of checkpw.
Created attachment 62116 [details] [review] Bug 12930 - Implemented invalid_username_or_password template variable being handed to Auth.tt. Removed indentation changes and modification to return value of checkpw
(in response to Jonathan Druart in comment 8) I have implemented a single patch meeting the requests you asked in comment 8. The comment 2 test plan is still applicable.
Created attachment 62117 [details] [review] Bug 12930: Web installer does not show login errors Implemented invalid_username_or_password template variable being handed to Auth.tt. Removed indentation changes and modification to return value of checkpw Test plan: See comment #2 Alternative test plan: - Clear session cookies or close / open your browser - Go to [MY SERVER]/cgi-bin/koha/installer/install.pl - Verify that message appears if wrong username/pw provided - Verify that you can log in with databas administrator account Tested with alternative test plan, works as expected. Title and description in commit message adapted. Signed-off-by: Marc Véron <veron@veron.ch>
Changed title and text of commit message, obsoleted previous patch, signed off.
Created attachment 62166 [details] [review] Bug 12930: Web installer does not show login errors Implemented invalid_username_or_password template variable being handed to Auth.tt. Removed indentation changes and modification to return value of checkpw Test plan: See comment #2 Alternative test plan: - Clear session cookies or close / open your browser - Go to [MY SERVER]/cgi-bin/koha/installer/install.pl - Verify that message appears if wrong username/pw provided - Verify that you can log in with databas administrator account Tested with alternative test plan, works as expected. Title and description in commit message adapted. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 17.05, thanks Alex!
This patch has been pushed to 16.11.x and will be in 16.11.07.
Pushed to 3.22.x for 3.22.20
Pushed to 16.05.x, for 16.05.12 release