If you login to staff interface with a user that has just 'catalogue' permission and nothing else you can view all the patrons' images, this should not be possible you should need to have the 'edit_borrowers' permission to do that. To reproduce: 1. Enable "patronimages" syspref 2. Upload first a photo to patron with borrower number XXX 3. With the user with just 'catalogue' permission go to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=XXX
I am getting "Error: You do not have permission to access this page." members/moremember.pl requires { borrowers => 'edit_borrowers' }
And I am getting a 403 on /cgi-bin/koha/members/patronimage.pl?borrowernumber=51
(In reply to Jonathan Druart from comment #2) > And I am getting a 403 on > /cgi-bin/koha/members/patronimage.pl?borrowernumber=51 Wrong, I am seeing the image.
I was expecting the following change to work, but it does not. diff --git a/members/patronimage.pl b/members/patronimage.pl index da62b875536..c4a6e5eb401 100755 --- a/members/patronimage.pl +++ b/members/patronimage.pl @@ -47,7 +47,7 @@ This script, when called from within HTML and passed a valid patron borrowernumb =cut -my ($status, $cookie, $sessionID) = check_api_auth($query, { catalogue => 1 } ); +my ($status, $cookie, $sessionID) = check_api_auth($query, [ { borrowers => '*' }, { circulate => '*' } ] ); unless ( $status eq 'ok' ) { print $query->header(-type => 'text/plain', -status => '403 Forbidden');
Created attachment 127932 [details] [review] Bug 29541: Restrict access to patron's image to borrowers => * and circulate => * The patron images is displayed on the 'circulation' and 'members' modules.
(In reply to Jonathan Druart from comment #4) > I was expecting the following change to work, but it does not. > > diff --git a/members/patronimage.pl b/members/patronimage.pl > index da62b875536..c4a6e5eb401 100755 > --- a/members/patronimage.pl > +++ b/members/patronimage.pl > @@ -47,7 +47,7 @@ This script, when called from within HTML and passed a > valid patron borrowernumb > > =cut > > -my ($status, $cookie, $sessionID) = check_api_auth($query, { catalogue => 1 > } ); > +my ($status, $cookie, $sessionID) = check_api_auth($query, [ { borrowers => > '*' }, { circulate => '*' } ] ); > > unless ( $status eq 'ok' ) { > print $query->header(-type => 'text/plain', -status => '403 Forbidden'); It actually works. I have been logged out at some point and haven't noticed...
borrowers => * and circulate => * would that works?
I am not sure if borrowers * is correct. I think if you are not allowed to see another library's patron data, you should also not be able to access the patron images for these patrons - so we probably need to add library groups into the equation here?
Created attachment 128276 [details] [review] Bug 29541: Prevent users from another group to access patron's images We should respect group restrictions here. Test plan: Create a patron from another group of libraries and don't let them access info from patrons outside of this group. Access the following link and confirm that you can see the image only for patrons from their group /cgi-bin/koha/members/patronimage.pl?borrowernumber=XX
Created attachment 128801 [details] [review] Bug 29541: Restrict access to patron's image to borrowers => * and circulate => * The patron images is displayed on the 'circulation' and 'members' modules. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 128802 [details] [review] Bug 29541: Prevent users from another group to access patron's images We should respect group restrictions here. Test plan: Create a patron from another group of libraries and don't let them access info from patrons outside of this group. Access the following link and confirm that you can see the image only for patrons from their group /cgi-bin/koha/members/patronimage.pl?borrowernumber=XX Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 129333 [details] [review] Bug 29541: Restrict access to patron's image to borrowers => * and circulate => * The patron images is displayed on the 'circulation' and 'members' modules. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 129334 [details] [review] Bug 29541: Prevent users from another group to access patron's images We should respect group restrictions here. Test plan: Create a patron from another group of libraries and don't let them access info from patrons outside of this group. Access the following link and confirm that you can see the image only for patrons from their group /cgi-bin/koha/members/patronimage.pl?borrowernumber=XX Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Backported: Pushed to 20.11.x branch for 20.11.14 Testing notes: for the 2nd patch the permission for the observing patron is the subpermission "Add, modify and view patron information (edit_borrowers)" and without the next one (view_borrower_infos_from_any_libraries) Then create a group with 2 libraries, confirm that it's possible to see another patron's (from another library) page and image URL. But when removing the library from the group, it's not possible to see the patron's page anymore. Without the patch, the image URL still works! And the patch fixes that.
*20.11.x-security branch
Backported to 19.11.x-security branch on the Security repo for 19.11.25 security release.
Pushed to security 21.05.x for 21.05.09