[FOLIO-2280] tenant superuser granted excessive okapi permissions Created: 24/Sep/19 Updated: 13/Jul/21 Resolved: 10/Mar/20 |
|
| Status: | Closed |
| Project: | FOLIO |
| Components: | None |
| Affects versions: | None |
| Fix versions: | None |
| Type: | Task | Priority: | P2 |
| Reporter: | John Malconian | Assignee: | Ian Hardy |
| Resolution: | Done | Votes: | 0 |
| Labels: | security | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original estimate: | Not Specified | ||
| Issue links: |
|
||||||||||||||||||||||||||||||||||||||||||||
| Sprint: | DevOps: sprint 84, CP: sprint 74, CP: sprint 73 | ||||||||||||||||||||||||||||||||||||||||||||
| Development Team: | Core: Platform | ||||||||||||||||||||||||||||||||||||||||||||
| Description |
|
Tenant super user is granted excessive Okapi permissions (okapi.*) on FOLIO instances built in the hosted CI environment and FOLIO vagrant-based images. For example, it is currently possible for a tenant super user, like diku_admin, to create new tenants via the Okapi /tenants endpoint. Before removing excessive permission:
|
| Comments |
| Comment by John Malconian [ 24/Sep/19 ] |
|
example:
login as diku_admin
curl https://folio-testing-okapi.aws.indexdata.com:443/authn/login -H 'Content-Type: application/json' -H 'X-Okapi-Tenant: diku' --data-binary '{"username":"diku_admin","password":"admin"}' -H 'Cache-control: no-cache' -v -w '\n' -o /dev/null
+ TOKEN=eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJ........
Without the token:
$ curl $OKAPI/_/proxy/tenants -H "Content-type: application/json" -XPOST -d '{ "id": "testing", "name": "POC tenant" }'
Access requires permission: okapi.proxy.tenants.post
With the token
$ curl $OKAPI/_/proxy/tenants -H "Content-type: application/json" -H "X-Okapi-Token: $TOKEN" -XPOST -d '{ "id": "testing", "name": "POC tenant" }'
{
"id" : "testing",
"name" : "POC tenant"
}
$ curl $OKAPI/_/proxy/tenants
[ {
"id" : "diku",
"name" : "Datalogisk Institut",
"description" : "Danish Library Technology Institute"
}, {
"id" : "invoicing_api_tests",
"name" : "Test invoices tenant",
"description" : "Tenant for test purpose"
}, {
"id" : "supertenant",
"name" : "supertenant",
"description" : "Okapi built-in super tenant"
}, {
"id" : "testing",
"name" : "POC tenant"
} ]
|
| Comment by John Malconian [ 24/Sep/19 ] |
|
Looks tenant admins currently get the 'okapi.all' permission. Refine this if possible. |
| Comment by Ian Hardy [ 09/Oct/19 ] |
|
There are some API tests run by hand in https://github.com/folio-org/folio-api-tests that use diku_admin to create tenants and will begin to fail when this is fixed. To provide a workaround, we're adding an additional user on the folio-testing build that can be used to create temporary tenants for these tests in its place. Tests can still run on the vagrant boxes without modification since the supertenant is unsecured there. |
| Comment by Ian Hardy [ 16/Oct/19 ] |
|
Hi Craig McNally would you be OK with setting a date to close up this loophole? What seems reasonable, a week or two, maybe longer? Could help people prioritize updating tests. |
| Comment by Craig McNally [ 18/Oct/19 ] |
|
I've created user stories - it's out of my hands at this point. I'll refer you to our PO Dennis Bridges to get this prioritized and sync up on a reasonable deadline. |
| Comment by Dennis Bridges [ 18/Oct/19 ] |
|
Hi Ian Hardy, we're planning to include the required updates defined by Craig for this issue in sprint 75. Thus, we will need at least two weeks to implement the changes. That said we have another sprint review Monday to confirm. If things change I will let you know. thx! |
| Comment by Ian Hardy [ 21/Oct/19 ] |
|
Thanks Dennis Bridges and Craig. I'll change the links to blocking and we'll handle this one after sprint 75 when those are resolved. |
| Comment by Ian Hardy [ 09/Mar/20 ] |
|
Update CQL to exclude okapi.* instead of okapi.all to protect against any future okapi permissions making there way in. |
| Comment by Ian Hardy [ 09/Mar/20 ] |
|
This ticket has been open for some time and we've reached a point where we need to merge it to improve security on the reference environments. Pulling into current sprint. |
| Comment by Ian Hardy [ 09/Mar/20 ] |
|
Struggling with some weird ansible behavior here. The tenant-admin-permissions role runs at least twice. Right after creating the tenant/admin user and again as a requirement for the ebsco-rmapi-config role. After updating the cql query in tenant-admin-permissions role on master, I see the new query is picked up the first time, but not the second time the role runs in the same build. For example, in this build: https://jenkins-aws.indexdata.com/job/Automation/job/folio-snapshot-test/111/consoleFull Search for "Get all permissionSets not included" and you'll get two hits. the first time there's the "excluding okapi" message I added when I update the cql query, the second time, not. Seems like when its included in ebsco-rmapi-config/meta/main.yml it's not running the latest version of master. Does this sound familiar Wayne Schneider John Malconian or David Crossley. I'll continue looking into it. Its strange and was wondering if any of you had seen it before. |
| Comment by Wayne Schneider [ 09/Mar/20 ] |
|
Ian Hardy it looks like there is a customized "tenant-admin-permissions" role in folio-infrastructure (https://github.com/folio-org-priv/folio-infrastructure/tree/master/CI/ansible/roles/tenant-admin-permissions) which gets called when ebsco-rmapi-config includes the role in the meta/main.yml file, because folio-ansible/roles is not on the roles path. I'm not sure why there is a customized tenant-admin-permissions role or how it differs from the stock one. |