Bug 38446

Summary: Permission error for additional fields
Product: Koha Reporter: Michaela Sieber <michaela.sieber>
Component: ERMAssignee: 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
With a "normal" libarian account (not superlibrarian) it is not possible to see or edit additional fields in erm . 
The error message "Something went wrong: Error: Authorization failure. Missing required permission(s)." appears.

The patron would need the manage_additional_fields permissions, otherwise the API blocks the request.

But the manage_additional_fields permissions should not be necessary as it is for administation.

To recreate:
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
Check that the error message mentioned above appears
Comment 1 Pedro Amorim 2024-11-14 14:35:21 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.
Comment 2 Matt Blenkinsop 2024-11-18 17:20:25 UTC
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
Comment 3 Matt Blenkinsop 2024-11-18 17:21:08 UTC
That way you should be able to access the ERM pages without needing to be given access to any of the authorised values pages
Comment 4 Matt Blenkinsop 2024-11-18 17:34:53 UTC
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
Comment 5 Matt Blenkinsop 2024-11-18 17:35:36 UTC
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
Comment 6 William Lavoie 2025-01-13 19:29:37 UTC
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>
Comment 7 Jonathan Druart 2025-01-21 09:03:49 UTC
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.
Comment 8 Mathieu Saby 2025-03-19 04:53:13 UTC
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)?
Comment 9 Katrin Fischer 2025-06-10 13:39:58 UTC
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
Comment 10 Katrin Fischer 2025-06-10 13:57:20 UTC
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.
Comment 11 Katrin Fischer 2025-06-10 14:13:02 UTC
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>
Comment 12 Katrin Fischer 2025-06-10 14:15:30 UTC
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?
Comment 13 Marcel de Rooy 2025-06-24 13:50:50 UTC
"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