Summary: | Permission error for additional fields | ||
---|---|---|---|
Product: | Koha | Reporter: | Michaela Sieber <michaela.sieber> |
Component: | ERM | Assignee: | Matt Blenkinsop <matt.blenkinsop> |
Status: | In Discussion --- | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P5 - low | CC: | clemens.tubach, jonathan.druart, jonathan.field, katrin.fischer, m.de.rooy, martin.renvoize, mathsabypro, matt.blenkinsop, pedro.amorim, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Bug Depends on: | 35287 | ||
Bug Blocks: | 38262 | ||
Attachments: |
Bug 38446: Allow those with ERM permissions to access extended attributes
Bug 38446: Allow those with ERM permissions to access extended attributes Bug 38446: Allow those with ERM permissions to access extended attributes |
Description
Michaela Sieber
2024-11-14 14:20:30 UTC
Adding Tomas here to the discussion. In extended_attributes_types, we have: x-koha-authorization: permissions: parameters: manage_additional_fields But staff members editing any of the additional fields supported resources e.g. ERM licenses may not have 'manage_additional_fields'. For Vue additional fields page (such as ERM licenses), this API endpoint needs to be retrieved in order to show configured additional fields on the form. To fix this for posterity, should we just have 'catalogue: "1"' for this endpoint? The UI page to manage additional fields should still be checking for manage_additional_fields anyway. If we want to be more granular with it we could make it an OR check on the permissions and just add the ERM permission. I think something like the below would work: x-koha-authorization: permissions: - parameters: - manage_additional_fields - erm: 1 That way you should be able to access the ERM pages without needing to be given access to any of the authorised values pages Created attachment 174758 [details] [review] Bug 38446: Allow those with ERM permissions to access extended attributes Test plan: 1. Log in as superlibrarian 2. Activate ERM 3. Add a new additional field for Licences under cgi-bin/koha/admin/additional-fields.pl 4. Go to ERM Module 5. Add a Licence and use the new additional field 6. Search for a user, for example borrowernumber=17 7. Give permissions for erm , acquisition and catalogue (staff access) 8. Log in with this account 9. Go to ERM Module 10. Try to edit the licence with the additional field you have created in step 5 11. Apply patch and run yarn api:bundle and then restart_all 12. Repeat steps 9 and 10 13. The permissions error should vanish Patch for discussion based on what I was planning for the vendors migration. If this is too granular then I think we should go with Pedro's suggestion and open up extended attributes more widely Created attachment 176471 [details] [review] Bug 38446: Allow those with ERM permissions to access extended attributes Test plan: 1. Log in as superlibrarian 2. Activate ERM 3. Add a new additional field for Licences under cgi-bin/koha/admin/additional-fields.pl 4. Go to ERM Module 5. Add a Licence and use the new additional field 6. Search for a user, for example borrowernumber=17 7. Give permissions for erm , acquisition and catalogue (staff access) 8. Log in with this account 9. Go to ERM Module 10. Try to edit the licence with the additional field you have created in step 5 11. Apply patch and run yarn api:bundle and then restart_all 12. Repeat steps 9 and 10 13. The permissions error should vanish Signed-off-by: William Lavoie <william.lavoie@inLibro.com> I don't this this is correct. We need a separate route to retrieve the ERM's attributes. Or should it be in /erm/config? IMO we don't want to give access to the attributes of other modules if the permissions is not set. I'm not sure I understand your exchanges. Why would you need a "manage" permission to view something in Koha (and not to manage it)? I have updated the severity as this prohibits libraries from using the additional fields in the ERM module in stable releases down to 24.11 at least. I believe we should go for the simple fix for backporting and work out a better solution for the future on a new bug. I'll test the attached patch and sign-off if it works. ---- For the permissions: manage_additional_fields Manage additional fields (requires the corresponding permission, one of edit_subscription, order_manage, edit_invoices, or remaining_permissions under updatecharges This permission is a sub permission to parameters and should only be applied when it's about changing the additional fields configuration, not for using them in the context of the modules they appear in (storing, adding, deleting data). I believe this might also re-appear as an issue when we are looking into additional fields for the vendors and every other place that is Vue/API based? Thinking of: Bug 38262 - Add additional fields to Vendors Looking deeper into this I understand Joubu's concern about the API a bit better, but this seems to only involve seeing the configuration of additional fields in other modules, not the data itself? I believe the data would be the sensitive bit. If so it seems forgiveable for a fix. This has been reported in November... and is a real bad blocker. Created attachment 183133 [details] [review] Bug 38446: Allow those with ERM permissions to access extended attributes Test plan: 1. Log in as superlibrarian 2. Activate ERM 3. Add a new additional field for Licences under cgi-bin/koha/admin/additional-fields.pl 4. Go to ERM Module 5. Add a Licence and use the new additional field 6. Search for a user, for example borrowernumber=17 7. Give permissions for erm , acquisition and catalogue (staff access) 8. Log in with this account 9. Go to ERM Module 10. Try to edit the licence with the additional field you have created in step 5 11. Apply patch and run yarn api:bundle and then restart_all 12. Repeat steps 9 and 10 13. The permissions error should vanish Signed-off-by: William Lavoie <william.lavoie@inLibro.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> If I understand correctly we'd need a way to filter the API for the different modules, so: - only expose ERM related additional field configuration if user has erm permissions - only expose acq related additional field configuration if user has acq permission Etc. For me it's hard to tell how we can do that with the API, but it seems a new concept that might need some more thinking? "Allow this until we solve the issue" sounds like something we did before? But are most of these issues solved yet? :) Giving access to the area of extended attribs only because you have ERM access, sounds like bad design. Shouldnt you add the extended attribs perms for those ERM users in a dbrev or so (release warning)? Are the extended attribs perms not granular enough? Should we add a level there? Discussion Time |