Summary: | Users with pretty basic staff interface permissions can see/add/remove API keys of any other user | ||
---|---|---|---|
Product: | Koha | Reporter: | Peter Vashchuk <stalkernoid> |
Component: | Architecture, internals, and plumbing | Assignee: | Peter Vashchuk <stalkernoid> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P5 - low | CC: | 1joynelson, bastien.gonckel, corinne.hayet, fridolin.somers, jonathan.druart, lucas, martin.renvoize, nick, nugged, tomascohen, victor, ztajoli |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28772 | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: |
21.11.00,21.05.04, 20.11.10, 20.05.16, 19.11.22
|
Circulation function: | |
Attachments: |
Bug 28759: limit accessibility for "Manage API keys"
Bug 28759: limit accessibility for "Manage API keys" Bug 28759: limit accessibility for "Manage API keys" |
Description
Peter Vashchuk
2021-07-26 11:48:00 UTC
As it has no workaround, the proposed solution is: limit accessibility for "Manage API keys" section only to superlibrarian and the owner of the API key account. Another open issue is that if user has permissions "Create SQL reports (create_reports)" user can create report that dumps API keys from the db with custom SQL report like "SELECT * FROM `api_keys`" (also reports allow to see hashed passwords but it's probably well known thing already) Even though it leads to the similar problem being caused by a different issue, it's better to create another bug for it. As it has no workaround, the proposed solution is: limit accessibility for "Manage API keys" section only to superlibrarian and the owner of the API key account. Another open issue is that if user has permissions "Create SQL reports (create_reports)" user can create report that dumps API keys from the db with custom SQL report like "SELECT * FROM `api_keys`" (also reports allow to see hashed passwords but it's probably well known thing already) Even though it leads to the similar problem being caused by a different issue, it has a workaround and it's better to create another ticket for it. (In reply to Peter Vashchuk from comment #2) > As it has no workaround, the proposed solution is: limit accessibility for > "Manage API keys" section only to superlibrarian and the owner of the API > key account. Yes, agreed! I don't think we need a separate permission. > Another open issue is that if user has permissions "Create SQL reports > (create_reports)" user can create report that dumps API keys from the db > with custom SQL report like "SELECT * FROM `api_keys`" > (also reports allow to see hashed passwords but it's probably well known > thing already) > Even though it leads to the similar problem being caused by a different > issue, it has a workaround and it's better to create another ticket for it. Yes, please a separate bug report for that one. (In reply to Jonathan Druart from comment #3) > (In reply to Peter Vashchuk from comment #2) > > Another open issue is that if user has permissions "Create SQL reports > > (create_reports)" user can create report that dumps API keys from the db > > with custom SQL report like "SELECT * FROM `api_keys`" > > (also reports allow to see hashed passwords but it's probably well known > > thing already) > > Even though it leads to the similar problem being caused by a different > > issue, it has a workaround and it's better to create another ticket for it. > > Yes, please a separate bug report for that one. Ha, you did already! Bug 28772. Created attachment 123701 [details] [review] Bug 28759: limit accessibility for "Manage API keys" This patch limits the accessibility for "Manage API keys" section only to superlibrarians and the owner of that said API key account. The way it does it is by checking if user is superlibrarian or if logged-in user is the same as a patron id/borrower number is the same as logged-in user number both in template and apikeys.pl and making sure the link is inaccessible or redirects to the 403 page if user tries to go there directly. To reproduce: 1) create/pick existing patron, set Staff access, allows viewing of catalogue in staff interface (catalogue)" and "Add, modify and iew patron information (borrowers)" permissions on; 2) enable "RESTOAuth2ClientCredentials" in sysprefs; 3) login with that user into staff interface; 4) check any other patron, go to the "More"->"Manage API keys" and check that you can see, add delete their API keys; 5) apply patch; 6) with that same user try to access "Manage API keys" page again. Ensure that you can't access that page of other patrons but can access your own page and manage your own API keys. 7) log in with superlibrarian now and ensure that you can access every "Manage API keys" page of every patron and apply changes there. Good job Created attachment 123782 [details] [review] Bug 28759: limit accessibility for "Manage API keys" This patch limits the accessibility for "Manage API keys" section only to superlibrarians and the owner of that said API key account. The way it does it is by checking if user is superlibrarian or if logged-in user is the same as a patron id/borrower number is the same as logged-in user number both in template and apikeys.pl and making sure the link is inaccessible or redirects to the 403 page if user tries to go there directly. To reproduce: 1) create/pick existing patron, set Staff access, allows viewing of catalogue in staff interface (catalogue)" and "Add, modify and iew patron information (borrowers)" permissions on; 2) enable "RESTOAuth2ClientCredentials" in sysprefs; 3) login with that user into staff interface; 4) check any other patron, go to the "More"->"Manage API keys" and check that you can see, add delete their API keys; 5) apply patch; 6) with that same user try to access "Manage API keys" page again. Ensure that you can't access that page of other patrons but can access your own page and manage your own API keys. 7) log in with superlibrarian now and ensure that you can access every "Manage API keys" page of every patron and apply changes there. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 124494 [details] [review] Bug 28759: limit accessibility for "Manage API keys" This patch limits the accessibility for "Manage API keys" section only to superlibrarians and the owner of that said API key account. The way it does it is by checking if user is superlibrarian or if logged-in user is the same as a patron id/borrower number is the same as logged-in user number both in template and apikeys.pl and making sure the link is inaccessible or redirects to the 403 page if user tries to go there directly. To reproduce: 1) create/pick existing patron, set Staff access, allows viewing of catalogue in staff interface (catalogue)" and "Add, modify and iew patron information (borrowers)" permissions on; 2) enable "RESTOAuth2ClientCredentials" in sysprefs; 3) login with that user into staff interface; 4) check any other patron, go to the "More"->"Manage API keys" and check that you can see, add delete their API keys; 5) apply patch; 6) with that same user try to access "Manage API keys" page again. Ensure that you can't access that page of other patrons but can access your own page and manage your own API keys. 7) log in with superlibrarian now and ensure that you can access every "Manage API keys" page of every patron and apply changes there. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Good call, works as expected.. Signing off. I meant PQA.. ;) Backported: Pushed to 20.05.x security branch for 20.05.16 Backported to 21.05.04, 20.11.10, 20.05.16, 19.11.22 Pushed to master for 21.11, thanks to everybody involved! |