Currently, Koha lacks a subpermission feature that allows libraries, especially those in correctional facilities utilizing inmate volunteers, to hide patron contact information (e.g., addresses) from certain users like volunteers. This privacy gap poses a significant risk, potentially exposing sensitive inmate data to unauthorized personnel. Implementing a subpermission feature would ensure that only authorized staff can access such sensitive information, addressing critical privacy concerns within these specialized library environments.
Created attachment 168282 [details] [review] Bug 37144 (still wip)
Created attachment 168283 [details] [review] Bug 37144 (still wip)
Created attachment 168284 [details] [review] Bug 37144 - Add a subpermission for hiding patron contact information
Created attachment 168387 [details] [review] Bug 37144: Hide address from patron lists
Created attachment 168388 [details] [review] Bug 37144 - Add a subpermission for hiding patron contact information To test: Apply the patch, updatedatabase, restart_all. 1. Create or ensure you have a patron (patron A) with all the contact information fields filled out (i.e. phone number, email, alternate contact information etc.) 2. Create another patron (patron B) with the following permissions: 1. Check out and check in items (circulate) 2. Staff access, allows viewing of catalogue in staff interface (catalogue) 3. Under Add, modify and view patron information 1. Search, list and view patrons (list_borrowers) 2. View patron infos from any libraries. If not set the logged in user could only access patron infos from its own library or group of libraries. (view_borrower_infos_from_any_libraries) 3. For now keep View patron contact information. (view_contact_information) unselected 4. Place and modify holds for patrons (reserveforothers) 3. Log in as patron B. 4. Confirm that patron contact information is not visible 1. View patron A 1. If you’re using autocomplete ensure that the address is not visible on the autocomplete dropdown. 2. If you’re using the Patron alphabet search to view the patron make sure that the address is not visible on the member result table. 3. Contact information cannot be viewed under the “brief information” on the right hand side of the patron account, below the patron image. 2. Holds 1. Place a hold for Patron A 1. Confirm that the address is not visible on the autocomplete when searching for a patron to place a hold for. 2. Build the holds queue 1. View the holds queue the patron phone number is not visible 2. View the holds waiting page. Contact information is not displayed either. Nor for ‘Holds waiting past their expiration date’ or ‘Holds with cancellation requests’. 3. Place a hold in transit. 1. The patron contact information is not visible for in-transit items, nor when they are recalled, or canceled. 4. Poke around to make sure that addresses are not visible anywhere else. 5. Log in with a user that has superlibrarian permissions. Make sure that addresses are visible in all the locations mentioned above, and of course in all their other regular locations. 6. Sign off and have a great day! :D
Created attachment 168389 [details] [review] Bug 37144 - Hide address on circ menu
Created attachment 168390 [details] [review] Bug 37144 - Add js variable
Created attachment 168391 [details] [review] Bug 37144 - Fix typo
Created attachment 168392 [details] [review] Bug 37144: Pass permission variable to the patron autocomplete
Created attachment 168393 [details] [review] Bug 37144: Fixed up the permission
Created attachment 168394 [details] [review] Bug 37144: Hide address from patron lists
After applying, I don't see the new subpermission and I'm not seeing any atomic update files attached.
Created attachment 168460 [details] [review] Bug 37144: Atomic update
Created attachment 168463 [details] [review] Bug 37144: Fixed circ.inc
Applying: Bug 37144: Fixed up the permission error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc). error: could not build fake ancestor Patch failed at 0001 Bug 37144: Fixed up the permission hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-37144-Fixed-up-the-permission-t9wm3ly_.patch
Created attachment 169320 [details] [review] Bug 37144: "Adding userpermissions" Same test plan
Created attachment 169321 [details] [review] Bug 37144: Add permission to circ menu
Created attachment 169322 [details] [review] Bug 37144: Added permission to patron search
Created attachment 169323 [details] [review] Bug 37144: Added the permission to Permissions.inc
Created attachment 169324 [details] [review] Bug 37144: Added variable to members menu.ic
Created attachment 169325 [details] [review] Bug 37144: Added permission to waiting holds
Created attachment 169326 [details] [review] Bug 37144: Added variable to circulation.tt
Created attachment 169327 [details] [review] Bug 37144: Added permission to circ returns
Created attachment 169328 [details] [review] Bug 37144: Added permission to holds queue
Created attachment 169329 [details] [review] Bug 37144: Added permission to member brief view`
Created attachment 169330 [details] [review] Bug 37144: Added variable and permission to moremember.tt
Created attachment 169331 [details] [review] Bug 37144: Added global js variable`
Created attachment 169332 [details] [review] Bug 37144: Passed permission through the autocomplete
Created attachment 169333 [details] [review] Bug 37144: Hide address in patron lists
Created attachment 169334 [details] [review] Bug 37144: Atomic update
I'm really interested in this use case, and I'll likely be working on this more in the future. I haven't reviewed these in depth, but at a glance it looks like you've added a new sub permission, but it's not clear to me how it relates to the other permissions. Like... it seems at odds with "list_borrowers" for instance. Can you tell us more about how you'd plan to use this new permission in practice?
Hi David, Thank you for your feedback. I understand your concern about how the new subpermission fits with existing permissions such as "list_borrowers." Here’s a more detailed explanation of how the new subpermission would work in practice and its relationship to current permissions: Currently, Koha does not offer granular control over sensitive patron information. This gap is particularly critical in environments such as correctional facilities, where there is a need to ensure that sensitive inmate information (like addresses) is not accessible to unauthorized users, such as volunteers who might assist with library operations but should not have access to personal data. The new subpermission I’m suggesting would be a refinement to control access specifically to sensitive patron information within the broader "list_borrowers" permission. Here's how it would function: 1. Granular Access Control: * "list_borrowers" Permission: Allows a user to see a list of all patrons in the system, including their basic information. * New Subpermission: Would be an additional layer that allows or restricts access to sensitive details (e.g., addresses, phone numbers) on a per-user basis. 2. Use Case Example: * Library Staff with Full Access: Users with full access (including the new subpermission) would be able to view and manage all patron details, including sensitive information. * Volunteers with Limited Access: Volunteers or other non-staff users would have the "list_borrowers" permission but would be restricted by the new subpermission from seeing sensitive information like addresses. 3. Implementation: * Separate Controls: The subpermission would be a toggle or separate setting within the permissions system, providing a more nuanced control over which users can see specific types of patron information. * Access Scenarios: For instance, a volunteer might need to see the list of patrons to check out books but should not be able to see the home addresses of the patrons. Practical Benefits: * Enhanced Privacy: Protects sensitive information from being exposed to unauthorized users. * Compliance: Helps in meeting privacy requirements and regulations specific to sensitive environments like correctional facilities. * Flexibility: Provides libraries with the flexibility to tailor access controls to their specific needs and user roles. In summary, the new subpermission would integrate with existing permissions by adding a layer of control over access to sensitive data, thus bridging the privacy gap without undermining the functionality of broader permissions like "list_borrowers." I hope this clarifies how the new subpermission complements and enhances the existing permission structure. Please let me know if you need further details or have additional questions.
(In reply to Laura Escamilla from comment #32) > In summary, the new subpermission would integrate with existing permissions > by adding a layer of control over access to sensitive data, thus bridging > the privacy gap without undermining the functionality of broader permissions > like "list_borrowers." > > I hope this clarifies how the new subpermission complements and enhances the > existing permission structure. Please let me know if you need further > details or have additional questions. Thanks for that, Laura. I think that I understand what you're saying. In other words, anyone currently with "list_borrowers" (also "edit_borrowers" for legacy reasons although we didn't give all "edit_borrowers" the "list_borrowers" permission when it came out as "edit_borrowers" could still do everything "list_borrowers" could do) would also need "view_contact_information" in order to continue seeing the information they're currently seeing. At a glance, I don't see a patch attached that performs that update. Without it, I think a lot of Koha staff users would become very unhappy in prod. In some ways, it's a shame this isn't implemented via Koha::Patron. If it were, it could be unit tested. (For instance, Koha::Patron could have a method that restricts the display of certain information, and ./members/moremember.pl could invoke that method based on the permission that the session user has.) That actually reminds me... I thought Tomas and Martin previously were working on something like that, but I can't remember the details now. With it as a template change, we should probably have some Selenium tests. Actually Selenium tests would be good in either case, since we're talking about restricting sensitive information.
Actually, that reminds me that this change doesn't take into account the REST API either, since it's just hiding information in the templates. A clever user could easily bypass this permission to view the contact information.
I think that the feature is a good idea, and would be very useful, but I think we might need to still do a bit more thinking on it. I don't think this change is all the way there yet.
I wanted to double-check things so I applied the patches, and it looks like there's at least one coding error. http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 staff-global_24.0600021.js:712 Uncaught SyntaxError: Unexpected token ')' (at staff-global_24.0600021.js:712:13)
And by visiting http://localhost:8081/api/v1/patrons/51 I bypass this change and get all the patron information.
(In reply to David Cook from comment #33) > That actually reminds me... I thought Tomas and > Martin previously were working on something like that, but I can't remember > the details now. I think it was bug 29523. However, that only covers the REST API. The REST API is arguably easier to deal with. If you added a patch to take care of the REST API... then these template based patches might be enough for now...
I have some concerns here as well: What is considered sensitive information might vary from library to library. Here it's listed as contact information. But I think it should be more clearly defined and describe. The other thing is that we now already have a lot of template changes and given the sensitivity of this feature I am worried about maintaining this and avoiding regressions. Having an option to filter out the information so it is never sent to the template in the first place would probably be much more elegant. I think that's what David meant by resolving this on the level of Patron.pm. With our mix of old and new code (C4/Koha) still not an easy task.
(In reply to Katrin Fischer from comment #39) > What is considered sensitive information might vary from library to library. > Here it's listed as contact information. But I think it should be more > clearly defined and describe. I was pondering that as well. For instance, many of our libraries would want to hide date of birth as well I think. > The other thing is that we now already have a lot of template changes and > given the sensitivity of this feature I am worried about maintaining this > and avoiding regressions. Having an option to filter out the information so > it is never sent to the template in the first place would probably be much > more elegant. I think that's what David meant by resolving this on the level > of Patron.pm. Yep!
> With our mix of old and new code (C4/Koha) still not an easy task. It is a tough one. I have a similar use case to Laura, which I'll likely need to code and deploy soon. I suspect I'll have to do something like this in the short-term, since it's the easier/quicker change to make. But I think we do need to put more thought into a design for a comprehensive solution. We need some way to associate the $logged_in_user with the Koha::Patron object, but there's lots of different ways of getting a Koha::Patron object even. On other projects with better MVC structures and smaller codebases, I've done some column-level security, although it was mostly in terms of restricting updates at the column-level. Using Koha::Proxy::Patron class to wrap Koha::Patron could work... except that there are ways of getting Koha::Patron objects from other objects which could bypass that. But maybe it would be enough... Actually, looking through './circ/circulation.pl' I'm thinking how we mostly want to use an unrestricted Koha::Patron for a lot of the business logic... but then for some things we'd want to use a Koha::Proxy::Patron with user restrictions for view logic... This is one of those topics where I think it's difficult to come up with a perfect design ahead of time... but where it's also easy to create a mess by having an incomplete design ahead of time...
Conceptually, I was just thinking about "patron_profile" rather than "patron", so instead of Koha::Proxy::Patron we'd have "Koha::Patron::Profile or something like that. Although I was thinking about that in terms of a whole separate object, which looking at this... would be easier said than done...