feat: SameAs generator#119
Conversation
0dbf9f5 to
8c8cd95
Compare
28c9a14 to
4ccd60e
Compare
max-moser
left a comment
There was a problem hiding this comment.
this looks amazing, and has the potential to massively simplify permission policy definitions!
thanks a lot for this 😃
i'll take it for a test ride and approve after 🙂
c3d3ad0 to
f057e35
Compare
* Changed permission policy's context to propagate policy instance to generators * Added CompositeGenerator class * Added implementation of SameAs with tests Co-authored-by: Ronald Krist <ronald.krist@cesnet.cz> Co-authored-by: Max <maximilian.moser@tuwien.ac.at>
f057e35 to
44627a4
Compare
|
FYI, I'm currently in the process of reshaping our own custom permission policy with this new generator, and preliminary results are very promising: https://gitlab.tuwien.ac.at/crdm/invenio-config-tuw/-/merge_requests/110 |
max-moser
left a comment
There was a problem hiding this comment.
With this new permission generator, I was able to cut down our custom TUWRecordPermissionPolicy from 131 lines of code down to 38 (and half of those are comments).
Now we only have 4 explicit action permission definitions left, instead of overriding almost every single one like before.
Massive work, thanks a ton!
Warning
This is a backward-incompatible change, as users might, in rare cases, rely on the static binding inside the base permission policy.
Please consider merging #120 for the same release as that would need major bump as well
Note
This PR has an accompanying one in invenio-rdm-records - inveniosoftware/invenio-rdm-records#2267
This PR has an accompanying one in invenio-requests - inveniosoftware/invenio-requests#582
This PR has an accompanying one in invenio-communities - inveniosoftware/invenio-communities#1355
Description
A permission policy can be a complicated thing - look for example at the RDM permission policy:
If we inherit from the policy and modify the
can_manageto include another group of users:can_curateis not updated as a user might expect. When evaluating permissions,can_curatewill not contain theUsersWithRolegenerator because it was created statically inside theRDMPermissionPolicyclass.The
SameAsgenerator addresses this issue by introducing dynamic binding — the delegation/inclusion happens at permission evaluation time. If the RDM policy were defined as follows:the example above would work as the user would intuitively expect.
Note:
SameAsis a permission generator, so that it can be included anywhere permission generator might go. To simplify permissions,SameAscan also be used outside lists and combined with other lists:Checklist
Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:
Frontend
Reminder
By using GitHub, you have already agreed to the GitHub’s Terms of Service including that: