[FOLIO-1153] Permissions consistency enforcement breaks CI builds Created: 21/Mar/18  Updated: 12/Nov/18  Resolved: 27/Mar/18

Status: Closed
Project: FOLIO
Components: None
Affects versions: None
Fix versions: None

Type: Bug Priority: P1
Reporter: Wayne Schneider Assignee: Wayne Schneider
Resolution: Done Votes: 0
Labels: ci, sprint34
Remaining Estimate: Not Specified
Time Spent: 1 day, 7 hours, 30 minutes
Original estimate: Not Specified

Issue links:
Blocks
is blocked by MODPERMS-32 Enabling ui-inventory causes tight lo... Closed
is blocked by MODPERMS-33 Allow for modules to load permissions... Closed
is blocked by UIIN-136 Okapi permissionSets contain non-exis... Closed
is blocked by UIREQ-73 Okapi permissionSets contain non-exis... Closed
Relates
relates to MODPERMS-29 keep references to users and parent p... Closed
Sprint:

 Comments   
Comment by Wayne Schneider [ 21/Mar/18 ]

Marc Johnson reported:
Overnight, the builds for folio-testing-backend (https://jenkins-aws.indexdata.com/job/Automation/job/folio-testing-backend01/298/consoleFull and hence also the UI) folio-snapshot (https://jenkins-aws.indexdata.com/job/Automation/job/folio-snapshot/156/console) and folio-blackbox (https://jenkins-aws.indexdata.com/job/Automation/job/folio-blackbox/501/consoleFull).

Symptoms

Most of the builds failed with an error similar to:

failed: [10.36.1.243] (item=ui-users.settings.permsets) => {"accept": "application/json", "accept_encoding": "identity", "changed": false, "connection": "close", "content": "Error attempting to update permissions metadata: Attempting to add non-existent permissions to user", "content_type": "text/plain", "failed": true, "host": "10.36.1.243:9130", "item": "ui-users.settings.permsets", "msg": "Status code was not [200, 422]: HTTP Error 500: Internal Server Error", "redirected": false, "status": 500, "transfer_encoding": "chunked", "url": "http://10.36.1.243:9130/perms/users/2408ae64-56ad-4177-9024-1e35fe5d895c/permissions", "user_agent": "ansible-httpget", "x_okapi_permissions": "[\"perms.users.item.post\"]", "x_okapi_request_id": "863699/perms", "x_okapi_tenant": "diku", "x_okapi_token": "eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiJkaWt1X2FkbWluIiwidXNlcl9pZCI6IjFhZDczN2IwLWQ4NDctMTFlNi1iZjI2LWNlYzBjOTMyY2UwMSIsInRlbmFudCI6ImRpa3UifQ.vrW69kPTGFlyAT6YJeddG8fP174wI-UH6mx3eWO70v4-D-Y0pQGTn1VegLoD5m6-caOY8jmMR74uUo_f1qTx3Q", "x_okapi_trace": "POST mod-authtoken-1.4.1-SNAPSHOT.20 http://10.36.1.243:9131/perms/users/2408ae64-56ad-4177-9024-1e35fe5d895c/permissions : 202 3580us, POST mod-permissions-5.1.0-SNAPSHOT.12 http://10.36.1.243:9132/perms/users/2408ae64-56ad-4177-9024-1e35fe5d895c/permissions : 500 8058us", "x_okapi_url": "http://10.36.1.243:9130", "x_okapi_user_id": "1ad737b0-d847-11e6-bf26-cec0c932ce01"}

folio-snapshot failed with a different error:

fatal: [10.36.1.22]: FAILED! => {"changed": false, "connection": "close", "content": "POST request for mod-permissions-5.1.0-SNAPSHOT.12 /_/tenantpermissions failed with Internal Server Error: Unable to satisfy dependencies for permission inventory-storage.all", "content_length": "174", "content_type": "text/plain", "failed": true, "msg": "Status code was not [200]: HTTP Error 500: Internal Server Error", "redirected": false, "status": 500, "url": "http://10.36.1.22:9130/_/proxy/tenants/diku/install"}

However this looks to be due to a related cause (I haven’t dug deeply into the code to check this assumption).

Initial Cause Investigation

The first error is produced here: https://github.com/folio-org/mod-permissions/blob/8e72f8b525457f6d7be39f925e5f240ec2321250/src/main/java/org/folio/rest/impl/PermsAPI.java#L1652

The second error is produced here: https://github.com/folio-org/mod-permissions/blob/8e72f8b525457f6d7be39f925e5f240ec2321250/src/main/java/org/folio/rest/impl/TenantPermsAPI.java#L97

Both of which were added in the same commit, which I think was merged yesterday (https://github.com/folio-org/mod-permissions/commit/8e72f8b525457f6d7be39f925e5f240ec2321250) - interestingly git blame cites the original commit which makes finding when it changed in master harder) as part of https://folio-org.atlassian.net/browse/MODPERMS-29.

It looks like we now reject assignments of permissions to users or permission sets where the permission does not exist.

Further Investigation

I’m starting to track down the permissions involved (and to confirm my understanding of the cause).

I had a quick look at trying to add which permission(s) caused the failure, or were involved in the activity. I don’t think I’m familiar enough with the code to make those changes without risking introducing other issues.

Kurt, I reckon you might be better placed to potentially do this, if you think it makes sense?

Possible Interim Mitigation

A tweak to mod-permissions to change the errors to be a 422 response should mean that folio-ansible marks these responses as success in some circumstances (the testing environment but not snapshot I think) which might temporarily mitigate some of the impact.

A consequence of that could be to mask the underlying issue of having undefined permissions which are attempted to be being granted.

Comment by Wayne Schneider [ 21/Mar/18 ]

Also from Marc Johnson:
Inventory Storage Investigation

I thought I’d start with the folio-snapshot build error message as this gave me an immediate hint at where to look (and it is an area I am familiar with):
Unable to satisfy dependencies for permission inventory-storage.all

I’ve found a typo in one of the permission names (see https://folio-org.atlassian.net/browse/MODINVSTOR-91) and am pushing a fix for that.

Will run the folio-snapshot (and then folio-testing) build to see if that is the only discrepancy.

(As an aside, I wrote a python script to help determine the discrepancies, with the help of a quick diff - https://github.com/k-int/mod-inventory-storage/blob/check-permissions/check-permission-discrepancies.py)

Comment by Wayne Schneider [ 21/Mar/18 ]

See https://folio-org.atlassian.net/browse/MODPERMS-29?focusedCommentId=76802&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel for an explanation of the effects of the change.

Comment by Wayne Schneider [ 21/Mar/18 ]

John Malconian has work on branch folio-1051a of folio-ansible that may take care of the issue of assigning permissions.

Comment by Wayne Schneider [ 22/Mar/18 ]

Temporary workaround: pinned the folio-testing and folio-snapshot builds to mod-permissions-5.0.1-SNAPSHOT.11.

Work is in progress on refactoring the Ansible build to use the tenant-admin-permissions role to assign admin permissions. Once the blocking issues are closed, that work should be able to be merged into master and we can remove the pinning for mod-permissions.

Comment by Wayne Schneider [ 22/Mar/18 ]

With the workarounds in place, I believe the blocked circ issues can be unblocked – Marc Johnson, Jakub Skoczen?

Comment by Marc Johnson [ 23/Mar/18 ]

Wayne Schneider Yup, I'll be unblocking and testing them this morning

Comment by Mike Taylor [ 23/Mar/18 ]

I see that one of the blockers, UIREQ-73 Closed , has been "fixed" by removing the notes.domain.requests permission from the Requests module; see https://github.com/folio-org/ui-requests/pull/58/files

This can't be the right solution, can it?. The module needs notes.domain.requests to make and maintain notes, does it not?

Comment by Marc Johnson [ 23/Mar/18 ]

Mike Taylor That is an example of an interesting scenario. (Including Heikki Levanto Kurt Nordstrom Matt Connolly and Jakub Skoczen as people who might have more knowledge / opinion about this)

The current changes to mod-permissions means that unknown permissions cannot be used, and as far as I can tell notes.domain.requests is not defined in any module.

What might make this especially interesting is the approach within mod-notes to use a wildcard permission mechanism ("permissionsDesired": ["notes.domain.*", "notes.domain.all"]). Which allows different domains of notes to be defined elsewhere to mod-notes.

Therefore, the initial question might be (assuming we want this late binding of domains for mod-notes and strong references to permissions within mod-permissions) where could / should those domain permissions be defined?

In this specific case, I guess UI requests could define the permission itself? Or to define the supported domains within mod-notes?

Comment by Mike Taylor [ 23/Mar/18 ]

Thanks, Marc, that's a nice summary of the problem.

I think the best solution is for the server-side module to define the permission for managing notes for that module – and then the client module can refer to that permission, including it in a higher-level one, if it wants to. For example, mod-users would define notes.domain.users, and ui-users would be at liberty to include this in ui-users.allPerms or something.

Comment by Heikki Levanto [ 23/Mar/18 ]

Yes, I was thinking along those lines when I designed the notes permissions.

Comment by Wayne Schneider [ 23/Mar/18 ]

In this particular case, because ui-requests.all contains the subPermission notes.domain.all, removing notes.domain.requests from the permissionSet should not break anything. That said, ui-requests.all probably shouldn't have notes.domain.all included in it.

I note that, AFAICT, no other module makes use of notes.domain.* permissions, and no atomic permissionSets for notes domains have been defined in any module that I could find. I have raised issue MODNOTES-36 Closed to continue this conversation, as I think it is only tangentially related to the current issue around the changed behavior in mod-permissions and how it affects the CI builds.

Comment by Wayne Schneider [ 25/Mar/18 ]

Changes merged to folio-ansible and folio-infrastructure.

Generated at Thu Feb 08 23:11:13 UTC 2024 using Jira 1001.0.0-SNAPSHOT#100246-sha1:7a5c50119eb0633d306e14180817ddef5e80c75d.