After a user has gone through the installer and is logging in for the first time when they enter the db user credentials it throws an error saying 'Error: You do not have permission to access this page' If you try entering in the db users credentials a second time then you can login successfully and the staff interface will appear. Similarly if once in the staff interface you log out then try logging back in again with the db user then you get the 'Error: You do not have permission to access this page' no matter how many times you try to log in. However if you open the URL in a new tab the staff interface appears.
I've had this occur on 16.11 also.
Created attachment 62423 [details] [review] Bug 18442 - Implemented assignment of the $userid variable before the haspermission call so that a defined $userid with a value is being passed to the haspermission() method. Previously the $userid variable was undefined when it was handed to the haspermission() method and this was causing 'Error: You do not have permission to access this page' errors when logging into the staff interface as database user. However if you viewed the url in a new tab you could view the staff interface. Test plan: 1. Drop and recreate your db 2. Clear memcached 3. Go through the installer (to speed up this test plan install all sample data so you dont have to create libraries, patron categories etc. later) 4. On the installer page login as the database user and notice that it does not work on the first attempt ( you get 'Error: You do not have permission to access this page') 5. Try logging in as database user for a second time and notice you are logged in successfully this time 4. In staff interface create a patron account with superlibrarian permissions 5. Logout of the staff interface 6. Login as database user 7. Notice you cant log in. You get the 'Error:: You do not have permission to access this page' error 8. Try a second attempt and notice you get the same error 9. Open the URL in a new tab and notice the staff interface appears showing that you are logged in 10. log out and log back in as the superlibrarian user you created and notice it works on first login attempt 11. Apply patch 12. Log out and try logging back in as database user and notice that you can login successfully on first attempt 13. Repeat steps 1,2,3 and login as database user and notice the login works on first attempt
This patch does not undo the security bug fixes of bug 18275 (opac-memberentry.pl security vulnerabilities) because all tests in the unit test t/db_dependent/Auth.t run successfully with this patch applied. See test output below: kohadev-koha@kohadevbox:/home/vagrant/kohaclone/t/db_dependent$ prove Auth.t Auth.t .. ok All tests successful. Files=1, Tests=21, 3 wallclock secs ( 0.03 usr 0.00 sys + 2.12 cusr 0.08 csys = 2.23 CPU) Result: PASS kohadev-koha@kohadevbox:/home/vagrant/kohaclone/t/db_dependent$
Created attachment 62448 [details] [review] Bug 18442 - Implemented assignment of the $userid variable before the haspermission call so that a defined $userid with a value is being passed to the haspermission() method. Previously the $userid variable was undefined when it was handed to the haspermission() method and this was causing 'Error: You do not have permission to access this page' errors when logging into the staff interface as database user. However if you viewed the url in a new tab you could view the staff interface. Test plan: 1. Drop and recreate your db 2. Clear memcached 3. Go through the installer (to speed up this test plan install all sample data so you dont have to create libraries, patron categories etc. later) 4. On the installer page login as the database user and notice that it does not work on the first attempt ( you get 'Error: You do not have permission to access this page') 5. Try logging in as database user for a second time and notice you are logged in successfully this time 4. In staff interface create a patron account with superlibrarian permissions 5. Logout of the staff interface 6. Login as database user 7. Notice you cant log in. You get the 'Error:: You do not have permission to access this page' error 8. Try a second attempt and notice you get the same error 9. Open the URL in a new tab and notice the staff interface appears showing that you are logged in 10. log out and log back in as the superlibrarian user you created and notice it works on first login attempt 11. Apply patch 12. Log out and try logging back in as database user and notice that you can login successfully on first attempt 13. Repeat steps 1,2,3 and login as database user and notice the login works on first attempt Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Introduced by bug 18275.
Version should be master?
I had more or less the same fix, but it was: $userid ||= $q_userid if $return == 2; It makes thing safer I *think* because it will only affect behaviours for DB user ($return == 2). On the other hand if does not really make sense because if $return == 2, $userid should not be set. So maybe just $userid ||= $q_userid; Could be enough. And especially it will not erase $userid if one of the checkpw calls return a userid different than the one we passed (possible??). Martin? Tomas? Kyle?
Ah yup would you like me to implement: $userid ||= $q_userid;
(In reply to Alex Buckley from comment #8) > Ah yup would you like me to implement: $userid ||= $q_userid; I'd love only 1 patch and avoid follow-up, not to make git log (more) complicated.
Created attachment 62567 [details] [review] Bug 18442 - Implemented assignment of the $userid variable before the haspermission call so that a defined $userid with a value is being passed to the haspermission() method. Previously the $userid variable was undefined when it was handed to the haspermission() method and this was causing 'Error: You do not have permission to access this page' errors when logging into the staff interface as database user. However if you viewed the url in a new tab you could view the staff interface. Test plan: 1. Drop and recreate your db 2. Clear memcached 3. Go through the installer (to speed up this test plan install all sample data so you dont have to create libraries, patron categories etc. later) 4. On the installer page login as the database user and notice that it does not work on the first attempt ( you get 'Error: You do not have permission to access this page') 5. Try logging in as database user for a second time and notice you are logged in successfully this time 4. In staff interface create a patron account with superlibrarian permissions 5. Logout of the staff interface 6. Login as database user 7. Notice you cant log in. You get the 'Error:: You do not have permission to access this page' error 8. Try a second attempt and notice you get the same error 9. Open the URL in a new tab and notice the staff interface appears showing that you are logged in 10. log out and log back in as the superlibrarian user you created and notice it works on first login attempt 11. Apply patch 12. Log out and try logging back in as database user and notice that you can login successfully on first attempt 13. Repeat steps 1,2,3 and login as database user and notice the login works on first attempt Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
(In reply to Jonathan Druart in comment 9) Hi Jonathan I have added your suggested alteration in my patch so now we just have a single patch attached to this bug report rather than a followup patch. I have run Auth.t again and all tests run successfully as this test output shows: kohadev-koha@kohadevbox:/home/vagrant/kohaclone/t/db_dependent$ prove Auth.t Auth.t .. ok All tests successful. Files=1, Tests=21, 3 wallclock secs ( 0.03 usr 0.00 sys + 2.33 cusr 0.11 csys = 2.47 CPU) Result: PASS kohadev-koha@kohadevbox:/home/vagrant/kohaclone/t/db_dependent$ exit exit
The patch is ok to me. I'd like opinion about $userid ||= $q_userid; vs $userid ||= $q_userid if $return == 2;
Checking the $return value would certainly be the belt and braces approach. I like it.
(In reply to Martin Renvoize in comment 13) Hi Martin So just to confirm you would like to see: $userid ||= $q_userid if $return == 2;
Correct. I'd like to see the `if $return == 2` added. I believe adding that should protect us from any unforeseen down the line action at a distance bugs creeping in.. or at least some ;).
Created attachment 62710 [details] [review] Bug 18442: Implemented $userid ||= $q_userid if $return == 2; in place of $userid ||= $q_userid; Test plan: 1. Drop and recreate your db 2. Clear memcached 3. Go through the installer (to speed up this test plan install all sample data so you dont have to create libraries, patron categories etc. later) 4. On the installer page login as the database user and notice that it does not work on the first attempt ( you get 'Error: You do not have permission to access this page') 5. Try logging in as database user for a second time and notice you are logged in successfully this time 4. In staff interface create a patron account with superlibrarian permissions 5. Logout of the staff interface 6. Login as database user 7. Notice you cant log in. You get the 'Error:: You do not have permission to access this page' error 8. Try a second attempt and notice you get the same error 9. Open the URL in a new tab and notice the staff interface appears showing that you are logged in 10. log out and log back in as the superlibrarian user you created and notice it works on first login attempt 11. Apply patch 12. Log out and try logging back in as database user and notice that you can login successfully on first attempt 13. Repeat steps 1,2,3 and login as database user and notice the login works on first attempt
(In reply to Martin Renvoize in comment 15) Thanks for the feedback Martin
I think the patch looks ok, meaning it makes sense. But we really need a small regression test, to make sure we are actually fixing the issue in the right place and not because of a collateral change of behaviour. And also because that's how we do it. So, in order to help I need to ask: what is the hypothesis for the bug? Answering that will make it easier to write that regression test.
Created attachment 62738 [details] [review] Bug 18442: Fix DB user loggin Test plan: 1. Drop and recreate your db 2. Clear memcached 3. Go through the installer (to speed up this test plan install all sample data so you dont have to create libraries, patron categories etc. later) 4. On the installer page login as the database user and notice that it does not work on the first attempt ( you get 'Error: You do not have permission to access this page') 5. Try logging in as database user for a second time and notice you are logged in successfully this time 4. In staff interface create a patron account with superlibrarian permissions 5. Logout of the staff interface 6. Login as database user 7. Notice you cant log in. You get the 'Error:: You do not have permission to access this page' error 8. Try a second attempt and notice you get the same error 9. Open the URL in a new tab and notice the staff interface appears showing that you are logged in 10. log out and log back in as the superlibrarian user you created and notice it works on first login attempt 11. Apply patch 12. Log out and try logging back in as database user and notice that you can login successfully on first attempt 13. Repeat steps 1,2,3 and login as database user and notice the login works on first attempt Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 62739 [details] [review] Bug 18442: Add a test Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 62826 [details] [review] Bug 18442: Fix DB user loggin Test plan: 1. Drop and recreate your db 2. Clear memcached 3. Go through the installer (to speed up this test plan install all sample data so you dont have to create libraries, patron categories etc. later) 4. On the installer page login as the database user and notice that it does not work on the first attempt ( you get 'Error: You do not have permission to access this page') 5. Try logging in as database user for a second time and notice you are logged in successfully this time 4. In staff interface create a patron account with superlibrarian permissions 5. Logout of the staff interface 6. Login as database user 7. Notice you cant log in. You get the 'Error:: You do not have permission to access this page' error 8. Try a second attempt and notice you get the same error 9. Open the URL in a new tab and notice the staff interface appears showing that you are logged in 10. log out and log back in as the superlibrarian user you created and notice it works on first login attempt 11. Apply patch 12. Log out and try logging back in as database user and notice that you can login successfully on first attempt 13. Repeat steps 1,2,3 and login as database user and notice the login works on first attempt Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 62827 [details] [review] Bug 18442: Fix DB user loggin Test plan: 1. Drop and recreate your db 2. Clear memcached 3. Go through the installer (to speed up this test plan install all sample data so you dont have to create libraries, patron categories etc. later) 4. On the installer page login as the database user and notice that it does not work on the first attempt ( you get 'Error: You do not have permission to access this page') 5. Try logging in as database user for a second time and notice you are logged in successfully this time 4. In staff interface create a patron account with superlibrarian permissions 5. Logout of the staff interface 6. Login as database user 7. Notice you cant log in. You get the 'Error:: You do not have permission to access this page' error 8. Try a second attempt and notice you get the same error 9. Open the URL in a new tab and notice the staff interface appears showing that you are logged in 10. log out and log back in as the superlibrarian user you created and notice it works on first login attempt 11. Apply patch 12. Log out and try logging back in as database user and notice that you can login successfully on first attempt 13. Repeat steps 1,2,3 and login as database user and notice the login works on first attempt Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 62828 [details] [review] Bug 18442: Add a test Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Looks solid to me, thanks for adding the test too.
Pushed to master for 17.05, thanks Alex, Jonathan!
These patches have been pushed to 16.11.x and will be in 16.11.08.
Pushed to 3.22.x for 3.22.21
Pushed to 16.05.x, for 16.05.13 release