Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Roles/permission not applied to migrations #114

Closed
bseeger opened this issue Apr 19, 2021 · 6 comments
Closed

Roles/permission not applied to migrations #114

bseeger opened this issue Apr 19, 2021 · 6 comments

Comments

@bseeger
Copy link

bseeger commented Apr 19, 2021

A user with access to run ingests can add collections and items in such ways that are beyond the permissions they should have access to.

To be clear, the roles that can access ingests are Collection Level Admins (CLA) and Global Admins. The Global Admin is not a concern, but the CLA's will only have access to certain content.

Right now a CLA can ingest material into any collection via an ingest (and UI). They cannot have edit access the item afterwards, but they can still create them, which is an issue.

This is tangential to this ticket. #108
Once that ticket is solved, I think the ingests will be better scoped to the user performing them and will fail unless they apply member_of and access terms the user has permission to use. I'm not sure if ticket 108 will fully solve the ingest issue though.

@birkland
Copy link

birkland commented Jun 9, 2021

At least one fundamental issue here is that validation for migrated content is not enabled (a related ticket #86 might be satisfied merely be enabling validation of migrations). Library staff will be populating Islandora primarily by defining csv files to migrate, so getting validation to work in the first place would be a big step. To the extent that this specific issue (roles/permission verification) is not satisfied merely be enabling validation, please track down and implement the missing pieces. I'm sure that existing migrations used in tests do are missing required fields, so it is likely an additional ticket would be needed to update those existing migrations, should migration validation prove to be possible to enable.

@jordandukart
Copy link

jordandukart commented Jun 14, 2021

In regards to the access use case(s) is there anything supplemental to this Google doc or direction to the existing implementation in code? Is this ticket referring to ingesting via the GUI only or does it also apply to Drupal Migrate migrations ran via CLI?

Given that ParseEntityLookup is invoking EntityLookup proper, accessCheck should being getting leveraged when performing lookups (at least on the migration side).

Assuming node access is being used (with an implementation to satisfy the use case(s)), entity_lookup should return nothing if a node was attempted to be referenced that a user did not have access to. At that point SkipOnEmpty may want to be leveraged to provide feedback that a value was looked up for and was either not found because it did not exist or the user did not have access to it. A decision would need to be made if this causes the entire row to be skipped or just the field itself.

From a GUI perspective it looks like field_member_of is leveraging Entity Reference Unpublished which is extending DefaultSelection where any access based implementations should be respected.

Lastly, setting validate: true is probably a good idea here anyway but as noted by @birkland this could uncover other issues.

@birkland
Copy link

Unfortunately, Bethany is on vacation for another couple weeks (and she's most familiar with this work), but here's a working doc she wrote up:
https://docs.google.com/document/d/12UBsXPWy9lG8owxg2cAA7ixxoqLgAi0oK1pSZoZSEWQ/edit#heading=h.aav7a08q05x7

Her work mostly leverages the workbench module

Ingests will be acheved by uploading CSVs via the migrate plus UI

@birkland
Copy link

Blocked awaiting advice on required validation for launch

@bseeger
Copy link
Author

bseeger commented Jul 20, 2021

This PR solves part of this ticket (the member_of part): #184

A fix for field_access_terms is being worked on by @jordandukart

@birkland
Copy link

birkland commented Aug 3, 2021

All parts are in place, as of jhu-idc/idc_defaults#4

@birkland birkland closed this as completed Aug 3, 2021
@birkland birkland removed the follow-up label Aug 3, 2021
jaredgalanis pushed a commit that referenced this issue Feb 15, 2022
Allow macs to be slower during initial build - fixes #113
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants