Adds Valkyrie native permissions and ACL handling #3900
Conversation
The actor stack should persist arbitrary permissions. This behavior is untested elsewhere, and may be at risk during the transition to Wings.
This is the beginning of a larger effort to establish Valkyrie-native permission management/ACLs.
Adds support for editing ACLs for a given resource using Valkyrie interfaces.
Adds a simple DSL for adding and removing permissions by mode/user pairs. This reduces the overhead/possibility for error in manually creating ACL policies by directly managing `Hyrax::Permission` objects.
There was a problem hiding this comment.
It looks like the approach here is to store each Hyrax::Permission as it's own object. Is this deliberate to mirror hydra-access-controls? If it doesn't need to mirror the existing fedora modeling, would collapsing them into the ACL so there is only one ACL object persisted for all Permissions be more performant?
More or less. We definitely want
Possibly. If so, this benefit won't be accessible until after Wings is deprecated (Wings must continue to save |
|
The switch to nested resources in the future might require a data migration though hence my interest in pushing for the ideal at the outset. But it is probably too early to see what is going to be best and my suggestion might just be premature optimization. |
i think whether this is the case will depend on the specific adapter. for now, with only Wings supported, the data layout will be the same regardless. do you feel confident the change will be an optimization for, e.g., the |
|
Maybe it would be useful to ask some other valkyrie developers for their opinion? |
|
it seems like maybe i should just develop this further before submitting? it doesn't feel productive to me to explore optimizations at this point, since this approach doesn't yet support round-tripping via Wings, so can't be used in production. |
|
|
||
| additions << permission | ||
|
|
||
| true |
There was a problem hiding this comment.
Why return boolean value instead of additions array?
There was a problem hiding this comment.
additions and deletions are internal implementation details that i don't expect to survive sustained development. it seems likely this code will use a ChangeSet or similar eventually.
For this reason, exposing them via the API seemed fragile.
| additions.delete(permission) | ||
| deletions << permission | ||
|
|
||
| true |
There was a problem hiding this comment.
Same question, why return boolean value instead of deletions array?
👍 I don't want to block Wings work over this. Maybe we could review this / think about optimization when it is getting closer to production-ready? |
Adds
Hyrax::PermissionandHyrax::AccessControlListas an initial implementation of ACLs and read/edit/discover grants for objects in Valkyrie.A simple grant/revoke DSL is added to encapsulate the most common practice of granting or revoking a particular access mode for a given user or group.
For now, this is only working against the abstract Valkyrie interface; further work is needed to allow the Wings adapter to translate the Valkyrie native approach to
ActiveFedora-compatible data/operations.This moves work forward toward #3584.
Changes proposed in this pull request:
Guidance for testing, such as acceptance criteria or new user interface behaviors:
@samvera/hyrax-code-reviewers