[FOLIO-2565] Misleading Permission Set Configuration Created: 16/Apr/20 Updated: 13/Jul/21 Resolved: 04/May/20 |
|
| Status: | Closed |
| Project: | FOLIO |
| Components: | None |
| Affects versions: | None |
| Fix versions: | None |
| Type: | Bug | Priority: | P2 |
| Reporter: | Jakub Skoczen | Assignee: | Craig McNally |
| Resolution: | Done | Votes: | 0 |
| Labels: | platform-backlog, security | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original estimate: | Not Specified | ||
| Issue links: |
|
||||||||||||||||||||||||||||
| Sprint: | CP: sprint 87 | ||||||||||||||||||||||||||||
| Story Points: | 2 | ||||||||||||||||||||||||||||
| Development Team: | Core: Platform | ||||||||||||||||||||||||||||
| Description |
| Comments |
| Comment by Julian Ladisch [ 21/Apr/20 ] |
|
mod-kb-ebsco-java has it's own configuration API endpoint. It stores the password to the EBSCO Knowledge Base but removes the password from the GET request: https://github.com/folio-org/mod-kb-ebsco-java/blob/v3.4.0/src/main/java/org/folio/rest/impl/EholdingsConfigurationImpl.java#L59 |
| Comment by Craig McNally [ 28/Apr/20 ] |
|
There are a few parts to this... 1) Permission sets named inappropriately, e.g. something.view suggests it provides the ability to view something. It shouldn't have subPermissions that allow for put/post/delete. Here's an actual example:
{
"permissionName": "ui-receiving.basic.view",
"displayName": "Receiving: Basic view",
"id": "5542bb26-9eff-4699-a7c5-6e6a049979d7",
"tags": [],
"subPermissions": [
"module.receiving.enabled",
"orders.item.get",
"orders.pieces.item.post",
"orders.pieces.item.put",
"orders.po-lines.collection.get",
"orders.titles.collection.get",
"orders.titles.item.get",
"ui-receiving.third-party-services"
],
"childOf": [
"ui-receiving.view"
],
"grantedTo": [],
"mutable": false,
"visible": false,
"dummy": false
}
The ERM modules use a naming convention that might be more appropriate for these types of permission set, e.g.
{
"permissionName": "ui-erm-usage.view-create-edit",
"displayName": "eUsage: Can view, create and edit usage data providers and COUNTER reports",
"id": "ade748af-66b5-4584-a319-3cac20899241",
"description": "Can view, create and edit usage data providers and COUNTER reports",
"tags": [],
"subPermissions": [
"module.erm-usage.enabled",
"usagedataproviders.collection.get",
"usagedataproviders.item.get",
"usagedataproviders.item.post",
"usagedataproviders.item.put",
"counterreports.collection.get",
"counterreports.item.get",
"counterreports.item.post",
"counterreports.item.put",
"aggregatorsettings.collection.get",
"aggregatorsettings.item.get"
],
"childOf": [],
"grantedTo": [
"fba0106d-e2ad-494e-8958-ce5b447ab2aa"
],
"mutable": false,
"visible": true,
"dummy": false
}
2) Permission sets provided by module A provide subPermissions for module B. The most glaring example of this is the inclusion of mod-configuration properties in permission sets created by other modules... $ curl $OKAPI/perms/permissions?length=99999 -H "X-Okapi-Token: $TOKEN" -w'\n' -s | jq '.permissions[] | select(.subPermissions[] | contains("configuration.entries"))' | grep permissionName "permissionName": "ui-inventory.settings.hrid-handling", "permissionName": "ui-oai-pmh.edit", "permissionName": "module.licenses.enabled", "permissionName": "modperms.circulation.loans.anonymize", "permissionName": "modperms.circulation.override-renewal-by-barcode.post", "permissionName": "modperms.circulation.renew-by-barcode.post", "permissionName": "modperms.circulation.requests.item.move.post", "permissionName": "module.agreements.enabled", "permissionName": "modperms.circulation.renew-by-id.post", "permissionName": "ui-calendar.view", "permissionName": "modperms.circulation.requests.item.post", "permissionName": "modperms.circulation.override-check-out-by-barcode.post", "permissionName": "modperms.circulation.requests.item.put", "permissionName": "modperms.circulation.requests.instances.item.post", "permissionName": "modperms.circulation.check-out-by-barcode.post", "permissionName": "module.erm-usage.enabled", "permissionName": "module.erm-usage.enabled", "permissionName": "ui-finance.third-party-services", "permissionName": "modperms.orders.item.post", "permissionName": "modperms.orders.item.put", "permissionName": "settings.erm-usage.enabled", "permissionName": "settings.erm-usage.enabled", "permissionName": "settings.erm-usage.enabled", "permissionName": "ui-inventory.instance.view", "permissionName": "ui-invoice.third-party-services", "permissionName": "ui-oai-pmh.view", "permissionName": "ui-orders.third-party-services", "permissionName": "ui-organizations.third-party-services", "permissionName": "ui-receiving.third-party-services", "permissionName": "configuration.all", "permissionName": "configuration.all", "permissionName": "configuration.all", "permissionName": "configuration.all", "permissionName": "configuration.all", "permissionName": "ui-users.settings.customfields.view", "permissionName": "ui-requests.view", "permissionName": "ui-users.settings.customfields.edit", "permissionName": "ui-users.settings.customfields.edit", "permissionName": "ui-users.view", This one can be categorized further...
3) mod-configuration permissions are not very granular. Granting a user to "configuration.entries.collection.get" so they can view a handful of relevant entries means that they can now also view ALL entries, regardless of their relevance and/or sensitivity. For this reason:
|
| Comment by Craig McNally [ 28/Apr/20 ] |
|
I've started a wiki page to capture the proposed guidelines: https://folio-org.atlassian.net/wiki/display/FOLIJET/Proposed+Permission+Set+Guidelines |
| Comment by Zak Burke [ 28/Apr/20 ] |
|
The mod-configuration issue is especially sticky IMHO as we don't have a way to segregate permissions based on user or module even though the permissions themselves are split out that way. Huh? It's like this: you can't grant users permission to read or write some configuration data without granting them permission to read or write all configuration data, regardless of module or userId. That seems ... unfortunate. |
| Comment by Craig McNally [ 04/May/20 ] |
|
Closing this per conversation with Jakub Skoczen. See linked tickets for follow-on work |